From 862159340f54beec98ba2a89a42ed613a02c7537 Mon Sep 17 00:00:00 2001 From: Mariusz Skamra Date: Mon, 29 Apr 2024 10:52:21 +0200 Subject: [PATCH] nimble: iso: Move Rx callback param to Setup ISO Data Path command This moves the ISO Rx callback to be provided by application as Setup ISO Data Path function parameter, as the callback will be used only for HCI data path, thus there is no point to require the callback when the broadcast sync is created. --- apps/btshell/src/cmd_iso.c | 70 +++++++++++++++++++++++++----- nimble/host/include/host/ble_iso.h | 17 ++++---- nimble/host/src/ble_iso.c | 56 ++++++++++++++++-------- nimble/include/nimble/hci_common.h | 2 + 4 files changed, 108 insertions(+), 37 deletions(-) diff --git a/apps/btshell/src/cmd_iso.c b/apps/btshell/src/cmd_iso.c index 27a9bc9d4..3383c379f 100644 --- a/apps/btshell/src/cmd_iso.c +++ b/apps/btshell/src/cmd_iso.c @@ -17,6 +17,7 @@ * under the License. */ +#include "host/ble_hs.h" #include "host/ble_iso.h" #include "cmd_iso.h" @@ -26,7 +27,7 @@ #if (MYNEWT_VAL(BLE_ISO)) static struct iso_rx_stats { - uint8_t bis_index; + uint16_t conn_handle; bool ts_valid; uint32_t ts; uint16_t seq_num; @@ -34,12 +35,60 @@ static struct iso_rx_stats { uint64_t valid_cnt; uint64_t error_cnt; uint64_t lost_cnt; -} rx_stats_pool[MYNEWT_VAL(BLE_ISO_MAX_BISES)]; +} rx_stats_pool[MYNEWT_VAL(BLE_ISO_MAX_BISES)] = { + [0 ... MYNEWT_VAL(BLE_ISO_MAX_BISES) - 1] = { + .conn_handle = BLE_HS_CONN_HANDLE_NONE + } +}; + +static struct iso_rx_stats * +iso_rx_stats_lookup_conn_handle(uint16_t conn_handle) +{ + for (size_t i = 0; i < ARRAY_SIZE(rx_stats_pool); i++) { + if (rx_stats_pool[i].conn_handle == conn_handle) { + return &rx_stats_pool[i]; + } + } + + return NULL; +} + +static struct iso_rx_stats * +iso_rx_stats_get_or_new(uint16_t conn_handle) +{ + struct iso_rx_stats *rx_stats; + + rx_stats = iso_rx_stats_lookup_conn_handle(conn_handle); + if (rx_stats == NULL) { + rx_stats = iso_rx_stats_lookup_conn_handle(BLE_HS_CONN_HANDLE_NONE); + if (rx_stats == NULL) { + return NULL; + } + } + + rx_stats->conn_handle = conn_handle; + + return rx_stats; +} + +static void +iso_rx_stats_reset(void) +{ + for (size_t i = 0; i < ARRAY_SIZE(rx_stats_pool); i++) { + memset(&rx_stats_pool[i], 0, sizeof(rx_stats_pool[i])); + rx_stats_pool[i].conn_handle = BLE_HS_CONN_HANDLE_NONE; + } +} static void -iso_rx_stats_update(uint16_t conn_handle, const struct ble_iso_rx_data_info *info, - struct iso_rx_stats *stats) +iso_rx_stats_update(uint16_t conn_handle, const struct ble_iso_rx_data_info *info, void *arg) { + struct iso_rx_stats *stats = arg; + + if (!stats) { + return; + } + stats->ts_valid = info->ts_valid; if (stats->ts_valid) { stats->ts = info->ts; @@ -58,9 +107,9 @@ iso_rx_stats_update(uint16_t conn_handle, const struct ble_iso_rx_data_info *inf stats->total_cnt++; if ((stats->total_cnt % 100) == 0) { - console_printf("BIS=%d, seq_num=%d, num_rx=%lld, " + console_printf("conn_handle=0x%04x, seq_num=%d, num_rx=%lld, " "(valid=%lld, error=%lld, lost=%lld) ", - stats->bis_index, stats->seq_num, + stats->conn_handle, stats->seq_num, stats->total_cnt, stats->valid_cnt, stats->error_cnt, stats->lost_cnt); @@ -122,6 +171,7 @@ ble_iso_event_handler(struct ble_iso_event *event, void *arg) console_printf("BIG Sync Terminated handle=0x%02x reason: %u\n", event->big_terminated.big_handle, event->big_terminated.reason); + iso_rx_stats_reset(); break; case BLE_ISO_EVENT_ISO_RX: @@ -349,12 +399,6 @@ cmd_iso_big_sync_create(int argc, char **argv) for (uint8_t i = 0; i < params.bis_cnt; i++) { bis_params[i].bis_index = bis_idxs[i]; - bis_params[i].cb = ble_iso_event_handler; - bis_params[i].cb_arg = &rx_stats_pool[i]; - - /* Reset stats */ - memset(&rx_stats_pool[i], 0, sizeof(rx_stats_pool[i])); - rx_stats_pool[i].bis_index = bis_idxs[i]; } params.bis_params = bis_params; @@ -457,6 +501,8 @@ cmd_iso_data_path_setup(int argc, char **argv) } /* For now, the Data Path ID is set to HCI by default */ + params.cb = ble_iso_event_handler; + params.cb_arg = iso_rx_stats_get_or_new(params.conn_handle); rc = ble_iso_data_path_setup(¶ms); if (rc != 0) { diff --git a/nimble/host/include/host/ble_iso.h b/nimble/host/include/host/ble_iso.h index ee99b962b..ad1d0ed9a 100644 --- a/nimble/host/include/host/ble_iso.h +++ b/nimble/host/include/host/ble_iso.h @@ -333,14 +333,6 @@ int ble_iso_terminate_big(uint8_t big_handle); struct ble_iso_bis_params { /** BIS index */ uint8_t bis_index; - - /** The callback to associate with the BIS. - * Received ISO data is reported through this callback. - */ - ble_iso_event_fn *cb; - - /** The optional argument to pass to the callback function */ - void *cb_arg; }; /** @brief BIG Sync parameters for @ref ble_iso_big_sync_create */ @@ -443,6 +435,15 @@ struct ble_iso_data_path_setup_params { /** Codec Configuration */ const uint8_t *codec_config; + + /** + * The ISO Data callback. Must be set if @p data_path_id is HCI. + * Received ISO data is reported through this callback. + */ + ble_iso_event_fn *cb; + + /** The optional argument to pass to the callback function */ + void *cb_arg; }; /** diff --git a/nimble/host/src/ble_iso.c b/nimble/host/src/ble_iso.c index af728d135..7e78cc960 100644 --- a/nimble/host/src/ble_iso.c +++ b/nimble/host/src/ble_iso.c @@ -233,6 +233,20 @@ ble_iso_big_free(struct ble_iso_big *big) return 0; } +static struct ble_iso_conn * +ble_iso_conn_lookup_handle(uint16_t handle) +{ + struct ble_iso_conn *conn; + + SLIST_FOREACH(conn, &ble_iso_conns, next) { + if (conn->handle == handle) { + return conn; + } + } + + return NULL; +} + #if MYNEWT_VAL(BLE_ISO_BROADCAST_SOURCE) int ble_iso_create_big(const struct ble_iso_create_big_params *create_params, @@ -496,20 +510,6 @@ ble_iso_tx(uint16_t conn_handle, void *data, uint16_t data_len) #endif /* BLE_ISO_BROADCAST_SOURCE */ #if MYNEWT_VAL(BLE_ISO_BROADCAST_SINK) -static struct ble_iso_conn * -ble_iso_conn_lookup_handle(uint16_t handle) -{ - struct ble_iso_conn *conn; - - SLIST_FOREACH(conn, &ble_iso_conns, next) { - if (conn->handle == handle) { - return conn; - } - } - - return NULL; -} - int ble_iso_big_sync_create(const struct ble_iso_big_sync_create_params *param, uint8_t *big_handle) @@ -553,9 +553,6 @@ ble_iso_big_sync_create(const struct ble_iso_big_sync_create_params *param, return BLE_HS_ENOMEM; } - bis->conn.cb = param->bis_params[i].cb; - bis->conn.cb_arg = param->bis_params[i].cb_arg; - cp->bis[i] = param->bis_params[i].bis_index; } @@ -867,8 +864,15 @@ ble_iso_data_path_setup(const struct ble_iso_data_path_setup_params *param) struct ble_hci_le_setup_iso_data_path_rp rp; struct ble_hci_le_setup_iso_data_path_cp *cp; uint8_t buf[sizeof(*cp) + UINT8_MAX]; + struct ble_iso_conn *conn; int rc; + conn = ble_iso_conn_lookup_handle(param->conn_handle); + if (conn == NULL) { + BLE_HS_LOG_ERROR("invalid conn_handle\n"); + return BLE_HS_ENOTCONN; + } + if (param->codec_config_len > 0 && param->codec_config == NULL) { BLE_HS_LOG_ERROR("Missing codec_config\n"); return BLE_HS_EINVAL; @@ -886,6 +890,14 @@ ble_iso_data_path_setup(const struct ble_iso_data_path_setup_params *param) if (param->data_path_dir & BLE_ISO_DATA_DIR_RX) { /* Output (Controller to Host) */ cp->data_path_dir |= BLE_HCI_ISO_DATA_PATH_DIR_OUTPUT; + + if (cp->data_path_id == BLE_HCI_ISO_DATA_PATH_ID_HCI && param->cb == NULL) { + BLE_HS_LOG_ERROR("param->cb is NULL\n"); + return BLE_HS_EINVAL; + } + + conn->cb = param->cb; + conn->cb_arg = param->cb_arg; } cp->data_path_id = param->data_path_id; @@ -910,8 +922,15 @@ ble_iso_data_path_remove(const struct ble_iso_data_path_remove_params *param) { struct ble_hci_le_remove_iso_data_path_rp rp; struct ble_hci_le_remove_iso_data_path_cp cp = { 0 }; + struct ble_iso_conn *conn; int rc; + conn = ble_iso_conn_lookup_handle(param->conn_handle); + if (conn == NULL) { + BLE_HS_LOG_ERROR("invalid conn_handle\n"); + return BLE_HS_ENOTCONN; + } + put_le16(&cp.conn_handle, param->conn_handle); if (param->data_path_dir & BLE_ISO_DATA_DIR_TX) { @@ -922,6 +941,9 @@ ble_iso_data_path_remove(const struct ble_iso_data_path_remove_params *param) if (param->data_path_dir & BLE_ISO_DATA_DIR_RX) { /* Output (Controller to Host) */ cp.data_path_dir |= BLE_HCI_ISO_DATA_PATH_DIR_OUTPUT; + + conn->cb = NULL; + conn->cb_arg = NULL; } rc = ble_hs_hci_cmd_tx(BLE_HCI_OP(BLE_HCI_OGF_LE, diff --git a/nimble/include/nimble/hci_common.h b/nimble/include/nimble/hci_common.h index aadfa37b5..80c4f2b3f 100644 --- a/nimble/include/nimble/hci_common.h +++ b/nimble/include/nimble/hci_common.h @@ -2423,6 +2423,8 @@ struct hci_data_hdr #define BLE_HCI_ISO_DATA_PATH_DIR_INPUT 0x00 #define BLE_HCI_ISO_DATA_PATH_DIR_OUTPUT 0x01 +#define BLE_HCI_ISO_DATA_PATH_ID_HCI 0x00 + struct ble_hci_iso { uint16_t handle; uint16_t length;