Skip to content

Commit 522e0b5

Browse files
erbr-otcarlescufi
authored andcommitted
Bluetooth: controller: fixing issue re. erroneous DLE changed events
Only apply change to effective DLE times if current max times are too small to accommodate. Similar to legacy implementation Update unit tests to new DLE ntf behavior Signed-off-by: Erik Brockhoff <[email protected]>
1 parent e1c2f2c commit 522e0b5

File tree

4 files changed

+26
-80
lines changed

4 files changed

+26
-80
lines changed

subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@ static uint8_t pu_update_eff_times(struct ll_conn *conn, struct proc_ctx *ctx)
286286
pu_calc_eff_time(lll->dle.eff.max_rx_octets, lll->phy_rx, max_rx_time);
287287
}
288288

289-
if ((eff_tx_time != lll->dle.eff.max_tx_time) ||
290-
(eff_rx_time != lll->dle.eff.max_rx_time)) {
289+
if ((eff_tx_time > lll->dle.eff.max_tx_time) ||
290+
(eff_rx_time > lll->dle.eff.max_rx_time)) {
291291
lll->dle.eff.max_tx_time = eff_tx_time;
292292
lll->dle.eff.max_rx_time = eff_rx_time;
293293
return 1U;

tests/bluetooth/controller/ctrl_collision/src/main.c

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,6 @@ void test_phy_update_central_loc_collision(void)
154154
struct pdu_data_llctrl_phy_upd_ind ind = { .instant = 9,
155155
.c_to_p_phy = PHY_2M,
156156
.p_to_c_phy = PHY_2M };
157-
struct pdu_data_llctrl_length_rsp length_ntf = {
158-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
159-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M)
160-
};
161157
uint16_t instant;
162158

163159
struct pdu_data_llctrl_reject_ext_ind reject_ext_ind = {
@@ -306,7 +302,6 @@ void test_phy_update_central_loc_collision(void)
306302

307303
/* There should be one host notification */
308304
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
309-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
310305
ut_rx_q_is_empty();
311306

312307
/* Release Ntf */
@@ -332,14 +327,6 @@ void test_phy_update_central_rem_collision(void)
332327
struct pdu_data_llctrl_phy_upd_ind ind_2 = { .instant = 14,
333328
.c_to_p_phy = PHY_2M,
334329
.p_to_c_phy = 0 };
335-
struct pdu_data_llctrl_length_rsp length_ntf_1 = {
336-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
337-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M)
338-
};
339-
struct pdu_data_llctrl_length_rsp length_ntf_2 = {
340-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
341-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M)
342-
};
343330
uint16_t instant;
344331

345332
struct node_rx_pu pu = { .status = BT_HCI_ERR_SUCCESS };
@@ -427,7 +414,6 @@ void test_phy_update_central_rem_collision(void)
427414

428415
/* There should be one host notification */
429416
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
430-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf_1);
431417
ut_rx_q_is_empty();
432418

433419
/* Release Ntf */
@@ -479,7 +465,6 @@ void test_phy_update_central_rem_collision(void)
479465

480466
/* There should be one host notification */
481467
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
482-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf_2);
483468
ut_rx_q_is_empty();
484469

485470
/* Release Ntf */
@@ -500,10 +485,6 @@ void test_phy_update_periph_loc_collision(void)
500485
struct pdu_data_llctrl_phy_upd_ind ind = { .instant = 7,
501486
.c_to_p_phy = PHY_2M,
502487
.p_to_c_phy = PHY_1M };
503-
struct pdu_data_llctrl_length_rsp length_ntf = {
504-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
505-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M)
506-
};
507488
uint16_t instant;
508489

