From ffd4fd571af68dad6ddbade646893b80c60ced3a Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Thu, 16 Dec 2021 12:02:34 +0100 Subject: [PATCH] Bluetooth: CSIS: Merge the two client discovery functions Merges bt_csis_client_discover and bt_csis_client_discover_sets, as they should be done together for the discovery procedure from the CSIP spec. Signed-off-by: Emil Gydesen --- include/bluetooth/audio/csis.h | 24 +---- subsys/bluetooth/audio/csis_client.c | 92 ++++++++++--------- subsys/bluetooth/shell/csis_client.c | 45 --------- .../bsim_test_audio/src/csis_client_test.c | 36 +------- 4 files changed, 52 insertions(+), 145 deletions(-) diff --git a/include/bluetooth/audio/csis.h b/include/bluetooth/audio/csis.h index fb73fa522d8..b2bf2815b71 100644 --- a/include/bluetooth/audio/csis.h +++ b/include/bluetooth/audio/csis.h @@ -222,7 +222,7 @@ struct bt_csis_client_set_info { * @brief Struct representing a coordinated set instance on a remote device * * The values in this struct will be populated during discovery of sets - * (bt_csis_client_discover_sets()). + * (bt_csis_client_discover()). */ struct bt_csis_client_csis_inst { struct bt_csis_client_set_info info; @@ -260,27 +260,6 @@ typedef void (*bt_csis_client_discover_cb)(struct bt_csis_client_set_member *mem */ int bt_csis_client_discover(struct bt_csis_client_set_member *member); -/** - * @typedef bt_csis_client_discover_sets_cb - * @brief Callback for discovering sets and their content. - * - * @param member Pointer to the set member. - * @param err 0 on success, or an errno value on error. - * @param set_count Number of sets on the member. - */ -typedef void (*bt_csis_client_discover_sets_cb)(struct bt_csis_client_set_member *member, - int err, uint8_t set_count); - -/** - * @brief Reads CSIS characteristics from a device, to find more information - * about the set(s) that the device is part of. - * - * @param member Pointer to a set member struct to store discovery results in. - * - * @return int Return 0 on success, or an errno value on error. - */ -int bt_csis_client_discover_sets(struct bt_csis_client_set_member *member); - /** * @typedef bt_csis_client_lock_set_cb * @brief Callback for locking a set across one or more devices @@ -322,7 +301,6 @@ struct bt_csis_client_cb { /* Set callbacks */ bt_csis_client_lock_set_cb lock_set; bt_csis_client_lock_set_cb release_set; - bt_csis_client_discover_sets_cb sets; bt_csis_client_lock_changed_cb lock_changed; /* Device specific callbacks */ diff --git a/subsys/bluetooth/audio/csis_client.c b/subsys/bluetooth/audio/csis_client.c index c7418fa6825..272304d84de 100644 --- a/subsys/bluetooth/audio/csis_client.c +++ b/subsys/bluetooth/audio/csis_client.c @@ -441,6 +441,25 @@ static int csis_client_read_rank(struct bt_conn *conn, uint8_t inst_idx, return bt_gatt_read(conn, &read_params); } +static int csis_client_discover_sets(struct bt_csis_client_set_member *member) +{ + int err; + + if (member->conn == NULL) { + BT_DBG("member->conn is NULL"); + return -EINVAL; + } else if (busy) { + return -EBUSY; + } + + /* Start reading values and call CB when done */ + err = read_set_sirk(member->insts[0].csis); + if (err == 0) { + busy = true; + } + return err; +} + static uint8_t discover_func(struct bt_conn *conn, const struct bt_gatt_attr *attr, struct bt_gatt_discover_params *params) @@ -478,11 +497,20 @@ static uint8_t discover_func(struct bt_conn *conn, } } else { + int err; + cur_inst = NULL; busy = false; - if (csis_client_cbs != NULL && csis_client_cbs->discover != NULL) { - csis_client_cbs->discover(client->set_member, 0, - client->inst_count); + err = csis_client_discover_sets(client->set_member); + if (err != 0) { + BT_DBG("Discover sets failed (err %d)", err); + cur_inst = NULL; + busy = false; + if (csis_client_cbs != NULL && + csis_client_cbs->discover != NULL) { + csis_client_cbs->discover(client->set_member, err, + client->inst_count); + } } } return BT_GATT_ITER_STOP; @@ -661,9 +689,10 @@ static uint8_t csis_client_discover_insts_read_rank_cb(struct bt_conn *conn, } if (cb_err != 0) { - if (csis_client_cbs != NULL && csis_client_cbs->sets != NULL) { - csis_client_cbs->sets(client->set_member, cb_err, - client->inst_count); + if (csis_client_cbs != NULL && + csis_client_cbs->discover != NULL) { + csis_client_cbs->discover(client->set_member, cb_err, + client->inst_count); } } @@ -703,9 +732,10 @@ static uint8_t csis_client_discover_insts_read_set_size_cb(struct bt_conn *conn, } if (cb_err != 0) { - if (csis_client_cbs != NULL && csis_client_cbs->sets != NULL) { - csis_client_cbs->sets(client->set_member, cb_err, - client->inst_count); + if (csis_client_cbs != NULL && + csis_client_cbs->discover != NULL) { + csis_client_cbs->discover(client->set_member, cb_err, + client->inst_count); } } @@ -790,9 +820,10 @@ static uint8_t csis_client_discover_insts_read_set_sirk_cb(struct bt_conn *conn, } if (cb_err != 0) { - if (csis_client_cbs != NULL && csis_client_cbs->sets != NULL) { - csis_client_cbs->sets(client->set_member, cb_err, - client->inst_count); + if (csis_client_cbs != NULL && + csis_client_cbs->discover != NULL) { + csis_client_cbs->discover(client->set_member, cb_err, + client->inst_count); } } @@ -839,18 +870,19 @@ static void discover_insts_resume(struct bt_conn *conn, uint16_t sirk_handle, /* Read next */ cb_err = read_set_sirk(cur_inst); } else if (csis_client_cbs != NULL && - csis_client_cbs->sets != NULL) { - csis_client_cbs->sets(client->set_member, 0, - client->inst_count); + csis_client_cbs->discover != NULL) { + csis_client_cbs->discover(client->set_member, 0, + client->inst_count); } return; } if (cb_err != 0) { - if (csis_client_cbs != NULL && csis_client_cbs->sets != NULL) { - csis_client_cbs->sets(client->set_member, cb_err, - client->inst_count); + if (csis_client_cbs != NULL && + csis_client_cbs->discover != NULL) { + csis_client_cbs->discover(client->set_member, cb_err, + client->inst_count); } } else { busy = true; @@ -1242,30 +1274,6 @@ int bt_csis_client_discover(struct bt_csis_client_set_member *member) return err; } -int bt_csis_client_discover_sets(struct bt_csis_client_set_member *member) -{ - int err; - - CHECKIF(member == NULL) { - BT_DBG("member is NULL"); - return -EINVAL; - } - - if (member->conn == NULL) { - BT_DBG("member->conn is NULL"); - return -EINVAL; - } else if (busy) { - return -EBUSY; - } - - /* Start reading values and call CB when done */ - err = read_set_sirk(member->insts[0].csis); - if (err == 0) { - busy = true; - } - return err; -} - static int verify_members_and_get_inst(const struct bt_csis_client_set_member **members, uint8_t count, const struct bt_csis_client_set_info *set_info, diff --git a/subsys/bluetooth/shell/csis_client.c b/subsys/bluetooth/shell/csis_client.c index 40bb8933190..f7e87d0c4b8 100644 --- a/subsys/bluetooth/shell/csis_client.c +++ b/subsys/bluetooth/shell/csis_client.c @@ -101,23 +101,6 @@ static void csis_discover_cb(struct bt_csis_client_set_member *member, int err, } } -static void csis_client_discover_sets_cb(struct bt_csis_client_set_member *member, - int err, - uint8_t set_count) -{ - if (err != 0) { - shell_error(ctx_shell, "Discover sets failed (%d)", err); - return; - } - - for (uint8_t i = 0; i < set_count; i++) { - struct bt_csis_client_csis_inst *inst = &member->insts[i]; - - shell_print(ctx_shell, "Set size %d (pointer: %p)", - inst[i].info.set_size, &inst[i]); - } -} - static void csis_client_lock_set_cb(int err) { if (err != 0) { @@ -157,7 +140,6 @@ static void csis_client_lock_state_read_cb(const struct bt_csis_client_set_info static struct bt_csis_client_cb cbs = { .lock_set = csis_client_lock_set_cb, .release_set = csis_client_release_set_cb, - .sets = csis_client_discover_sets_cb, .discover = csis_discover_cb, .lock_state_read = csis_client_lock_state_read_cb }; @@ -280,30 +262,6 @@ static int cmd_csis_client_discover(const struct shell *sh, size_t argc, return err; } -static int cmd_csis_client_discover_sets(const struct shell *sh, size_t argc, - char *argv[]) -{ - int err; - long member_index = 0; - - if (argc > 1) { - member_index = strtol(argv[1], NULL, 0); - - if (member_index < 0 || member_index > CONFIG_BT_MAX_CONN) { - shell_error(sh, "Invalid member_index %ld", - member_index); - return -ENOEXEC; - } - } - - err = bt_csis_client_discover_sets(&set_members[member_index]); - if (err != 0) { - shell_error(sh, "Fail: %d", err); - } - - return err; -} - static int cmd_csis_client_discover_members(const struct shell *sh, size_t argc, char *argv[]) { @@ -520,9 +478,6 @@ SHELL_STATIC_SUBCMD_SET_CREATE(csis_client_cmds, SHELL_CMD_ARG(discover, NULL, "Run discover for CSIS on peer device [member_index]", cmd_csis_client_discover, 1, 1), - SHELL_CMD_ARG(discover_sets, NULL, - "Read all set values on connected device [member_index]", - cmd_csis_client_discover_sets, 1, 1), SHELL_CMD_ARG(discover_members, NULL, "Scan for set members ", cmd_csis_client_discover_members, 2, 0), diff --git a/tests/bluetooth/bsim_bt/bsim_test_audio/src/csis_client_test.c b/tests/bluetooth/bsim_bt/bsim_test_audio/src/csis_client_test.c index 5b8a4b5e3cb..f140efcfaa5 100644 --- a/tests/bluetooth/bsim_bt/bsim_test_audio/src/csis_client_test.c +++ b/tests/bluetooth/bsim_bt/bsim_test_audio/src/csis_client_test.c @@ -12,7 +12,6 @@ extern enum bst_result_t bst_result; static volatile bool is_connected; static volatile bool discovered; -static volatile bool sets_discovered; static volatile bool members_discovered; static volatile bool set_locked; static volatile bool set_unlocked; @@ -51,21 +50,6 @@ static void csis_client_lock_set_cb(int err) set_locked = true; } -static void csis_client_discover_sets_cb(struct bt_csis_client_set_member *member, - int err, - uint8_t set_count) -{ - printk("%s\n", __func__); - - if (err != 0) { - FAIL("Discover sets failed (%d)\n", err); - return; - } - - inst = &member->insts[0]; - sets_discovered = true; -} - static void csis_discover_cb(struct bt_csis_client_set_member *member, int err, uint8_t set_count) { @@ -76,6 +60,7 @@ static void csis_discover_cb(struct bt_csis_client_set_member *member, int err, return; } + inst = &member->insts[0]; discovered = true; } @@ -129,7 +114,6 @@ static struct bt_conn_cb conn_callbacks = { static struct bt_csis_client_cb cbs = { .lock_set = csis_client_lock_set_cb, .release_set = csis_client_lock_release_cb, - .sets = csis_client_discover_sets_cb, .discover = csis_discover_cb, .lock_changed = csis_lock_changed_cb, .lock_state_read = csis_client_lock_state_read_cb, @@ -285,14 +269,6 @@ static void test_main(void) WAIT_FOR(discovered); - err = bt_csis_client_discover_sets(&set_members[0]); - if (err != 0) { - FAIL("Failed to do CSIS client discovery sets (%d)\n", err); - return; - } - - WAIT_FOR(sets_discovered); - err = bt_le_scan_start(BT_LE_SCAN_ACTIVE, NULL); if (err != 0) { FAIL("Could not start scan: %d", err); @@ -343,16 +319,6 @@ static void test_main(void) } WAIT_FOR(discovered); - - sets_discovered = false; - printk("Doing sets discovery on member[%u]", i); - err = bt_csis_client_discover_sets(&set_members[i]); - if (err != 0) { - FAIL("Failed to do CSIS client discovery sets (%d)\n", err); - return; - } - - WAIT_FOR(sets_discovered); } for (size_t i = 0; i < ARRAY_SIZE(locked_members); i++) {