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 <emil.gydesen@nordicsemi.no>

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 */

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) {
+			err = csis_client_discover_sets(client->set_member);
-				csis_client_cbs->discover(client->set_member, 0,
+			if (err != 0) {
-							  client->inst_count);
+				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) {
+		if (csis_client_cbs != NULL &&
-			csis_client_cbs->sets(client->set_member, cb_err,
+		    csis_client_cbs->discover != NULL) {
-					      client->inst_count);
+			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) {
+		if (csis_client_cbs != NULL &&
-			csis_client_cbs->sets(client->set_member, cb_err,
+		    csis_client_cbs->discover != NULL) {
-					      client->inst_count);
+			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) {
+		if (csis_client_cbs != NULL &&
-			csis_client_cbs->sets(client->set_member, cb_err,
+		    csis_client_cbs->discover != NULL) {
-					      client->inst_count);
+			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->discover != NULL) {
-			csis_client_cbs->sets(client->set_member, 0,
+			csis_client_cbs->discover(client->set_member, 0,
-					      client->inst_count);
+						  client->inst_count);
 		}
 
 		return;
 	}
 
 	if (cb_err != 0) {
-		if (csis_client_cbs != NULL && csis_client_cbs->sets != NULL) {
+		if (csis_client_cbs != NULL &&
-			csis_client_cbs->sets(client->set_member, cb_err,
+		    csis_client_cbs->discover != NULL) {
-					      client->inst_count);
+			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,

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 <set_pointer>",
 		      cmd_csis_client_discover_members, 2, 0),

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++) {