diff --git a/include/zephyr/bluetooth/audio/bap.h b/include/zephyr/bluetooth/audio/bap.h index f18c034a1b3..ccae126e0bc 100644 --- a/include/zephyr/bluetooth/audio/bap.h +++ b/include/zephyr/bluetooth/audio/bap.h @@ -1156,7 +1156,7 @@ struct bt_bap_unicast_group_param { }; /** - * @brief Create audio unicast group. + * @brief Create unicast group. * * Create a new audio unicast group with one or more audio streams as a unicast client. * All streams shall share the same framing. @@ -1171,6 +1171,24 @@ struct bt_bap_unicast_group_param { int bt_bap_unicast_group_create(struct bt_bap_unicast_group_param *param, struct bt_bap_unicast_group **unicast_group); +/** + * @brief Reconfigure unicast group. + * + * Reconfigure a unicast group with one or more audio streams as a unicast client. + * All streams shall share the same framing. + * All streams in the same direction shall share the same interval and latency (see + * @ref bt_audio_codec_qos). + * All streams in @p param shall already belong to @p unicast_group. + * Use bt_bap_unicast_group_add_streams() to add additional streams. + * + * @param unicast_group Pointer to the unicast group created. + * @param param The unicast group reconfigure parameters. + * + * @return Zero on success or (negative) error code otherwise. + */ +int bt_bap_unicast_group_reconfig(struct bt_bap_unicast_group *unicast_group, + const struct bt_bap_unicast_group_param *param); + /** * @brief Add streams to a unicast group as a unicast client * diff --git a/subsys/bluetooth/audio/bap_endpoint.h b/subsys/bluetooth/audio/bap_endpoint.h index cd6165b4483..85d3cea3e4f 100644 --- a/subsys/bluetooth/audio/bap_endpoint.h +++ b/subsys/bluetooth/audio/bap_endpoint.h @@ -85,6 +85,10 @@ struct bt_bap_unicast_group { /* Unicast group fields */ uint8_t index; bool allocated; + /* Used to determine whether any stream in this group has been started which will prevent + * reconfiguring it + */ + bool has_been_connected; struct bt_iso_cig *cig; /* The ISO API for CIG creation requires an array of pointers to ISO channels */ struct bt_iso_chan *cis[UNICAST_GROUP_STREAM_CNT]; diff --git a/subsys/bluetooth/audio/bap_unicast_client.c b/subsys/bluetooth/audio/bap_unicast_client.c index 51a34d92d8f..9b51d43b2f0 100644 --- a/subsys/bluetooth/audio/bap_unicast_client.c +++ b/subsys/bluetooth/audio/bap_unicast_client.c @@ -318,6 +318,10 @@ static void unicast_client_ep_iso_connected(struct bt_bap_ep *ep) const struct bt_bap_stream_ops *stream_ops; struct bt_bap_stream *stream; + if (ep->unicast_group != NULL) { + ep->unicast_group->has_been_connected = true; + } + if (ep->status.state != BT_BAP_EP_STATE_ENABLING) { LOG_DBG("endpoint not in enabling state: %s", bt_bap_ep_state_str(ep->status.state)); @@ -2171,17 +2175,12 @@ static void bt_audio_codec_qos_to_cig_param(struct bt_iso_cig_param *cig_param, } } -static int bt_audio_cig_create(struct bt_bap_unicast_group *group) +static uint8_t unicast_group_get_cis_count(const struct bt_bap_unicast_group *unicast_group) { - struct bt_iso_cig_param param = {0}; - uint8_t cis_count; - int err; + uint8_t cis_count = 0U; - LOG_DBG("group %p", group); - - cis_count = 0U; - for (size_t i = 0U; i < ARRAY_SIZE(group->cis); i++) { - if (group->cis[i] == NULL) { + for (size_t i = 0U; i < ARRAY_SIZE(unicast_group->cis); i++) { + if (unicast_group->cis[i] == NULL) { /* A NULL CIS acts as a NULL terminator */ break; } @@ -2189,7 +2188,17 @@ static int bt_audio_cig_create(struct bt_bap_unicast_group *group) cis_count++; } - param.num_cis = cis_count; + return cis_count; +} + +static int bt_audio_cig_create(struct bt_bap_unicast_group *group) +{ + struct bt_iso_cig_param param = {0}; + int err; + + LOG_DBG("group %p", group); + + param.num_cis = unicast_group_get_cis_count(group); param.cis_channels = group->cis; bt_audio_codec_qos_to_cig_param(¶m, group); @@ -2352,6 +2361,27 @@ static void unicast_client_codec_qos_to_iso_qos(struct bt_bap_iso *iso, } } +static void unicast_group_set_iso_stream_param(struct bt_bap_unicast_group *group, + struct bt_bap_iso *iso, + struct bt_audio_codec_qos *qos, + enum bt_audio_dir dir) +{ + /* Store the stream Codec QoS in the bap_iso */ + unicast_client_codec_qos_to_iso_qos(iso, qos, dir); + + /* Store the group Codec QoS in the group - This assume thats the parameters have been + * verified first + */ + group->cig_param.framing = qos->framing; + if (dir == BT_AUDIO_DIR_SOURCE) { + group->cig_param.p_to_c_interval = qos->interval; + group->cig_param.p_to_c_latency = qos->latency; + } else { + group->cig_param.c_to_p_interval = qos->interval; + group->cig_param.c_to_p_latency = qos->latency; + } +} + static void unicast_group_add_stream(struct bt_bap_unicast_group *group, struct bt_bap_unicast_group_stream_param *param, struct bt_bap_iso *iso, enum bt_audio_dir dir) @@ -2372,20 +2402,7 @@ static void unicast_group_add_stream(struct bt_bap_unicast_group *group, bt_bap_iso_bind_ep(iso, stream->ep); } - /* Store the stream Codec QoS in the bap_iso */ - unicast_client_codec_qos_to_iso_qos(iso, qos, dir); - - /* Store the group Codec QoS in the group - This assume thats the parameters have been - * verified first - */ - group->cig_param.framing = qos->framing; - if (dir == BT_AUDIO_DIR_SOURCE) { - group->cig_param.p_to_c_interval = qos->interval; - group->cig_param.p_to_c_latency = qos->latency; - } else { - group->cig_param.c_to_p_interval = qos->interval; - group->cig_param.c_to_p_latency = qos->latency; - } + unicast_group_set_iso_stream_param(group, iso, qos, dir); sys_slist_append(&group->streams, &stream->_node); } @@ -2574,7 +2591,8 @@ static int stream_pair_param_check(const struct bt_bap_unicast_group_stream_pair } /** Validates that the stream parameter does not contain invalid values */ -static bool valid_unicast_group_stream_param(const struct bt_bap_unicast_group_stream_param *param, +static bool valid_unicast_group_stream_param(const struct bt_bap_unicast_group *unicast_group, + const struct bt_bap_unicast_group_stream_param *param, struct bt_bap_unicast_group_cig_param *cig_param, enum bt_audio_dir dir) { @@ -2591,7 +2609,13 @@ static bool valid_unicast_group_stream_param(const struct bt_bap_unicast_group_s } if (param->stream != NULL && param->stream->group != NULL) { - LOG_DBG("stream %p already part of group %p", param->stream, param->stream->group); + if (unicast_group != NULL && param->stream->group != unicast_group) { + LOG_DBG("stream %p not part of group %p (%p)", param->stream, unicast_group, + param->stream->group); + } else { + LOG_DBG("stream %p already part of group %p", param->stream, + param->stream->group); + } return -EALREADY; } @@ -2659,15 +2683,15 @@ valid_group_stream_pair_param(const struct bt_bap_unicast_group *unicast_group, } if (pair_param->rx_param != NULL) { - if (!valid_unicast_group_stream_param(pair_param->rx_param, &cig_param, - BT_AUDIO_DIR_SOURCE)) { + if (!valid_unicast_group_stream_param(unicast_group, pair_param->rx_param, + &cig_param, BT_AUDIO_DIR_SOURCE)) { return false; } } if (pair_param->tx_param != NULL) { - if (!valid_unicast_group_stream_param(pair_param->tx_param, &cig_param, - BT_AUDIO_DIR_SINK)) { + if (!valid_unicast_group_stream_param(unicast_group, pair_param->tx_param, + &cig_param, BT_AUDIO_DIR_SINK)) { return false; } } @@ -2675,7 +2699,7 @@ valid_group_stream_pair_param(const struct bt_bap_unicast_group *unicast_group, return true; } -static bool valid_unicast_group_param(const struct bt_bap_unicast_group *unicast_group, +static bool valid_unicast_group_param(struct bt_bap_unicast_group *unicast_group, const struct bt_bap_unicast_group_param *param) { CHECKIF(param == NULL) { @@ -2689,6 +2713,17 @@ static bool valid_unicast_group_param(const struct bt_bap_unicast_group *unicast return false; } + if (unicast_group != NULL) { + const size_t group_cis_cnt = unicast_group_get_cis_count(unicast_group); + + if (param->params_count != group_cis_cnt) { + LOG_DBG("Mismatch between group CIS count (%zu) and params_count (%zu)", + group_cis_cnt, param->params_count); + + return false; + } + } + for (size_t i = 0U; i < param->params_count; i++) { if (!valid_group_stream_pair_param(unicast_group, ¶m->params[i])) { return false; @@ -2718,7 +2753,7 @@ int bt_bap_unicast_group_create(struct bt_bap_unicast_group_param *param, return -ENOMEM; } - if (!valid_unicast_group_param(unicast_group, param)) { + if (!valid_unicast_group_param(NULL, param)) { unicast_group_free(unicast_group); return -EINVAL; @@ -2758,6 +2793,91 @@ int bt_bap_unicast_group_create(struct bt_bap_unicast_group_param *param, return 0; } +int bt_bap_unicast_group_reconfig(struct bt_bap_unicast_group *unicast_group, + const struct bt_bap_unicast_group_param *param) +{ + struct bt_iso_chan_io_qos rx_io_qos_backup[UNICAST_GROUP_STREAM_CNT]; + struct bt_iso_chan_io_qos tx_io_qos_backup[UNICAST_GROUP_STREAM_CNT]; + struct bt_bap_unicast_group_cig_param cig_param_backup; + struct bt_bap_stream *tmp_stream; + size_t idx; + int err; + + IF_ENABLED(CONFIG_BT_ISO_TEST_PARAMS, + (uint8_t num_subevents_backup[UNICAST_GROUP_STREAM_CNT])); + + CHECKIF(unicast_group == NULL) { + LOG_DBG("unicast_group is NULL"); + return -EINVAL; + } + + if (unicast_group->has_been_connected) { + LOG_DBG("Cannot modify a unicast_group where a CIS has been connected"); + return -EINVAL; + } + + if (!valid_unicast_group_param(unicast_group, param)) { + return -EINVAL; + } + + /* Make backups of the values in case that the reconfigure request is rejected by e.g. the + * controller + */ + idx = 0U; + SYS_SLIST_FOR_EACH_CONTAINER(&unicast_group->streams, tmp_stream, _node) { + memcpy(&rx_io_qos_backup[idx], tmp_stream->bap_iso->chan.qos->rx, + sizeof(rx_io_qos_backup[idx])); + memcpy(&tx_io_qos_backup[idx], tmp_stream->bap_iso->chan.qos->tx, + sizeof(tx_io_qos_backup[idx])); + IF_ENABLED( + CONFIG_BT_ISO_TEST_PARAMS, + (num_subevents_backup[idx] = tmp_stream->bap_iso->chan.qos->num_subevents)); + idx++; + } + memcpy(&cig_param_backup, &unicast_group->cig_param, sizeof(cig_param_backup)); + + /* Update the stream and group parameters */ + for (size_t i = 0U; i < param->params_count; i++) { + struct bt_bap_unicast_group_stream_pair_param *stream_param = ¶m->params[i]; + struct bt_bap_unicast_group_stream_param *rx_param = stream_param->rx_param; + struct bt_bap_unicast_group_stream_param *tx_param = stream_param->tx_param; + + if (rx_param != NULL) { + unicast_group_set_iso_stream_param(unicast_group, rx_param->stream->bap_iso, + rx_param->qos, BT_AUDIO_DIR_SOURCE); + } + + if (tx_param != NULL) { + unicast_group_set_iso_stream_param(unicast_group, tx_param->stream->bap_iso, + tx_param->qos, BT_AUDIO_DIR_SOURCE); + } + } + + /* Reconfigure the CIG based on the above new values */ + err = bt_audio_cig_reconfigure(unicast_group); + if (err != 0) { + LOG_DBG("bt_audio_cig_reconfigure failed: %d", err); + + /* Revert any changes above */ + memcpy(&unicast_group->cig_param, &cig_param_backup, sizeof(cig_param_backup)); + idx = 0U; + SYS_SLIST_FOR_EACH_CONTAINER(&unicast_group->streams, tmp_stream, _node) { + memcpy(tmp_stream->bap_iso->chan.qos->rx, &rx_io_qos_backup[idx], + sizeof(rx_io_qos_backup[idx])); + memcpy(tmp_stream->bap_iso->chan.qos->tx, &tx_io_qos_backup[idx], + sizeof(tx_io_qos_backup[idx])); + IF_ENABLED(CONFIG_BT_ISO_TEST_PARAMS, + (tmp_stream->bap_iso->chan.qos->num_subevents = + num_subevents_backup[idx])); + idx++; + } + + return err; + } + + return 0; +} + int bt_bap_unicast_group_add_streams(struct bt_bap_unicast_group *unicast_group, struct bt_bap_unicast_group_stream_pair_param params[], size_t num_param) @@ -2774,6 +2894,11 @@ int bt_bap_unicast_group_add_streams(struct bt_bap_unicast_group *unicast_group, return -EINVAL; } + if (unicast_group->has_been_connected) { + LOG_DBG("Cannot modify a unicast_group where a CIS has been connected"); + return -EINVAL; + } + CHECKIF(params == NULL) { LOG_DBG("params is NULL"); diff --git a/subsys/bluetooth/audio/shell/bap.c b/subsys/bluetooth/audio/shell/bap.c index 9dcffab1635..64d2b8e1a9b 100644 --- a/subsys/bluetooth/audio/shell/bap.c +++ b/subsys/bluetooth/audio/shell/bap.c @@ -1437,19 +1437,14 @@ static int cmd_stream_qos(const struct shell *sh, size_t argc, char *argv[]) return 0; } -static int create_unicast_group(const struct shell *sh) +static int set_group_param( + const struct shell *sh, struct bt_bap_unicast_group_param *group_param, + struct bt_bap_unicast_group_stream_pair_param pair_param[ARRAY_SIZE(unicast_streams)], + struct bt_bap_unicast_group_stream_param stream_params[ARRAY_SIZE(unicast_streams)]) { - struct bt_bap_unicast_group_stream_pair_param pair_param[ARRAY_SIZE(unicast_streams)]; - struct bt_bap_unicast_group_stream_param stream_params[ARRAY_SIZE(unicast_streams)]; - struct bt_bap_unicast_group_param group_param; size_t source_cnt = 0; size_t sink_cnt = 0; size_t cnt = 0; - int err; - - memset(pair_param, 0, sizeof(pair_param)); - memset(stream_params, 0, sizeof(stream_params)); - memset(&group_param, 0, sizeof(group_param)); for (size_t i = 0U; i < ARRAY_SIZE(unicast_streams); i++) { struct bt_bap_stream *stream = bap_stream_from_shell_stream(&unicast_streams[i]); @@ -1479,16 +1474,50 @@ static int create_unicast_group(const struct shell *sh) return -ENOEXEC; } - group_param.packing = BT_ISO_PACKING_SEQUENTIAL; - group_param.params = pair_param; - group_param.params_count = MAX(source_cnt, sink_cnt); + group_param->packing = BT_ISO_PACKING_SEQUENTIAL; + group_param->params = pair_param; + group_param->params_count = MAX(source_cnt, sink_cnt); - err = bt_bap_unicast_group_create(&group_param, - &default_unicast_group); + return 0; +} + +static int create_unicast_group(const struct shell *sh) +{ + struct bt_bap_unicast_group_stream_pair_param pair_param[ARRAY_SIZE(unicast_streams)] = {0}; + struct bt_bap_unicast_group_stream_param stream_params[ARRAY_SIZE(unicast_streams)] = {0}; + struct bt_bap_unicast_group_param group_param = {0}; + int err; + + err = set_group_param(sh, &group_param, pair_param, stream_params); if (err != 0) { - shell_error(sh, - "Unable to create default unicast group: %d", - err); + return err; + } + + err = bt_bap_unicast_group_create(&group_param, &default_unicast_group); + if (err != 0) { + shell_error(sh, "Unable to create default unicast group: %d", err); + + return -ENOEXEC; + } + + return 0; +} + +static int reconfig_unicast_group(const struct shell *sh) +{ + struct bt_bap_unicast_group_stream_pair_param pair_param[ARRAY_SIZE(unicast_streams)] = {0}; + struct bt_bap_unicast_group_stream_param stream_params[ARRAY_SIZE(unicast_streams)] = {0}; + struct bt_bap_unicast_group_param group_param = {0}; + int err; + + err = set_group_param(sh, &group_param, pair_param, stream_params); + if (err != 0) { + return err; + } + + err = bt_bap_unicast_group_reconfig(default_unicast_group, &group_param); + if (err != 0) { + shell_error(sh, "Unable to create default unicast group: %d", err); return -ENOEXEC; } @@ -1515,6 +1544,11 @@ static int cmd_qos(const struct shell *sh, size_t argc, char *argv[]) if (err != 0) { return err; } + } else { + err = reconfig_unicast_group(sh); + if (err != 0) { + return err; + } } err = bt_bap_stream_qos(default_conn, default_unicast_group); diff --git a/tests/bsim/bluetooth/audio/src/bap_unicast_client_test.c b/tests/bsim/bluetooth/audio/src/bap_unicast_client_test.c index a92fd610bb1..f96b3064186 100644 --- a/tests/bsim/bluetooth/audio/src/bap_unicast_client_test.c +++ b/tests/bsim/bluetooth/audio/src/bap_unicast_client_test.c @@ -1233,6 +1233,53 @@ static void test_main_async_group(void) PASS("Unicast client async group parameters passed\n"); } +static void test_main_reconf_group(void) +{ + static struct bt_bap_lc3_preset preset_16_2_2 = BT_BAP_LC3_UNICAST_PRESET_16_2_2( + BT_AUDIO_LOCATION_FRONT_LEFT, BT_AUDIO_CONTEXT_TYPE_UNSPECIFIED); + struct bt_bap_stream rx_stream = {0}; + struct bt_bap_stream tx_stream = {0}; + struct bt_bap_unicast_group_stream_param rx_param = { + .qos = &preset_16_2_1.qos, + .stream = &rx_stream, + }; + struct bt_bap_unicast_group_stream_param tx_param = { + .qos = &preset_16_2_1.qos, + .stream = &tx_stream, + }; + struct bt_bap_unicast_group_stream_pair_param pair_param = { + .rx_param = &rx_param, + .tx_param = &tx_param, + }; + struct bt_bap_unicast_group_param param = { + .params = &pair_param, + .params_count = 1U, + .packing = BT_ISO_PACKING_SEQUENTIAL, + }; + struct bt_bap_unicast_group *unicast_group; + int err; + + init(); + + err = bt_bap_unicast_group_create(¶m, &unicast_group); + if (err != 0) { + FAIL("Unable to create unicast group: %d", err); + + return; + } + + rx_param.qos = &preset_16_2_2.qos; + tx_param.qos = &preset_16_2_2.qos; + err = bt_bap_unicast_group_reconfig(unicast_group, ¶m); + if (err != 0) { + FAIL("Unable to reconfigure unicast group: %d", err); + + return; + } + + PASS("Unicast client async group parameters passed\n"); +} + static const struct bst_test_instance test_unicast_client[] = { { .test_id = "unicast_client", @@ -1255,6 +1302,13 @@ static const struct bst_test_instance test_unicast_client[] = { "values in each direction, such as 10000us SDU interval in C to P " "and 7500us for P to C", }, + { + .test_id = "unicast_client_reconf_group", + .test_pre_init_f = test_init, + .test_tick_f = test_tick, + .test_main_f = test_main_reconf_group, + .test_descr = "Tests that a unicast group (CIG) can be reconfigred with new values", + }, BSTEST_END_MARKER, }; diff --git a/tests/bsim/bluetooth/audio/test_scripts/bap_unicast_client_reconf_group.sh b/tests/bsim/bluetooth/audio/test_scripts/bap_unicast_client_reconf_group.sh new file mode 100755 index 00000000000..3b66ce83158 --- /dev/null +++ b/tests/bsim/bluetooth/audio/test_scripts/bap_unicast_client_reconf_group.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2021-2024 Nordic Semiconductor ASA +# +# SPDX-License-Identifier: Apache-2.0 + +SIMULATION_ID="bap_unicast_client_reconf_group" +VERBOSITY_LEVEL=2 + +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +cd ${BSIM_OUT_PATH}/bin + +printf "\n\n======== BAP Unicast Client Reconfigure Group parameters test =========\n\n" + +Execute ./bs_${BOARD_TS}_tests_bsim_bluetooth_audio_prj_conf \ + -v=${VERBOSITY_LEVEL} -s=${SIMULATION_ID} -d=0 -testid=unicast_client_reconf_group -rs=23 -D=1 + +# Simulation time should be larger than the WAIT_TIME in common.h +Execute ./bs_2G4_phy_v1 -v=${VERBOSITY_LEVEL} -s=${SIMULATION_ID} \ + -D=1 -sim_length=60e6 $@ + +wait_for_background_jobs