From 85dfc739d74acb152c088586f664a10b1d07212d Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Mon, 28 Oct 2024 15:36:52 +0100 Subject: [PATCH 01/10] [nrf fromtree] tests: Bluetooth: Tester: Fix use of uninitialized cig_id for CAP The CAP tests used u_group->cig->index but the u_group->cig may have been deleted when the stream is released, which meant that u_group->cig == NULL when e.g. unicast_stop_complete_cb was called and that could cause failing tests as the index would just be a uninitialized value. Signed-off-by: Emil Gydesen (cherry picked from commit 5ebe1194be5daf2c8a023de53d6fe3766a807525) --- tests/bluetooth/tester/src/audio/btp_bap_unicast.c | 1 + tests/bluetooth/tester/src/audio/btp_bap_unicast.h | 4 ++++ tests/bluetooth/tester/src/audio/btp_cap.c | 8 ++++---- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/bluetooth/tester/src/audio/btp_bap_unicast.c b/tests/bluetooth/tester/src/audio/btp_bap_unicast.c index 0a78441f013..f71096bc167 100644 --- a/tests/bluetooth/tester/src/audio/btp_bap_unicast.c +++ b/tests/bluetooth/tester/src/audio/btp_bap_unicast.c @@ -1131,6 +1131,7 @@ int btp_bap_unicast_group_create(uint8_t cig_id, } cigs[cig_id].in_use = true; + cigs[cig_id].cig_id = cig_id; *out_unicast_group = &cigs[cig_id]; return 0; diff --git a/tests/bluetooth/tester/src/audio/btp_bap_unicast.h b/tests/bluetooth/tester/src/audio/btp_bap_unicast.h index 1ac0bac1aa9..24651ec5a19 100644 --- a/tests/bluetooth/tester/src/audio/btp_bap_unicast.h +++ b/tests/bluetooth/tester/src/audio/btp_bap_unicast.h @@ -2,10 +2,13 @@ /* * Copyright (c) 2023 Codecoup + * Copyright (c) 2024 Nordic Semiconductor ASA * * SPDX-License-Identifier: Apache-2.0 */ +#include + #include #define BTP_BAP_UNICAST_MAX_SNK_STREAMS_COUNT MIN(CONFIG_BT_BAP_UNICAST_CLIENT_ASE_SNK_COUNT, \ @@ -20,6 +23,7 @@ struct btp_bap_unicast_group { struct bt_audio_codec_qos qos[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT]; struct bt_bap_unicast_group *cig; + uint8_t cig_id; bool in_use; }; diff --git a/tests/bluetooth/tester/src/audio/btp_cap.c b/tests/bluetooth/tester/src/audio/btp_cap.c index 19986060b94..3c42ad292f7 100644 --- a/tests/bluetooth/tester/src/audio/btp_cap.c +++ b/tests/bluetooth/tester/src/audio/btp_cap.c @@ -104,13 +104,13 @@ static void unicast_start_complete_cb(int err, struct bt_conn *conn) if (err != 0) { LOG_DBG("Failed to unicast-start, err %d", err); - btp_send_cap_unicast_start_completed_ev(u_group->cig->index, + btp_send_cap_unicast_start_completed_ev(u_group->cig_id, BTP_CAP_UNICAST_START_STATUS_FAILED); return; } - btp_send_cap_unicast_start_completed_ev(u_group->cig->index, + btp_send_cap_unicast_start_completed_ev(u_group->cig_id, BTP_CAP_UNICAST_START_STATUS_SUCCESS); } @@ -129,13 +129,13 @@ static void unicast_stop_complete_cb(int err, struct bt_conn *conn) if (err != 0) { LOG_DBG("Failed to unicast-stop, err %d", err); - btp_send_cap_unicast_stop_completed_ev(u_group->cig->index, + btp_send_cap_unicast_stop_completed_ev(u_group->cig_id, BTP_CAP_UNICAST_START_STATUS_FAILED); return; } - btp_send_cap_unicast_stop_completed_ev(u_group->cig->index, + btp_send_cap_unicast_stop_completed_ev(u_group->cig_id, BTP_CAP_UNICAST_START_STATUS_SUCCESS); } From c9b647d9a20bca4dadefeeb2da8ea2eac41cbe8e Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Fri, 1 Nov 2024 15:05:30 +0100 Subject: [PATCH 02/10] [nrf fromtree] Bluetooth: CSIP: Handle disconnects while in procedure If a device disconnects while we are in a procedure then get_next_active_instance would return a service instance pointer with the `conn` set to NULL. The issue was caused by the set_info being potentially memset when the device that disconnected was the one that held the set_info pointer. The solution is to not use a pointer, but rather a copy of the set_info, so that the active.set_info value is still valid after a disconnect. Since the set_info is not longer a pointer to a specific set_info from one of the members, the logs have been updated as well, as the pointer of the active.set_info is useless for debugging. Signed-off-by: Emil Gydesen (cherry picked from commit 1f1e4afa4f145699677f8ee50dac5c5890d42dd9) --- subsys/bluetooth/audio/csip_set_coordinator.c | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/subsys/bluetooth/audio/csip_set_coordinator.c b/subsys/bluetooth/audio/csip_set_coordinator.c index 2b0c53db23c..b09d32dd890 100644 --- a/subsys/bluetooth/audio/csip_set_coordinator.c +++ b/subsys/bluetooth/audio/csip_set_coordinator.c @@ -58,7 +58,7 @@ LOG_MODULE_REGISTER(bt_csip_set_coordinator, CONFIG_BT_CSIP_SET_COORDINATOR_LOG_ static struct active_members { struct bt_csip_set_coordinator_set_member *members[CONFIG_BT_MAX_CONN]; - const struct bt_csip_set_coordinator_set_info *info; + struct bt_csip_set_coordinator_set_info info; uint8_t members_count; uint8_t members_handled; uint8_t members_restored; @@ -169,7 +169,7 @@ static struct bt_csip_set_coordinator_svc_inst *lookup_instance_by_set_info( member_set_info = &member->insts[i].info; if (member_set_info->set_size == set_info->set_size && - memcmp(&member_set_info->sirk, &set_info->sirk, sizeof(set_info->sirk)) == 0) { + memcmp(member_set_info->sirk, set_info->sirk, sizeof(set_info->sirk)) == 0) { return bt_csip_set_coordinator_lookup_instance_by_index(inst->conn, i); } } @@ -184,9 +184,9 @@ static struct bt_csip_set_coordinator_svc_inst *get_next_active_instance(void) member = active.members[active.members_handled]; - svc_inst = lookup_instance_by_set_info(member, active.info); + svc_inst = lookup_instance_by_set_info(member, &active.info); if (svc_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); } return svc_inst; @@ -201,8 +201,8 @@ static int member_rank_compare_asc(const void *m1, const void *m2) struct bt_csip_set_coordinator_svc_inst *svc_inst_1; struct bt_csip_set_coordinator_svc_inst *svc_inst_2; - svc_inst_1 = lookup_instance_by_set_info(member_1, active.info); - svc_inst_2 = lookup_instance_by_set_info(member_2, active.info); + svc_inst_1 = lookup_instance_by_set_info(member_1, &active.info); + svc_inst_2 = lookup_instance_by_set_info(member_2, &active.info); if (svc_inst_1 == NULL) { LOG_ERR("svc_inst_1 was NULL for member %p", member_1); @@ -232,7 +232,7 @@ static void active_members_store_ordered(const struct bt_csip_set_coordinator_se { (void)memcpy(active.members, members, count * sizeof(members[0U])); active.members_count = count; - active.info = info; + memcpy(&active.info, info, sizeof(active.info)); if (count > 1U && CONFIG_BT_MAX_CONN > 1) { qsort(active.members, count, sizeof(members[0U]), @@ -1080,7 +1080,7 @@ static void csip_set_coordinator_write_restore_cb(struct bt_conn *conn, int csip_err; member = active.members[active.members_handled - active.members_restored - 1]; - client->cur_inst = lookup_instance_by_set_info(member, active.info); + client->cur_inst = lookup_instance_by_set_info(member, &active.info); if (client->cur_inst == NULL) { release_set_complete(-ENOENT); @@ -1115,9 +1115,9 @@ static void csip_set_coordinator_write_lock_cb(struct bt_conn *conn, active.members_restored = 0; member = active.members[active.members_handled - active.members_restored]; - client->cur_inst = lookup_instance_by_set_info(member, active.info); + client->cur_inst = lookup_instance_by_set_info(member, &active.info); if (client->cur_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); lock_set_complete(-ENOENT); return; @@ -1215,7 +1215,7 @@ static void csip_set_coordinator_write_release_cb(struct bt_conn *conn, uint8_t static void csip_set_coordinator_lock_state_read_cb(int err, bool locked) { - const struct bt_csip_set_coordinator_set_info *info = active.info; + const struct bt_csip_set_coordinator_set_info *info = &active.info; struct bt_csip_set_coordinator_set_member *cur_member = NULL; if (err || locked) { @@ -1607,9 +1607,9 @@ csip_set_coordinator_get_lock_state(const struct bt_csip_set_coordinator_set_mem for (uint8_t i = 0U; i < count; i++) { struct bt_csip_set_coordinator_svc_inst *svc_inst; - svc_inst = lookup_instance_by_set_info(active.members[i], active.info); + svc_inst = lookup_instance_by_set_info(active.members[i], &active.info); if (svc_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); active_members_reset(); return -ENOENT; @@ -1633,11 +1633,11 @@ csip_set_coordinator_get_lock_state(const struct bt_csip_set_coordinator_set_mem * here. */ if (active.oap_cb == NULL || - !active.oap_cb(active.info, active.members, active.members_count)) { + !active.oap_cb(&active.info, active.members, active.members_count)) { err = -ECANCELED; } - ordered_access_complete(active.info, err, false, NULL); + ordered_access_complete(&active.info, err, false, NULL); } return err; @@ -1690,9 +1690,9 @@ int bt_csip_set_coordinator_lock( active_members_store_ordered(members, count, set_info, true); - svc_inst = lookup_instance_by_set_info(active.members[0], active.info); + svc_inst = lookup_instance_by_set_info(active.members[0], &active.info); if (svc_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); active_members_reset(); return -ENOENT; @@ -1732,9 +1732,9 @@ int bt_csip_set_coordinator_release(const struct bt_csip_set_coordinator_set_mem active_members_store_ordered(members, count, set_info, false); - svc_inst = lookup_instance_by_set_info(active.members[0], active.info); + svc_inst = lookup_instance_by_set_info(active.members[0], &active.info); if (svc_inst == NULL) { - LOG_DBG("Failed to lookup instance by set_info %p", active.info); + LOG_DBG("Failed to lookup instance by set_info"); active_members_reset(); return -ENOENT; From b674d88e7e60bd468ebc00a20412a29f49e1af6c Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Tue, 5 Nov 2024 17:53:56 +0100 Subject: [PATCH 03/10] [nrf fromtree] Bluetooth: CSIP: Fix off-by-one in in lock restore If the lock request was rejected by a set member we should restore any previously written logs in reverse order. However there was a off-by-one error in csip_set_coordinator_write_lock_cb which caused us to attempt to release member[1] instead of member[0] if member[1] was the one that rejected the lock request. Additionally, the lock_set_complete would be called prematurely before we get the response from the restore request. Signed-off-by: Emil Gydesen (cherry picked from commit 7c40b071e448f57868b56b67d2f5bd23443c232a) --- subsys/bluetooth/audio/csip_set_coordinator.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/subsys/bluetooth/audio/csip_set_coordinator.c b/subsys/bluetooth/audio/csip_set_coordinator.c index b09d32dd890..2d83aa41c6a 100644 --- a/subsys/bluetooth/audio/csip_set_coordinator.c +++ b/subsys/bluetooth/audio/csip_set_coordinator.c @@ -1114,7 +1114,7 @@ static void csip_set_coordinator_write_lock_cb(struct bt_conn *conn, active.members_restored = 0; - member = active.members[active.members_handled - active.members_restored]; + member = active.members[active.members_handled - 1]; client->cur_inst = lookup_instance_by_set_info(member, &active.info); if (client->cur_inst == NULL) { LOG_DBG("Failed to lookup instance by set_info"); @@ -1131,10 +1131,10 @@ static void csip_set_coordinator_write_lock_cb(struct bt_conn *conn, active_members_reset(); return; } + } else { + lock_set_complete(err); } - lock_set_complete(err); - return; } From 749e6f95700d97c8ba50ebd924a04cc28fe25eab Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Wed, 12 Jun 2024 19:09:24 +0200 Subject: [PATCH 04/10] [nrf fromtree] Bluetooth: CAP: Add support for doing just disable for unicast stop The unicast_stop function is changed to primarily do a BAP disable instead of a release, with optional support for releasing the streams once they have been disabled. This also adds unittests for the procedure which also allow us to remove the invalid param testing from the BSIM tests. Signed-off-by: Emil Gydesen (cherry picked from commit fa4f2ffc47d49a17e04f11da14d12ca317124ecc) --- include/zephyr/bluetooth/audio/cap.h | 8 +- subsys/bluetooth/audio/bap_stream.c | 4 +- subsys/bluetooth/audio/cap_common.c | 10 + subsys/bluetooth/audio/cap_initiator.c | 484 ++++++++++++++---- subsys/bluetooth/audio/cap_internal.h | 8 + subsys/bluetooth/audio/cap_stream.c | 10 + subsys/bluetooth/audio/shell/cap_initiator.c | 1 + .../audio/cap_initiator/CMakeLists.txt | 1 + tests/bluetooth/audio/cap_initiator/prj.conf | 2 + .../bluetooth/audio/cap_initiator/src/main.c | 2 - .../audio/cap_initiator/src/test_common.c | 2 + .../cap_initiator/src/test_unicast_start.c | 2 - .../cap_initiator/src/test_unicast_stop.c | 482 +++++++++++++++++ .../cap_initiator/uut/bap_unicast_client.c | 95 +++- tests/bluetooth/tester/src/audio/btp_cap.c | 1 + .../audio/src/cap_initiator_unicast_test.c | 34 +- .../bsim/bluetooth/audio/src/gmap_ugg_test.c | 1 + 17 files changed, 1022 insertions(+), 125 deletions(-) create mode 100644 tests/bluetooth/audio/cap_initiator/src/test_unicast_stop.c diff --git a/include/zephyr/bluetooth/audio/cap.h b/include/zephyr/bluetooth/audio/cap.h index 50c29640a73..4417477860d 100644 --- a/include/zephyr/bluetooth/audio/cap.h +++ b/include/zephyr/bluetooth/audio/cap.h @@ -312,6 +312,9 @@ struct bt_cap_unicast_audio_stop_param { /** Array of streams to stop */ struct bt_cap_stream **streams; + + /** Whether to release the streams after they have stopped */ + bool release; }; /** @@ -379,7 +382,10 @@ int bt_cap_initiator_unicast_audio_update(const struct bt_cap_unicast_audio_upda * * @param param Stop parameters. * - * @return 0 on success or negative error value on failure. + * @return 0 on success + * @retval -EBUSY if a CAP procedure is already in progress + * @retval -EINVAL if any parameter is invalid + * @retval -EALREADY if no state changes will occur */ int bt_cap_initiator_unicast_audio_stop(const struct bt_cap_unicast_audio_stop_param *param); diff --git a/subsys/bluetooth/audio/bap_stream.c b/subsys/bluetooth/audio/bap_stream.c index dbc6a6d5a18..ebc9a87712a 100644 --- a/subsys/bluetooth/audio/bap_stream.c +++ b/subsys/bluetooth/audio/bap_stream.c @@ -445,7 +445,7 @@ void bt_bap_stream_detach(struct bt_bap_stream *stream) { const bool is_broadcast = bt_bap_stream_is_broadcast(stream); - LOG_DBG("stream %p", stream); + LOG_DBG("stream %p conn %p ep %p", stream, (void *)stream->conn, (void *)stream->ep); if (stream->conn != NULL) { bt_conn_unref(stream->conn); @@ -523,7 +523,7 @@ int bt_bap_stream_config(struct bt_conn *conn, struct bt_bap_stream *stream, str codec_cfg, codec_cfg ? codec_cfg->id : 0, codec_cfg ? codec_cfg->cid : 0, codec_cfg ? codec_cfg->vid : 0); - CHECKIF(conn == NULL || stream == NULL || codec_cfg == NULL) { + CHECKIF(conn == NULL || stream == NULL || codec_cfg == NULL || ep == NULL) { LOG_DBG("NULL value(s) supplied)"); return -EINVAL; } diff --git a/subsys/bluetooth/audio/cap_common.c b/subsys/bluetooth/audio/cap_common.c index 56e7741d2ee..8780a3e357a 100644 --- a/subsys/bluetooth/audio/cap_common.c +++ b/subsys/bluetooth/audio/cap_common.c @@ -64,6 +64,11 @@ void bt_cap_common_set_subproc(enum bt_cap_common_subproc_type subproc_type) active_proc.subproc_type = subproc_type; } +bool bt_cap_common_proc_is_type(enum bt_cap_common_proc_type proc_type) +{ + return active_proc.proc_type == proc_type; +} + bool bt_cap_common_subproc_is_type(enum bt_cap_common_subproc_type subproc_type) { return active_proc.subproc_type == subproc_type; @@ -122,7 +127,12 @@ void bt_cap_common_abort_proc(struct bt_conn *conn, int err) return; } +#if defined(CONFIG_BT_CAP_INITIATOR_UNICAST) + LOG_DBG("Aborting proc %d with subproc %d for %p: %d", active_proc.proc_type, + active_proc.subproc_type, (void *)conn, err); +#else /* !CONFIG_BT_CAP_INITIATOR_UNICAST */ LOG_DBG("Aborting proc %d for %p: %d", active_proc.proc_type, (void *)conn, err); +#endif /* CONFIG_BT_CAP_INITIATOR_UNICAST */ active_proc.err = err; active_proc.failed_conn = conn; diff --git a/subsys/bluetooth/audio/cap_initiator.c b/subsys/bluetooth/audio/cap_initiator.c index 20d5011ebde..23295848b14 100644 --- a/subsys/bluetooth/audio/cap_initiator.c +++ b/subsys/bluetooth/audio/cap_initiator.c @@ -513,6 +513,17 @@ static void update_proc_done_cnt(struct bt_cap_common_proc *active_proc) state = stream_get_state(bap_stream); switch (subproc_type) { + case BT_CAP_COMMON_SUBPROC_TYPE_DISABLE: + if (state == BT_BAP_EP_STATE_QOS_CONFIGURED || + state == BT_BAP_EP_STATE_DISABLING) { + proc_done_cnt++; + } + break; + case BT_CAP_COMMON_SUBPROC_TYPE_STOP: + if (state == BT_BAP_EP_STATE_QOS_CONFIGURED) { + proc_done_cnt++; + } + break; case BT_CAP_COMMON_SUBPROC_TYPE_RELEASE: if (state == BT_BAP_EP_STATE_IDLE || state == BT_BAP_EP_STATE_CODEC_CONFIGURED) { @@ -574,47 +585,60 @@ get_next_proc_param(struct bt_cap_common_proc *active_proc) struct bt_cap_initiator_proc_param *proc_param; struct bt_cap_stream *cap_stream; struct bt_bap_stream *bap_stream; + enum bt_bap_ep_state state; proc_param = &active_proc->proc_param.initiator[i]; cap_stream = proc_param->stream; bap_stream = &cap_stream->bap_stream; + state = stream_get_state(bap_stream); switch (subproc_type) { case BT_CAP_COMMON_SUBPROC_TYPE_CODEC_CONFIG: - if (stream_is_in_state(bap_stream, BT_BAP_EP_STATE_IDLE)) { + if (state == BT_BAP_EP_STATE_IDLE) { return proc_param; } break; case BT_CAP_COMMON_SUBPROC_TYPE_QOS_CONFIG: - if (stream_is_in_state(bap_stream, BT_BAP_EP_STATE_CODEC_CONFIGURED)) { + if (state == BT_BAP_EP_STATE_CODEC_CONFIGURED) { return proc_param; } break; case BT_CAP_COMMON_SUBPROC_TYPE_ENABLE: - if (stream_is_in_state(bap_stream, BT_BAP_EP_STATE_QOS_CONFIGURED)) { + if (state == BT_BAP_EP_STATE_QOS_CONFIGURED) { return proc_param; } break; case BT_CAP_COMMON_SUBPROC_TYPE_CONNECT: - if (stream_is_in_state(bap_stream, BT_BAP_EP_STATE_ENABLING) && - !proc_param->start.connected) { + if (state == BT_BAP_EP_STATE_ENABLING && !proc_param->start.connected) { return proc_param; } break; case BT_CAP_COMMON_SUBPROC_TYPE_START: - if (stream_is_in_state(bap_stream, BT_BAP_EP_STATE_ENABLING)) { + if (state == BT_BAP_EP_STATE_ENABLING) { /* TODO: Add check for connected */ return proc_param; } break; case BT_CAP_COMMON_SUBPROC_TYPE_META_UPDATE: - if (stream_is_in_state(bap_stream, BT_BAP_EP_STATE_ENABLING) || - stream_is_in_state(bap_stream, BT_BAP_EP_STATE_STREAMING)) { + if (state == BT_BAP_EP_STATE_ENABLING || + state == BT_BAP_EP_STATE_STREAMING) { + return proc_param; + } + break; + case BT_CAP_COMMON_SUBPROC_TYPE_DISABLE: + if (state == BT_BAP_EP_STATE_ENABLING || + state == BT_BAP_EP_STATE_STREAMING) { + return proc_param; + } + break; + case BT_CAP_COMMON_SUBPROC_TYPE_STOP: + if (state == BT_BAP_EP_STATE_DISABLING) { return proc_param; } break; case BT_CAP_COMMON_SUBPROC_TYPE_RELEASE: - if (!stream_is_in_state(bap_stream, BT_BAP_EP_STATE_IDLE)) { + if (proc_param->stop.release && state != BT_BAP_EP_STATE_IDLE && + state != BT_BAP_EP_STATE_CODEC_CONFIGURED) { return proc_param; } break; @@ -848,6 +872,7 @@ static int cap_initiator_unicast_audio_configure( /* Since BAP operations may require a write long or a read long on the notification, * we cannot assume that we can do multiple streams at once, thus do it one at a time. * TODO: We should always be able to do one per ACL, so there is room for optimization. + * This applies to all BAP calls in this file. */ err = bt_bap_stream_config(conn, bap_stream, ep, codec_cfg); if (err != 0) { @@ -946,12 +971,6 @@ void bt_cap_initiator_codec_configured(struct bt_cap_stream *cap_stream) next_bap_stream = &next_cap_stream->bap_stream; active_proc->proc_initiated_cnt++; - /* Since BAP operations may require a write long or a read long on the notification, - * we cannot assume that we can do multiple streams at once, thus do it one at a - * time. - * TODO: We should always be able to do one per ACL, so there is room for - * optimization. - */ err = bt_bap_stream_config(conn, next_bap_stream, ep, codec_cfg); if (err != 0) { LOG_DBG("Failed to config stream %p: %d", next_cap_stream, err); @@ -1042,10 +1061,6 @@ void bt_cap_initiator_codec_configured(struct bt_cap_stream *cap_stream) void bt_cap_initiator_qos_configured(struct bt_cap_stream *cap_stream) { struct bt_cap_common_proc *active_proc = bt_cap_common_get_active_proc(); - struct bt_cap_initiator_proc_param *proc_param; - struct bt_cap_stream *next_cap_stream; - struct bt_bap_stream *bap_stream; - int err; if (!bt_cap_common_stream_in_active_proc(cap_stream)) { /* State change happened outside of a procedure; ignore */ @@ -1054,7 +1069,9 @@ void bt_cap_initiator_qos_configured(struct bt_cap_stream *cap_stream) LOG_DBG("cap_stream %p", cap_stream); - if (!bt_cap_common_subproc_is_type(BT_CAP_COMMON_SUBPROC_TYPE_QOS_CONFIG)) { + if (!(bt_cap_common_proc_is_type(BT_CAP_COMMON_PROC_TYPE_START) && + bt_cap_common_subproc_is_type(BT_CAP_COMMON_SUBPROC_TYPE_QOS_CONFIG)) && + !(bt_cap_common_proc_is_type(BT_CAP_COMMON_PROC_TYPE_STOP))) { /* Unexpected callback - Abort */ bt_cap_common_abort_proc(cap_stream->bap_stream.conn, -EBADMSG); } else { @@ -1072,36 +1089,60 @@ void bt_cap_initiator_qos_configured(struct bt_cap_stream *cap_stream) return; } - if (!bt_cap_common_proc_is_done()) { - /* Not yet finished, wait for all */ - return; - } + if (bt_cap_common_proc_is_type(BT_CAP_COMMON_PROC_TYPE_START)) { + struct bt_cap_initiator_proc_param *proc_param; + struct bt_cap_stream *next_cap_stream; + struct bt_bap_stream *bap_stream; + int err; - bt_cap_common_set_subproc(BT_CAP_COMMON_SUBPROC_TYPE_ENABLE); - proc_param = get_next_proc_param(active_proc); - if (proc_param == NULL) { - /* If proc_param is NULL then this step is a no-op and we can skip to the next step - */ - bt_cap_initiator_enabled(active_proc->proc_param.initiator[0].stream); + if (!bt_cap_common_proc_is_done()) { + /* Not yet finished, wait for all */ + return; + } - return; - } + bt_cap_common_set_subproc(BT_CAP_COMMON_SUBPROC_TYPE_ENABLE); + proc_param = get_next_proc_param(active_proc); + if (proc_param == NULL) { + /* If proc_param is NULL then this step is a no-op and we can skip to the + * next step + */ + bt_cap_initiator_enabled(active_proc->proc_param.initiator[0].stream); - next_cap_stream = proc_param->stream; - bap_stream = &next_cap_stream->bap_stream; - active_proc->proc_initiated_cnt++; + return; + } - /* Since BAP operations may require a write long or a read long on the notification, we - * cannot assume that we can do multiple streams at once, thus do it one at a time. - * TODO: We should always be able to do one per ACL, so there is room for optimization. - */ - err = bt_bap_stream_enable(bap_stream, bap_stream->codec_cfg->meta, - bap_stream->codec_cfg->meta_len); - if (err != 0) { - LOG_DBG("Failed to enable stream %p: %d", next_cap_stream, err); + next_cap_stream = proc_param->stream; + bap_stream = &next_cap_stream->bap_stream; + active_proc->proc_initiated_cnt++; - bt_cap_common_abort_proc(bap_stream->conn, err); - cap_initiator_unicast_audio_proc_complete(); + err = bt_bap_stream_enable(bap_stream, bap_stream->codec_cfg->meta, + bap_stream->codec_cfg->meta_len); + if (err != 0) { + LOG_DBG("Failed to enable stream %p: %d", next_cap_stream, err); + + bt_cap_common_abort_proc(bap_stream->conn, err); + cap_initiator_unicast_audio_proc_complete(); + } + } else if (bt_cap_common_subproc_is_type(BT_CAP_COMMON_SUBPROC_TYPE_RELEASE)) { + struct bt_cap_initiator_proc_param *proc_param; + struct bt_cap_stream *next_cap_stream; + struct bt_bap_stream *next_bap_stream; + int err; + + proc_param = get_next_proc_param(active_proc); + __ASSERT(proc_param != NULL, "proc is not done, but could not get next proc_param"); + + next_cap_stream = proc_param->stream; + next_bap_stream = &next_cap_stream->bap_stream; + active_proc->proc_initiated_cnt++; + + err = bt_bap_stream_release(next_bap_stream); + if (err != 0) { + LOG_DBG("Failed to release stream %p: %d", next_cap_stream, err); + + bt_cap_common_abort_proc(next_bap_stream->conn, err); + cap_initiator_unicast_audio_proc_complete(); + } } } @@ -1148,12 +1189,6 @@ void bt_cap_initiator_enabled(struct bt_cap_stream *cap_stream) active_proc->proc_initiated_cnt++; - /* Since BAP operations may require a write long or a read long on the notification, - * we cannot assume that we can do multiple streams at once, thus do it one at a - * time. - * TODO: We should always be able to do one per ACL, so there is room for - * optimization. - */ err = bt_bap_stream_enable(next_bap_stream, next_bap_stream->codec_cfg->meta, next_bap_stream->codec_cfg->meta_len); if (err != 0) { @@ -1276,12 +1311,6 @@ void bt_cap_initiator_connected(struct bt_cap_stream *cap_stream) bap_stream = &proc_param->stream->bap_stream; if (stream_is_dir(bap_stream, BT_AUDIO_DIR_SOURCE)) { - /* Since BAP operations may require a write long or a read long on the notification, - * we cannot assume that we can do multiple streams at once, thus do it one at a - * time. - * TODO: We should always be able to do one per ACL, so there is room for - * optimization. - */ err = bt_bap_stream_start(bap_stream); if (err != 0) { LOG_DBG("Failed to start stream %p: %d", proc_param->stream, err); @@ -1330,12 +1359,6 @@ void bt_cap_initiator_started(struct bt_cap_stream *cap_stream) if (stream_is_dir(next_bap_stream, BT_AUDIO_DIR_SOURCE)) { int err; - /* Since BAP operations may require a write long or a read long on - * the notification, we cannot assume that we can do multiple - * streams at once, thus do it one at a time. - * TODO: We should always be able to do one per ACL, so there is - * room for optimization. - */ err = bt_bap_stream_start(next_bap_stream); if (err != 0) { LOG_DBG("Failed to start stream %p: %d", next_cap_stream, err); @@ -1478,6 +1501,9 @@ int bt_cap_initiator_unicast_audio_update(const struct bt_cap_unicast_audio_upda ¶m->stream_params[i]; struct bt_cap_stream *cap_stream = stream_param->stream; + /* Ensure that ops are registered before any procedures are started */ + bt_cap_stream_ops_register_bap(cap_stream); + active_proc->proc_param.initiator[i].stream = cap_stream; active_proc->proc_param.initiator[i].meta_update.meta_len = stream_param->meta_len; memcpy(&active_proc->proc_param.initiator[i].meta_update.meta, stream_param->meta, @@ -1562,13 +1588,6 @@ void bt_cap_initiator_metadata_updated(struct bt_cap_stream *cap_stream) bap_stream = &next_cap_stream->bap_stream; active_proc->proc_initiated_cnt++; - /* Since BAP operations may require a write long or a read long on the notification, - * we cannot assume that we can do multiple streams at once, thus do it one at a - * time. - * TODO: We should always be able to do one per ACL, so there is room for - * optimization. - */ - err = bt_bap_stream_metadata(bap_stream, meta, meta_len); if (err != 0) { LOG_DBG("Failed to update metadata for stream %p: %d", next_cap_stream, @@ -1584,13 +1603,43 @@ void bt_cap_initiator_metadata_updated(struct bt_cap_stream *cap_stream) cap_initiator_unicast_audio_proc_complete(); } -static bool can_release(const struct bt_bap_stream *bap_stream) +static bool can_release_stream(const struct bt_bap_stream *bap_stream) +{ + enum bt_bap_ep_state state; + + if (bap_stream->conn == NULL) { + return false; + } + + state = stream_get_state(bap_stream); + + /* We cannot release idle endpoints. + * We do not know if we can release endpoints in the Codec Configured state as servers may + * cache it, so treat it as idle + */ + return state != BT_BAP_EP_STATE_IDLE && state != BT_BAP_EP_STATE_CODEC_CONFIGURED; +} + +static bool can_disable_stream(const struct bt_bap_stream *bap_stream) +{ + enum bt_bap_ep_state state; + + if (bap_stream->conn == NULL) { + return false; + } + + state = stream_get_state(bap_stream); + + return state == BT_BAP_EP_STATE_STREAMING || state == BT_BAP_EP_STATE_ENABLING; +} + +static bool can_stop_stream(const struct bt_bap_stream *bap_stream) { if (bap_stream->conn == NULL) { return false; } - return !stream_is_in_state(bap_stream, BT_BAP_EP_STATE_IDLE); + return stream_is_in_state(bap_stream, BT_BAP_EP_STATE_DISABLING); } static bool valid_unicast_audio_stop_param(const struct bt_cap_unicast_audio_stop_param *param) @@ -1637,6 +1686,18 @@ static bool valid_unicast_audio_stop_param(const struct bt_cap_unicast_audio_sto return -EINVAL; } + if (param->type == BT_CAP_SET_TYPE_CSIP) { + struct bt_cap_common_client *client = bt_cap_common_get_client_by_acl(conn); + + if (client->csis_inst == NULL) { + LOG_DBG("param->streams[%zu]->bap_stream.conn not part of a " + "coordinated set", + i); + + return false; + } + } + CHECKIF(bap_stream->group == NULL) { LOG_DBG("param->streams[%zu] is not in a unicast group", i); return false; @@ -1653,12 +1714,6 @@ static bool valid_unicast_audio_stop_param(const struct bt_cap_unicast_audio_sto } } - if (!can_release(bap_stream)) { - LOG_DBG("Cannot stop param->streams[%zu]", i); - - return false; - } - for (size_t j = 0U; j < i; j++) { if (param->streams[j] == cap_stream) { LOG_DBG("param->stream_params[%zu] (%p) is " @@ -1676,8 +1731,9 @@ static bool valid_unicast_audio_stop_param(const struct bt_cap_unicast_audio_sto int bt_cap_initiator_unicast_audio_stop(const struct bt_cap_unicast_audio_stop_param *param) { struct bt_cap_common_proc *active_proc = bt_cap_common_get_active_proc(); - struct bt_cap_initiator_proc_param *proc_param; - struct bt_bap_stream *bap_stream; + bool can_release = false; + bool can_disable = false; + bool can_stop = false; int err; if (bt_cap_common_proc_is_active()) { @@ -1692,34 +1748,259 @@ int bt_cap_initiator_unicast_audio_stop(const struct bt_cap_unicast_audio_stop_p for (size_t i = 0U; i < param->count; i++) { struct bt_cap_stream *cap_stream = param->streams[i]; + struct bt_bap_stream *bap_stream = &cap_stream->bap_stream; + + /* Ensure that ops are registered before any procedures are started */ + bt_cap_stream_ops_register_bap(cap_stream); active_proc->proc_param.initiator[i].stream = cap_stream; - } + active_proc->proc_param.initiator[i].stop.release = param->release; - bt_cap_common_start_proc(BT_CAP_COMMON_PROC_TYPE_STOP, param->count); + if (!can_disable && can_disable_stream(bap_stream)) { + can_disable = true; + } - bt_cap_common_set_subproc(BT_CAP_COMMON_SUBPROC_TYPE_RELEASE); + if (!can_stop && can_stop_stream(bap_stream)) { + can_stop = true; + } + if (!can_release && param->release && can_release_stream(bap_stream)) { + can_release = true; + } + } + + if (!can_disable && !can_stop && !can_release) { + LOG_DBG("Cannot %s any streams", !can_disable ? "disable" + : !can_stop ? "stop" + : "release"); + return -EALREADY; + } + + bt_cap_common_start_proc(BT_CAP_COMMON_PROC_TYPE_STOP, param->count); /** TODO: If this is a CSIP set, then the order of the procedures may * not match the order in the parameters, and the CSIP ordered access * procedure should be used. */ - proc_param = get_next_proc_param(active_proc); - __ASSERT(proc_param != NULL, "proc is not done, but could not get next proc_param"); - bap_stream = &proc_param->stream->bap_stream; - active_proc->proc_initiated_cnt++; + if (can_disable) { + struct bt_cap_initiator_proc_param *proc_param; + struct bt_bap_stream *bap_stream; - err = bt_bap_stream_release(bap_stream); - if (err != 0) { - LOG_DBG("Failed to stop bap_stream %p: %d", proc_param->stream, err); + bt_cap_common_set_subproc(BT_CAP_COMMON_SUBPROC_TYPE_DISABLE); - bt_cap_common_clear_active_proc(); + proc_param = get_next_proc_param(active_proc); + __ASSERT(proc_param != NULL, + "proc is not started, but could not get next proc_param"); + bap_stream = &proc_param->stream->bap_stream; + active_proc->proc_initiated_cnt++; + + err = bt_bap_stream_disable(bap_stream); + if (err != 0) { + LOG_DBG("Failed to disable bap_stream %p: %d", proc_param->stream, err); + + bt_cap_common_clear_active_proc(); + } + } else if (can_stop) { + struct bt_cap_initiator_proc_param *proc_param; + struct bt_bap_stream *bap_stream; + + bt_cap_common_set_subproc(BT_CAP_COMMON_SUBPROC_TYPE_STOP); + + proc_param = get_next_proc_param(active_proc); + __ASSERT(proc_param != NULL, + "proc is not started, but could not get next proc_param"); + bap_stream = &proc_param->stream->bap_stream; + active_proc->proc_initiated_cnt++; + + err = bt_bap_stream_stop(bap_stream); + if (err != 0) { + LOG_DBG("Failed to stop bap_stream %p: %d", proc_param->stream, err); + + bt_cap_common_clear_active_proc(); + } + } else { + struct bt_cap_initiator_proc_param *proc_param; + struct bt_bap_stream *bap_stream; + + bt_cap_common_set_subproc(BT_CAP_COMMON_SUBPROC_TYPE_RELEASE); + + proc_param = get_next_proc_param(active_proc); + __ASSERT(proc_param != NULL, + "proc is not started, but could not get next proc_param"); + bap_stream = &proc_param->stream->bap_stream; + active_proc->proc_initiated_cnt++; + + err = bt_bap_stream_release(bap_stream); + if (err != 0) { + LOG_DBG("Failed to release bap_stream %p: %d", proc_param->stream, err); + + bt_cap_common_clear_active_proc(); + } } return err; } +void bt_cap_initiator_disabled(struct bt_cap_stream *cap_stream) +{ + struct bt_cap_common_proc *active_proc = bt_cap_common_get_active_proc(); + + if (!bt_cap_common_stream_in_active_proc(cap_stream)) { + /* State change happened outside of a procedure; ignore */ + return; + } + + if (!bt_cap_common_subproc_is_type(BT_CAP_COMMON_SUBPROC_TYPE_DISABLE)) { + /* Unexpected callback - Abort */ + bt_cap_common_abort_proc(cap_stream->bap_stream.conn, -EBADMSG); + } else { + update_proc_done_cnt(active_proc); + + LOG_DBG("Stream %p disabled (%zu/%zu streams done)", cap_stream, + active_proc->proc_done_cnt, active_proc->proc_cnt); + } + + if (bt_cap_common_proc_is_aborted()) { + if (bt_cap_common_proc_all_handled()) { + cap_initiator_unicast_audio_proc_complete(); + } + + return; + } + + if (!bt_cap_common_proc_is_done()) { + struct bt_cap_initiator_proc_param *proc_param; + struct bt_cap_stream *next_cap_stream; + struct bt_bap_stream *next_bap_stream; + int err; + + proc_param = get_next_proc_param(active_proc); + __ASSERT(proc_param != NULL, "proc is not done, but could not get next proc_param"); + next_cap_stream = proc_param->stream; + next_bap_stream = &next_cap_stream->bap_stream; + active_proc->proc_initiated_cnt++; + + err = bt_bap_stream_disable(next_bap_stream); + if (err != 0) { + LOG_DBG("Failed to disable stream %p: %d", next_cap_stream, err); + + bt_cap_common_abort_proc(next_bap_stream->conn, err); + cap_initiator_unicast_audio_proc_complete(); + } + } else { + struct bt_cap_initiator_proc_param *proc_param; + struct bt_cap_stream *next_cap_stream; + struct bt_bap_stream *next_bap_stream; + int err; + + bt_cap_common_set_subproc(BT_CAP_COMMON_SUBPROC_TYPE_STOP); + + proc_param = get_next_proc_param(active_proc); + if (proc_param == NULL) { + /* If proc_param is NULL then this step is a no-op and we can skip to the + * next step + */ + bt_cap_initiator_stopped(active_proc->proc_param.initiator[0].stream); + + return; + } + + next_cap_stream = proc_param->stream; + next_bap_stream = &next_cap_stream->bap_stream; + active_proc->proc_initiated_cnt++; + + err = bt_bap_stream_stop(next_bap_stream); + if (err != 0) { + LOG_DBG("Failed to stop stream %p: %d", next_cap_stream, err); + + bt_cap_common_abort_proc(next_bap_stream->conn, err); + cap_initiator_unicast_audio_proc_complete(); + } + } +} + +void bt_cap_initiator_stopped(struct bt_cap_stream *cap_stream) +{ + struct bt_cap_common_proc *active_proc = bt_cap_common_get_active_proc(); + + if (!bt_cap_common_stream_in_active_proc(cap_stream)) { + /* State change happened outside of a procedure; ignore */ + return; + } + + if (!bt_cap_common_proc_is_type(BT_CAP_COMMON_PROC_TYPE_STOP)) { + /* Unexpected callback - Abort */ + bt_cap_common_abort_proc(cap_stream->bap_stream.conn, -EBADMSG); + } else { + if (bt_cap_common_subproc_is_type(BT_CAP_COMMON_SUBPROC_TYPE_STOP)) { + update_proc_done_cnt(active_proc); + + LOG_DBG("Stream %p stopped (%zu/%zu streams done)", cap_stream, + active_proc->proc_done_cnt, active_proc->proc_cnt); + } else { + /* We are still doing disable - Wait for those to be done, as stopped may + * also be called when we are disabling sink ASEs + */ + return; + } + } + + if (bt_cap_common_proc_is_aborted()) { + if (bt_cap_common_proc_all_handled()) { + cap_initiator_unicast_audio_proc_complete(); + } + + return; + } + + if (!bt_cap_common_proc_is_done()) { + struct bt_cap_initiator_proc_param *proc_param; + struct bt_cap_stream *next_cap_stream; + struct bt_bap_stream *next_bap_stream; + int err; + + proc_param = get_next_proc_param(active_proc); + __ASSERT(proc_param != NULL, "proc is not done, but could not get next proc_param"); + next_cap_stream = proc_param->stream; + next_bap_stream = &next_cap_stream->bap_stream; + + active_proc->proc_initiated_cnt++; + + err = bt_bap_stream_stop(next_bap_stream); + if (err != 0) { + LOG_DBG("Failed to stop stream %p: %d", next_cap_stream, err); + + bt_cap_common_abort_proc(next_bap_stream->conn, err); + cap_initiator_unicast_audio_proc_complete(); + } + } else { + /* We are done stopping streams now - We mark the next subproc. If + * get_next_proc_param returns a NULL value it means that we are complete done. If + * it returns a non-NULL value, it means that we need to start releasing streams. + * However, since the QoS Configured state is better suited to trigger this, we + * simply wait until bt_cap_initiator_qos_configured is called. + */ + struct bt_cap_initiator_proc_param *proc_param; + + if (!bt_cap_common_proc_is_done()) { + /* We are still disabling or stopping some */ + return; + } + + bt_cap_common_set_subproc(BT_CAP_COMMON_SUBPROC_TYPE_RELEASE); + + proc_param = get_next_proc_param(active_proc); + if (proc_param == NULL) { + /* If proc_param is NULL then this step is a no-op and we can finish the + * procedure + */ + cap_initiator_unicast_audio_proc_complete(); + + return; + } /* wait for bt_cap_initiator_qos_configured */ + } +} + void bt_cap_initiator_released(struct bt_cap_stream *cap_stream) { struct bt_cap_common_proc *active_proc = bt_cap_common_get_active_proc(); @@ -1748,23 +2029,22 @@ void bt_cap_initiator_released(struct bt_cap_stream *cap_stream) } if (!bt_cap_common_proc_is_done()) { - struct bt_cap_stream *next_cap_stream = - active_proc->proc_param.initiator[active_proc->proc_done_cnt].stream; - struct bt_bap_stream *bap_stream = &next_cap_stream->bap_stream; + struct bt_cap_initiator_proc_param *proc_param; + struct bt_cap_stream *next_cap_stream; + struct bt_bap_stream *next_bap_stream; int err; + proc_param = get_next_proc_param(active_proc); + __ASSERT(proc_param != NULL, "proc is not done, but could not get next proc_param"); + next_cap_stream = proc_param->stream; + next_bap_stream = &next_cap_stream->bap_stream; active_proc->proc_initiated_cnt++; - /* Since BAP operations may require a write long or a read long on the - * notification, we cannot assume that we can do multiple streams at once, - * thus do it one at a time. - * TODO: We should always be able to do one per ACL, so there is room for - * optimization. - */ - err = bt_bap_stream_release(bap_stream); + + err = bt_bap_stream_release(next_bap_stream); if (err != 0) { LOG_DBG("Failed to release stream %p: %d", next_cap_stream, err); - bt_cap_common_abort_proc(bap_stream->conn, err); + bt_cap_common_abort_proc(next_bap_stream->conn, err); cap_initiator_unicast_audio_proc_complete(); } } else { diff --git a/subsys/bluetooth/audio/cap_internal.h b/subsys/bluetooth/audio/cap_internal.h index 839d4a31757..3543b35c387 100644 --- a/subsys/bluetooth/audio/cap_internal.h +++ b/subsys/bluetooth/audio/cap_internal.h @@ -30,6 +30,8 @@ void bt_cap_initiator_enabled(struct bt_cap_stream *cap_stream); void bt_cap_initiator_started(struct bt_cap_stream *cap_stream); void bt_cap_initiator_connected(struct bt_cap_stream *cap_stream); void bt_cap_initiator_metadata_updated(struct bt_cap_stream *cap_stream); +void bt_cap_initiator_disabled(struct bt_cap_stream *cap_stream); +void bt_cap_initiator_stopped(struct bt_cap_stream *cap_stream); void bt_cap_initiator_released(struct bt_cap_stream *cap_stream); void bt_cap_stream_ops_register_bap(struct bt_cap_stream *cap_stream); @@ -61,6 +63,8 @@ enum bt_cap_common_subproc_type { BT_CAP_COMMON_SUBPROC_TYPE_CONNECT, BT_CAP_COMMON_SUBPROC_TYPE_START, BT_CAP_COMMON_SUBPROC_TYPE_META_UPDATE, + BT_CAP_COMMON_SUBPROC_TYPE_DISABLE, + BT_CAP_COMMON_SUBPROC_TYPE_STOP, BT_CAP_COMMON_SUBPROC_TYPE_RELEASE, }; @@ -79,6 +83,9 @@ struct bt_cap_initiator_proc_param { /** Codec Specific Capabilities Metadata */ uint8_t meta[CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE]; } meta_update; + struct { + bool release; + } stop; }; }; @@ -173,6 +180,7 @@ struct bt_cap_common_proc *bt_cap_common_get_active_proc(void); void bt_cap_common_clear_active_proc(void); void bt_cap_common_start_proc(enum bt_cap_common_proc_type proc_type, size_t proc_cnt); void bt_cap_common_set_subproc(enum bt_cap_common_subproc_type subproc_type); +bool bt_cap_common_proc_is_type(enum bt_cap_common_proc_type proc_type); bool bt_cap_common_subproc_is_type(enum bt_cap_common_subproc_type subproc_type); struct bt_conn *bt_cap_common_get_member_conn(enum bt_cap_set_type type, const union bt_cap_set_member *member); diff --git a/subsys/bluetooth/audio/cap_stream.c b/subsys/bluetooth/audio/cap_stream.c index e1ee733a448..bbc645daa3e 100644 --- a/subsys/bluetooth/audio/cap_stream.c +++ b/subsys/bluetooth/audio/cap_stream.c @@ -132,6 +132,11 @@ static void cap_stream_disabled_cb(struct bt_bap_stream *bap_stream) LOG_DBG("%p", cap_stream); + if (IS_ENABLED(CONFIG_BT_CAP_INITIATOR) && IS_ENABLED(CONFIG_BT_BAP_UNICAST_CLIENT) && + stream_is_central(bap_stream)) { + bt_cap_initiator_disabled(cap_stream); + } + if (ops != NULL && ops->disabled != NULL) { ops->disabled(bap_stream); } @@ -188,6 +193,11 @@ static void cap_stream_stopped_cb(struct bt_bap_stream *bap_stream, uint8_t reas LOG_DBG("%p", cap_stream); + if (IS_ENABLED(CONFIG_BT_CAP_INITIATOR) && IS_ENABLED(CONFIG_BT_BAP_UNICAST_CLIENT) && + stream_is_central(bap_stream)) { + bt_cap_initiator_stopped(cap_stream); + } + if (ops != NULL && ops->stopped != NULL) { ops->stopped(bap_stream, reason); } diff --git a/subsys/bluetooth/audio/shell/cap_initiator.c b/subsys/bluetooth/audio/shell/cap_initiator.c index 197c46059be..3027edf4d73 100644 --- a/subsys/bluetooth/audio/shell/cap_initiator.c +++ b/subsys/bluetooth/audio/shell/cap_initiator.c @@ -532,6 +532,7 @@ static int cmd_cap_initiator_unicast_stop(const struct shell *sh, size_t argc, param.streams = streams; param.type = BT_CAP_SET_TYPE_AD_HOC; + param.release = true; err = bt_cap_initiator_unicast_audio_stop(¶m); if (err != 0) { diff --git a/tests/bluetooth/audio/cap_initiator/CMakeLists.txt b/tests/bluetooth/audio/cap_initiator/CMakeLists.txt index 2c7096e8c32..8502038e975 100644 --- a/tests/bluetooth/audio/cap_initiator/CMakeLists.txt +++ b/tests/bluetooth/audio/cap_initiator/CMakeLists.txt @@ -17,4 +17,5 @@ target_sources(testbinary src/main.c src/test_common.c src/test_unicast_start.c + src/test_unicast_stop.c ) diff --git a/tests/bluetooth/audio/cap_initiator/prj.conf b/tests/bluetooth/audio/cap_initiator/prj.conf index 18afc819e24..7f34c72a09a 100644 --- a/tests/bluetooth/audio/cap_initiator/prj.conf +++ b/tests/bluetooth/audio/cap_initiator/prj.conf @@ -22,3 +22,5 @@ CONFIG_BT_BAP_UNICAST_CLIENT_ASE_SRC_COUNT=2 CONFIG_ASSERT=y CONFIG_ASSERT_LEVEL=2 CONFIG_ASSERT_VERBOSE=y + +CONFIG_BT_BAP_STREAM_LOG_LEVEL_DBG=y diff --git a/tests/bluetooth/audio/cap_initiator/src/main.c b/tests/bluetooth/audio/cap_initiator/src/main.c index 54b78f0efe2..a15c1fc1c1f 100644 --- a/tests/bluetooth/audio/cap_initiator/src/main.c +++ b/tests/bluetooth/audio/cap_initiator/src/main.c @@ -24,8 +24,6 @@ #include "ztest_assert.h" #include "ztest_test.h" -DEFINE_FFF_GLOBALS; - static void mock_init_rule_before(const struct ztest_unit_test *test, void *fixture) { test_mocks_init(); diff --git a/tests/bluetooth/audio/cap_initiator/src/test_common.c b/tests/bluetooth/audio/cap_initiator/src/test_common.c index dbb0c8a9470..fc8d2b10ae2 100644 --- a/tests/bluetooth/audio/cap_initiator/src/test_common.c +++ b/tests/bluetooth/audio/cap_initiator/src/test_common.c @@ -18,6 +18,8 @@ #include "test_common.h" #include "ztest_assert.h" +DEFINE_FFF_GLOBALS; + void test_mocks_init(void) { mock_cap_initiator_init(); diff --git a/tests/bluetooth/audio/cap_initiator/src/test_unicast_start.c b/tests/bluetooth/audio/cap_initiator/src/test_unicast_start.c index 7ff7f2472ff..27eb4773926 100644 --- a/tests/bluetooth/audio/cap_initiator/src/test_unicast_start.c +++ b/tests/bluetooth/audio/cap_initiator/src/test_unicast_start.c @@ -30,8 +30,6 @@ #include "ztest_assert.h" #include "ztest_test.h" -#define FFF_GLOBALS - struct cap_initiator_test_unicast_start_fixture { struct bt_cap_stream cap_streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT]; struct bt_bap_ep eps[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT]; diff --git a/tests/bluetooth/audio/cap_initiator/src/test_unicast_stop.c b/tests/bluetooth/audio/cap_initiator/src/test_unicast_stop.c new file mode 100644 index 00000000000..50175f588d0 --- /dev/null +++ b/tests/bluetooth/audio/cap_initiator/src/test_unicast_stop.c @@ -0,0 +1,482 @@ +/* test_unicast_stop.c - unit test for unicast stop procedure */ + +/* + * Copyright (c) 2024 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "bap_endpoint.h" +#include "cap_initiator.h" +#include "conn.h" +#include "expects_util.h" +#include "test_common.h" +#include "ztest_assert.h" +#include "ztest_test.h" + +struct cap_initiator_test_unicast_stop_fixture { + struct bt_cap_stream cap_streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT]; + struct bt_bap_ep eps[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT]; + struct bt_bap_unicast_group unicast_group; + struct bt_conn conns[CONFIG_BT_MAX_CONN]; + struct bt_bap_lc3_preset preset; +}; + +static void cap_initiator_test_unicast_stop_fixture_init( + struct cap_initiator_test_unicast_stop_fixture *fixture) +{ + fixture->preset = (struct bt_bap_lc3_preset)BT_BAP_LC3_UNICAST_PRESET_16_2_1( + BT_AUDIO_LOCATION_MONO_AUDIO, BT_AUDIO_CONTEXT_TYPE_UNSPECIFIED); + + for (size_t i = 0U; i < ARRAY_SIZE(fixture->conns); i++) { + test_conn_init(&fixture->conns[i]); + } + + for (size_t i = 0U; i < ARRAY_SIZE(fixture->eps); i++) { + fixture->eps[i].dir = (i & 1) + 1; /* Makes it either 1 or 2 (sink or source)*/ + } + + for (size_t i = 0U; i < ARRAY_SIZE(fixture->cap_streams); i++) { + struct bt_cap_stream *cap_stream = &fixture->cap_streams[i]; + struct bt_bap_stream *bap_stream = &cap_stream->bap_stream; + + sys_slist_append(&fixture->unicast_group.streams, &bap_stream->_node); + bap_stream->group = &fixture->unicast_group; + } +} + +static void *cap_initiator_test_unicast_stop_setup(void) +{ + struct cap_initiator_test_unicast_stop_fixture *fixture; + + fixture = malloc(sizeof(*fixture)); + zassert_not_null(fixture); + + return fixture; +} + +static void cap_initiator_test_unicast_stop_before(void *f) +{ + int err; + + memset(f, 0, sizeof(struct cap_initiator_test_unicast_stop_fixture)); + cap_initiator_test_unicast_stop_fixture_init(f); + + err = bt_cap_initiator_register_cb(&mock_cap_initiator_cb); + zassert_equal(0, err, "Unexpected return value %d", err); +} + +static void cap_initiator_test_unicast_stop_after(void *f) +{ + struct cap_initiator_test_unicast_stop_fixture *fixture = f; + + bt_cap_initiator_unregister_cb(&mock_cap_initiator_cb); + + for (size_t i = 0; i < ARRAY_SIZE(fixture->conns); i++) { + mock_bt_conn_disconnected(&fixture->conns[i], BT_HCI_ERR_REMOTE_USER_TERM_CONN); + } + + /* In the case of a test failing, we cancel the procedure so that subsequent won't fail */ + bt_cap_initiator_unicast_audio_cancel(); +} + +static void cap_initiator_test_unicast_stop_teardown(void *f) +{ + free(f); +} + +ZTEST_SUITE(cap_initiator_test_unicast_stop, NULL, cap_initiator_test_unicast_stop_setup, + cap_initiator_test_unicast_stop_before, cap_initiator_test_unicast_stop_after, + cap_initiator_test_unicast_stop_teardown); + +static ZTEST_F(cap_initiator_test_unicast_stop, + test_initiator_unicast_stop_disable_state_codec_configured) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = false, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_CODEC_CONFIGURED); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, -EALREADY, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 0, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + const struct bt_bap_stream *bap_stream = &fixture->cap_streams[i].bap_stream; + const enum bt_bap_ep_state state = bap_stream->ep->status.state; + + zassert_equal(state, BT_BAP_EP_STATE_CODEC_CONFIGURED, + "[%zu]: Stream %p unexpected state: %d", i, bap_stream, state); + } +} + +static ZTEST_F(cap_initiator_test_unicast_stop, + test_initiator_unicast_stop_disable_state_qos_configured) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = false, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_QOS_CONFIGURED); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, -EALREADY, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 0, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + const struct bt_bap_stream *bap_stream = &fixture->cap_streams[i].bap_stream; + const enum bt_bap_ep_state state = bap_stream->ep->status.state; + + zassert_equal(state, BT_BAP_EP_STATE_QOS_CONFIGURED, + "[%zu]: Stream %p unexpected state: %d", i, bap_stream, state); + } +} + +static ZTEST_F(cap_initiator_test_unicast_stop, test_initiator_unicast_stop_disable_state_enabling) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = false, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_ENABLING); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, 0, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 1, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + const struct bt_bap_stream *bap_stream = &fixture->cap_streams[i].bap_stream; + const enum bt_bap_ep_state state = bap_stream->ep->status.state; + + zassert_equal(state, BT_BAP_EP_STATE_QOS_CONFIGURED, + "[%zu]: Stream %p unexpected state: %d", i, bap_stream, state); + } +} + +static ZTEST_F(cap_initiator_test_unicast_stop, test_initiator_unicast_stop_disable_state_streaming) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = false, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_STREAMING); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, 0, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 1, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); + + for (size_t i = 0U; i < ARRAY_SIZE(fixture->cap_streams); i++) { + const struct bt_bap_stream *bap_stream = &fixture->cap_streams[i].bap_stream; + const enum bt_bap_ep_state state = bap_stream->ep->status.state; + + zassert_equal(state, BT_BAP_EP_STATE_QOS_CONFIGURED, + "[%zu]: Stream %p unexpected state: %d", i, bap_stream, state); + } +} + +static ZTEST_F(cap_initiator_test_unicast_stop, + test_initiator_unicast_stop_release_state_codec_configured) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = true, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_CODEC_CONFIGURED); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, -EALREADY, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 0, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + const struct bt_bap_stream *bap_stream = &fixture->cap_streams[i].bap_stream; + const enum bt_bap_ep_state state = bap_stream->ep->status.state; + + zassert_equal(state, BT_BAP_EP_STATE_CODEC_CONFIGURED, + "[%zu]: Stream %p unexpected state: %d", i, bap_stream, state); + } +} + +static ZTEST_F(cap_initiator_test_unicast_stop, + test_initiator_unicast_stop_release_state_qos_configured) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = true, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_QOS_CONFIGURED); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, 0, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 1, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + const struct bt_bap_stream *bap_stream = &fixture->cap_streams[i].bap_stream; + const enum bt_bap_ep_state state = fixture->eps[i].status.state; + + zassert_equal(state, BT_BAP_EP_STATE_IDLE, "[%zu]: Stream %p unexpected state: %d", + i, bap_stream, state); + } +} + +static ZTEST_F(cap_initiator_test_unicast_stop, test_initiator_unicast_stop_release_state_enabling) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = true, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_ENABLING); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, 0, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 1, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + const struct bt_bap_stream *bap_stream = &fixture->cap_streams[i].bap_stream; + const enum bt_bap_ep_state state = fixture->eps[i].status.state; + + zassert_equal(state, BT_BAP_EP_STATE_IDLE, "[%zu]: Stream %p unexpected state: %d", + i, bap_stream, state); + } +} + +static ZTEST_F(cap_initiator_test_unicast_stop, test_initiator_unicast_stop_release_state_streaming) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = true, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_ENABLING); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, 0, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 1, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); + + for (size_t i = 0U; i < ARRAY_SIZE(fixture->cap_streams); i++) { + const struct bt_bap_stream *bap_stream = &fixture->cap_streams[i].bap_stream; + const enum bt_bap_ep_state state = fixture->eps[i].status.state; + + zassert_equal(state, BT_BAP_EP_STATE_IDLE, "[%zu]: Stream %p unexpected state: %d", + i, bap_stream, state); + } +} + +static ZTEST_F(cap_initiator_test_unicast_stop, test_initiator_unicast_stop_inval_param_null) +{ + int err; + + err = bt_cap_initiator_unicast_audio_stop(NULL); + zassert_equal(err, -EINVAL, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 0, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); +} + +static ZTEST_F(cap_initiator_test_unicast_stop, + test_initiator_unicast_stop_inval_param_null_streams) +{ + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT, + .streams = NULL, + .release = true, + }; + int err; + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, -EINVAL, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 0, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); +} + +static ZTEST_F(cap_initiator_test_unicast_stop, test_initiator_unicast_stop_inval_missing_cas) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_CSIP, /* CSIP requires CAS */ + .count = ARRAY_SIZE(streams), + .streams = streams, + .release = true, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + + test_unicast_set_state(streams[i], &fixture->conns[i % ARRAY_SIZE(fixture->conns)], + &fixture->eps[i], &fixture->preset, + BT_BAP_EP_STATE_STREAMING); + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, -EINVAL, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 0, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); +} + +static ZTEST_F(cap_initiator_test_unicast_stop, test_initiator_unicast_stop_inval_param_zero_count) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = 0U, + .streams = streams, + .release = true, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, -EINVAL, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 0, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); +} + +static ZTEST_F(cap_initiator_test_unicast_stop, test_initiator_unicast_stop_inval_param_inval_count) +{ + struct bt_cap_stream *streams[CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT] = {0}; + const struct bt_cap_unicast_audio_stop_param param = { + .type = BT_CAP_SET_TYPE_AD_HOC, + .count = CONFIG_BT_BAP_UNICAST_CLIENT_GROUP_STREAM_COUNT + 1U, + .streams = streams, + .release = true, + }; + int err; + + for (size_t i = 0U; i < ARRAY_SIZE(streams); i++) { + streams[i] = &fixture->cap_streams[i]; + } + + err = bt_cap_initiator_unicast_audio_stop(¶m); + zassert_equal(err, -EINVAL, "Unexpected return value %d", err); + + zexpect_call_count("bt_cap_initiator_cb.unicast_stop_complete_cb", 0, + mock_cap_initiator_unicast_stop_complete_cb_fake.call_count); +} diff --git a/tests/bluetooth/audio/cap_initiator/uut/bap_unicast_client.c b/tests/bluetooth/audio/cap_initiator/uut/bap_unicast_client.c index 52feeeb15d3..387aeaf41ba 100644 --- a/tests/bluetooth/audio/cap_initiator/uut/bap_unicast_client.c +++ b/tests/bluetooth/audio/cap_initiator/uut/bap_unicast_client.c @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -149,7 +150,6 @@ int bt_bap_unicast_client_connect(struct bt_bap_stream *stream) if (stream->ep != NULL && stream->ep->dir == BT_AUDIO_DIR_SINK) { /* Mocking that the unicast server automatically starts the stream */ stream->ep->status.state = BT_BAP_EP_STATE_STREAMING; - printk("A %s %p\n", __func__, stream); if (stream->ops != NULL && stream->ops->started != NULL) { stream->ops->started(stream); @@ -184,18 +184,105 @@ int bt_bap_unicast_client_start(struct bt_bap_stream *stream) int bt_bap_unicast_client_disable(struct bt_bap_stream *stream) { - zassert_unreachable("Unexpected call to '%s()' occurred", __func__); + printk("%s %p %d\n", __func__, stream, stream->ep->dir); + + if (stream == NULL || stream->ep == NULL) { + return -EINVAL; + } + + switch (stream->ep->status.state) { + case BT_BAP_EP_STATE_ENABLING: + case BT_BAP_EP_STATE_STREAMING: + break; + default: + return -EINVAL; + } + + /* Even though the ASCS spec does not have the disabling state for sink ASEs, the unicast + * client implementation fakes the behavior of it and always calls the disabled callback + * when leaving the streaming state in a non-release manner + */ + + /* Disabled sink ASEs go directly to the QoS configured state */ + if (stream->ep->dir == BT_AUDIO_DIR_SINK) { + stream->ep->status.state = BT_BAP_EP_STATE_QOS_CONFIGURED; + + if (stream->ops != NULL && stream->ops->disabled != NULL) { + stream->ops->disabled(stream); + } + + if (stream->ops != NULL && stream->ops->stopped != NULL) { + stream->ops->stopped(stream, BT_HCI_ERR_LOCALHOST_TERM_CONN); + } + + if (stream->ops != NULL && stream->ops->qos_set != NULL) { + stream->ops->qos_set(stream); + } + } else if (stream->ep->dir == BT_AUDIO_DIR_SOURCE) { + stream->ep->status.state = BT_BAP_EP_STATE_DISABLING; + + if (stream->ops != NULL && stream->ops->disabled != NULL) { + stream->ops->disabled(stream); + } + } + return 0; } int bt_bap_unicast_client_stop(struct bt_bap_stream *stream) { - zassert_unreachable("Unexpected call to '%s()' occurred", __func__); + printk("%s %p\n", __func__, stream); + + /* As per the ASCS spec, only source streams can be stopped by the client */ + if (stream == NULL || stream->ep == NULL || stream->ep->dir == BT_AUDIO_DIR_SINK) { + return -EINVAL; + } + + switch (stream->ep->status.state) { + case BT_BAP_EP_STATE_DISABLING: + break; + default: + return -EINVAL; + } + + stream->ep->status.state = BT_BAP_EP_STATE_QOS_CONFIGURED; + + if (stream->ops != NULL && stream->ops->stopped != NULL) { + stream->ops->stopped(stream, BT_HCI_ERR_LOCALHOST_TERM_CONN); + } + + if (stream->ops != NULL && stream->ops->qos_set != NULL) { + stream->ops->qos_set(stream); + } + return 0; } int bt_bap_unicast_client_release(struct bt_bap_stream *stream) { - zassert_unreachable("Unexpected call to '%s()' occurred", __func__); + printk("%s %p\n", __func__, stream); + + if (stream == NULL || stream->ep == NULL) { + return -EINVAL; + } + + switch (stream->ep->status.state) { + case BT_BAP_EP_STATE_CODEC_CONFIGURED: + case BT_BAP_EP_STATE_QOS_CONFIGURED: + case BT_BAP_EP_STATE_ENABLING: + case BT_BAP_EP_STATE_STREAMING: + case BT_BAP_EP_STATE_DISABLING: + break; + default: + return -EINVAL; + } + + stream->ep->status.state = BT_BAP_EP_STATE_IDLE; + bt_bap_stream_reset(stream); + + if (stream->ops != NULL && stream->ops->released != NULL) { + stream->ops->released(stream); + } + return 0; } diff --git a/tests/bluetooth/tester/src/audio/btp_cap.c b/tests/bluetooth/tester/src/audio/btp_cap.c index 3c42ad292f7..fda96ee3602 100644 --- a/tests/bluetooth/tester/src/audio/btp_cap.c +++ b/tests/bluetooth/tester/src/audio/btp_cap.c @@ -438,6 +438,7 @@ static uint8_t btp_cap_unicast_audio_stop(const void *cmd, uint16_t cmd_len, param.streams = streams; param.count = stream_cnt; param.type = BT_CAP_SET_TYPE_AD_HOC; + param.release = true; err = bt_cap_initiator_unicast_audio_stop(¶m); if (err != 0) { diff --git a/tests/bsim/bluetooth/audio/src/cap_initiator_unicast_test.c b/tests/bsim/bluetooth/audio/src/cap_initiator_unicast_test.c index 24c697e0339..719ef3f742c 100644 --- a/tests/bsim/bluetooth/audio/src/cap_initiator_unicast_test.c +++ b/tests/bsim/bluetooth/audio/src/cap_initiator_unicast_test.c @@ -682,17 +682,6 @@ static void unicast_audio_update(void) printk("READ LONG META\n"); } -static void unicast_audio_stop_inval(void) -{ - int err; - - err = bt_cap_initiator_unicast_audio_stop(NULL); - if (err == 0) { - FAIL("bt_cap_initiator_unicast_audio_stop with NULL param did not fail\n"); - return; - } -} - static void unicast_audio_stop(struct bt_bap_unicast_group *unicast_group) { struct bt_cap_unicast_audio_stop_param param; @@ -701,8 +690,30 @@ static void unicast_audio_stop(struct bt_bap_unicast_group *unicast_group) param.type = BT_CAP_SET_TYPE_AD_HOC; param.count = non_idle_streams_cnt; param.streams = non_idle_streams; + param.release = false; + + /* Stop without release first to verify that we enter the QoS Configured state */ + UNSET_FLAG(flag_stopped); + + err = bt_cap_initiator_unicast_audio_stop(¶m); + if (err != 0) { + FAIL("Failed to stop unicast audio without release: %d\n", err); + return; + } + + WAIT_FOR_FLAG(flag_stopped); + + /* Verify that it cannot be stopped twice */ + err = bt_cap_initiator_unicast_audio_stop(¶m); + if (err == 0) { + FAIL("bt_cap_initiator_unicast_audio_stop without release with already-stopped " + "streams did not fail\n"); + return; + } + /* Stop with release first to verify that we enter the idle state */ UNSET_FLAG(flag_stopped); + param.release = true; err = bt_cap_initiator_unicast_audio_stop(¶m); if (err != 0) { @@ -820,7 +831,6 @@ static void test_main_cap_initiator_unicast_inval(void) unicast_audio_update_inval(); unicast_audio_update(); - unicast_audio_stop_inval(); unicast_audio_stop(unicast_group); unicast_group_delete_inval(); diff --git a/tests/bsim/bluetooth/audio/src/gmap_ugg_test.c b/tests/bsim/bluetooth/audio/src/gmap_ugg_test.c index be16111858e..a9d80d39dd6 100644 --- a/tests/bsim/bluetooth/audio/src/gmap_ugg_test.c +++ b/tests/bsim/bluetooth/audio/src/gmap_ugg_test.c @@ -928,6 +928,7 @@ static void unicast_audio_stop(struct bt_bap_unicast_group *unicast_group) param.type = BT_CAP_SET_TYPE_AD_HOC; param.count = started_unicast_streams_cnt; param.streams = started_unicast_streams; + param.release = true; err = bt_cap_initiator_unicast_audio_stop(¶m); if (err != 0) { From d76d148a1a0fb06b11ba8055047d8de96c75a510 Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Thu, 10 Oct 2024 21:03:33 +0200 Subject: [PATCH 05/10] [nrf fromtree] Bluetooth: Tester: Added flag parameter to CAP stop cmd Added a flag parameter so that it is possible to use the bt_cap_initiator_unicast_audio_stop to perform disable+stop without releasing the streams by setting the RELEASE flag. This allows us to use the bt_cap_initiator_unicast_audio_stop function to just disable streams without releasing them, as that is requested by some PTS tests such as CAP/INI/UST/BV-40-C. Signed-off-by: Emil Gydesen (cherry picked from commit 0fa9701a260d49f946e46841a607b14065b715c3) --- tests/bluetooth/tester/src/audio/btp/btp_cap.h | 2 ++ tests/bluetooth/tester/src/audio/btp_cap.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/bluetooth/tester/src/audio/btp/btp_cap.h b/tests/bluetooth/tester/src/audio/btp/btp_cap.h index 4369e87536a..1d0f9037abb 100644 --- a/tests/bluetooth/tester/src/audio/btp/btp_cap.h +++ b/tests/bluetooth/tester/src/audio/btp/btp_cap.h @@ -60,7 +60,9 @@ struct btp_cap_unicast_audio_update_data { #define BTP_CAP_UNICAST_AUDIO_STOP 0x06 struct btp_cap_unicast_audio_stop_cmd { uint8_t cig_id; + uint8_t flags; } __packed; +#define BTP_CAP_UNICAST_AUDIO_STOP_FLAG_RELEASE BIT(0) #define BTP_CAP_BROADCAST_SOURCE_SETUP_STREAM 0x07 struct btp_cap_broadcast_source_setup_stream_cmd { diff --git a/tests/bluetooth/tester/src/audio/btp_cap.c b/tests/bluetooth/tester/src/audio/btp_cap.c index fda96ee3602..0009abedb63 100644 --- a/tests/bluetooth/tester/src/audio/btp_cap.c +++ b/tests/bluetooth/tester/src/audio/btp_cap.c @@ -438,7 +438,7 @@ static uint8_t btp_cap_unicast_audio_stop(const void *cmd, uint16_t cmd_len, param.streams = streams; param.count = stream_cnt; param.type = BT_CAP_SET_TYPE_AD_HOC; - param.release = true; + param.release = (cp->flags & BTP_CAP_UNICAST_AUDIO_STOP_FLAG_RELEASE) != 0; err = bt_cap_initiator_unicast_audio_stop(¶m); if (err != 0) { From 31b1560afc31fbb1cfed03e9e35a142c759860aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Rist=20Sk=C3=B8ien?= Date: Sun, 8 Sep 2024 13:44:26 +0200 Subject: [PATCH 06/10] [nrf fromtree] shell: Added assert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assert to check columns. Averts a potential divide by zero Signed-off-by: Kristoffer Rist Skøien (cherry picked from commit c73c5d985e15c728729f9ef47a1abceae4fe61c4) --- subsys/shell/Kconfig | 1 + subsys/shell/shell.c | 1 + subsys/shell/shell_cmds.c | 7 ++----- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/subsys/shell/Kconfig b/subsys/shell/Kconfig index 626431b4cae..444e953dddd 100644 --- a/subsys/shell/Kconfig +++ b/subsys/shell/Kconfig @@ -91,6 +91,7 @@ config SHELL_PRINTF_BUFF_SIZE config SHELL_DEFAULT_TERMINAL_WIDTH int "Default terminal width" + range 1 $(UINT16_MAX) default 80 help Default terminal width is used to break lines. diff --git a/subsys/shell/shell.c b/subsys/shell/shell.c index 3ffe2691417..f55d5002a28 100644 --- a/subsys/shell/shell.c +++ b/subsys/shell/shell.c @@ -141,6 +141,7 @@ static void tab_item_print(const struct shell *sh, const char *option, columns = (sh->ctx->vt100_ctx.cons.terminal_wid - z_shell_strlen(tab)) / longest_option; + __ASSERT_NO_MSG(columns != 0); diff = longest_option - z_shell_strlen(option); if (sh->ctx->vt100_ctx.printed_cmd++ % columns == 0U) { diff --git a/subsys/shell/shell_cmds.c b/subsys/shell/shell_cmds.c index b1a29799a88..ef82c2b1c68 100644 --- a/subsys/shell/shell_cmds.c +++ b/subsys/shell/shell_cmds.c @@ -69,9 +69,6 @@ /* 10 == {esc, [, 2, 5, 0, ;, 2, 5, 0, '\0'} */ #define SHELL_CURSOR_POSITION_BUFFER (10u) -#define SHELL_DEFAULT_TERMINAL_WIDTH 80 -#define SHELL_DEFAULT_TERMINAL_HEIGHT 24 - /* Function reads cursor position from terminal. */ static int cursor_position_get(const struct shell *sh, uint16_t *x, uint16_t *y) { @@ -408,8 +405,8 @@ static int cmd_resize_default(const struct shell *sh, ARG_UNUSED(argv); Z_SHELL_VT100_CMD(sh, SHELL_VT100_SETCOL_80); - sh->ctx->vt100_ctx.cons.terminal_wid = SHELL_DEFAULT_TERMINAL_WIDTH; - sh->ctx->vt100_ctx.cons.terminal_hei = SHELL_DEFAULT_TERMINAL_HEIGHT; + sh->ctx->vt100_ctx.cons.terminal_wid = CONFIG_SHELL_DEFAULT_TERMINAL_WIDTH; + sh->ctx->vt100_ctx.cons.terminal_hei = CONFIG_SHELL_DEFAULT_TERMINAL_HEIGHT; return 0; } From a0f105d163f2c0bccccde8f59f969df888073d20 Mon Sep 17 00:00:00 2001 From: Alberto Escolar Piedras Date: Mon, 21 Oct 2024 15:45:16 +0200 Subject: [PATCH 07/10] [nrf fromtree] boards nrf_bsim: Add NVIC_GetPendingIRQ() equivalent Add a substitute for NVIC_GetPendingIRQ() Signed-off-by: Alberto Escolar Piedras (cherry picked from commit 1c7540883fb3d2c1c80ef416724e75acc260e762) --- boards/native/nrf_bsim/common/cmsis/cmsis.c | 5 +++++ boards/native/nrf_bsim/common/cmsis/cmsis.h | 1 + 2 files changed, 6 insertions(+) diff --git a/boards/native/nrf_bsim/common/cmsis/cmsis.c b/boards/native/nrf_bsim/common/cmsis/cmsis.c index e80aea6b4ff..dac5f82e699 100644 --- a/boards/native/nrf_bsim/common/cmsis/cmsis.c +++ b/boards/native/nrf_bsim/common/cmsis/cmsis.c @@ -30,6 +30,11 @@ void NVIC_DisableIRQ(IRQn_Type IRQn) hw_irq_ctrl_disable_irq(CONFIG_NATIVE_SIMULATOR_MCU_N, IRQn); } +uint32_t NVIC_GetPendingIRQ(IRQn_Type IRQn) +{ + return hw_irq_ctrl_is_irq_pending(CONFIG_NATIVE_SIMULATOR_MCU_N, IRQn); +} + void NVIC_EnableIRQ(IRQn_Type IRQn) { hw_irq_ctrl_enable_irq(CONFIG_NATIVE_SIMULATOR_MCU_N, IRQn); diff --git a/boards/native/nrf_bsim/common/cmsis/cmsis.h b/boards/native/nrf_bsim/common/cmsis/cmsis.h index ff9030ca547..2afc6e9ae50 100644 --- a/boards/native/nrf_bsim/common/cmsis/cmsis.h +++ b/boards/native/nrf_bsim/common/cmsis/cmsis.h @@ -30,6 +30,7 @@ void __set_PRIMASK(uint32_t primask); void NVIC_SetPendingIRQ(IRQn_Type IRQn); void NVIC_ClearPendingIRQ(IRQn_Type IRQn); void NVIC_DisableIRQ(IRQn_Type IRQn); +uint32_t NVIC_GetPendingIRQ(IRQn_Type IRQn); void NVIC_EnableIRQ(IRQn_Type IRQn); void NVIC_SetPriority(IRQn_Type IRQn, uint32_t priority); uint32_t NVIC_GetPriority(IRQn_Type IRQn); From 588489164e7978fe846eb67a0e0886a406fa6840 Mon Sep 17 00:00:00 2001 From: Alberto Escolar Piedras Date: Mon, 21 Oct 2024 14:27:59 +0200 Subject: [PATCH 08/10] [nrf fromtree] boards nrf_bsim: Provide IRQ_PRIO_LOWEST Some apps use it, so let's provide it. Signed-off-by: Alberto Escolar Piedras (cherry picked from commit c4e6ec89a7db966a5d36fd70d3aa8cfb7a78e6a1) --- boards/native/nrf_bsim/board_irq.h | 1 + 1 file changed, 1 insertion(+) diff --git a/boards/native/nrf_bsim/board_irq.h b/boards/native/nrf_bsim/board_irq.h index e1a385194fb..38f359fdf62 100644 --- a/boards/native/nrf_bsim/board_irq.h +++ b/boards/native/nrf_bsim/board_irq.h @@ -18,6 +18,7 @@ void nrfbsim_WFE_model(void); void nrfbsim_SEV_model(void); #define IRQ_ZERO_LATENCY BIT(1) /* Unused in this board*/ +#define IRQ_PRIO_LOWEST UINT8_MAX #ifdef __cplusplus } From 8132da7f59520e3628787caf1a4a20a8d0bbd1b8 Mon Sep 17 00:00:00 2001 From: Alberto Escolar Piedras Date: Mon, 21 Oct 2024 14:25:34 +0200 Subject: [PATCH 09/10] [nrf fromtree] manifest: Update nRF hw models to latest Update the HW models module to: aca798cf7cf0c5dc1fd89c66cf62670051feb8d0 Including the following: * aca798c IRQ controller: Add missing prototype * 4f108bc IRQ controller: Add API to check if int is pending * a514448 MDK: provide replacement for SystemCoreClock* Signed-off-by: Alberto Escolar Piedras (cherry picked from commit 15a7819a9bab2682efcb6aad320e374aa4693d66) --- west.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/west.yml b/west.yml index dd0275031a6..d5545988bef 100644 --- a/west.yml +++ b/west.yml @@ -300,7 +300,7 @@ manifest: groups: - tools - name: nrf_hw_models - revision: eeed2591d38e5e9bf89658df67555f2777249fc0 + revision: aca798cf7cf0c5dc1fd89c66cf62670051feb8d0 path: modules/bsim_hw_models/nrf_hw_models - name: open-amp revision: b735edbc739ad59156eb55bb8ce2583d74537719 From a84f11976d80c6f9338a48f16f3bcfc3acfb1408 Mon Sep 17 00:00:00 2001 From: Alberto Escolar Piedras Date: Mon, 21 Oct 2024 13:42:20 +0200 Subject: [PATCH 10/10] [nrf fromtree] native_simulator: Get latest from upstream Align with native_simulator's upstream main 51b27b67addd0073dc86e3d83f492c5cac5c3361 Which includes: * 51b27b nsi_utils: Add macro for weak declarations Signed-off-by: Alberto Escolar Piedras (cherry picked from commit 6ffbb8c72878c3f3f605f13455b28e23d8d36de0) --- scripts/native_simulator/common/src/include/nsi_utils.h | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/native_simulator/common/src/include/nsi_utils.h b/scripts/native_simulator/common/src/include/nsi_utils.h index 997eac30005..43d171a8e89 100644 --- a/scripts/native_simulator/common/src/include/nsi_utils.h +++ b/scripts/native_simulator/common/src/include/nsi_utils.h @@ -28,6 +28,7 @@ #define NSI_CODE_UNREACHABLE __builtin_unreachable() #define NSI_FUNC_NORETURN __attribute__((__noreturn__)) +#define NSI_WEAK __attribute__((__weak__)) #if defined(__clang__) /* The address sanitizer in llvm adds padding (redzones) after data