Skip to content

Commit c0ceb01

Browse files
committed
Bluetooth: CSIP: Set member: Fix issue with re-registration
The bt_csip_set_member_register kept a counter that was not decreased when bt_csip_set_member_unregister was called. This meant that we could register and unregister CSIS, but we could not re-register once it had been unregistered. This commit fixes this by removing the counter and instead rely on the service instance state, which also requires restoring the original service definition, as well as adding a test that would have failed with the previous version. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
1 parent 3e09a40 commit c0ceb01

File tree

3 files changed

+91
-11
lines changed

3 files changed

+91
-11
lines changed

subsys/bluetooth/audio/csip_set_member.c

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
/*
44
* Copyright (c) 2019 Bose Corporation
5-
* Copyright (c) 2020-2022 Nordic Semiconductor ASA
5+
* Copyright (c) 2020-2025 Nordic Semiconductor ASA
66
*
77
* SPDX-License-Identifier: Apache-2.0
88
*/
@@ -822,15 +822,9 @@ int bt_csip_set_member_register(const struct bt_csip_set_member_register_param *
822822
struct bt_csip_set_member_svc_inst **svc_inst)
823823
{
824824
static bool first_register;
825-
static uint8_t instance_cnt;
826825
struct bt_csip_set_member_svc_inst *inst;
827826
int err;
828827

829-
if (instance_cnt == ARRAY_SIZE(svc_insts)) {
830-
LOG_DBG("Too many set member registrations");
831-
return -ENOMEM;
832-
}
833-
834828
CHECKIF(param == NULL) {
835829
LOG_DBG("NULL param");
836830
return -EINVAL;
@@ -841,9 +835,19 @@ int bt_csip_set_member_register(const struct bt_csip_set_member_register_param *
841835
return -EINVAL;
842836
}
843837

844-
inst = &svc_insts[instance_cnt];
845-
inst->service_p = &csip_set_member_service_list[instance_cnt];
846-
instance_cnt++;
838+
inst = NULL;
839+
ARRAY_FOR_EACH(svc_insts, i) {
840+
if (svc_insts[i].service_p == NULL) {
841+
inst = &svc_insts[i];
842+
inst->service_p = &csip_set_member_service_list[i];
843+
break;
844+
}
845+
}
846+
847+
if (inst == NULL) {
848+
LOG_DBG("Too many set member registrations");
849+
return -ENOMEM;
850+
}
847851

848852
if (!first_register) {
849853
bt_conn_cb_register(&conn_callbacks);
@@ -918,6 +922,16 @@ int bt_csip_set_member_unregister(struct bt_csip_set_member_svc_inst *svc_inst)
918922
return err;
919923
}
920924

925+
/* Restore original declaration */
926+
927+
/* attrs_0 is an array of the original attributes, and while the actual number of attributes
928+
* may change, the size of the array stays the same, so we can use that to restore the
929+
* original attribute count
930+
*/
931+
(void)memcpy(svc_inst->service_p->attrs,
932+
(struct bt_gatt_attr[])BT_CSIP_SERVICE_DEFINITION(svc_inst), sizeof(attrs_0));
933+
svc_inst->service_p->attr_count = ARRAY_SIZE(attrs_0);
934+
921935
(void)k_work_cancel_delayable(&svc_inst->set_lock_timer);
922936
memset(svc_inst, 0, sizeof(*svc_inst));
923937

tests/bsim/bluetooth/audio/src/csip_set_member_test.c

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* Copyright (c) 2019 Bose Corporation
3-
* Copyright (c) 2020-2024 Nordic Semiconductor ASA
3+
* Copyright (c) 2020-2025 Nordic Semiconductor ASA
44
*
55
* SPDX-License-Identifier: Apache-2.0
66
*/
@@ -288,6 +288,41 @@ static void test_new_set_size_and_rank(void)
288288
PASS("CSIP Set member passed: Client successfully disconnected\n");
289289
}
290290

291+
static void test_register(void)
292+
{
293+
struct bt_csip_set_member_svc_inst *svc_insts[CONFIG_BT_CSIP_SET_MEMBER_MAX_INSTANCE_COUNT];
294+
int err;
295+
296+
for (size_t iteration = 1; iteration <= 5; iteration++) {
297+
printk("Running iteration %zu\n", iteration);
298+
299+
ARRAY_FOR_EACH(svc_insts, i) {
300+
err = bt_csip_set_member_register(&param, &svc_insts[i]);
301+
if (err != 0) {
302+
FAIL("[%zu]: Could not register CSIS (err %d)\n", i, err);
303+
return;
304+
}
305+
}
306+
307+
err = bt_csip_set_member_register(&param, &svc_inst);
308+
if (err == 0) {
309+
FAIL("Registered more CSIS than expected\n");
310+
return;
311+
}
312+
313+
ARRAY_FOR_EACH(svc_insts, i) {
314+
err = bt_csip_set_member_unregister(svc_insts[i]);
315+
if (err != 0) {
316+
FAIL("[%zu]: Could not unregister CSIS (err %d)\n", i, err);
317+
return;
318+
}
319+
svc_insts[i] = NULL;
320+
}
321+
}
322+
323+
PASS("CSIP Set member register passed\n");
324+
}
325+
291326
static void test_args(int argc, char *argv[])
292327
{
293328
for (size_t argn = 0; argn < argc; argn++) {
@@ -352,6 +387,13 @@ static const struct bst_test_instance test_connect[] = {
352387
.test_main_f = test_new_set_size_and_rank,
353388
.test_args_f = test_args,
354389
},
390+
{
391+
.test_id = "csip_set_member_register",
392+
.test_pre_init_f = test_init,
393+
.test_tick_f = test_tick,
394+
.test_main_f = test_register,
395+
.test_args_f = test_args,
396+
},
355397
BSTEST_END_MARKER,
356398
};
357399

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
#!/usr/bin/env bash
2+
#
3+
# Copyright (c) 2025 Nordic Semiconductor ASA
4+
#
5+
# SPDX-License-Identifier: Apache-2.0
6+
7+
source ${ZEPHYR_BASE}/tests/bsim/sh_common.source
8+
9+
VERBOSITY_LEVEL=2
10+
EXECUTE_TIMEOUT=30
11+
12+
cd ${BSIM_OUT_PATH}/bin
13+
14+
SIMULATION_ID="csip_register"
15+
16+
Execute ./bs_${BOARD_TS}_tests_bsim_bluetooth_audio_prj_conf \
17+
-v=${VERBOSITY_LEVEL} -s=${SIMULATION_ID} -d=0 -testid=csip_set_member_register \
18+
-RealEncryption=1 -rs=20 -D=1 -argstest rank 1 size 2
19+
20+
# Simulation time should be larger than the WAIT_TIME in common.h
21+
Execute ./bs_2G4_phy_v1 -v=${VERBOSITY_LEVEL} -s=${SIMULATION_ID} \
22+
-D=1 -sim_length=60e6 $@
23+
24+
wait_for_background_jobs

0 commit comments

Comments
 (0)