509490
struct pdu_data_llctrl_reject_ext_ind reject_ext_ind = {
@@ -608,7 +589,6 @@ void test_phy_update_periph_loc_collision(void)
608589
/* There should be one host notification */
609590
pu.status = BT_HCI_ERR_SUCCESS;
610591
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
611-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
612592
ut_rx_q_is_empty();
613593

614594
/* Release Ntf */
@@ -626,10 +606,6 @@ void test_phy_conn_update_central_loc_collision(void)
626606
struct pdu_data *pdu;
627607
uint16_t instant;
628608

629-
struct pdu_data_llctrl_length_rsp length_ntf = {
630-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
631-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M)
632-
};
633609
struct pdu_data_llctrl_reject_ext_ind reject_ext_ind = {
634610
.reject_opcode = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ,
635611
.error_code = BT_HCI_ERR_DIFF_TRANS_COLLISION
@@ -751,7 +727,6 @@ void test_phy_conn_update_central_loc_collision(void)
751727

752728
/* (A) There should be one host notification */
753729
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
754-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
755730

756731
ut_rx_q_is_empty();
757732

tests/bluetooth/controller/ctrl_cte_req/src/main.c

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,7 @@ void wait_for_phy_update_instant(uint8_t instant)
883883

884884
void check_phy_update_and_cte_req_complete(bool is_local, struct pdu_data_llctrl_cte_req *cte_req,
885885
struct pdu_data_llctrl_phy_req *phy_req,
886-
uint8_t ctx_num_at_end)
886+
uint8_t ctx_num_at_end, bool dle_ntf)
887887
{
888888
struct pdu_data_llctrl_length_rsp length_ntf = {
889889
PDU_PDU_MAX_OCTETS, PDU_DC_MAX_US(PDU_PDU_MAX_OCTETS, phy_req->tx_phys),
@@ -918,7 +918,9 @@ void check_phy_update_and_cte_req_complete(bool is_local, struct pdu_data_llctrl
918918

919919
/* There should be two host notifications, one pu and one dle */
920920
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
921-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
921+
if (dle_ntf) {
922+
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
923+
}
922924

923925
/* Release Ntf */
924926
ull_cp_release_ntf(ntf);
@@ -966,7 +968,7 @@ void check_phy_update_and_cte_req_complete(bool is_local, struct pdu_data_llctrl
966968
*/
967969
static void run_phy_update_central(bool is_local, struct pdu_data_llctrl_cte_req *cte_req,
968970
struct pdu_data_llctrl_phy_req *phy_req, uint8_t events_at_start,
969-
uint8_t ctx_num_at_end)
971+
uint8_t ctx_num_at_end, bool dle_ntf)
970972
{
971973
struct pdu_data_llctrl_phy_req rsp = { .rx_phys = PHY_PREFER_ANY,
972974
.tx_phys = PHY_PREFER_ANY };
@@ -1030,7 +1032,7 @@ static void run_phy_update_central(bool is_local, struct pdu_data_llctrl_cte_req
10301032

10311033
wait_for_phy_update_instant(instant);
10321034

1033-
check_phy_update_and_cte_req_complete(is_local, cte_req, phy_req, ctx_num_at_end);
1035+
check_phy_update_and_cte_req_complete(is_local, cte_req, phy_req, ctx_num_at_end, dle_ntf);
10341036
}
10351037

10361038
/**
@@ -1050,7 +1052,8 @@ static void run_phy_update_central(bool is_local, struct pdu_data_llctrl_cte_req
10501052
*/
10511053
static void run_phy_update_peripheral(bool is_local, struct pdu_data_llctrl_cte_req *cte_req,
10521054
struct pdu_data_llctrl_phy_req *phy_req,
1053-
uint8_t events_at_start, uint8_t ctx_num_at_end)
1055+
uint8_t events_at_start, uint8_t ctx_num_at_end,
1056+
bool dle_ntf)
10541057
{
10551058
struct pdu_data_llctrl_phy_req rsp = { .rx_phys = PHY_PREFER_ANY,
10561059
.tx_phys = PHY_PREFER_ANY };
@@ -1122,7 +1125,7 @@ static void run_phy_update_peripheral(bool is_local, struct pdu_data_llctrl_cte_
11221125

11231126
wait_for_phy_update_instant(instant);
11241127

1125-
check_phy_update_and_cte_req_complete(is_local, cte_req, phy_req, ctx_num_at_end);
1128+
check_phy_update_and_cte_req_complete(is_local, cte_req, phy_req, ctx_num_at_end, dle_ntf);
11261129
}
11271130

11281131
static void test_local_cte_req_wait_for_phy_update_complete_and_disable(uint8_t role)
@@ -1155,10 +1158,10 @@ static void test_local_cte_req_wait_for_phy_update_complete_and_disable(uint8_t
11551158

11561159
if (role == BT_HCI_ROLE_CENTRAL) {
11571160
run_phy_update_central(true, NULL, &phy_req, pu_event_counter(&conn),
1158-
test_ctx_buffers_cnt() - 1);
1161+
test_ctx_buffers_cnt() - 1, true);
11591162
} else {
11601163
run_phy_update_peripheral(true, NULL, &phy_req, pu_event_counter(&conn),
1161-
test_ctx_buffers_cnt() - 1);
1164+
test_ctx_buffers_cnt() - 1, true);
11621165
}
11631166

11641167
/* In this test CTE request is local procedure. Local procedures are handled after remote
@@ -1222,10 +1225,10 @@ static void test_local_cte_req_wait_for_phy_update_complete(uint8_t role)
12221225

12231226
if (role == BT_HCI_ROLE_CENTRAL) {
12241227
run_phy_update_central(true, &local_cte_req, &phy_req, pu_event_counter(&conn),
1225-
test_ctx_buffers_cnt() - 1);
1228+
test_ctx_buffers_cnt() - 1, false);
12261229
} else {
12271230
run_phy_update_peripheral(true, &local_cte_req, &phy_req, pu_event_counter(&conn),
1228-
test_ctx_buffers_cnt() - 1);
1231+
test_ctx_buffers_cnt() - 1, false);
12291232
}
12301233

12311234
/* PHY update was completed. Handle CTE request */
@@ -1280,10 +1283,10 @@ static void test_local_phy_update_wait_for_cte_req_complete(uint8_t role)
12801283

12811284
if (role == BT_HCI_ROLE_CENTRAL) {
12821285
run_phy_update_central(true, NULL, &phy_req, pu_event_counter(&conn),
1283-
test_ctx_buffers_cnt());
1286+
test_ctx_buffers_cnt(), true);
12841287
} else {
12851288
run_phy_update_peripheral(true, NULL, &phy_req, pu_event_counter(&conn),
1286-
test_ctx_buffers_cnt());
1289+
test_ctx_buffers_cnt(), true);
12871290
}
12881291
}
12891292

