From c99e0c738caf5b7e88c117ebb4515053cf2d54dc Mon Sep 17 00:00:00 2001 From: Ryan Chu Date: Fri, 21 Mar 2025 15:03:00 +0100 Subject: [PATCH 1/2] [nrf fromtree] bluetooth: host: Report status of Channel Sounding complete events If the HCI status of a complete event is not BT_HCI_ERR_SUCCESS, the remaining parameters could be invalid. In this case, the params is passed as NULL pointer to the callbacks. - LE CS Read Remote Supported Capabilities Complete event - LE CS Read Remote FAE Table Complete event - LE CS Config Complete event - LE CS Security Enable Complete event - LE CS Procedure Enable Complete event This change avoids forwarding the invalid fileds to the applications. Signed-off-by: Ryan Chu (cherry picked from commit c9240cc99f939a8b42cc62725e1c3037d848105e) --- include/zephyr/bluetooth/conn.h | 56 ++++-- subsys/bluetooth/host/conn.c | 53 +++--- subsys/bluetooth/host/conn_internal.h | 13 +- subsys/bluetooth/host/cs.c | 248 ++++++++++++++------------ tests/bluetooth/host/cs/mocks/conn.c | 12 +- tests/bluetooth/host/cs/mocks/conn.h | 11 +- 6 files changed, 223 insertions(+), 170 deletions(-) diff --git a/include/zephyr/bluetooth/conn.h b/include/zephyr/bluetooth/conn.h index da72b2b506d..dc088068645 100644 --- a/include/zephyr/bluetooth/conn.h +++ b/include/zephyr/bluetooth/conn.h @@ -1847,35 +1847,50 @@ struct bt_conn_cb { #if defined(CONFIG_BT_CHANNEL_SOUNDING) /** @brief LE CS Read Remote Supported Capabilities Complete event. * - * This callback notifies the application that the remote channel + * This callback notifies the application that a Channel Sounding + * Capabilities Exchange procedure has completed. + * + * If status is BT_HCI_ERR_SUCCESS, the remote channel * sounding capabilities have been received from the peer. * * @param conn Connection object. - * @param remote_cs_capabilities Remote Channel Sounding Capabilities. + * @param status HCI status of complete event. + * @param remote_cs_capabilities Pointer to CS Capabilities on success or NULL otherwise. */ - void (*le_cs_remote_capabilities_available)(struct bt_conn *conn, - struct bt_conn_le_cs_capabilities *params); + void (*le_cs_read_remote_capabilities_complete)(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_capabilities *params); /** @brief LE CS Read Remote FAE Table Complete event. * - * This callback notifies the application that the remote mode-0 + * This callback notifies the application that a Channel Sounding + * Mode-0 FAE Table Request procedure has completed. + * + * If status is BT_HCI_ERR_SUCCESS, the remote mode-0 * FAE Table has been received from the peer. * * @param conn Connection object. - * @param params FAE Table. + * @param status HCI status of complete event. + * @param params Pointer to FAE Table on success or NULL otherwise. */ - void (*le_cs_remote_fae_table_available)(struct bt_conn *conn, - struct bt_conn_le_cs_fae_table *params); + void (*le_cs_read_remote_fae_table_complete)(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_fae_table *params); /** @brief LE CS Config created. * * This callback notifies the application that a Channel Sounding - * Configuration procedure has completed and a new CS config is created + * Configuration procedure has completed. + * + * If status is BT_HCI_ERR_SUCCESS, a new CS config is created. * * @param conn Connection object. - * @param config CS configuration. + * @param status HCI status of complete event. + * @param config Pointer to CS configuration on success or NULL otherwise. */ - void (*le_cs_config_created)(struct bt_conn *conn, struct bt_conn_le_cs_config *config); + void (*le_cs_config_complete)(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_config *config); /** @brief LE CS Config removed. * @@ -1901,22 +1916,29 @@ struct bt_conn_cb { /** @brief LE CS Security Enabled. * * This callback notifies the application that a Channel Sounding - * Security Enable procedure has completed + * Security Enable procedure has completed. + * + * If status is BT_HCI_ERR_SUCCESS, CS Security is enabled. * * @param conn Connection object. + * @param status HCI status of complete event. */ - void (*le_cs_security_enabled)(struct bt_conn *conn); + void (*le_cs_security_enable_complete)(struct bt_conn *conn, uint8_t status); /** @brief LE CS Procedure Enabled. * * This callback notifies the application that a Channel Sounding - * Procedure Enable procedure has completed + * Procedure Enable procedure has completed. + * + * If status is BT_HCI_ERR_SUCCESS, CS procedure is enabled. * * @param conn Connection object. - * @param params CS Procedure Enable parameters + * @param status HCI status. + * @param params Pointer to CS Procedure Enable parameters on success or NULL otherwise. */ - void (*le_cs_procedure_enabled)( - struct bt_conn *conn, struct bt_conn_le_cs_procedure_enable_complete *params); + void (*le_cs_procedure_enable_complete)( + struct bt_conn *conn, uint8_t status, + struct bt_conn_le_cs_procedure_enable_complete *params); #endif diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 40d91c3e8d6..f10f1aceed8 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -3340,53 +3340,56 @@ int bt_conn_le_subrate_request(struct bt_conn *conn, #endif /* CONFIG_BT_SUBRATING */ #if defined(CONFIG_BT_CHANNEL_SOUNDING) -void notify_remote_cs_capabilities(struct bt_conn *conn, struct bt_conn_le_cs_capabilities params) +void notify_remote_cs_capabilities(struct bt_conn *conn, uint8_t status, + struct bt_conn_le_cs_capabilities *params) { struct bt_conn_cb *callback; SYS_SLIST_FOR_EACH_CONTAINER(&conn_cbs, callback, _node) { - if (callback->le_cs_remote_capabilities_available) { - callback->le_cs_remote_capabilities_available(conn, ¶ms); + if (callback->le_cs_read_remote_capabilities_complete) { + callback->le_cs_read_remote_capabilities_complete(conn, status, params); } } STRUCT_SECTION_FOREACH(bt_conn_cb, cb) { - if (cb->le_cs_remote_capabilities_available) { - cb->le_cs_remote_capabilities_available(conn, ¶ms); + if (cb->le_cs_read_remote_capabilities_complete) { + cb->le_cs_read_remote_capabilities_complete(conn, status, params); } } } -void notify_remote_cs_fae_table(struct bt_conn *conn, struct bt_conn_le_cs_fae_table params) +void notify_remote_cs_fae_table(struct bt_conn *conn, uint8_t status, + struct bt_conn_le_cs_fae_table *params) { struct bt_conn_cb *callback; SYS_SLIST_FOR_EACH_CONTAINER(&conn_cbs, callback, _node) { - if (callback->le_cs_remote_fae_table_available) { - callback->le_cs_remote_fae_table_available(conn, ¶ms); + if (callback->le_cs_read_remote_fae_table_complete) { + callback->le_cs_read_remote_fae_table_complete(conn, status, params); } } STRUCT_SECTION_FOREACH(bt_conn_cb, cb) { - if (cb->le_cs_remote_fae_table_available) { - cb->le_cs_remote_fae_table_available(conn, ¶ms); + if (cb->le_cs_read_remote_fae_table_complete) { + cb->le_cs_read_remote_fae_table_complete(conn, status, params); } } } -void notify_cs_config_created(struct bt_conn *conn, struct bt_conn_le_cs_config *params) +void notify_cs_config_created(struct bt_conn *conn, uint8_t status, + struct bt_conn_le_cs_config *params) { struct bt_conn_cb *callback; SYS_SLIST_FOR_EACH_CONTAINER(&conn_cbs, callback, _node) { - if (callback->le_cs_config_created) { - callback->le_cs_config_created(conn, params); + if (callback->le_cs_config_complete) { + callback->le_cs_config_complete(conn, status, params); } } STRUCT_SECTION_FOREACH(bt_conn_cb, cb) { - if (cb->le_cs_config_created) { - cb->le_cs_config_created(conn, params); + if (cb->le_cs_config_complete) { + cb->le_cs_config_complete(conn, status, params); } } } @@ -3408,37 +3411,37 @@ void notify_cs_config_removed(struct bt_conn *conn, uint8_t config_id) } } -void notify_cs_security_enable_available(struct bt_conn *conn) +void notify_cs_security_enable_available(struct bt_conn *conn, uint8_t status) { struct bt_conn_cb *callback; SYS_SLIST_FOR_EACH_CONTAINER(&conn_cbs, callback, _node) { - if (callback->le_cs_security_enabled) { - callback->le_cs_security_enabled(conn); + if (callback->le_cs_security_enable_complete) { + callback->le_cs_security_enable_complete(conn, status); } } STRUCT_SECTION_FOREACH(bt_conn_cb, cb) { - if (cb->le_cs_security_enabled) { - cb->le_cs_security_enabled(conn); + if (cb->le_cs_security_enable_complete) { + cb->le_cs_security_enable_complete(conn, status); } } } -void notify_cs_procedure_enable_available(struct bt_conn *conn, +void notify_cs_procedure_enable_available(struct bt_conn *conn, uint8_t status, struct bt_conn_le_cs_procedure_enable_complete *params) { struct bt_conn_cb *callback; SYS_SLIST_FOR_EACH_CONTAINER(&conn_cbs, callback, _node) { - if (callback->le_cs_procedure_enabled) { - callback->le_cs_procedure_enabled(conn, params); + if (callback->le_cs_procedure_enable_complete) { + callback->le_cs_procedure_enable_complete(conn, status, params); } } STRUCT_SECTION_FOREACH(bt_conn_cb, cb) { - if (cb->le_cs_procedure_enabled) { - cb->le_cs_procedure_enabled(conn, params); + if (cb->le_cs_procedure_enable_complete) { + cb->le_cs_procedure_enable_complete(conn, status, params); } } } diff --git a/subsys/bluetooth/host/conn_internal.h b/subsys/bluetooth/host/conn_internal.h index cd4eb2b8045..12ab95a8ac4 100644 --- a/subsys/bluetooth/host/conn_internal.h +++ b/subsys/bluetooth/host/conn_internal.h @@ -498,20 +498,25 @@ void notify_subrate_change(struct bt_conn *conn, struct bt_conn_le_subrate_changed params); void notify_remote_cs_capabilities(struct bt_conn *conn, - struct bt_conn_le_cs_capabilities params); + uint8_t status, + struct bt_conn_le_cs_capabilities *params); void notify_remote_cs_fae_table(struct bt_conn *conn, - struct bt_conn_le_cs_fae_table params); + uint8_t status, + struct bt_conn_le_cs_fae_table *params); -void notify_cs_config_created(struct bt_conn *conn, struct bt_conn_le_cs_config *params); +void notify_cs_config_created(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_config *params); void notify_cs_config_removed(struct bt_conn *conn, uint8_t config_id); void notify_cs_subevent_result(struct bt_conn *conn, struct bt_conn_le_cs_subevent_result *result); -void notify_cs_security_enable_available(struct bt_conn *conn); +void notify_cs_security_enable_available(struct bt_conn *conn, uint8_t status); void notify_cs_procedure_enable_available(struct bt_conn *conn, + uint8_t status, struct bt_conn_le_cs_procedure_enable_complete *params); #if defined(CONFIG_BT_SMP) diff --git a/subsys/bluetooth/host/cs.c b/subsys/bluetooth/host/cs.c index 8114659d822..3a2e588bdd0 100644 --- a/subsys/bluetooth/host/cs.c +++ b/subsys/bluetooth/host/cs.c @@ -310,7 +310,6 @@ void bt_hci_le_cs_read_remote_supported_capabilities_complete(struct net_buf *bu evt = net_buf_pull_mem(buf, sizeof(*evt)); if (evt->status) { LOG_WRN("Read Remote Supported Capabilities failed (status 0x%02X)", evt->status); - return; } conn = bt_conn_lookup_handle(sys_le16_to_cpu(evt->conn_handle), BT_CONN_TYPE_LE); @@ -319,95 +318,106 @@ void bt_hci_le_cs_read_remote_supported_capabilities_complete(struct net_buf *bu return; } - remote_cs_capabilities.num_config_supported = evt->num_config_supported; - remote_cs_capabilities.max_consecutive_procedures_supported = - sys_le16_to_cpu(evt->max_consecutive_procedures_supported); - remote_cs_capabilities.num_antennas_supported = evt->num_antennas_supported; - remote_cs_capabilities.max_antenna_paths_supported = evt->max_antenna_paths_supported; - - remote_cs_capabilities.initiator_supported = - evt->roles_supported & BT_HCI_LE_CS_INITIATOR_ROLE_MASK; - remote_cs_capabilities.reflector_supported = - evt->roles_supported & BT_HCI_LE_CS_REFLECTOR_ROLE_MASK; - remote_cs_capabilities.mode_3_supported = - evt->modes_supported & BT_HCI_LE_CS_MODES_SUPPORTED_MODE_3_MASK; - - remote_cs_capabilities.rtt_aa_only_n = evt->rtt_aa_only_n; - remote_cs_capabilities.rtt_sounding_n = evt->rtt_sounding_n; - remote_cs_capabilities.rtt_random_payload_n = evt->rtt_random_payload_n; - - if (evt->rtt_aa_only_n) { - if (evt->rtt_capability & BT_HCI_LE_CS_RTT_AA_ONLY_N_10NS_MASK) { - remote_cs_capabilities.rtt_aa_only_precision = - BT_CONN_LE_CS_RTT_AA_ONLY_10NS; + if (evt->status == BT_HCI_ERR_SUCCESS) { + remote_cs_capabilities.num_config_supported = evt->num_config_supported; + remote_cs_capabilities.max_consecutive_procedures_supported = + sys_le16_to_cpu(evt->max_consecutive_procedures_supported); + remote_cs_capabilities.num_antennas_supported = evt->num_antennas_supported; + remote_cs_capabilities.max_antenna_paths_supported = + evt->max_antenna_paths_supported; + + remote_cs_capabilities.initiator_supported = + evt->roles_supported & BT_HCI_LE_CS_INITIATOR_ROLE_MASK; + remote_cs_capabilities.reflector_supported = + evt->roles_supported & BT_HCI_LE_CS_REFLECTOR_ROLE_MASK; + remote_cs_capabilities.mode_3_supported = + evt->modes_supported & BT_HCI_LE_CS_MODES_SUPPORTED_MODE_3_MASK; + + remote_cs_capabilities.rtt_aa_only_n = evt->rtt_aa_only_n; + remote_cs_capabilities.rtt_sounding_n = evt->rtt_sounding_n; + remote_cs_capabilities.rtt_random_payload_n = evt->rtt_random_payload_n; + + if (evt->rtt_aa_only_n) { + if (evt->rtt_capability & BT_HCI_LE_CS_RTT_AA_ONLY_N_10NS_MASK) { + remote_cs_capabilities.rtt_aa_only_precision = + BT_CONN_LE_CS_RTT_AA_ONLY_10NS; + } else { + remote_cs_capabilities.rtt_aa_only_precision = + BT_CONN_LE_CS_RTT_AA_ONLY_150NS; + } } else { remote_cs_capabilities.rtt_aa_only_precision = - BT_CONN_LE_CS_RTT_AA_ONLY_150NS; + BT_CONN_LE_CS_RTT_AA_ONLY_NOT_SUPP; } - } else { - remote_cs_capabilities.rtt_aa_only_precision = BT_CONN_LE_CS_RTT_AA_ONLY_NOT_SUPP; - } - if (evt->rtt_sounding_n) { - if (evt->rtt_capability & BT_HCI_LE_CS_RTT_SOUNDING_N_10NS_MASK) { - remote_cs_capabilities.rtt_sounding_precision = - BT_CONN_LE_CS_RTT_SOUNDING_10NS; + if (evt->rtt_sounding_n) { + if (evt->rtt_capability & BT_HCI_LE_CS_RTT_SOUNDING_N_10NS_MASK) { + remote_cs_capabilities.rtt_sounding_precision = + BT_CONN_LE_CS_RTT_SOUNDING_10NS; + } else { + remote_cs_capabilities.rtt_sounding_precision = + BT_CONN_LE_CS_RTT_SOUNDING_150NS; + } } else { remote_cs_capabilities.rtt_sounding_precision = - BT_CONN_LE_CS_RTT_SOUNDING_150NS; + BT_CONN_LE_CS_RTT_SOUNDING_NOT_SUPP; } - } else { - remote_cs_capabilities.rtt_sounding_precision = BT_CONN_LE_CS_RTT_SOUNDING_NOT_SUPP; - } - if (evt->rtt_random_payload_n) { - if (evt->rtt_capability & BT_HCI_LE_CS_RTT_RANDOM_PAYLOAD_N_10NS_MASK) { - remote_cs_capabilities.rtt_random_payload_precision = - BT_CONN_LE_CS_RTT_RANDOM_PAYLOAD_10NS; + if (evt->rtt_random_payload_n) { + if (evt->rtt_capability & BT_HCI_LE_CS_RTT_RANDOM_PAYLOAD_N_10NS_MASK) { + remote_cs_capabilities.rtt_random_payload_precision = + BT_CONN_LE_CS_RTT_RANDOM_PAYLOAD_10NS; + } else { + remote_cs_capabilities.rtt_random_payload_precision = + BT_CONN_LE_CS_RTT_RANDOM_PAYLOAD_150NS; + } } else { remote_cs_capabilities.rtt_random_payload_precision = - BT_CONN_LE_CS_RTT_RANDOM_PAYLOAD_150NS; + BT_CONN_LE_CS_RTT_RANDOM_PAYLOAD_NOT_SUPP; } - } else { - remote_cs_capabilities.rtt_random_payload_precision = - BT_CONN_LE_CS_RTT_RANDOM_PAYLOAD_NOT_SUPP; - } - remote_cs_capabilities.phase_based_nadm_sounding_supported = - sys_le16_to_cpu(evt->nadm_sounding_capability) & - BT_HCI_LE_CS_NADM_SOUNDING_CAPABILITY_PHASE_BASED_MASK; + remote_cs_capabilities.phase_based_nadm_sounding_supported = + sys_le16_to_cpu(evt->nadm_sounding_capability) & + BT_HCI_LE_CS_NADM_SOUNDING_CAPABILITY_PHASE_BASED_MASK; - remote_cs_capabilities.phase_based_nadm_random_supported = - sys_le16_to_cpu(evt->nadm_random_capability) & - BT_HCI_LE_CS_NADM_RANDOM_CAPABILITY_PHASE_BASED_MASK; + remote_cs_capabilities.phase_based_nadm_random_supported = + sys_le16_to_cpu(evt->nadm_random_capability) & + BT_HCI_LE_CS_NADM_RANDOM_CAPABILITY_PHASE_BASED_MASK; - remote_cs_capabilities.cs_sync_2m_phy_supported = - evt->cs_sync_phys_supported & BT_HCI_LE_CS_SYNC_PHYS_2M_MASK; + remote_cs_capabilities.cs_sync_2m_phy_supported = + evt->cs_sync_phys_supported & BT_HCI_LE_CS_SYNC_PHYS_2M_MASK; - remote_cs_capabilities.cs_sync_2m_2bt_phy_supported = - evt->cs_sync_phys_supported & BT_HCI_LE_CS_SYNC_PHYS_2M_2BT_MASK; + remote_cs_capabilities.cs_sync_2m_2bt_phy_supported = + evt->cs_sync_phys_supported & BT_HCI_LE_CS_SYNC_PHYS_2M_2BT_MASK; - remote_cs_capabilities.cs_without_fae_supported = - sys_le16_to_cpu(evt->subfeatures_supported) & - BT_HCI_LE_CS_SUBFEATURE_NO_TX_FAE_MASK; + remote_cs_capabilities.cs_without_fae_supported = + sys_le16_to_cpu(evt->subfeatures_supported) & + BT_HCI_LE_CS_SUBFEATURE_NO_TX_FAE_MASK; - remote_cs_capabilities.chsel_alg_3c_supported = - sys_le16_to_cpu(evt->subfeatures_supported) & - BT_HCI_LE_CS_SUBFEATURE_CHSEL_ALG_3C_MASK; + remote_cs_capabilities.chsel_alg_3c_supported = + sys_le16_to_cpu(evt->subfeatures_supported) & + BT_HCI_LE_CS_SUBFEATURE_CHSEL_ALG_3C_MASK; - remote_cs_capabilities.pbr_from_rtt_sounding_seq_supported = - sys_le16_to_cpu(evt->subfeatures_supported) & - BT_HCI_LE_CS_SUBFEATURE_PBR_FROM_RTT_SOUNDING_SEQ_MASK; + remote_cs_capabilities.pbr_from_rtt_sounding_seq_supported = + sys_le16_to_cpu(evt->subfeatures_supported) & + BT_HCI_LE_CS_SUBFEATURE_PBR_FROM_RTT_SOUNDING_SEQ_MASK; - remote_cs_capabilities.t_ip1_times_supported = sys_le16_to_cpu(evt->t_ip1_times_supported); - remote_cs_capabilities.t_ip2_times_supported = sys_le16_to_cpu(evt->t_ip2_times_supported); - remote_cs_capabilities.t_fcs_times_supported = sys_le16_to_cpu(evt->t_fcs_times_supported); - remote_cs_capabilities.t_pm_times_supported = sys_le16_to_cpu(evt->t_pm_times_supported); + remote_cs_capabilities.t_ip1_times_supported = + sys_le16_to_cpu(evt->t_ip1_times_supported); + remote_cs_capabilities.t_ip2_times_supported = + sys_le16_to_cpu(evt->t_ip2_times_supported); + remote_cs_capabilities.t_fcs_times_supported = + sys_le16_to_cpu(evt->t_fcs_times_supported); + remote_cs_capabilities.t_pm_times_supported = + sys_le16_to_cpu(evt->t_pm_times_supported); - remote_cs_capabilities.t_sw_time = evt->t_sw_time_supported; - remote_cs_capabilities.tx_snr_capability = evt->tx_snr_capability; + remote_cs_capabilities.t_sw_time = evt->t_sw_time_supported; + remote_cs_capabilities.tx_snr_capability = evt->tx_snr_capability; - notify_remote_cs_capabilities(conn, remote_cs_capabilities); + notify_remote_cs_capabilities(conn, BT_HCI_ERR_SUCCESS, &remote_cs_capabilities); + } else { + notify_remote_cs_capabilities(conn, evt->status, NULL); + } bt_conn_unref(conn); } @@ -470,7 +480,6 @@ void bt_hci_le_cs_read_remote_fae_table_complete(struct net_buf *buf) evt = net_buf_pull_mem(buf, sizeof(*evt)); if (evt->status) { LOG_WRN("Read Remote FAE Table failed with status 0x%02X", evt->status); - return; } conn = bt_conn_lookup_handle(sys_le16_to_cpu(evt->conn_handle), BT_CONN_TYPE_LE); @@ -479,8 +488,13 @@ void bt_hci_le_cs_read_remote_fae_table_complete(struct net_buf *buf) return; } - fae_table.remote_fae_table = evt->remote_fae_table; - notify_remote_cs_fae_table(conn, fae_table); + if (evt->status == BT_HCI_ERR_SUCCESS) { + fae_table.remote_fae_table = evt->remote_fae_table; + + notify_remote_cs_fae_table(conn, BT_HCI_ERR_SUCCESS, &fae_table); + } else { + notify_remote_cs_fae_table(conn, evt->status, NULL); + } bt_conn_unref(conn); } @@ -810,7 +824,6 @@ void bt_hci_le_cs_config_complete_event(struct net_buf *buf) evt = net_buf_pull_mem(buf, sizeof(*evt)); if (evt->status) { LOG_WRN("CS Config failed (status 0x%02X)", evt->status); - return; } conn = bt_conn_lookup_handle(sys_le16_to_cpu(evt->handle), BT_CONN_TYPE_LE); @@ -819,33 +832,38 @@ void bt_hci_le_cs_config_complete_event(struct net_buf *buf) return; } - if (evt->action == BT_HCI_LE_CS_CONFIG_ACTION_REMOVED) { - notify_cs_config_removed(conn, evt->config_id); - bt_conn_unref(conn); - return; + if (evt->status == BT_HCI_ERR_SUCCESS) { + if (evt->action == BT_HCI_LE_CS_CONFIG_ACTION_REMOVED) { + notify_cs_config_removed(conn, evt->config_id); + bt_conn_unref(conn); + return; + } + + config.id = evt->config_id; + config.main_mode_type = evt->main_mode_type; + config.sub_mode_type = evt->sub_mode_type; + config.min_main_mode_steps = evt->min_main_mode_steps; + config.max_main_mode_steps = evt->max_main_mode_steps; + config.main_mode_repetition = evt->main_mode_repetition; + config.mode_0_steps = evt->mode_0_steps; + config.role = evt->role; + config.rtt_type = evt->rtt_type; + config.cs_sync_phy = evt->cs_sync_phy; + config.channel_map_repetition = evt->channel_map_repetition; + config.channel_selection_type = evt->channel_selection_type; + config.ch3c_shape = evt->ch3c_shape; + config.ch3c_jump = evt->ch3c_jump; + config.t_ip1_time_us = evt->t_ip1_time; + config.t_ip2_time_us = evt->t_ip2_time; + config.t_fcs_time_us = evt->t_fcs_time; + config.t_pm_time_us = evt->t_pm_time; + memcpy(config.channel_map, evt->channel_map, ARRAY_SIZE(config.channel_map)); + + notify_cs_config_created(conn, BT_HCI_ERR_SUCCESS, &config); + } else { + notify_cs_config_created(conn, evt->status, NULL); } - config.id = evt->config_id; - config.main_mode_type = evt->main_mode_type; - config.sub_mode_type = evt->sub_mode_type; - config.min_main_mode_steps = evt->min_main_mode_steps; - config.max_main_mode_steps = evt->max_main_mode_steps; - config.main_mode_repetition = evt->main_mode_repetition; - config.mode_0_steps = evt->mode_0_steps; - config.role = evt->role; - config.rtt_type = evt->rtt_type; - config.cs_sync_phy = evt->cs_sync_phy; - config.channel_map_repetition = evt->channel_map_repetition; - config.channel_selection_type = evt->channel_selection_type; - config.ch3c_shape = evt->ch3c_shape; - config.ch3c_jump = evt->ch3c_jump; - config.t_ip1_time_us = evt->t_ip1_time; - config.t_ip2_time_us = evt->t_ip2_time; - config.t_fcs_time_us = evt->t_fcs_time; - config.t_pm_time_us = evt->t_pm_time; - memcpy(config.channel_map, evt->channel_map, ARRAY_SIZE(config.channel_map)); - - notify_cs_config_created(conn, &config); bt_conn_unref(conn); } @@ -1208,7 +1226,6 @@ void bt_hci_le_cs_security_enable_complete(struct net_buf *buf) evt = net_buf_pull_mem(buf, sizeof(*evt)); if (evt->status) { LOG_WRN("Security Enable failed with status 0x%02X", evt->status); - return; } conn = bt_conn_lookup_handle(sys_le16_to_cpu(evt->handle), BT_CONN_TYPE_LE); @@ -1217,7 +1234,7 @@ void bt_hci_le_cs_security_enable_complete(struct net_buf *buf) return; } - notify_cs_security_enable_available(conn); + notify_cs_security_enable_available(conn, evt->status); bt_conn_unref(conn); } @@ -1237,7 +1254,6 @@ void bt_hci_le_cs_procedure_enable_complete(struct net_buf *buf) evt = net_buf_pull_mem(buf, sizeof(*evt)); if (evt->status) { LOG_WRN("Procedure Enable failed with status 0x%02X", evt->status); - return; } conn = bt_conn_lookup_handle(sys_le16_to_cpu(evt->handle), BT_CONN_TYPE_LE); @@ -1255,19 +1271,23 @@ void bt_hci_le_cs_procedure_enable_complete(struct net_buf *buf) } } - params.config_id = evt->config_id; - params.state = evt->state; - params.tone_antenna_config_selection = evt->tone_antenna_config_selection; - params.selected_tx_power = evt->selected_tx_power; - params.subevent_len = sys_get_le24(evt->subevent_len); - params.subevents_per_event = evt->subevents_per_event; - params.subevent_interval = sys_le16_to_cpu(evt->subevent_interval); - params.event_interval = sys_le16_to_cpu(evt->event_interval); - params.procedure_interval = sys_le16_to_cpu(evt->procedure_interval); - params.procedure_count = sys_le16_to_cpu(evt->procedure_count); - params.max_procedure_len = sys_le16_to_cpu(evt->max_procedure_len); - - notify_cs_procedure_enable_available(conn, ¶ms); + if (evt->status == BT_HCI_ERR_SUCCESS) { + params.config_id = evt->config_id; + params.state = evt->state; + params.tone_antenna_config_selection = evt->tone_antenna_config_selection; + params.selected_tx_power = evt->selected_tx_power; + params.subevent_len = sys_get_le24(evt->subevent_len); + params.subevents_per_event = evt->subevents_per_event; + params.subevent_interval = sys_le16_to_cpu(evt->subevent_interval); + params.event_interval = sys_le16_to_cpu(evt->event_interval); + params.procedure_interval = sys_le16_to_cpu(evt->procedure_interval); + params.procedure_count = sys_le16_to_cpu(evt->procedure_count); + params.max_procedure_len = sys_le16_to_cpu(evt->max_procedure_len); + + notify_cs_procedure_enable_available(conn, BT_HCI_ERR_SUCCESS, ¶ms); + } else { + notify_cs_procedure_enable_available(conn, evt->status, NULL); + } bt_conn_unref(conn); } diff --git a/tests/bluetooth/host/cs/mocks/conn.c b/tests/bluetooth/host/cs/mocks/conn.c index 910a06e3ce3..c0bfd06944d 100644 --- a/tests/bluetooth/host/cs/mocks/conn.c +++ b/tests/bluetooth/host/cs/mocks/conn.c @@ -11,12 +11,14 @@ DEFINE_FAKE_VOID_FUNC(bt_conn_unref, struct bt_conn *); DEFINE_FAKE_VALUE_FUNC(struct bt_conn *, bt_conn_lookup_handle, uint16_t, enum bt_conn_type); DEFINE_FAKE_VOID_FUNC(notify_remote_cs_capabilities, struct bt_conn *, - struct bt_conn_le_cs_capabilities); -DEFINE_FAKE_VOID_FUNC(notify_remote_cs_fae_table, struct bt_conn *, struct bt_conn_le_cs_fae_table); -DEFINE_FAKE_VOID_FUNC(notify_cs_config_created, struct bt_conn *, struct bt_conn_le_cs_config *); + uint8_t, struct bt_conn_le_cs_capabilities *); +DEFINE_FAKE_VOID_FUNC(notify_remote_cs_fae_table, struct bt_conn *, + uint8_t, struct bt_conn_le_cs_fae_table *); +DEFINE_FAKE_VOID_FUNC(notify_cs_config_created, struct bt_conn *, + uint8_t, struct bt_conn_le_cs_config *); DEFINE_FAKE_VOID_FUNC(notify_cs_config_removed, struct bt_conn *, uint8_t); DEFINE_FAKE_VOID_FUNC(notify_cs_subevent_result, struct bt_conn *, struct bt_conn_le_cs_subevent_result *); -DEFINE_FAKE_VOID_FUNC(notify_cs_security_enable_available, struct bt_conn *); +DEFINE_FAKE_VOID_FUNC(notify_cs_security_enable_available, struct bt_conn *, uint8_t); DEFINE_FAKE_VOID_FUNC(notify_cs_procedure_enable_available, struct bt_conn *, - struct bt_conn_le_cs_procedure_enable_complete *); + uint8_t, struct bt_conn_le_cs_procedure_enable_complete *); diff --git a/tests/bluetooth/host/cs/mocks/conn.h b/tests/bluetooth/host/cs/mocks/conn.h index 70aa9bdfd26..10d8bccd603 100644 --- a/tests/bluetooth/host/cs/mocks/conn.h +++ b/tests/bluetooth/host/cs/mocks/conn.h @@ -25,13 +25,14 @@ DECLARE_FAKE_VOID_FUNC(bt_conn_unref, struct bt_conn *); DECLARE_FAKE_VALUE_FUNC(struct bt_conn *, bt_conn_lookup_handle, uint16_t, enum bt_conn_type); DECLARE_FAKE_VOID_FUNC(notify_remote_cs_capabilities, struct bt_conn *, - struct bt_conn_le_cs_capabilities); + uint8_t, struct bt_conn_le_cs_capabilities *); DECLARE_FAKE_VOID_FUNC(notify_remote_cs_fae_table, struct bt_conn *, - struct bt_conn_le_cs_fae_table); -DECLARE_FAKE_VOID_FUNC(notify_cs_config_created, struct bt_conn *, struct bt_conn_le_cs_config *); + uint8_t, struct bt_conn_le_cs_fae_table *); +DECLARE_FAKE_VOID_FUNC(notify_cs_config_created, struct bt_conn *, + uint8_t, struct bt_conn_le_cs_config *); DECLARE_FAKE_VOID_FUNC(notify_cs_config_removed, struct bt_conn *, uint8_t); DECLARE_FAKE_VOID_FUNC(notify_cs_subevent_result, struct bt_conn *, struct bt_conn_le_cs_subevent_result *); -DECLARE_FAKE_VOID_FUNC(notify_cs_security_enable_available, struct bt_conn *); +DECLARE_FAKE_VOID_FUNC(notify_cs_security_enable_available, struct bt_conn *, uint8_t); DECLARE_FAKE_VOID_FUNC(notify_cs_procedure_enable_available, struct bt_conn *, - struct bt_conn_le_cs_procedure_enable_complete *); + uint8_t, struct bt_conn_le_cs_procedure_enable_complete *); From 99581226440d460d34ce596894ad509997691437 Mon Sep 17 00:00:00 2001 From: Ryan Chu Date: Fri, 28 Mar 2025 11:51:27 +0100 Subject: [PATCH 2/2] [nrf fromtree] bluetooth: host: Apply callback changes of CS complete events The CS complete callbacks provide both status and params. In the case of errors, NULL pointer is passed to the params of callbacks. Signed-off-by: Ryan Chu (cherry picked from commit 9ba60d3eb742a0e6f22df32768cbf82f20beef67) --- .../src/connected_cs_initiator.c | 56 ++++++++++++------ .../src/connected_cs_reflector.c | 58 +++++++++++++------ subsys/bluetooth/host/shell/bt.c | 33 +++++++++-- 3 files changed, 106 insertions(+), 41 deletions(-) diff --git a/samples/bluetooth/channel_sounding/src/connected_cs_initiator.c b/samples/bluetooth/channel_sounding/src/connected_cs_initiator.c index 1a012665c75..5f5250bad2c 100644 --- a/samples/bluetooth/channel_sounding/src/connected_cs_initiator.c +++ b/samples/bluetooth/channel_sounding/src/connected_cs_initiator.c @@ -138,32 +138,54 @@ static void security_changed_cb(struct bt_conn *conn, bt_security_t level, enum k_sem_give(&sem_acl_encryption_enabled); } -static void remote_capabilities_cb(struct bt_conn *conn, struct bt_conn_le_cs_capabilities *params) +static void remote_capabilities_cb(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_capabilities *params) { ARG_UNUSED(params); - printk("CS capability exchange completed.\n"); - k_sem_give(&sem_remote_capabilities_obtained); + + if (status == BT_HCI_ERR_SUCCESS) { + printk("CS capability exchange completed.\n"); + k_sem_give(&sem_remote_capabilities_obtained); + } else { + printk("CS capability exchange failed. (HCI status 0x%02x)\n", status); + } } -static void config_created_cb(struct bt_conn *conn, struct bt_conn_le_cs_config *config) +static void config_create_cb(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_config *config) { - printk("CS config creation complete. ID: %d\n", config->id); - k_sem_give(&sem_config_created); + if (status == BT_HCI_ERR_SUCCESS) { + printk("CS config creation complete. ID: %d\n", config->id); + k_sem_give(&sem_config_created); + } else { + printk("CS config creation failed. (HCI status 0x%02x)\n", status); + } } -static void security_enabled_cb(struct bt_conn *conn) +static void security_enable_cb(struct bt_conn *conn, uint8_t status) { - printk("CS security enabled.\n"); - k_sem_give(&sem_cs_security_enabled); + if (status == BT_HCI_ERR_SUCCESS) { + printk("CS security enabled.\n"); + k_sem_give(&sem_cs_security_enabled); + } else { + printk("CS security enable failed. (HCI status 0x%02x)\n", status); + } } -static void procedure_enabled_cb(struct bt_conn *conn, +static void procedure_enable_cb(struct bt_conn *conn, + uint8_t status, struct bt_conn_le_cs_procedure_enable_complete *params) { - if (params->state == 1) { - printk("CS procedures enabled.\n"); + if (status == BT_HCI_ERR_SUCCESS) { + if (params->state == 1) { + printk("CS procedures enabled.\n"); + } else { + printk("CS procedures disabled.\n"); + } } else { - printk("CS procedures disabled.\n"); + printk("CS procedures enable failed. (HCI status 0x%02x)\n", status); } } @@ -223,10 +245,10 @@ BT_CONN_CB_DEFINE(conn_cb) = { .connected = connected_cb, .disconnected = disconnected_cb, .security_changed = security_changed_cb, - .le_cs_remote_capabilities_available = remote_capabilities_cb, - .le_cs_config_created = config_created_cb, - .le_cs_security_enabled = security_enabled_cb, - .le_cs_procedure_enabled = procedure_enabled_cb, + .le_cs_read_remote_capabilities_complete = remote_capabilities_cb, + .le_cs_config_complete = config_create_cb, + .le_cs_security_enable_complete = security_enable_cb, + .le_cs_procedure_enable_complete = procedure_enable_cb, .le_cs_subevent_data_available = subevent_result_cb, }; diff --git a/samples/bluetooth/channel_sounding/src/connected_cs_reflector.c b/samples/bluetooth/channel_sounding/src/connected_cs_reflector.c index 2563d604387..404ee1ad4e3 100644 --- a/samples/bluetooth/channel_sounding/src/connected_cs_reflector.c +++ b/samples/bluetooth/channel_sounding/src/connected_cs_reflector.c @@ -89,32 +89,54 @@ static void disconnected_cb(struct bt_conn *conn, uint8_t reason) connection = NULL; } -static void remote_capabilities_cb(struct bt_conn *conn, struct bt_conn_le_cs_capabilities *params) +static void remote_capabilities_cb(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_capabilities *params) { ARG_UNUSED(params); - printk("CS capability exchange completed.\n"); - k_sem_give(&sem_remote_capabilities_obtained); + + if (status == BT_HCI_ERR_SUCCESS) { + printk("CS capability exchange completed.\n"); + k_sem_give(&sem_remote_capabilities_obtained); + } else { + printk("CS capability exchange failed. (HCI status 0x%02x)\n", status); + } } -static void config_created_cb(struct bt_conn *conn, struct bt_conn_le_cs_config *config) +static void config_create_cb(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_config *config) { - printk("CS config creation complete. ID: %d\n", config->id); - k_sem_give(&sem_config_created); + if (status == BT_HCI_ERR_SUCCESS) { + printk("CS config creation complete. ID: %d\n", config->id); + k_sem_give(&sem_config_created); + } else { + printk("CS config creation failed. (HCI status 0x%02x)\n", status); + } } -static void security_enabled_cb(struct bt_conn *conn) +static void security_enable_cb(struct bt_conn *conn, uint8_t status) { - printk("CS security enabled.\n"); - k_sem_give(&sem_cs_security_enabled); + if (status == BT_HCI_ERR_SUCCESS) { + printk("CS security enabled.\n"); + k_sem_give(&sem_cs_security_enabled); + } else { + printk("CS security enable failed. (HCI status 0x%02x)\n", status); + } } -static void procedure_enabled_cb(struct bt_conn *conn, - struct bt_conn_le_cs_procedure_enable_complete *params) +static void procedure_enable_cb(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_procedure_enable_complete *params) { - if (params->state == 1) { - printk("CS procedures enabled.\n"); + if (status == BT_HCI_ERR_SUCCESS) { + if (params->state == 1) { + printk("CS procedures enabled.\n"); + } else { + printk("CS procedures disabled.\n"); + } } else { - printk("CS procedures disabled.\n"); + printk("CS procedures enable failed. (HCI status 0x%02x)\n", status); } } @@ -160,10 +182,10 @@ static void write_func(struct bt_conn *conn, uint8_t err, struct bt_gatt_write_p BT_CONN_CB_DEFINE(conn_cb) = { .connected = connected_cb, .disconnected = disconnected_cb, - .le_cs_remote_capabilities_available = remote_capabilities_cb, - .le_cs_config_created = config_created_cb, - .le_cs_security_enabled = security_enabled_cb, - .le_cs_procedure_enabled = procedure_enabled_cb, + .le_cs_read_remote_capabilities_complete = remote_capabilities_cb, + .le_cs_config_complete = config_create_cb, + .le_cs_security_enable_complete = security_enable_cb, + .le_cs_procedure_enable_complete = procedure_enable_cb, .le_cs_subevent_data_available = subevent_result_cb, }; diff --git a/subsys/bluetooth/host/shell/bt.c b/subsys/bluetooth/host/shell/bt.c index 7386e14f0d1..c35e7c226e0 100644 --- a/subsys/bluetooth/host/shell/bt.c +++ b/subsys/bluetooth/host/shell/bt.c @@ -988,8 +988,15 @@ void subrate_changed(struct bt_conn *conn, #endif #if defined(CONFIG_BT_CHANNEL_SOUNDING) -void print_remote_cs_capabilities(struct bt_conn *conn, struct bt_conn_le_cs_capabilities *params) +void print_remote_cs_capabilities(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_capabilities *params) { + if (status != BT_HCI_ERR_SUCCESS) { + bt_shell_print("Read Remote CS Capabilities failed (HCI status 0x%02x)", status); + return; + } + bt_shell_print( "Received remote channel sounding capabilities:\n" "- Num CS configurations: %d\n" @@ -1051,14 +1058,28 @@ void print_remote_cs_capabilities(struct bt_conn *conn, struct bt_conn_le_cs_cap params->tx_snr_capability); } -void print_remote_cs_fae_table(struct bt_conn *conn, struct bt_conn_le_cs_fae_table *params) +void print_remote_cs_fae_table(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_fae_table *params) { + if (status != BT_HCI_ERR_SUCCESS) { + bt_shell_print("Read Remote CS FAE Table failed (HCI status 0x%02x)", status); + return; + } + bt_shell_print("Received FAE Table: "); bt_shell_hexdump(params->remote_fae_table, 72); } -static void le_cs_config_created(struct bt_conn *conn, struct bt_conn_le_cs_config *config) +static void le_cs_config_created(struct bt_conn *conn, + uint8_t status, + struct bt_conn_le_cs_config *config) { + if (status != BT_HCI_ERR_SUCCESS) { + bt_shell_print("Create CS Config failed (HCI status 0x%02x)", status); + return; + } + const char *mode_str[5] = {"Unused", "1 (RTT)", "2 (PBR)", "3 (RTT + PBR)", "Invalid"}; const char *role_str[3] = {"Initiator", "Reflector", "Invalid"}; const char *rtt_type_str[8] = {"AA only", "32-bit sounding", "96-bit sounding", @@ -1146,9 +1167,9 @@ static struct bt_conn_cb conn_callbacks = { .subrate_changed = subrate_changed, #endif #if defined(CONFIG_BT_CHANNEL_SOUNDING) - .le_cs_remote_capabilities_available = print_remote_cs_capabilities, - .le_cs_remote_fae_table_available = print_remote_cs_fae_table, - .le_cs_config_created = le_cs_config_created, + .le_cs_read_remote_capabilities_complete = print_remote_cs_capabilities, + .le_cs_read_remote_fae_table_complete = print_remote_cs_fae_table, + .le_cs_config_complete = le_cs_config_created, .le_cs_config_removed = le_cs_config_removed, #endif };