diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp.c b/subsys/bluetooth/controller/ll_sw/ull_llcp.c index 20c216713d4..4fa91c5661a 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp.c @@ -622,7 +622,9 @@ uint8_t ull_cp_encryption_start(struct ll_conn *conn, const uint8_t rand[8], con { struct proc_ctx *ctx; - /* TODO(thoh): Proper checks for role, parameters etc. */ + if (conn->lll.role != BT_HCI_ROLE_CENTRAL) { + return BT_HCI_ERR_CMD_DISALLOWED; + } ctx = llcp_create_local_procedure(PROC_ENCRYPTION_START); if (!ctx) { @@ -646,7 +648,9 @@ uint8_t ull_cp_encryption_pause(struct ll_conn *conn, const uint8_t rand[8], con { struct proc_ctx *ctx; - /* TODO(thoh): Proper checks for role, parameters etc. */ + if (conn->lll.role != BT_HCI_ROLE_CENTRAL) { + return BT_HCI_ERR_CMD_DISALLOWED; + } ctx = llcp_create_local_procedure(PROC_ENCRYPTION_PAUSE); if (!ctx) { diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c index f1a68064add..aae07938542 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c @@ -933,6 +933,9 @@ static void rp_comm_send_rsp(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t case PROC_VERSION_EXCHANGE: /* The Link Layer shall only queue for transmission a maximum of one * LL_VERSION_IND PDU during a connection. + * If the Link Layer receives an LL_VERSION_IND PDU and has already sent an + * LL_VERSION_IND PDU then the Link Layer shall not send another + * LL_VERSION_IND PDU to the peer device. */ if (!conn->llcp.vex.sent) { if (ctx->pause || !llcp_tx_alloc_peek(conn, ctx)) { @@ -944,13 +947,12 @@ static void rp_comm_send_rsp(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t ctx->state = RP_COMMON_STATE_IDLE; } } else { - /* Protocol Error. - * + /* Invalid behaviour * A procedure already sent a LL_VERSION_IND and received a LL_VERSION_IND. + * For now we chose to ignore the 'out of order' PDU */ - /* TODO */ - LL_ASSERT(0); } + break; #if defined(CONFIG_BT_CTLR_MIN_USED_CHAN) && defined(CONFIG_BT_CENTRAL) case PROC_MIN_USED_CHANS: @@ -959,17 +961,18 @@ static void rp_comm_send_rsp(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t * The procedure has completed when the Link Layer acknowledgment of the * LL_MIN_USED_CHANNELS_IND PDU is sent or received. * In effect, for this procedure, this is equivalent to RX of PDU + * + * Also: + * If the Link Layer receives an LL_MIN_USED_CHANNELS_IND PDU, it should ensure + * that, whenever the Peripheral-to-Central PHY is one of those specified, + * the connection uses at least the number of channels given in the + * MinUsedChannels field of the PDU. + * + * The 'should' is here interpreted as 'permission' to do nothing + * + * Future improvement could implement logic to support this */ - /* Inititate a chmap update, but only if acting as central, just in case ... */ - if (conn->lll.role == BT_HCI_ROLE_CENTRAL && - ull_conn_lll_phy_active(conn, conn->llcp.muc.phys)) { - uint8_t chmap[5]; - ull_chan_map_get((uint8_t *const)chmap); - ull_cp_chan_map_update(conn, chmap); - /* TODO - what to do on failure of ull_cp_chan_map_update() */ - } - /* No response */ llcp_rr_complete(conn); ctx->state = RP_COMMON_STATE_IDLE; break; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c index 97ea073b9f6..e0dad483b70 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c @@ -336,7 +336,6 @@ static void lp_cu_st_wait_rx_conn_param_rsp(struct ll_conn *conn, struct proc_ct lp_cu_send_conn_update_ind(conn, ctx, evt, param); break; case LP_CU_EVT_REJECT: - /* TODO(tosk): Select between LL_REJECT_IND and LL_REJECT_EXT_IND */ if (pdu->llctrl.reject_ext_ind.error_code == BT_HCI_ERR_UNSUPP_REMOTE_FEATURE) { /* Remote legacy Host */ llcp_rr_set_incompat(conn, INCOMPAT_RESERVED); @@ -547,7 +546,6 @@ static void rp_cu_tx(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t opcode) break; #if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ) case PDU_DATA_LLCTRL_TYPE_REJECT_EXT_IND: - /* TODO(thoh): Select between LL_REJECT_IND and LL_REJECT_EXT_IND */ llcp_pdu_encode_reject_ext_ind(pdu, PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ, ctx->data.cu.error); break; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c index e44ac121302..08fd5d2c2de 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c @@ -73,37 +73,23 @@ enum { static bool proc_with_instant(struct proc_ctx *ctx) { - /* - * TODO: should we combine all the cases that return 0 - * and all the cases that return 1? - */ switch (ctx->proc) { case PROC_UNKNOWN: - return 0U; case PROC_FEATURE_EXCHANGE: - return 0U; case PROC_MIN_USED_CHANS: - return 0U; case PROC_LE_PING: - return 0U; case PROC_VERSION_EXCHANGE: - return 0U; case PROC_ENCRYPTION_START: case PROC_ENCRYPTION_PAUSE: - return 0U; - case PROC_PHY_UPDATE: - return 1U; - case PROC_CONN_UPDATE: - case PROC_CONN_PARAM_REQ: - return 1U; case PROC_TERMINATE: - return 0U; - case PROC_CHAN_MAP_UPDATE: - return 1U; case PROC_DATA_LENGTH_UPDATE: - return 0U; case PROC_CTE_REQ: return 0U; + case PROC_PHY_UPDATE: + case PROC_CONN_UPDATE: + case PROC_CONN_PARAM_REQ: + case PROC_CHAN_MAP_UPDATE: + return 1U; default: /* Unknown procedure */ LL_ASSERT(0); diff --git a/tests/bluetooth/controller/common/include/helper_features.h b/tests/bluetooth/controller/common/include/helper_features.h index 059f04c3741..60c23e187e7 100644 --- a/tests/bluetooth/controller/common/include/helper_features.h +++ b/tests/bluetooth/controller/common/include/helper_features.h @@ -237,13 +237,8 @@ /* * The following two are defined as per - * Core Spec V5.2 Volume 6, Part B, chapter 4.6 - * LL_FEAT_BIT_MASK_VALID does not account for the bits - * for the new features in V5.2 - * TODO: EXPECTED_FEAT_EXCH_VALID is not used at the moment - * but probably LL_FEAT_BIT_MASK_VALID should get this - * value in ll_feat.h + * Core Spec V5.3 Volume 6, Part B, chapter 4.6 */ -#define EXPECTED_FEAT_EXCH_VALID 0x0000000FF787CF2F +#define EXPECTED_FEAT_EXCH_VALID 0x000000EFF787CF2F #define FEAT_FILTER_OCTET0 0xFFFFFFFFFFFFFF00 #define COMMON_FEAT_OCTET0(x) (FEAT_FILTER_OCTET0 | ((x) & ~FEAT_FILTER_OCTET0)) diff --git a/tests/bluetooth/controller/ctrl_chmu/src/main.c b/tests/bluetooth/controller/ctrl_chmu/src/main.c index fee2c62fc4e..439bdc4c51c 100644 --- a/tests/bluetooth/controller/ctrl_chmu/src/main.c +++ b/tests/bluetooth/controller/ctrl_chmu/src/main.c @@ -53,8 +53,7 @@ static bool is_instant_reached(struct ll_conn *conn, uint16_t instant) void test_channel_map_update_central_loc(void) { uint8_t chm[5] = { 0x00, 0x04, 0x05, 0x06, 0x00 }; - /* TODO should test setup set this to valid value? */ - uint8_t defchm[5] = {}; + uint8_t initial_chm[5]; uint8_t err; struct node_tx *tx; struct pdu_data *pdu; @@ -64,6 +63,9 @@ void test_channel_map_update_central_loc(void) .chm = { 0x00, 0x04, 0x05, 0x06, 0x00 }, }; + /* Store initial channel map */ + memcpy(initial_chm, conn.lll.data_chan_map, sizeof(conn.lll.data_chan_map)); + /* Role */ test_set_role(&conn, BT_HCI_ROLE_CENTRAL); @@ -104,8 +106,9 @@ void test_channel_map_update_central_loc(void) /* There should NOT be a host notification */ ut_rx_q_is_empty(); - /* check if using old channel map */ - zassert_mem_equal(conn.lll.data_chan_map, defchm, sizeof(conn.lll.data_chan_map), + /* check if still using initial channel map */ + zassert_mem_equal(conn.lll.data_chan_map, initial_chm, + sizeof(conn.lll.data_chan_map), "Channel map invalid"); } @@ -132,14 +135,16 @@ void test_channel_map_update_central_loc(void) void test_channel_map_update_periph_rem(void) { uint8_t chm[5] = { 0x00, 0x04, 0x05, 0x06, 0x00 }; - /* TODO should test setup set this to valid value? */ - uint8_t defchm[5] = {}; + uint8_t initial_chm[5]; struct pdu_data_llctrl_chan_map_ind chmu_ind = { .instant = 6, .chm = { 0x00, 0x04, 0x05, 0x06, 0x00 }, }; uint16_t instant = 6; + /* Store initial channel map */ + memcpy(initial_chm, conn.lll.data_chan_map, sizeof(conn.lll.data_chan_map)); + /* Role */ test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL); @@ -173,7 +178,8 @@ void test_channel_map_update_periph_rem(void) ut_rx_q_is_empty(); /* check if using old channel map */ - zassert_mem_equal(conn.lll.data_chan_map, defchm, sizeof(conn.lll.data_chan_map), + zassert_mem_equal(conn.lll.data_chan_map, initial_chm, + sizeof(conn.lll.data_chan_map), "Channel map invalid"); } diff --git a/tests/bluetooth/controller/ctrl_min_used_chans/src/main.c b/tests/bluetooth/controller/ctrl_min_used_chans/src/main.c index df5907d4229..a9a615f074d 100644 --- a/tests/bluetooth/controller/ctrl_min_used_chans/src/main.c +++ b/tests/bluetooth/controller/ctrl_min_used_chans/src/main.c @@ -126,10 +126,6 @@ void test_min_used_chans_central_rem(void) { struct pdu_data_llctrl_min_used_chans_ind remote_muc_ind = { .phys = 1, .min_used_chans = 2 }; - struct pdu_data_llctrl_chan_map_ind ch_map_ind = { .chm = { 0xff, 0xff, 0xff, 0xff, 0x1f }, - .instant = 7 }; - - struct node_tx *tx; /* Role */ test_set_role(&conn, BT_HCI_ROLE_CENTRAL); @@ -143,17 +139,13 @@ void test_min_used_chans_central_rem(void) /* Rx */ lt_tx(LL_MIN_USED_CHANS_IND, &conn, &remote_muc_ind); - /* Emulate a phy to trigger channel map update */ - conn.lll.phy_tx = 0x7; - /* Done */ event_done(&conn); /* Prepare */ event_prepare(&conn); - /* Tx Queue should have one LL Control PDU */ - lt_rx(LL_CHAN_MAP_UPDATE_IND, &conn, &tx, &ch_map_ind); + /* Tx Queue should have no LL Control PDU */ lt_rx_q_is_empty(&conn); /* Done */ @@ -162,7 +154,7 @@ void test_min_used_chans_central_rem(void) /* There should not be a host notifications */ ut_rx_q_is_empty(); - zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt() - 1, + zassert_equal(ctx_buffers_free(), test_ctx_buffers_cnt(), "Free CTX buffers %d", ctx_buffers_free()); } diff --git a/tests/bluetooth/controller/mock_ctrl/src/util.c b/tests/bluetooth/controller/mock_ctrl/src/util.c index 9d05fe008cb..ad180ebb866 100644 --- a/tests/bluetooth/controller/mock_ctrl/src/util.c +++ b/tests/bluetooth/controller/mock_ctrl/src/util.c @@ -17,9 +17,6 @@ /** * @brief Population count: Count the number of bits set to 1 - * @details - * TODO: Faster methods available at [1]. - * [1] http://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel * * @param octets Data to count over * @param octets_len Must not be bigger than 255/8 = 31 bytes