[nrf fromtree] bluetooth: audio: scan_delegator: Reject remove when PA synced

- BASS mandates that a remove_source operation is not done if
  we are synced to PA or BIS. This PR fixes that
- Update BabbleSim tests to reflect this behavior
- Fixes BASS/SR/SPE/BI-05-C

Signed-off-by: Alexander Svensen <[email protected]>
(cherry picked from commit 87997af)

Author: alexsven

subsys/bluetooth/audio/bap_scan_delegator.c

@@ -1051,7 +1051,6 @@ static int scan_delegator_rem_src(struct bt_conn *conn,
 {
 	struct bass_recv_state_internal *internal_state;
 	struct bt_bap_scan_delegator_recv_state *state;
-	bool bis_sync_was_requested = false;
 	uint8_t src_id;
 	int err;
 
@@ -1078,38 +1077,40 @@ static int scan_delegator_rem_src(struct bt_conn *conn,
 
 	state = &internal_state->state;
 
-	/* If conn == NULL then it's a local operation and we do not need to ask the application */
-	if (conn != NULL) {
-
-		if (scan_delegator_cbs != NULL && scan_delegator_cbs->remove_source != NULL) {
-			err = scan_delegator_cbs->remove_source(conn, src_id);
-			if (err != 0) {
-				LOG_DBG("Remove Source rejected with reason 0x%02x", err);
-				err = k_mutex_unlock(&internal_state->mutex);
-				__ASSERT(err == 0, "Failed to unlock mutex: %d", err);
-				return BT_GATT_ERR(BT_ATT_ERR_WRITE_REQ_REJECTED);
-			}
-		}
-
-		if (state->pa_sync_state == BT_BAP_PA_STATE_INFO_REQ ||
-		    state->pa_sync_state == BT_BAP_PA_STATE_SYNCED) {
-			/* Terminate PA sync */
-			err = pa_sync_term_request(conn, &internal_state->state);
-			if (err != 0) {
-				LOG_DBG("PA sync term from %p was rejected with reason %d",
-					(void *)conn, err);
-				err = k_mutex_unlock(&internal_state->mutex);
-				__ASSERT(err == 0, "Failed to unlock mutex: %d", err);
-				return BT_GATT_ERR(BT_ATT_ERR_WRITE_REQ_REJECTED);
-			}
-		}
+	if (state->pa_sync_state == BT_BAP_PA_STATE_SYNCED) {
+		LOG_DBG("Cannot remove source ID 0x%02x while PA is synced", state->src_id);
+		err = k_mutex_unlock(&internal_state->mutex);
+		__ASSERT(err == 0, "Failed to unlock mutex: %d", err);
+		/* We shouldn't return a success here, but the Test Spec requires it at the moment,
+		 * Errata to fix this: https://bluetooth.atlassian.net/browse/ES-28445
+		 */
+		return BT_GATT_ERR(BT_ATT_ERR_SUCCESS);
 	}
 
 	for (uint8_t i = 0U; i < state->num_subgroups; i++) {
 		if (internal_state->requested_bis_sync[i] != 0U &&
 		    internal_state->state.subgroups[i].bis_sync != 0U) {
-			bis_sync_was_requested = true;
-			break;
+			LOG_DBG("Cannot remove source ID 0x%02x while BIS is synced",
+				state->src_id);
+			err = k_mutex_unlock(&internal_state->mutex);
+			__ASSERT(err == 0, "Failed to unlock mutex: %d", err);
+			/* We shouldn't return a success here, but the Test Spec requires it at the
+			 * moment, Errata to fix this:
+			 * https://bluetooth.atlassian.net/browse/ES-28445
+			 */
+			return BT_GATT_ERR(BT_ATT_ERR_SUCCESS);
+		}
+	}
+
+	/* If conn == NULL then it's a local operation and we do not need to ask the application */
+	if (conn != NULL && scan_delegator_cbs != NULL &&
+	    scan_delegator_cbs->remove_source != NULL) {
+		err = scan_delegator_cbs->remove_source(conn, src_id);
+		if (err != 0) {
+			LOG_DBG("Remove Source rejected with reason 0x%02x", err);
+			err = k_mutex_unlock(&internal_state->mutex);
+			__ASSERT(err == 0, "Failed to unlock mutex: %d", err);
+			return BT_GATT_ERR(BT_ATT_ERR_WRITE_REQ_REJECTED);
 		}
 	}
 
@@ -1128,10 +1129,6 @@ static int scan_delegator_rem_src(struct bt_conn *conn,
 	err = k_mutex_unlock(&internal_state->mutex);
 	__ASSERT(err == 0, "Failed to unlock mutex: %d", err);
 
-	if (bis_sync_was_requested) {
-		bis_sync_request_updated(conn, internal_state);
-	}
-
 	/* app callback */
 	receive_state_updated(conn, internal_state);
 

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

@@ -502,7 +502,7 @@ static void test_bass_add_source(void)
 	printk("Source added\n");
 }
 
-static void test_bass_mod_source(uint32_t bis_sync)
+static void test_bass_mod_source(bool pa_sync, uint32_t bis_sync)
 {
 	int err;
 	struct bt_bap_broadcast_assistant_mod_src_param mod_src_param = { 0 };
@@ -515,7 +515,7 @@ static void test_bass_mod_source(uint32_t bis_sync)
 	UNSET_FLAG(flag_recv_state_updated);
 	mod_src_param.src_id = recv_state.src_id;
 	mod_src_param.num_subgroups = 1;
-	mod_src_param.pa_sync = true;
+	mod_src_param.pa_sync = pa_sync;
 	mod_src_param.subgroups = &subgroup;
 	mod_src_param.pa_interval = g_broadcaster_info.interval;
 	subgroup.bis_sync = bis_sync;
@@ -542,14 +542,16 @@ static void test_bass_mod_source(uint32_t bis_sync)
 		WAIT_FOR_AND_CLEAR_FLAG(flag_recv_state_updated);
 	}
 
-	if (recv_state.pa_sync_state != BT_BAP_PA_STATE_SYNCED) {
-		FAIL("Unexpected PA sync state: %d\n", recv_state.pa_sync_state);
-		return;
-	}
+	if (pa_sync) {
+		if (recv_state.pa_sync_state != BT_BAP_PA_STATE_SYNCED) {
+			FAIL("Unexpected PA sync state: %d\n", recv_state.pa_sync_state);
+			return;
+		}
 
-	if (recv_state.encrypt_state != BT_BAP_BIG_ENC_STATE_NO_ENC) {
-		FAIL("Unexpected BIG encryption state: %d\n", recv_state.pa_sync_state);
-		return;
+		if (recv_state.encrypt_state != BT_BAP_BIG_ENC_STATE_NO_ENC) {
+			FAIL("Unexpected BIG encryption state: %d\n", recv_state.pa_sync_state);
+			return;
+		}
 	}
 
 	if (recv_state.num_subgroups != mod_src_param.num_subgroups) {
@@ -563,19 +565,15 @@ static void test_bass_mod_source(uint32_t bis_sync)
 		WAIT_FOR_AND_CLEAR_FLAG(flag_recv_state_updated);
 	}
 
-	remote_bis_sync = recv_state.subgroups[0].bis_sync;
-	if (subgroup.bis_sync == 0) {
-		if (remote_bis_sync != 0U) {
-			FAIL("Unexpected BIS sync value: %u\n", remote_bis_sync);
-			return;
-		}
-	} else {
+	if (bis_sync != 0) {
+		remote_bis_sync = recv_state.subgroups[0].bis_sync;
 		printk("Waiting for BIS sync\n");
 
 		if (remote_bis_sync == 0U &&
 		    recv_state.encrypt_state == BT_BAP_BIG_ENC_STATE_NO_ENC) {
-			/* Wait for another notification, which will either request a broadcast code
-			 * for encrypted broadcasts, or have the BIS sync values set
+			/* Wait for another notification, which will either request a
+			 * broadcast code for encrypted broadcasts, or have the BIS sync
+			 * values set
 			 */
 			printk("Waiting for another receive state update with BIS sync\n");
 			WAIT_FOR_AND_CLEAR_FLAG(flag_recv_state_updated);
@@ -770,14 +768,15 @@ static void test_main_client_sync(void)
 	test_bass_scan_stop();
 	test_bass_create_pa_sync();
 	test_bass_add_source();
-	test_bass_mod_source(0);
+	test_bass_mod_source(true, 0);
 	test_bass_mod_source_long_meta();
-	test_bass_mod_source(BT_ISO_BIS_INDEX_BIT(1) | BT_ISO_BIS_INDEX_BIT(2));
+	test_bass_mod_source(true, BT_ISO_BIS_INDEX_BIT(1) | BT_ISO_BIS_INDEX_BIT(2));
 	test_bass_broadcast_code(BROADCAST_CODE);
 
 	printk("Waiting for receive state with BIS sync\n");
 	WAIT_FOR_FLAG(flag_recv_state_updated_with_bis_sync);
 
+	test_bass_mod_source(false, 0);
 	test_bass_remove_source();
 
 	PASS("BAP Broadcast Assistant Client Sync Passed\n");
@@ -797,11 +796,12 @@ static void test_main_client_sync_incorrect_code(void)
 	test_bass_scan_stop();
 	test_bass_create_pa_sync();
 	test_bass_add_source();
-	test_bass_mod_source(BT_ISO_BIS_INDEX_BIT(1));
+	test_bass_mod_source(true, BT_ISO_BIS_INDEX_BIT(1));
 	WAIT_FOR_FLAG(flag_broadcast_code_requested);
 	test_bass_broadcast_code(INCORRECT_BROADCAST_CODE);
 	WAIT_FOR_FLAG(flag_incorrect_broadcast_code);
 
+	test_bass_mod_source(false, 0);
 	test_bass_remove_source();
 
 	PASS("BAP Broadcast Assistant Client Sync Passed\n");
@@ -825,6 +825,7 @@ static void test_main_server_sync_client_rem(void)
 	WAIT_FOR_FLAG(flag_recv_state_updated_with_bis_sync);
 
 	printk("Attempting to remove source for the first time\n");
+	test_bass_mod_source(false, 0);
 	test_bass_remove_source();
 
 	WAIT_FOR_FLAG(flag_remove_source_rejected);

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

@@ -213,6 +213,8 @@ static int pa_sync_term(struct sync_state *state)
 
 	printk("Deleting PA sync\n");
 
+	UNSET_FLAG(flag_pa_terminated);
+
 	err = bt_le_per_adv_sync_delete(state->pa_sync);
 	if (err != 0) {
 		FAIL("Could not delete per adv sync: %d\n", err);
@@ -221,6 +223,8 @@ static int pa_sync_term(struct sync_state *state)
 		state->pa_sync = NULL;
 	}
 
+	WAIT_FOR_FLAG(flag_pa_terminated);
+
 	return err;
 }
 
@@ -708,13 +712,48 @@ static void remove_all_sources(void)
 
 			printk("[%zu]: Source removed with id %u\n",
 			       i, state->src_id);
+		}
+	}
+}
 
-			printk("Terminating PA sync\n");
-			err = pa_sync_term(state);
-			if (err) {
-				FAIL("[%zu]: PA sync term failed (err %d)\n", err);
-				return;
-			}
+static void terminate_all_pa(void)
+{
+	for (size_t i = 0U; i < ARRAY_SIZE(sync_states); i++) {
+		int err;
+		struct sync_state *state = &sync_states[i];
+
+		if (state->pa_sync == NULL) {
+			continue;
+		}
+
+		err = pa_sync_term(state);
+		if (err != 0) {
+			FAIL("[%zu]: PA sync term failed (err %d)\n", i, err);
+			return;
+		}
+	}
+}
+
+static void set_bis_sync_state(struct sync_state *state,
+			       uint32_t bis_sync_req[CONFIG_BT_BAP_BASS_MAX_SUBGROUPS])
+{
+	int err;
+
+	err = bt_bap_scan_delegator_set_bis_sync_state(state->src_id, bis_sync_req);
+	if (err != 0) {
+		FAIL("Could not set BIS sync state: %d\n", err);
+		return;
+	}
+}
+
+static void set_all_bis_sync_states(uint32_t bis_sync_req[CONFIG_BT_BAP_BASS_MAX_SUBGROUPS])
+{
+	for (size_t i = 0U; i < ARRAY_SIZE(sync_states); i++) {
+		struct sync_state *state = &sync_states[i];
+
+		if (state->recv_state != NULL) {
+			printk("[%zu]: Setting BIS sync state\n", i);
+			set_bis_sync_state(state, bis_sync_req);
 		}
 	}
 }
@@ -914,7 +953,12 @@ static void test_main_server_sync_server_rem(void)
 	/* Set the BIS sync state */
 	sync_all_broadcasts();
 
-	/* Remote all sources, causing PA sync term request to trigger */
+	uint32_t bis_sync_req[CONFIG_BT_BAP_BASS_MAX_SUBGROUPS] = {0};
+
+	set_all_bis_sync_states(bis_sync_req);
+	terminate_all_pa();
+
+	/* Remove all sources, causing PA sync term request to trigger */
 	remove_all_sources();
 
 	/* Wait for PA sync to be terminated */