@@ -1370,10 +1373,10 @@ static void test_phy_update_wait_for_remote_cte_req_complete(uint8_t role)
13701373

13711374
if (role == BT_HCI_ROLE_CENTRAL) {
13721375
run_phy_update_central(true, NULL, &phy_req, pu_event_counter(&conn),
1373-
test_ctx_buffers_cnt());
1376+
test_ctx_buffers_cnt(), true);
13741377
} else {
13751378
run_phy_update_peripheral(true, NULL, &phy_req, pu_event_counter(&conn),
1376-
test_ctx_buffers_cnt());
1379+
test_ctx_buffers_cnt(), true);
13771380
}
13781381
}
13791382

@@ -1422,10 +1425,10 @@ static void test_cte_req_wait_for_remote_phy_update_complete_and_disable(uint8_t
14221425

14231426
if (role == BT_HCI_ROLE_CENTRAL) {
14241427
run_phy_update_central(false, NULL, &phy_req, pu_event_counter(&conn),
1425-
test_ctx_buffers_cnt());
1428+
test_ctx_buffers_cnt(), true);
14261429
} else {
14271430
run_phy_update_peripheral(false, NULL, &phy_req, pu_event_counter(&conn),
1428-
test_ctx_buffers_cnt());
1431+
test_ctx_buffers_cnt(), true);
14291432
}
14301433

14311434
/* There is no special handling of CTE REQ completion. It is done when instant happens just
@@ -1478,10 +1481,10 @@ static void test_cte_req_wait_for_remote_phy_update_complete(uint8_t role)
14781481

14791482
if (role == BT_HCI_ROLE_CENTRAL) {
14801483
run_phy_update_central(false, &local_cte_req, &phy_req, pu_event_counter(&conn),
1481-
test_ctx_buffers_cnt());
1484+
test_ctx_buffers_cnt(), false);
14821485
} else {
14831486
run_phy_update_peripheral(false, &local_cte_req, &phy_req, pu_event_counter(&conn),
1484-
test_ctx_buffers_cnt());
1487+
test_ctx_buffers_cnt(), false);
14851488
}
14861489

14871490
/* There is no special handling of CTE REQ completion here. It is done when instant happens

tests/bluetooth/controller/ctrl_phy_update/src/main.c

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ void test_phy_update_central_loc(void)
125125

126126
struct node_rx_pu pu = { .status = BT_HCI_ERR_SUCCESS };
127127

128+
/* 'Trigger' DLE ntf on PHY update, as this forces change to eff tx/rx times */
129+
conn.lll.dle.eff.max_rx_time = 0;
130+
128131
/* Role */
129132
test_set_role(&conn, BT_HCI_ROLE_CENTRAL);
130133

@@ -329,10 +332,6 @@ void test_phy_update_central_rem(void)
329332
struct pdu_data_llctrl_phy_upd_ind ind = { .instant = 7,
330333
.c_to_p_phy = 0,
331334
.p_to_c_phy = PHY_2M };
332-
struct pdu_data_llctrl_length_rsp length_ntf = {
333-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
334-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M)
335-
};
336335
uint16_t instant;
337336

338337
struct node_rx_pu pu = { .status = BT_HCI_ERR_SUCCESS };
@@ -407,7 +406,6 @@ void test_phy_update_central_rem(void)
407406

408407
/* There should be one host notification */
409408
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
410-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
411409
ut_rx_q_is_empty();
412410

