The bap scan delegator module's remove source operation does not return a gatt error when the sink is PA and/or BIS synchronized as it should according to the BAP spec

[jilu-ot]

The bap scan delegator module's remove source operation does not return a gatt error when the sink is PA and/or BIS synchronized as it should according to the BAP spec

Describe the bug

The BAP specicification version 1.02 section 6.5.8. says the following:

The Broadcast Assistant shall initiate the Remove Source operation only if the Broadcast Assistant has determined that the Scan Delegator collocated with the Broadcast Sink is not synchronized to a PA and/or a BIS, as defined by the values of the PA_Sync_State field and/or the BIS_Sync_State[i] field for the Broadcast Receive State characteristic, which contains the Source_ID that the Broadcast Assistant intends to use in the Remove Source operation.

The function scan_delegator_rem_src is used to handle the remove source operation initiated by a broadcast assistant.

The current version of the bap_scan_delegator function scan_delegator_rem_src terminates periodic advertisement synchronization if it is active. But section 6.5.8 in the bap specification version 1.0.2 says that the broadcast assistant must only remove the source if the broadcast assistant has determined that the sink is neither periodic advertisement synkronized and/or bis synkronised.

The scan_delegator_rem_src function should therefore not terminate any synkronization, but return a gatt error instead.

To Reproduce

Setup:

• 1 Broadcast source.
• 1 Broadcast assistant.
• 1 Broadcast sink with the BASS service.

1. Use the assistant to add and start bis synkronization with the broadcast source.
2. Use the assistant to remove the source.
   1. This should make the broadcast sink return a gatt error to the assistant. But it does not.

Expected behavior

That the broadcast sink returns a gatt error when the broadcast assistant attempts to remove a source that is periodical advertisement synkronized and/or BIS synkronized.

Impact

1. The BAP specification for the remove source operation is not followed.
2. Broadcast assistant developer are not made aware that the BAP specification is not followed for the remove operation because they do not get a gatt error.

Logs and console output

None.

Environment (please complete the following information):

• OS: Zephyr
• Zephyr
• SHA:e1b43207224e2226dc692bd294afaf869e05a201

Additional context

None.

[Thalley]

I don't disagree that we perhaps should add this to the scan delegator, but not convinced we can consider it a bug.

The requirement is for the Broadcast Assistant, and not the Scan Delegator or the BASS server.

Since we do not store the remove receive states in the broadcast assistant, we cannot perform this check in the stack, and it's up to the user of the broadcast assistant to ensure this.

I have brought this (or something similar) to the attention of the GAWG, but they declined adding the requirement on the Scan Delegator/BASS server.

Additionally, since this is a requirement added by BAP, and is not a requirement by BASS, and the Scan Delegator is unaware of whether the BASS client is a BAP Broadcast Assistant or another (possibly vendor specific) profile. That is simply a caveat of how Bluetooth profiles (unfortunately) work.

In summary:

• I don't think we can call this a bug, as it was purposely not done as it is not a requirement
• Given the use cases of the scan delegator, I'm OK adding such a check and reject the request on the server

We can consider making this a callback on the scan delegator, so that we can leave it up to the application to decide, as the application is ultimately the one who "owns" the sync.

[jilu-ot]

I suppose that the BAP spec. puts a requirement on the assistant and not the scan delegator. But I assumed that it implies that the scan delegator should not comply with the remove source request since the BAP spec. requires the assistant to stop the synchronization first.

But if not, then it sounds like a good compromise to add a callback for the application, so it can decide.

[Thalley]

But if not, then it sounds like a good compromise to add a callback for the application, so it can decide.

It's worth mentioning that this already exists, although in a less obvious way.

If there is a remove request and the PA is synced, then the pa_sync_term_req callback is called.
If there is a remove request and the PA is synced, then the bis_sync_req with all-zeroes callback is called.

The context (remove versus modify) isn't there though, and we may want to reconsider refactoring the callbacks for the scan delegator, so that the application is aware of when a source is being added, modified and removed.

[jilu-ot]

Some refactoring of the callbacks would be good to make it more intuitive.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.