From 06078ee54edc79abfd09333114035036e14f50ca Mon Sep 17 00:00:00 2001 From: Erik Brockhoff Date: Wed, 6 Apr 2022 11:03:21 +0200 Subject: [PATCH] Bluetooth: controller: DO TODO, compile out members of struct re. CPR Compile out misc. members not used when Connection Parameter Request is not supported. Implement missing tests re. unsupported features in CU/CPR procedure Signed-off-by: Erik Brockhoff --- .../controller/ll_sw/ull_llcp_internal.h | 3 +- .../bluetooth/controller/ll_sw/ull_llcp_pdu.c | 2 + .../controller/ctrl_conn_update/src/main.c | 157 ++++++++++++------ 3 files changed, 107 insertions(+), 55 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h index 6f702fd4fd4..64d4561574e 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h @@ -190,7 +190,6 @@ struct proc_ctx { } pu; #endif /* CONFIG_BT_CTLR_PHY */ - /* TODO(tosk): leave out some params below if !CONFIG_BT_CTLR_CONN_PARAM_REQ */ /* Connection Update & Connection Parameter Request */ struct { uint8_t error; @@ -202,6 +201,7 @@ struct proc_ctx { uint16_t interval_max; uint16_t latency; uint16_t timeout; +#if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) uint8_t preferred_periodicity; uint16_t reference_conn_event_count; uint16_t offset0; @@ -210,6 +210,7 @@ struct proc_ctx { uint16_t offset3; uint16_t offset4; uint16_t offset5; +#endif /* defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) */ } cu; /* Use by ACL Termination Procedure */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c index 5ca9bed11ce..3fb09588646 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_pdu.c @@ -504,6 +504,7 @@ void llcp_pdu_decode_phy_rsp(struct proc_ctx *ctx, struct pdu_data *pdu) #endif /* CONFIG_BT_CENTRAL */ #endif /* CONFIG_BT_CTLR_PHY */ +#if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) /* * Connection Update Procedure Helper */ @@ -592,6 +593,7 @@ void llcp_pdu_decode_conn_param_rsp(struct proc_ctx *ctx, struct pdu_data *pdu) ctx->data.cu.offset4 = sys_le16_to_cpu(p->offset4); ctx->data.cu.offset5 = sys_le16_to_cpu(p->offset5); } +#endif /* defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) */ void llcp_pdu_encode_conn_update_ind(struct proc_ctx *ctx, struct pdu_data *pdu) { diff --git a/tests/bluetooth/controller/ctrl_conn_update/src/main.c b/tests/bluetooth/controller/ctrl_conn_update/src/main.c index c21879a6684..71ee12daa79 100644 --- a/tests/bluetooth/controller/ctrl_conn_update/src/main.c +++ b/tests/bluetooth/controller/ctrl_conn_update/src/main.c @@ -51,7 +51,6 @@ struct pdu_data_llctrl_conn_update_ind conn_update_ind = { .win_size = 1U, .timeout = TIMEOUT, .instant = 6U }; -#if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) /* Default conn_param_req PDU */ struct pdu_data_llctrl_conn_param_req conn_param_req = { .interval_min = INTVL_MIN, .interval_max = INTVL_MAX, @@ -66,6 +65,7 @@ struct pdu_data_llctrl_conn_param_req conn_param_req = { .interval_min = INTVL_M .offset4 = 0xffffU, .offset5 = 0xffffU }; +#if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) /* Default conn_param_rsp PDU */ struct pdu_data_llctrl_conn_param_rsp conn_param_rsp = { .interval_min = INTVL_MIN, .interval_max = INTVL_MAX, @@ -1276,28 +1276,6 @@ void test_conn_update_central_rem_reject(void) "Free CTX buffers %d", ctx_buffers_free()); } -/* Peripheral-initiated Connection Parameters Request procedure. - * Peripheral requests change in LE connection parameters, central’s Controller do not - * support Connection Parameters Request procedure. - * - * +-----+ +-------+ +-----+ - * | UT | | LL_C | | LT | - * +-----+ +-------+ +-----+ - * | | | - * | | LL_CONNECTION_PARAM_REQ | - * | |<--------------------------| - * | | | - * | | LL_UNKNOWN_RSP | - * | |-------------------------->| - * | | | - */ -void test_conn_update_central_rem_unsupp_feat(void) -{ - /* TODO(thoh): Implement when Remote Request machine has feature - * checking - */ -} - /* * (A) * Peripheral-initiated Connection Parameters Request procedure. @@ -2525,29 +2503,6 @@ void test_conn_update_periph_rem_reject(void) "Free CTX buffers %d", ctx_buffers_free()); } -/* - * Central-initiated Connection Parameters Request procedure. - * Central requests change in LE connection parameters, peripheral’s Controller do not - * support Connection Parameters Request procedure. - * - * +-----+ +-------+ +-----+ - * | UT | | LL_P | | LT | - * +-----+ +-------+ +-----+ - * | | | - * | | LL_CONNECTION_PARAM_REQ | - * | |<--------------------------| - * | | | - * | | LL_UNKNOWN_RSP | - * | |-------------------------->| - * | | | - */ -void test_conn_update_periph_rem_unsupp_feat(void) -{ - /* TODO(thoh): Implement when Remote Request machine has feature - * checking - */ -} - /* * (A) * Central-initiated Connection Parameters Request procedure. @@ -2890,7 +2845,7 @@ void test_conn_update_central_loc_accept_no_param_req(void) /* * Parameter Request Procedure not supported. - * Peripheral-initiated Connection Update procedure. + * Peripheral-initiated Connection Update/Connection Parameter Request procedure * Central receives Connection Update parameters. * * +-----+ +-------+ +-----+ @@ -2904,8 +2859,15 @@ void test_conn_update_central_loc_accept_no_param_req(void) * | |-------------------------->| * | | | * | | | + * | | LL_CONNECTION_PARAM_REQ | + * | |<--------------------------| + * | | | + * | | LL_UNKNOWN_RSP | + * | |-------------------------->| + * | | | + * | | | */ -void test_conn_update_central_rem_accept_no_param_req(void) +void test_conn_update_central_rem_unknown_no_param_req(void) { struct node_tx *tx; @@ -2943,6 +2905,93 @@ void test_conn_update_central_rem_accept_no_param_req(void) zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), "Free CTX buffers %d", ctx_buffers_free()); + + /* Check UNKNOWN_RSP on Connection Parameter Request */ + unknown_rsp.type = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ; + /* Prepare */ + event_prepare(&conn); + + /* Rx */ + lt_tx(LL_CONNECTION_PARAM_REQ, &conn, &conn_param_req); + + /* Done */ + event_done(&conn); + + /* Prepare */ + event_prepare(&conn); + + /* Tx Queue should have one LL Control PDU */ + lt_rx(LL_UNKNOWN_RSP, &conn, &tx, &unknown_rsp); + lt_rx_q_is_empty(&conn); + + /* Done */ + event_done(&conn); + + /* There should NOT be a host notification */ + ut_rx_q_is_empty(); + + zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), + "Free CTX buffers %d", ctx_buffers_free()); + +} + +/* + * Parameter Request Procedure not supported. + * Peripheral-initiated Connection Update/Connection Parameter Request procedure + * Central receives Connection Update parameters. + * + * +-----+ +-------+ +-----+ + * | UT | | LL_M | | LT | + * +-----+ +-------+ +-----+ + * | | | + * | | | + * | | LL_CONNECTION_PARAM_REQ | + * | |<--------------------------| + * | | | + * | | LL_UNKNOWN_RSP | + * | |-------------------------->| + * | | | + * | | | + */ +void test_conn_update_periph_rem_unknown_no_param_req(void) +{ + struct node_tx *tx; + + struct pdu_data_llctrl_unknown_rsp unknown_rsp = { + .type = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ + }; + + /* Role */ + test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL); + + /* Connect */ + ull_cp_state_set(&conn, ULL_CP_CONNECTED); + + /* Prepare */ + event_prepare(&conn); + + /* Rx */ + lt_tx(LL_CONNECTION_PARAM_REQ, &conn, &conn_param_req); + + /* Done */ + event_done(&conn); + + /* Prepare */ + event_prepare(&conn); + + /* Tx Queue should have one LL Control PDU */ + lt_rx(LL_UNKNOWN_RSP, &conn, &tx, &unknown_rsp); + lt_rx_q_is_empty(&conn); + + /* Done */ + event_done(&conn); + + /* There should NOT be a host notification */ + ut_rx_q_is_empty(); + + zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), + "Free CTX buffers %d", ctx_buffers_free()); + } /* @@ -3110,8 +3159,6 @@ void test_main(void) setup, unit_test_noop), ztest_unit_test_setup_teardown(test_conn_update_central_rem_reject, setup, unit_test_noop), - ztest_unit_test_setup_teardown(test_conn_update_central_rem_unsupp_feat, - setup, unit_test_noop), ztest_unit_test_setup_teardown(test_conn_update_central_rem_collision, setup, unit_test_noop)); @@ -3137,8 +3184,6 @@ void test_main(void) setup, unit_test_noop), ztest_unit_test_setup_teardown(test_conn_update_periph_rem_reject, setup, unit_test_noop), - ztest_unit_test_setup_teardown(test_conn_update_periph_rem_unsupp_feat, - setup, unit_test_noop), ztest_unit_test_setup_teardown(test_conn_update_periph_rem_collision, setup, unit_test_noop)); @@ -3154,7 +3199,7 @@ void test_main(void) setup, unit_test_noop)); ztest_test_suite(central_rem_no_param_req, ztest_unit_test_setup_teardown( - test_conn_update_central_rem_accept_no_param_req, + test_conn_update_central_rem_unknown_no_param_req, setup, unit_test_noop)); ztest_test_suite( @@ -3162,8 +3207,12 @@ void test_main(void) ztest_unit_test_setup_teardown(test_conn_update_periph_loc_disallowed_no_param_req, setup, unit_test_noop)); - ztest_test_suite(periph_rem_no_param_req, ztest_unit_test_setup_teardown( + ztest_test_suite(periph_rem_no_param_req, + ztest_unit_test_setup_teardown( test_conn_update_periph_rem_accept_no_param_req, + setup, unit_test_noop), + ztest_unit_test_setup_teardown( + test_conn_update_periph_rem_unknown_no_param_req, setup, unit_test_noop)); ztest_run_test_suite(central_loc_no_param_req);