Bluetooth: BAP: SD: Add check for mixing NO_PREF with specific BIS #89936
Conversation
Thalley
force-pushed
the
scan_delegator_no_pref_fix
branch
from
13a779f to
ba0ccfc
Compare
github-actions
bot
requested review from
asbjornsabo,
babrsn,
Casper-Bonde-Bose,
fredrikdanebjer,
jhedberg,
larsgk,
MariuszSkamra,
niym-ot,
pin-zephyr and
sjanc
There was a problem hiding this comment.
Pull Request Overview
This PR adds a check to ensure that a Bluetooth broadcast assistant does not mix specific BIS indexes with the BT_BAP_BIS_SYNC_NO_PREF flag in the request.
- Introduces a new helper function, valid_bis_sync_request(), to consolidate validation for BIS sync requests.
- Updates calls in bap_scan_delegator.c and bap_broadcast_assistant.c to use the new function for improved clarity and correctness.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| subsys/bluetooth/audio/bap_scan_delegator.c | New valid_bis_sync_request() introduced and existing validation logic updated. |
| subsys/bluetooth/audio/bap_broadcast_assistant.c | Refactored BIS sync validation to use valid_bis_sync_request() for consistency. |
|
Since the check is duplicated code, I did consider moving it into a common place. We don't really have a bap_common.c or similar, and making it a common function would also perhaps make it harder to interpret the logs, as it loses the context of the role. Reviewers: Please let me know if you want me to make it a common function |
Thalley
force-pushed
the
scan_delegator_no_pref_fix
branch
from
ba0ccfc to
1dfe2a7
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds additional validation logic for BIS sync index requests to prevent mixing the special BT_BAP_BIS_SYNC_NO_PREF value with specific BIS indexes, as required by Bluetooth specification updates.
- Introduces a helper function valid_bis_sync_request in two files to perform combined duplicate and consistency checks.
- Replaces previous isolated checks with calls to valid_bis_sync_request, ensuring consistent logging and validation behavior.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| subsys/bluetooth/audio/bap_scan_delegator.c | Replaces individual BIS sync validations with a unified function call |
| subsys/bluetooth/audio/bap_broadcast_assistant.c | Incorporates the new valid_bis_sync_request to validate BIS sync indexes in subgroups |
Comments suppressed due to low confidence (1)
subsys/bluetooth/audio/bap_scan_delegator.c:159
- [nitpick] For clarity and consistency, consider using uniform hexadecimal formatting (e.g., using 0x%08x for both requested and aggregated values) in the log message.
LOG_DBG("Duplicate BIS index 0x%08x (aggregated %x)", requested_bis_syncs, aggregated_bis_syncs);
| @@ -152,6 +152,30 @@ static bool bis_syncs_unique_or_no_pref(uint32_t requested_bis_syncs, | |||
| return (requested_bis_syncs & aggregated_bis_syncs) != 0U; | |||
| } | |||
|
|
|||
| static bool valid_bis_sync_request(uint32_t requested_bis_syncs, uint32_t aggregated_bis_syncs) | |||
There was a problem hiding this comment.
Consider centralizing the valid_bis_sync_request implementation into a common utility module to reduce code duplication across files.
Copilot uses AI. Check for mistakes.
Based on a dicussion on the Bluetooth SIG GAWG reflector, it is not allowed for a broadcast assistant to request specific BIS indexes as well as BT_BAP_BIS_SYNC_NO_PREF in the same request. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Based on a dicussion on the Bluetooth SIG GAWG reflector, it is not allowed for a broadcast assistant to request specific BIS indexes as well as BT_BAP_BIS_SYNC_NO_PREF in the same request. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Thalley
force-pushed
the
scan_delegator_no_pref_fix
branch
from
1dfe2a7 to
1d7c54c
Compare
|
Based on a dicussion on the Bluetooth SIG GAWG reflector, it is not allowed for a broadcast assistant to request specific BIS indexes as well as BT_BAP_BIS_SYNC_NO_PREF in the same request.