Bluetooth: BAP: Bap conn checks #96282
Open
+382
−245
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We can actually already when we bind the endpoint to the stream perform the call to bt_bap_iso_bind_ep, if the stream has been added to a group. If the stream has not yet been added to a group, then when the stream is added to a group, then bt_bap_iso_bind_ep will be done there (existing behavior). Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add a check that verifies that 2 endpoints from different connection don't share a CIS, which would be invalid. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Adds assert for bt_bap_iso_bind_ep to ensure that endpoints from different connection don't share a CIS (a bt_bap_iso). Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Thalley
force-pushed
the
bap_conn_fix
branch
from
d96871d to
d1fb90d
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request adds additional connection checks to ensure that endpoints sharing the same CIS (Connected Isochronous Stream) also share the same connection in the Bluetooth Audio Profile (BAP) unicast client implementation.
Key Changes
- Added connection validation between paired endpoints in the same CIS during QoS configuration
- Refactored unicast group creation to use stream pairs with proper ISO channel allocation
- Enhanced test infrastructure to create proper unicast groups with connection validation
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/bluetooth/audio/cap_initiator/uut/bap_unicast_client.c | Refactored group creation logic with ISO allocation and stream pairing |
| tests/bluetooth/audio/cap_initiator/src/test_unicast_start.c | Updated test fixture to create proper unicast groups and simplified test parameters |
| tests/bluetooth/audio/cap_initiator/src/test_common.c | Added stream configuration call to test state setup |
| subsys/bluetooth/audio/bap_unicast_client.c | Added connection validation for paired endpoints during QoS configuration |
| subsys/bluetooth/audio/bap_stream.c | Enhanced stream configuration with endpoint binding and error handling |
| subsys/bluetooth/audio/bap_iso.c | Added connection assertion for paired endpoints during ISO binding |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| /* Store the stream Codec QoS in the bap_iso */ | ||
| unicast_client_qos_cfg_to_iso_qos(iso, qos, dir); | ||
|
|
||
| /* Store the group Codec QoS in the group - This assume thats the parameters have been |
There was a problem hiding this comment.
There are spelling and grammatical errors in the comment. 'assume thats' should be 'assumes that' and 'parameters have' should be 'parameters have'.
Suggested change
| /* Store the group Codec QoS in the group - This assume thats the parameters have been | |
| /* Store the group Codec QoS in the group - This assumes that the parameters have been |
Copilot uses AI. Check for mistakes.
| /* CAP requires stream context - Let's remove it */ | ||
| memset(stream_param.codec_cfg->meta, 0, sizeof(stream_param.codec_cfg->meta)); | ||
| stream_param.codec_cfg->meta_len = 0U; | ||
| (void)memset(fixture->audio_start_stream_params[0].codec_cfg, 0, |
There was a problem hiding this comment.
The memset is targeting the codec_cfg pointer itself instead of the meta field. It should be (void)memset(fixture->audio_start_stream_params[0].codec_cfg->meta, 0, sizeof(fixture->audio_start_stream_params[0].codec_cfg->meta));
Suggested change
| (void)memset(fixture->audio_start_stream_params[0].codec_cfg, 0, | |
| (void)memset(fixture->audio_start_stream_params[0].codec_cfg->meta, 0, |
Copilot uses AI. Check for mistakes.
zephyrbot
requested review from
asbjornsabo,
babrsn,
fredrikdanebjer,
jhedberg,
jthm-ot,
larsgk,
niym-ot and
sjanc
Update the CAP initiator audio start unittest. The reason for this, is that the recent change to endpoints means that we need to properly generate a BAP unicast group and call bt_bap_stream_config, instead of trying to manage the bap_iso values directly. As part of updating this, the tests were also updated to significantly reduce code duplication, but generating a default start param that can easily be reused. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Thalley
force-pushed
the
bap_conn_fix
branch
from
3a00c81 to
c1610b4
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add additional checks to ensure that 2 endpoints sharing the same CIS also share the same connection (the CIS does not yet have a connection at this point, so the stream->conn is used for this).