diff --git a/components/bt/esp_ble_audio/api/include/esp_ble_audio_bap_lc3_preset_defs.h b/components/bt/esp_ble_audio/api/include/esp_ble_audio_bap_lc3_preset_defs.h index e2d922ce9d3..ee9446b497d 100644 --- a/components/bt/esp_ble_audio/api/include/esp_ble_audio_bap_lc3_preset_defs.h +++ b/components/bt/esp_ble_audio/api/include/esp_ble_audio_bap_lc3_preset_defs.h @@ -40,19 +40,26 @@ typedef struct bt_bap_lc3_preset esp_ble_audio_bap_lc3_preset_t; * @param _rtn Number of retransmissions. * @param _latency Maximum transport latency (in milliseconds). * @param _pd Presentation delay (in microseconds). + * + * Backing buffers are sized to `CONFIG_BT_AUDIO_CODEC_CFG_MAX_*_SIZE` so + * the preset is safe to mutate via `esp_ble_audio_codec_cfg_*_set_*`. */ #define ESP_BLE_AUDIO_BAP_LC3_PRESET_DEFINE(_name, _freq, _duration, _loc, _len, \ _frames_per_sdu, _stream_context, \ _interval, _sdu, _rtn, _latency, _pd) \ - static uint8_t codec_cfg_data_##_name[] = \ + static uint8_t codec_cfg_data_##_name[CONFIG_BT_AUDIO_CODEC_CFG_MAX_DATA_SIZE] = \ ESP_BLE_AUDIO_CODEC_CFG_LC3_DATA(_freq, _duration, _loc, _len, _frames_per_sdu); \ - static uint8_t codec_cfg_meta_##_name[] = \ + static uint8_t codec_cfg_meta_##_name[CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE] = \ ESP_BLE_AUDIO_CODEC_CFG_LC3_META(_stream_context); \ static esp_ble_audio_bap_lc3_preset_t _name = \ - ESP_BLE_AUDIO_BAP_LC3_PRESET(ESP_BLE_AUDIO_CODEC_CFG_LC3(codec_cfg_data_##_name, \ - codec_cfg_meta_##_name), \ - ESP_BLE_AUDIO_BAP_QOS_CFG_UNFRAMED(_interval, _sdu, _rtn, \ - _latency, _pd)); + ESP_BLE_AUDIO_BAP_LC3_PRESET( \ + ESP_BLE_AUDIO_CODEC_CFG_LC3_LEN( \ + codec_cfg_data_##_name, \ + sizeof((uint8_t[])ESP_BLE_AUDIO_CODEC_CFG_LC3_DATA(_freq, _duration, _loc, \ + _len, _frames_per_sdu)), \ + codec_cfg_meta_##_name, \ + sizeof((uint8_t[])ESP_BLE_AUDIO_CODEC_CFG_LC3_META(_stream_context))), \ + ESP_BLE_AUDIO_BAP_QOS_CFG_UNFRAMED(_interval, _sdu, _rtn, _latency, _pd)); /** * @brief Helper to declare LC3 Unicast 8_1_1 codec configuration. diff --git a/components/bt/esp_ble_audio/api/include/esp_ble_audio_codec_api.h b/components/bt/esp_ble_audio/api/include/esp_ble_audio_codec_api.h index 12545623c5a..8f0fb2604d1 100644 --- a/components/bt/esp_ble_audio/api/include/esp_ble_audio_codec_api.h +++ b/components/bt/esp_ble_audio/api/include/esp_ble_audio_codec_api.h @@ -77,6 +77,35 @@ typedef enum bt_audio_codec_cfg_target_phy esp_ble_audio_codec_cfg_ .meta = (_meta), \ }) +/** + * @brief Variant of ESP_BLE_AUDIO_CODEC_CFG with explicit LVT content lengths. + * + * @param _id Codec ID. + * @param _cid Company ID. + * @param _vid Vendor ID. + * @param _data Codec Specific Data buffer in LVT format. + * @param _data_len Current LVT content length in `_data`. + * @param _meta Codec Specific Metadata buffer in LVT format. + * @param _meta_len Current LVT content length in `_meta`. + */ +#define ESP_BLE_AUDIO_CODEC_CFG_LEN(_id, _cid, _vid, _data, _data_len, _meta, _meta_len) \ + ((esp_ble_audio_codec_cfg_t){ \ + /* Use HCI data path as default, can be overwritten by application */ \ + .path_id = ESP_BLE_ISO_DATA_PATH_HCI, \ + .ctlr_transcode = false, \ + COND_CODE_1(IS_ENABLED(CONFIG_BT_BAP_UNICAST), \ + (.target_latency = ESP_BLE_AUDIO_CODEC_CFG_TARGET_LATENCY_BALANCED, \ + .target_phy = ESP_BLE_AUDIO_CODEC_CFG_TARGET_PHY_2M,), \ + ()) \ + .id = (_id), \ + .cid = (_cid), \ + .vid = (_vid), \ + .data_len = (_data_len), \ + .data = (_data), \ + .meta_len = (_meta_len), \ + .meta = (_meta), \ + }) + /** * @brief Helper to declare esp_ble_audio_codec_cap_t. * @@ -100,6 +129,23 @@ typedef enum bt_audio_codec_cfg_target_phy esp_ble_audio_codec_cfg_ .meta = (_meta), \ }) +/** + * @brief Variant of ESP_BLE_AUDIO_CODEC_CAP with explicit LVT content lengths. + */ +#define ESP_BLE_AUDIO_CODEC_CAP_LEN(_id, _cid, _vid, _data, _data_len, _meta, _meta_len) \ + ((esp_ble_audio_codec_cap_t){ \ + /* Use HCI data path as default, can be overwritten by application */ \ + .path_id = ESP_BLE_ISO_DATA_PATH_HCI, \ + .ctlr_transcode = false, \ + .id = (_id), \ + .cid = (_cid), \ + .vid = (_vid), \ + .data_len = (_data_len), \ + .data = (_data), \ + .meta_len = (_meta_len), \ + .meta = (_meta), \ + }) + /*!< Supported sampling frequencies */ #define ESP_BLE_AUDIO_CODEC_CAP_TYPE_FREQ BT_AUDIO_CODEC_CAP_TYPE_FREQ /*!< Supported frame durations */ diff --git a/components/bt/esp_ble_audio/api/include/esp_ble_audio_gmap_lc3_preset_defs.h b/components/bt/esp_ble_audio/api/include/esp_ble_audio_gmap_lc3_preset_defs.h index 138ff509083..a983d7ff0e2 100644 --- a/components/bt/esp_ble_audio/api/include/esp_ble_audio_gmap_lc3_preset_defs.h +++ b/components/bt/esp_ble_audio/api/include/esp_ble_audio_gmap_lc3_preset_defs.h @@ -31,19 +31,26 @@ extern "C" { * @param _rtn Maximum number of retransmissions. * @param _latency Maximum latency in milliseconds. * @param _pd Presentation delay in microseconds. + * + * Backing buffers are sized to `CONFIG_BT_AUDIO_CODEC_CFG_MAX_*_SIZE` so + * the preset is safe to mutate via `esp_ble_audio_codec_cfg_*_set_*`. */ #define ESP_BLE_AUDIO_GMAP_LC3_PRESET_DEFINE(_name, _freq, _duration, _loc, _len, \ _frames_per_sdu, _stream_context, \ _interval, _sdu, _rtn, _latency, _pd) \ - static uint8_t codec_cfg_data_##_name[] = \ + static uint8_t codec_cfg_data_##_name[CONFIG_BT_AUDIO_CODEC_CFG_MAX_DATA_SIZE] = \ ESP_BLE_AUDIO_CODEC_CFG_LC3_DATA(_freq, _duration, _loc, _len, _frames_per_sdu); \ - static uint8_t codec_cfg_meta_##_name[] = \ + static uint8_t codec_cfg_meta_##_name[CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE] = \ ESP_BLE_AUDIO_CODEC_CFG_LC3_META(_stream_context); \ static esp_ble_audio_bap_lc3_preset_t _name = \ - ESP_BLE_AUDIO_BAP_LC3_PRESET(ESP_BLE_AUDIO_CODEC_CFG_LC3(codec_cfg_data_##_name, \ - codec_cfg_meta_##_name), \ - ESP_BLE_AUDIO_BAP_QOS_CFG_UNFRAMED(_interval, _sdu, _rtn, \ - _latency, _pd)); + ESP_BLE_AUDIO_BAP_LC3_PRESET( \ + ESP_BLE_AUDIO_CODEC_CFG_LC3_LEN( \ + codec_cfg_data_##_name, \ + sizeof((uint8_t[])ESP_BLE_AUDIO_CODEC_CFG_LC3_DATA(_freq, _duration, _loc, \ + _len, _frames_per_sdu)), \ + codec_cfg_meta_##_name, \ + sizeof((uint8_t[])ESP_BLE_AUDIO_CODEC_CFG_LC3_META(_stream_context))), \ + ESP_BLE_AUDIO_BAP_QOS_CFG_UNFRAMED(_interval, _sdu, _rtn, _latency, _pd)); /** * @brief Gaming Reliable (GR) preset at 32kHz, 7.5ms frame duration. diff --git a/components/bt/esp_ble_audio/api/include/esp_ble_audio_lc3_defs.h b/components/bt/esp_ble_audio/api/include/esp_ble_audio_lc3_defs.h index 2fca7f5905e..bdc686edeac 100644 --- a/components/bt/esp_ble_audio/api/include/esp_ble_audio_lc3_defs.h +++ b/components/bt/esp_ble_audio/api/include/esp_ble_audio_lc3_defs.h @@ -52,6 +52,13 @@ extern "C" { #define ESP_BLE_AUDIO_CODEC_CAP_LC3(_data, _meta) \ ESP_BLE_AUDIO_CODEC_CAP(ESP_BLE_ISO_CODING_FORMAT_LC3, 0x0000, 0x0000, _data, _meta) +/** + * @brief Variant of ESP_BLE_AUDIO_CODEC_CAP_LC3 with explicit LVT content lengths. + */ +#define ESP_BLE_AUDIO_CODEC_CAP_LC3_LEN(_data, _data_len, _meta, _meta_len) \ + ESP_BLE_AUDIO_CODEC_CAP_LEN(ESP_BLE_ISO_CODING_FORMAT_LC3, 0x0000, 0x0000, \ + _data, _data_len, _meta, _meta_len) + /** * @brief Helper to declare LC3 codec data configuration. * @@ -82,6 +89,13 @@ extern "C" { #define ESP_BLE_AUDIO_CODEC_CFG_LC3(_data, _meta) \ ESP_BLE_AUDIO_CODEC_CFG(ESP_BLE_ISO_CODING_FORMAT_LC3, 0x0000, 0x0000, _data, _meta) +/** + * @brief Variant of ESP_BLE_AUDIO_CODEC_CFG_LC3 with explicit LVT content lengths. + */ +#define ESP_BLE_AUDIO_CODEC_CFG_LC3_LEN(_data, _data_len, _meta, _meta_len) \ + ESP_BLE_AUDIO_CODEC_CFG_LEN(ESP_BLE_ISO_CODING_FORMAT_LC3, 0x0000, 0x0000, \ + _data, _data_len, _meta, _meta_len) + #ifdef __cplusplus } #endif diff --git a/components/bt/esp_ble_audio/host/common/init.c b/components/bt/esp_ble_audio/host/common/init.c index a1de419d75c..34b4e6be044 100644 --- a/components/bt/esp_ble_audio/host/common/init.c +++ b/components/bt/esp_ble_audio/host/common/init.c @@ -178,7 +178,7 @@ static const uint16_t ext_structs[] = { sizeof(struct bt_bond_info), }; -#define LEA_VERSION (0x20260430) +#define LEA_VERSION (0x20260512) struct lib_ext_cfgs { /* BLE */ @@ -971,6 +971,12 @@ struct lib_ext_funcs { void (*_log_wrn)(const char *format, ...); void (*_log_err)(const char *format, ...); + /* Fatal assert: log + abort with tag/info/file/line/func context. + * Mirrors lib-side lib_ext_funcs._assert in init.h. ABI must match. + */ + void (*_assert)(const char *tag, size_t info, + const char *file, int line, const char *func); + /* Memory */ void *(*_malloc)(size_t size); void *(*_calloc)(size_t n, size_t size); @@ -1180,12 +1186,30 @@ static void log_error(const char *format, ...) #endif /* (CONFIG_BT_AUDIO_LOG_LEVEL >= BT_ISO_LOG_ERROR) */ } +/* Fatal assert handler registered into lib_ext_funcs._assert. + * Always logged (no LOG_LEVEL gate) — this is the last message before + * abort, and the user needs the context to diagnose. + */ +static void assert_fatal(const char *tag, size_t info, + const char *file, int line, const char *func) +{ + esp_log_write(ESP_LOG_ERROR, LEA_TAG, + BT_ISO_LOG_COLOR_E + "E (%lu) %s: LibAssert[%s][info=%u][%s:%d][%s]" + BT_ISO_LOG_RESET_COLOR "\n", + esp_log_timestamp(), LEA_TAG, + tag, (unsigned)info, file, line, func); + abort(); +} + static const struct lib_ext_funcs ext_funcs = { ._log_dbg = (void *)log_debug, ._log_inf = (void *)log_info, ._log_wrn = (void *)log_warn, ._log_err = (void *)log_error, + ._assert = (void *)assert_fatal, + ._malloc = (void *)malloc, ._calloc = (void *)calloc, ._free = (void *)free, diff --git a/components/bt/esp_ble_audio/lib/lib b/components/bt/esp_ble_audio/lib/lib index ad9b161851a..3796f0e4ef5 160000 --- a/components/bt/esp_ble_audio/lib/lib +++ b/components/bt/esp_ble_audio/lib/lib @@ -1 +1 @@ -Subproject commit ad9b161851a008f6cf60b3b693fb206883a5bc01 +Subproject commit 3796f0e4ef56f7b36fc51a32f41bb968c1e56081 diff --git a/components/bt/esp_ble_iso/host/adapter/nimble/gap.c b/components/bt/esp_ble_iso/host/adapter/nimble/gap.c index 9e435760fe3..59d4843887f 100644 --- a/components/bt/esp_ble_iso/host/adapter/nimble/gap.c +++ b/components/bt/esp_ble_iso/host/adapter/nimble/gap.c @@ -245,7 +245,7 @@ int bt_le_nimble_scan_start(const struct bt_le_scan_param *param, ble_gap_event_ scan_param.filter_duplicates = 0; return nimble_err_to_errno(ble_gap_disc(BLE_OWN_ADDR_PUBLIC, BLE_HS_FOREVER, - &scan_param, cb, NULL)); + &scan_param, cb, NULL)); } int bt_le_nimble_scan_stop(void) diff --git a/components/bt/esp_ble_iso/host/adapter/nimble/gatt/gatt.nrp.c b/components/bt/esp_ble_iso/host/adapter/nimble/gatt/gatt.nrp.c index c58ace5253a..d21413d9728 100644 --- a/components/bt/esp_ble_iso/host/adapter/nimble/gatt/gatt.nrp.c +++ b/components/bt/esp_ble_iso/host/adapter/nimble/gatt/gatt.nrp.c @@ -34,10 +34,17 @@ struct gatt_nrp_node { union { struct { struct bt_gatt_read_params *params; + /* read_by_uuid is multi-event (one per matching attr + EDONE); + * cb may return BT_GATT_ITER_STOP early, after which we must + * not invoke it again for trailing events on this procedure. */ + bool iter_stopped; } read_by_uuid; struct { struct bt_gatt_read_params *params; + /* Same multi-event story as read_by_uuid; per-fragment events + * may precede EDONE and the cb may STOP early. */ + bool iter_stopped; } read_long; struct { @@ -46,6 +53,13 @@ struct gatt_nrp_node { struct { struct bt_gatt_write_params *params; + /* Heap copy of params->data captured at insert time. Callers may pass + * a pointer to a stack-allocated buffer (e.g. NET_BUF_SIMPLE_DEFINE + * inside a helper that returns immediately after bt_gatt_write); when + * the request gets queued behind another in-flight GATT op, the + * caller's buffer is gone by the time we re-dispatch. Hold our own + * copy and dispatch from it. NULL when length == 0. */ + uint8_t *data_copy; } write_req; struct { @@ -53,7 +67,11 @@ struct gatt_nrp_node { } subscribe; struct { + /* Deep-copied at insert: callers may reuse the params slot or + * pass stack-backed data while we are queued/in-flight. */ struct bt_gatt_indicate_params *params; + struct bt_gatt_indicate_params params_copy; + uint8_t *data_copy; } indicate; }; @@ -70,13 +88,19 @@ static struct gatt_nrp { }, }; +static struct gatt_nrp *gatt_nrp_find(uint16_t conn_handle); + static int gattc_nrp_read_by_uuid_cb_safe(uint16_t conn_handle, const struct ble_gatt_error *error, struct ble_gatt_attr *attr, void *arg) { struct bt_gatt_read_params *read_params; + struct gatt_nrp_node *nrp_node; + struct gatt_nrp *nrp; struct bt_conn *conn; + sys_snode_t *node; + bool iter_stopped; int rc = 0; read_params = arg; @@ -95,15 +119,21 @@ static int gattc_nrp_read_by_uuid_cb_safe(uint16_t conn_handle, if (conn == NULL || conn->state != BT_CONN_CONNECTED) { LOG_ERR("[N]GattcNrpNotConn[%d]", __LINE__); rc = -ENOTCONN; - /* Note: - * Cannot remove NRP node or invoke callback without valid conn. - * Consider if bt_le_nimble_gatt_nrp_clear() should be invoked. - */ goto end; } - /* Shall be invoked before the read_params->func is invoked */ - bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_READ_BY_UUID, read_params); + /* Peek (don't pop) the in-flight head; READ_BY_UUID delivers per-attr + * events followed by EDONE, so the head must survive across events. + * It is removed only on terminal status (EDONE / error) below. */ + nrp = gatt_nrp_find(conn->handle); + assert(nrp); + node = sys_slist_peek_head(&nrp->list); + assert(node); + nrp_node = CONTAINER_OF(node, struct gatt_nrp_node, node); + assert(nrp_node->type == GATTC_NRP_READ_BY_UUID); + assert(nrp_node->read_by_uuid.params == read_params); + + iter_stopped = nrp_node->read_by_uuid.iter_stopped; switch (error->status) { case 0: @@ -118,20 +148,28 @@ static int gattc_nrp_read_by_uuid_cb_safe(uint16_t conn_handle, read_params->by_uuid.start_handle = attr->handle; } - read_params->func(conn, 0, read_params, attr->om->om_data, attr->om->om_len); + if (!iter_stopped) { + uint8_t cb_ret = read_params->func(conn, 0, read_params, + attr->om->om_data, attr->om->om_len); + if (cb_ret == BT_GATT_ITER_STOP) { + nrp_node->read_by_uuid.iter_stopped = true; + } + } break; case BLE_HS_EDONE: - read_params->func(conn, 0, read_params, NULL, 0); + bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_READ_BY_UUID, read_params, 0); + if (!iter_stopped) { + read_params->func(conn, 0, read_params, NULL, 0); + } break; default: LOG_WRN("[N]GattcNrpStatus[%04x]", error->status); - if (error->status & BLE_HS_ERR_ATT_BASE) { + bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_READ_BY_UUID, read_params, 0); + if ((error->status & BLE_HS_ERR_ATT_BASE) && !iter_stopped) { read_params->func(conn, (uint8_t)error->status, read_params, NULL, 0); - } else { - /* Note: for other errors, currently we just ignore it. */ } rc = error->status; @@ -150,7 +188,11 @@ static int gattc_nrp_read_long_cb_safe(uint16_t conn_handle, void *arg) { struct bt_gatt_read_params *read_params; + struct gatt_nrp_node *nrp_node; + struct gatt_nrp *nrp; struct bt_conn *conn; + sys_snode_t *node; + bool iter_stopped; int rc = 0; read_params = arg; @@ -170,8 +212,17 @@ static int gattc_nrp_read_long_cb_safe(uint16_t conn_handle, goto end; } - /* Shall be invoked before the read_params->func is invoked */ - bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_READ_LONG, read_params); + /* Peek (don't pop) — per-fragment events may precede EDONE; head is + * removed only on terminal status below. */ + nrp = gatt_nrp_find(conn->handle); + assert(nrp); + node = sys_slist_peek_head(&nrp->list); + assert(node); + nrp_node = CONTAINER_OF(node, struct gatt_nrp_node, node); + assert(nrp_node->type == GATTC_NRP_READ_LONG); + assert(nrp_node->read_long.params == read_params); + + iter_stopped = nrp_node->read_long.iter_stopped; switch (error->status) { case 0: @@ -181,20 +232,28 @@ static int gattc_nrp_read_long_cb_safe(uint16_t conn_handle, LOG_DBG("[N]GattcNrpHdl[%u][%u]Offset[%u]Len[%u]", attr->handle, read_params->single.handle, attr->offset, attr->om->om_len); - read_params->func(conn, 0, read_params, attr->om->om_data, attr->om->om_len); + if (!iter_stopped) { + uint8_t cb_ret = read_params->func(conn, 0, read_params, + attr->om->om_data, attr->om->om_len); + if (cb_ret == BT_GATT_ITER_STOP) { + nrp_node->read_long.iter_stopped = true; + } + } break; case BLE_HS_EDONE: - read_params->func(conn, 0, read_params, NULL, 0); + bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_READ_LONG, read_params, 0); + if (!iter_stopped) { + read_params->func(conn, 0, read_params, NULL, 0); + } break; default: LOG_WRN("[N]GattcNrpStatus[%04x]", error->status); - if (error->status & BLE_HS_ERR_ATT_BASE) { + bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_READ_LONG, read_params, 0); + if ((error->status & BLE_HS_ERR_ATT_BASE) && !iter_stopped) { read_params->func(conn, (uint8_t)error->status, read_params, NULL, 0); - } else { - /* Note: for other errors, currently we just ignore it. */ } rc = error->status; @@ -234,7 +293,7 @@ static int gattc_nrp_read_single_cb_safe(uint16_t conn_handle, } /* Shall be invoked before the read_params->func is invoked */ - bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_READ_SINGLE, read_params); + bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_READ_SINGLE, read_params, 0); switch (error->status) { case 0: @@ -263,8 +322,6 @@ static int gattc_nrp_read_single_cb_safe(uint16_t conn_handle, if (error->status & BLE_HS_ERR_ATT_BASE) { read_params->func(conn, (uint8_t)error->status, read_params, NULL, 0); - } else { - /* Note: for other errors, currently we just ignore it. */ } rc = error->status; @@ -353,7 +410,7 @@ static int gattc_nrp_write_cb_safe(uint16_t conn_handle, } /* Shall be invoked before the write_params->func is invoked */ - bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_WRITE_REQ, write_params); + bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_WRITE_REQ, write_params, 0); switch (error->status) { case 0: @@ -372,8 +429,6 @@ static int gattc_nrp_write_cb_safe(uint16_t conn_handle, if (error->status & BLE_HS_ERR_ATT_BASE) { write_params->func(conn, (uint8_t)error->status, write_params); - } else { - /* Note: for other errors, currently we just ignore it. */ } rc = error->status; @@ -386,17 +441,18 @@ end: return rc; } -static int gattc_nrp_write(struct bt_conn *conn, struct bt_gatt_write_params *params) +static int gattc_nrp_write(struct bt_conn *conn, struct bt_gatt_write_params *params, + const uint8_t *data, uint16_t length) { int rc; LOG_DBG("[N]GattcNrpWr[%u][%u][%u]", - conn->handle, params->handle, params->length); + conn->handle, params->handle, length); rc = ble_gattc_write_flat(conn->handle, params->handle, - params->data, - params->length, + data, + length, gattc_nrp_write_cb_safe, params); if (rc) { @@ -430,7 +486,7 @@ static int gattc_nrp_subscribe_cb_safe(uint16_t conn_handle, } /* Should be invoked here */ - bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_SUBSCRIBE, sub_params); + bt_le_nimble_gatt_nrp_remove(conn, GATTC_NRP_SUBSCRIBE, sub_params, 0); switch (error->status) { case 0: @@ -521,10 +577,13 @@ void bt_le_nimble_gatts_nrp_indicate_cb(uint16_t conn_handle, } if (status) { + /* EDONE = peer confirmed → success; other non-zero = failure. */ + uint8_t err = (status == BLE_HS_EDONE) ? 0 : status; + attr.handle = attr_handle; params.attr = &attr; - bt_le_nimble_gatt_nrp_remove(conn, GATTS_NRP_INDICATE, ¶ms); + bt_le_nimble_gatt_nrp_remove(conn, GATTS_NRP_INDICATE, ¶ms, err); } } @@ -536,8 +595,11 @@ static int gatts_nrp_indicate(struct bt_conn *conn, struct bt_gatt_indicate_para assert(conn && params); assert(params->attr); - assert(params->data); - assert(params->len); + /* 0-length indications are valid (spec); ble_hs_mbuf_from_flat accepts + * (NULL, 0). But (NULL, len>0) would manifest as a misleading -ENOMEM, + * so catch the programmer error here as a defense-in-depth guard. + */ + assert(params->len == 0 || params->data != NULL); data.attr = params->attr; data.handle = bt_gatt_attr_get_handle(data.attr); @@ -633,6 +695,11 @@ static void gatt_nrp_del(struct gatt_nrp *nrp) LOG_DBG("[N]GattNrpFree[%p]", nrp_head); + if (nrp_head->type == GATTC_NRP_WRITE_REQ) { + free(nrp_head->write_req.data_copy); + } else if (nrp_head->type == GATTS_NRP_INDICATE) { + free(nrp_head->indicate.data_copy); + } free(nrp_head); } @@ -671,15 +738,44 @@ static int gatt_nrp_insert(struct bt_conn *conn, uint8_t type, void *params) case GATTC_NRP_READ_SINGLE: nrp_node->read_single.params = params; break; - case GATTC_NRP_WRITE_REQ: - nrp_node->write_req.params = params; + case GATTC_NRP_WRITE_REQ: { + struct bt_gatt_write_params *wp = params; + nrp_node->write_req.params = wp; + nrp_node->write_req.data_copy = NULL; + if (wp->length > 0) { + assert(wp->data); + nrp_node->write_req.data_copy = malloc(wp->length); + if (nrp_node->write_req.data_copy == NULL) { + LOG_ERR("[N]GattNrpWrDataAllocFail[%u]", wp->length); + free(nrp_node); + return -ENOMEM; + } + memcpy(nrp_node->write_req.data_copy, wp->data, wp->length); + } break; + } case GATTC_NRP_SUBSCRIBE: nrp_node->subscribe.params = params; break; - case GATTS_NRP_INDICATE: - nrp_node->indicate.params = params; + case GATTS_NRP_INDICATE: { + struct bt_gatt_indicate_params *ip = params; + + nrp_node->indicate.params_copy = *ip; + nrp_node->indicate.data_copy = NULL; + if (ip->len > 0) { + assert(ip->data); + nrp_node->indicate.data_copy = malloc(ip->len); + if (nrp_node->indicate.data_copy == NULL) { + LOG_ERR("[N]GattNrpIndDataAllocFail[%u]", ip->len); + free(nrp_node); + return -ENOMEM; + } + memcpy(nrp_node->indicate.data_copy, ip->data, ip->len); + } + nrp_node->indicate.params_copy.data = nrp_node->indicate.data_copy; + nrp_node->indicate.params = &nrp_node->indicate.params_copy; break; + } default: assert(0); } @@ -692,11 +788,13 @@ static int gatt_nrp_insert(struct bt_conn *conn, uint8_t type, void *params) type == GATTC_NRP_READ_SINGLE) { rc = gattc_nrp_read(conn, params); } else if (type == GATTC_NRP_WRITE_REQ) { - rc = gattc_nrp_write(conn, params); + rc = gattc_nrp_write(conn, params, + nrp_node->write_req.data_copy, + ((struct bt_gatt_write_params *)params)->length); } else if (type == GATTC_NRP_SUBSCRIBE) { rc = gattc_nrp_subscribe(conn, params); } else if (type == GATTS_NRP_INDICATE) { - rc = gatts_nrp_indicate(conn, params); + rc = gatts_nrp_indicate(conn, nrp_node->indicate.params); } } else { LOG_DBG("[N]GattNrpListNotEmpty"); @@ -710,6 +808,11 @@ static int gatt_nrp_insert(struct bt_conn *conn, uint8_t type, void *params) if (rc != BLE_HS_ENOTCONN) { LOG_ERR("[N]GattNrpInsertFail[%d]", rc); } + if (type == GATTC_NRP_WRITE_REQ) { + free(nrp_node->write_req.data_copy); + } else if (type == GATTS_NRP_INDICATE) { + free(nrp_node->indicate.data_copy); + } free(nrp_node); } @@ -756,7 +859,7 @@ int bt_le_nimble_gatt_nrp_insert(struct bt_conn *conn, uint8_t type, void *param return nimble_err_to_errno(rc); } -int bt_le_nimble_gatt_nrp_remove(struct bt_conn *conn, uint8_t type, void *params) +int bt_le_nimble_gatt_nrp_remove(struct bt_conn *conn, uint8_t type, void *params, uint8_t err) { struct gatt_nrp_node *nrp_head; struct gatt_nrp *nrp; @@ -790,6 +893,8 @@ int bt_le_nimble_gatt_nrp_remove(struct bt_conn *conn, uint8_t type, void *param break; case GATTC_NRP_WRITE_REQ: assert(nrp_head->write_req.params == params); + free(nrp_head->write_req.data_copy); + nrp_head->write_req.data_copy = NULL; break; case GATTC_NRP_SUBSCRIBE: assert(nrp_head->subscribe.params == params); @@ -805,11 +910,13 @@ int bt_le_nimble_gatt_nrp_remove(struct bt_conn *conn, uint8_t type, void *param assert(0); /* Should not happen */ } - /* Note: - * The params may be shared by multiple connections. And currently - * in this case, the params will not be updated in the func callback. - */ - nrp_head->indicate.params->func(conn, nrp_head->indicate.params, 0); + /* params is the deep copy taken at insert time; cb sees the copy's + * address, not the caller's original (which may have been reused). + * err propagates the mapped NimBLE status (0 on EDONE, else err). */ + nrp_head->indicate.params->func(conn, nrp_head->indicate.params, err); + + free(nrp_head->indicate.data_copy); + nrp_head->indicate.data_copy = NULL; break; default: assert(0); @@ -853,7 +960,9 @@ int bt_le_nimble_gatt_nrp_remove(struct bt_conn *conn, uint8_t type, void *param nrp_head->read_single.params->func(conn, rc, nrp_head->read_single.params, NULL, 0); } } else if (nrp_head->type == GATTC_NRP_WRITE_REQ) { - rc = gattc_nrp_write(conn, nrp_head->write_req.params); + rc = gattc_nrp_write(conn, nrp_head->write_req.params, + nrp_head->write_req.data_copy, + nrp_head->write_req.params->length); if (rc) { LOG_ERR("[N]GattcNrpWrFail[%d]", rc); @@ -887,6 +996,11 @@ int bt_le_nimble_gatt_nrp_remove(struct bt_conn *conn, uint8_t type, void *param */ sys_slist_remove(&nrp->list, NULL, node); + if (nrp_head->type == GATTC_NRP_WRITE_REQ) { + free(nrp_head->write_req.data_copy); + } else if (nrp_head->type == GATTS_NRP_INDICATE) { + free(nrp_head->indicate.data_copy); + } free(nrp_head); } diff --git a/components/bt/esp_ble_iso/host/adapter/nimble/include/nimble/gatt.h b/components/bt/esp_ble_iso/host/adapter/nimble/include/nimble/gatt.h index 6adb58869d9..01f246e04a0 100644 --- a/components/bt/esp_ble_iso/host/adapter/nimble/include/nimble/gatt.h +++ b/components/bt/esp_ble_iso/host/adapter/nimble/include/nimble/gatt.h @@ -113,7 +113,9 @@ void bt_le_nimble_gatts_nrp_indicate_cb(uint16_t conn_handle, int bt_le_nimble_gatt_nrp_insert(struct bt_conn *conn, uint8_t type, void *params); -int bt_le_nimble_gatt_nrp_remove(struct bt_conn *conn, uint8_t type, void *params); +/* err is forwarded only to the INDICATE func cb; other NRP types invoke their + * own func with err in cb_safe and pass 0 here. */ +int bt_le_nimble_gatt_nrp_remove(struct bt_conn *conn, uint8_t type, void *params, uint8_t err); void bt_le_nimble_gatt_nrp_clear(uint16_t conn_handle); diff --git a/components/bt/esp_ble_iso/host/common/gatt.c b/components/bt/esp_ble_iso/host/common/gatt.c index a45bb6548a6..8c4d6779e9b 100644 --- a/components/bt/esp_ble_iso/host/common/gatt.c +++ b/components/bt/esp_ble_iso/host/common/gatt.c @@ -409,6 +409,8 @@ ssize_t bt_gatt_attr_read(struct bt_conn *conn, const struct bt_gatt_attr *attr, _LIB_ONLY int bt_gatt_notify_cb(struct bt_conn *conn, struct bt_gatt_notify_params *params) { + int err; + assert(params); LOG_DBG("GattNtfCb[%u]", params->len); @@ -418,7 +420,16 @@ int bt_gatt_notify_cb(struct bt_conn *conn, struct bt_gatt_notify_params *params return -ENOTCONN; } - return bt_le_nimble_gatts_notify(conn, params); + err = bt_le_nimble_gatts_notify(conn, params); + + /* gatts_notify is synchronous (mbuf-copy + dispatch on return); fire the + * caller's completion cb here so state machines like PACS_FLAG_NOTIFY_RDY + * advance. */ + if (err == 0 && params->func != NULL) { + params->func(conn, params->user_data); + } + + return err; } _LIB_IDF diff --git a/components/bt/esp_ble_iso/host/common/host.c b/components/bt/esp_ble_iso/host/common/host.c index 42e5b44971b..5cf78899924 100644 --- a/components/bt/esp_ble_iso/host/common/host.c +++ b/components/bt/esp_ble_iso/host/common/host.c @@ -7,8 +7,10 @@ #include #include +#include #include +#include #include #include "common/host.h" @@ -28,7 +30,18 @@ void bt_le_host_lock(void) #endif /* HOST_LOCK_DEBUG */ { /* LOG_DBG("%s: %d", func, line); */ - k_mutex_lock(&host_mutex, TIMEOUT_MS); + + int err = k_mutex_lock(&host_mutex, TIMEOUT_MS); + if (err) { + /* 5s wait failed: the host stack is wedged. k_mutex_lock has + * already logged self/holder task names. Use libc abort() rather + * than assert(0) — assert is a no-op under NDEBUG, which would + * let the caller enter the critical section without the mutex + * held and cause races. abort() halts in every build. + */ + LOG_ERR("HostLockTimeout"); + abort(); + } } #if HOST_LOCK_DEBUG @@ -38,6 +51,16 @@ void bt_le_host_unlock(void) #endif /* HOST_LOCK_DEBUG */ { /* LOG_DBG("%s: %d", func, line); */ + + /* Defense-in-depth: bt_le_host_lock now aborts on timeout, so this + * branch is unreachable in normal flow. Keep the check to catch any + * unbalanced unlock (callers releasing without prior lock). + */ + if (xSemaphoreGetMutexHolder(host_mutex.handle) != xTaskGetCurrentTaskHandle()) { + LOG_WRN("HostUnlockNotHolder"); + return; + } + k_mutex_unlock(&host_mutex); } diff --git a/components/bt/esp_ble_iso/host/utils/timer.c b/components/bt/esp_ble_iso/host/utils/timer.c index 30a8f8307e9..49a5fe13c33 100644 --- a/components/bt/esp_ble_iso/host/utils/timer.c +++ b/components/bt/esp_ble_iso/host/utils/timer.c @@ -28,8 +28,6 @@ static void iso_timer_cb(void *arg) struct k_work *work = arg; int err; - LOG_DBG("IsoTimerCb[%p]", work); - assert(work); assert(work->timer); assert(work->handler); @@ -310,7 +308,5 @@ void bt_le_timer_handle_event(void *arg) { struct k_work *work = arg; - LOG_DBG("HdlTimerEvt[%p]", work); - k_work_submit_safe(work); } diff --git a/components/bt/esp_ble_iso/include/zephyr/bluetooth/gatt.h b/components/bt/esp_ble_iso/include/zephyr/bluetooth/gatt.h index 3eb73708864..7a0b8c325d8 100644 --- a/components/bt/esp_ble_iso/include/zephyr/bluetooth/gatt.h +++ b/components/bt/esp_ble_iso/include/zephyr/bluetooth/gatt.h @@ -1216,6 +1216,8 @@ struct bt_gatt_notify_params { uint16_t len; /** Notification Value callback */ bt_gatt_complete_func_t func; + /** Data passed to @ref func when the notification completes. */ + void *user_data; /** Att channel options. */ enum bt_att_chan_opt chan_opt; }; diff --git a/components/bt/esp_ble_iso/include/zephyr/kernel.h b/components/bt/esp_ble_iso/include/zephyr/kernel.h index b4b2e74a672..ec6f0e82f10 100644 --- a/components/bt/esp_ble_iso/include/zephyr/kernel.h +++ b/components/bt/esp_ble_iso/include/zephyr/kernel.h @@ -67,12 +67,23 @@ static inline int k_mutex_lock(struct k_mutex *mutex, uint32_t timeout) assert(mutex); assert(mutex->handle); - if (xSemaphoreTakeRecursive(mutex->handle, timeout) != pdTRUE) { - K_MUTEX_LOG_ERR("LockFail"); - return -EIO; + if (xSemaphoreTakeRecursive(mutex->handle, timeout) == pdTRUE) { + return 0; } - return 0; +#if !CONFIG_BT_ISO_NO_LOG && (CONFIG_BT_ISO_LOG_LEVEL >= BT_ISO_LOG_ERROR) + /* On timeout, dump who timed out and who is currently holding the + * mutex so the caller's log makes wedge diagnosis a one-look job + * instead of a multi-task scavenger hunt. + */ + TaskHandle_t holder = xSemaphoreGetMutexHolder(mutex->handle); + K_MUTEX_LOG_ERR("LockFail[self=%s][holder=%s]", + pcTaskGetName(NULL), + holder ? pcTaskGetName(holder) : ""); +#else + K_MUTEX_LOG_ERR("LockFail"); +#endif + return -EIO; } static inline int k_mutex_unlock(struct k_mutex *mutex)