413411
/* Release Ntf */
@@ -425,10 +423,6 @@ void test_phy_update_periph_loc(void)
425423
struct node_tx *tx;
426424
struct node_rx_pdu *ntf;
427425
struct pdu_data_llctrl_phy_req req = { .rx_phys = PHY_2M, .tx_phys = PHY_2M };
428-
struct pdu_data_llctrl_length_rsp length_ntf = {
429-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
430-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M)
431-
};
432426
uint16_t instant;
433427

434428
struct node_rx_pu pu = { .status = BT_HCI_ERR_SUCCESS };
@@ -501,7 +495,6 @@ void test_phy_update_periph_loc(void)
501495

502496
/* There should be one host notification */
503497
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
504-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
505498
ut_rx_q_is_empty();
506499

507500
/* Release Ntf */
@@ -523,10 +516,6 @@ void test_phy_update_periph_rem(void)
523516
struct pdu_data_llctrl_phy_upd_ind ind = { .instant = 7,
524517
.c_to_p_phy = 0,
525518
.p_to_c_phy = PHY_2M };
526-
struct pdu_data_llctrl_length_rsp length_ntf = {
527-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M),
528-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M)
529-
};
530519
uint16_t instant;
531520

532521
struct node_rx_pu pu = { .status = BT_HCI_ERR_SUCCESS };
@@ -604,7 +593,6 @@ void test_phy_update_periph_rem(void)
604593

605594
/* There should be one host notification */
606595
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
607-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
608596
ut_rx_q_is_empty();
609597

610598
/* Release Ntf */
@@ -685,10 +673,6 @@ void test_phy_update_central_loc_collision(void)
685673
struct pdu_data_llctrl_phy_upd_ind ind = { .instant = 9,
686674
.c_to_p_phy = PHY_2M,
687675
.p_to_c_phy = PHY_2M };
688-
struct pdu_data_llctrl_length_rsp length_ntf = {
689-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
690-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M)
691-
};
692676
uint16_t instant;
693677

694678
struct pdu_data_llctrl_reject_ext_ind reject_ext_ind = {
@@ -837,7 +821,6 @@ void test_phy_update_central_loc_collision(void)
837821

838822
/* There should be one host notification */
839823
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
840-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
841824
ut_rx_q_is_empty();
842825

843826
/* Release Ntf */
@@ -863,14 +846,6 @@ void test_phy_update_central_rem_collision(void)
863846
struct pdu_data_llctrl_phy_upd_ind ind_2 = { .instant = 14,
864847
.c_to_p_phy = PHY_2M,
865848
.p_to_c_phy = 0 };
866-
struct pdu_data_llctrl_length_rsp length_ntf_1 = {
867-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
868-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M)
869-
};
870-
struct pdu_data_llctrl_length_rsp length_ntf_2 = {
871-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
872-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M)
873-
};
874849
uint16_t instant;
875850

876851
struct node_rx_pu pu = { .status = BT_HCI_ERR_SUCCESS };
@@ -958,7 +933,6 @@ void test_phy_update_central_rem_collision(void)
958933

959934
/* There should be one host notification */
960935
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
961-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf_1);
962936
ut_rx_q_is_empty();
963937

964938
/* Release Ntf */
@@ -1010,7 +984,6 @@ void test_phy_update_central_rem_collision(void)
1010984

1011985
/* There should be one host notification */
1012986
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
1013-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf_2);
1014987
ut_rx_q_is_empty();
1015988

1016989
/* Release Ntf */
@@ -1031,10 +1004,6 @@ void test_phy_update_periph_loc_collision(void)
10311004
struct pdu_data_llctrl_phy_upd_ind ind = { .instant = 7,
10321005
.c_to_p_phy = PHY_2M,
10331006
.p_to_c_phy = PHY_1M };
1034-
struct pdu_data_llctrl_length_rsp length_ntf = {
1035-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_2M),
1036-
3 * PDU_DC_PAYLOAD_SIZE_MIN, PDU_DC_MAX_US(3 * PDU_DC_PAYLOAD_SIZE_MIN, PHY_1M)
1037-
};
10381007
uint16_t instant;
10391008

10401009
struct pdu_data_llctrl_reject_ext_ind reject_ext_ind = {
@@ -1139,7 +1108,6 @@ void test_phy_update_periph_loc_collision(void)
11391108
/* There should be one host notification */
11401109
pu.status = BT_HCI_ERR_SUCCESS;
11411110
ut_rx_node(NODE_PHY_UPDATE, &ntf, &pu);
1142-
ut_rx_pdu(LL_LENGTH_RSP, &ntf, &length_ntf);
11431111
ut_rx_q_is_empty();
11441112

11451113
/* Release Ntf */

0 commit comments

Comments
 (0)