From fa1be6436a27ba558a50fde602d75493af34d1e7 Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Tue, 23 Nov 2021 13:32:04 +0100 Subject: [PATCH] Bluetooth: CSIS: Refactor bt_csis_client_discover_sets to use member Refactor bt_csis_client_discover_sets to use the bt_csis_client_set_member struct instead of a bt_conn. The bt_csis_client_set_member represents a remote server (set member), and make it possible to avoid sending indexes of instances around instead of bt_csis. Signed-off-by: Emil Gydesen --- include/bluetooth/audio/csis.h | 11 ++-- subsys/bluetooth/audio/csis_client.c | 66 +++++++++---------- subsys/bluetooth/shell/csis_client.c | 25 ++----- .../bsim_test_audio/src/csis_client_test.c | 18 ++--- 4 files changed, 49 insertions(+), 71 deletions(-) diff --git a/include/bluetooth/audio/csis.h b/include/bluetooth/audio/csis.h index 4e3e822fdd5..0691d391b02 100644 --- a/include/bluetooth/audio/csis.h +++ b/include/bluetooth/audio/csis.h @@ -218,25 +218,24 @@ typedef void (*bt_csis_client_discover_cb)(struct bt_csis_client_set_member *mem * @brief Initialise the csis_client instance for a connection. This will do a * discovery on the device and prepare the instance for following commands. * - * @param member Pointer to a set member struct to store discovery results in + * @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(struct bt_csis_client_set_member *member); -typedef void (*bt_csis_client_discover_sets_cb)(struct bt_conn *conn, - int err, uint8_t set_count, - struct bt_csis_client_set *sets); +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 conn The connection to the device to read CSIS characteristics + * @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_conn *conn); +int bt_csis_client_discover_sets(struct bt_csis_client_set_member *member); typedef void (*bt_csis_client_lock_set_cb)(int err); diff --git a/subsys/bluetooth/audio/csis_client.c b/subsys/bluetooth/audio/csis_client.c index 8ccf9a98ec7..77d1bba031d 100644 --- a/subsys/bluetooth/audio/csis_client.c +++ b/subsys/bluetooth/audio/csis_client.c @@ -68,7 +68,7 @@ static struct bt_uuid_16 uuid = BT_UUID_INIT_16(0); static struct bt_csis_client_cb *csis_client_cbs; static struct bt_csis_client_inst client_insts[CONFIG_BT_MAX_CONN]; -static int read_set_sirk(struct bt_conn *conn, uint8_t inst_idx); +static int read_set_sirk(struct bt_csis *csis); static int csis_client_read_set_size(struct bt_conn *conn, uint8_t inst_idx, bt_gatt_read_func_t cb); @@ -353,34 +353,27 @@ static int csis_client_write_set_lock(struct bt_csis *inst, return bt_gatt_write(inst->cli.conn, &write_params); } -static int read_set_sirk(struct bt_conn *conn, uint8_t inst_idx) +static int read_set_sirk(struct bt_csis *csis) { - if (inst_idx >= CONFIG_BT_CSIS_CLIENT_MAX_CSIS_INSTANCES) { - return -EINVAL; - } else if (cur_inst != NULL) { - if (cur_inst != lookup_instance_by_index(conn, inst_idx)) { + if (cur_inst != NULL) { + if (cur_inst != csis) { return -EBUSY; } } else { - cur_inst = lookup_instance_by_index(conn, inst_idx); - if (cur_inst == NULL) { - BT_DBG("Inst not found"); - return -EINVAL; - } + cur_inst = csis; } - if (cur_inst->cli.set_sirk_handle == 0) { + if (csis->cli.set_sirk_handle == 0) { BT_DBG("Handle not set"); - cur_inst = NULL; return -EINVAL; } read_params.func = csis_client_discover_sets_read_set_sirk_cb; read_params.handle_count = 1; - read_params.single.handle = cur_inst->cli.set_sirk_handle; + read_params.single.handle = csis->cli.set_sirk_handle; read_params.single.offset = 0U; - return bt_gatt_read(conn, &read_params); + return bt_gatt_read(csis->cli.conn, &read_params); } static int csis_client_read_set_size(struct bt_conn *conn, uint8_t inst_idx, @@ -666,9 +659,8 @@ static uint8_t csis_client_discover_sets_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(conn, cb_err, - client->inst_count, - client->set_member->sets); + csis_client_cbs->sets(client->set_member, cb_err, + client->inst_count); } } @@ -709,9 +701,8 @@ static uint8_t csis_client_discover_sets_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(conn, cb_err, - client->inst_count, - client->set_member->sets); + csis_client_cbs->sets(client->set_member, cb_err, + client->inst_count); } } @@ -797,9 +788,8 @@ static uint8_t csis_client_discover_sets_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(conn, cb_err, - client->inst_count, - client->set_member->sets); + csis_client_cbs->sets(client->set_member, cb_err, + client->inst_count); } } @@ -841,12 +831,14 @@ static void discover_sets_resume(struct bt_conn *conn, uint16_t sirk_handle, cur_inst = NULL; if (next_idx < client->inst_count) { + cur_inst = lookup_instance_by_index(conn, next_idx); + /* Read next */ - cb_err = read_set_sirk(conn, next_idx); + cb_err = read_set_sirk(cur_inst); } else if (csis_client_cbs != NULL && csis_client_cbs->sets != NULL) { - csis_client_cbs->sets(conn, 0, client->inst_count, - client->set_member->sets); + csis_client_cbs->sets(client->set_member, 0, + client->inst_count); } return; @@ -854,9 +846,8 @@ static void discover_sets_resume(struct bt_conn *conn, uint16_t sirk_handle, if (cb_err != 0) { if (csis_client_cbs != NULL && csis_client_cbs->sets != NULL) { - csis_client_cbs->sets(conn, cb_err, - client->inst_count, - client->set_member->sets); + csis_client_cbs->sets(client->set_member, cb_err, + client->inst_count); } } else { busy = true; @@ -1177,19 +1168,24 @@ int bt_csis_client_discover(struct bt_csis_client_set_member *member) return err; } -int bt_csis_client_discover_sets(struct bt_conn *conn) +int bt_csis_client_discover_sets(struct bt_csis_client_set_member *member) { int err; - if (conn == NULL) { - BT_DBG("Not connected"); - return -ENOTCONN; + 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(conn, 0); + err = read_set_sirk(member->sets[0].csis); if (err == 0) { busy = true; } diff --git a/subsys/bluetooth/shell/csis_client.c b/subsys/bluetooth/shell/csis_client.c index 1dccdc1cc0a..a1d098484e9 100644 --- a/subsys/bluetooth/shell/csis_client.c +++ b/subsys/bluetooth/shell/csis_client.c @@ -100,9 +100,9 @@ static void csis_discover_cb(struct bt_csis_client_set_member *member, int err, } } -static void csis_client_discover_sets_cb(struct bt_conn *conn, int err, - uint8_t set_count, - struct bt_csis_client_set *sets) +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); @@ -110,21 +110,10 @@ static void csis_client_discover_sets_cb(struct bt_conn *conn, int err, } for (uint8_t i = 0; i < set_count; i++) { + struct bt_csis_client_set *set = &member->sets[i]; + shell_print(ctx_shell, "Set size %d (pointer: %p)", - sets[i].set_size, &sets[i]); - } - - for (size_t i = 0; i < ARRAY_SIZE(set_members); i++) { - struct bt_csis_client_set_member *member; - - member = &set_members[i]; - - if (member->conn == conn) { - for (uint8_t j = 0; j < set_count; j++) { - (void)memcpy(&member->sets[j], &sets[j], - sizeof(sets[j])); - } - } + set[i].set_size, &set[i]); } } @@ -302,7 +291,7 @@ static int cmd_csis_client_discover_sets(const struct shell *sh, size_t argc, } } - err = bt_csis_client_discover_sets(set_members[member_index].conn); + err = bt_csis_client_discover_sets(&set_members[member_index]); if (err != 0) { shell_error(sh, "Fail: %d", err); } 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 d127d61cc93..732394740d3 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 @@ -47,9 +47,9 @@ static void csis_client_lock_set_cb(int err) set_locked = true; } -static void csis_client_discover_sets_cb(struct bt_conn *conn, int err, - uint8_t set_count, - struct bt_csis_client_set *sets) +static void csis_client_discover_sets_cb(struct bt_csis_client_set_member *member, + int err, + uint8_t set_count) { printk("%s\n", __func__); @@ -58,13 +58,7 @@ static void csis_client_discover_sets_cb(struct bt_conn *conn, int err, return; } - for (uint8_t i = 0; i < set_count; i++) { - printk("Set %u: size %d\n", i, sets[i].set_size); - memcpy(&set_members[bt_conn_index(conn)].sets[i], &sets[i], - sizeof(sets[i])); - } - - set = sets; + set = &member->sets[0]; sets_discovered = true; } @@ -242,7 +236,7 @@ static void test_main(void) WAIT_FOR(discovered); - err = bt_csis_client_discover_sets(set_members[0].conn); + err = bt_csis_client_discover_sets(&set_members[0]); if (err != 0) { FAIL("Failed to do CSIS client discovery sets (%d)\n", err); return; @@ -303,7 +297,7 @@ static void test_main(void) sets_discovered = false; printk("Doing sets discovery on member[%u]", i); - err = bt_csis_client_discover_sets(set_members[i].conn); + err = bt_csis_client_discover_sets(&set_members[i]); if (err != 0) { FAIL("Failed to do CSIS client discovery sets (%d)\n", err); return;