Bluetooth: Audio: Fixing discovery and subscription #67629
Merged
Conversation
This file contains 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
Thalley
force-pushed
the
auto_ccc_discover_fix
branch
5 times, most recently
from
3edc1cd to
fb3e299
Compare
Thalley
changed the title
Thalley
changed the title
Thalley
force-pushed
the
auto_ccc_discover_fix
branch
8 times, most recently
from
02a8197 to
8803061
Compare
zephyrbot
requested review from
asbjornsabo,
Casper-Bonde-Bose,
fredrikdanebjer,
jhedberg,
kruithofa,
larsgk,
MariuszSkamra and
sjanc
Thalley
force-pushed
the
auto_ccc_discover_fix
branch
2 times, most recently
from
48a5d1e to
0659d04
Compare
fredrikdanebjer
previously approved these changes
Feb 1, 2024
Thalley
force-pushed
the
auto_ccc_discover_fix
branch
from
0659d04 to
668bbf3
Compare
|
Rebased so solve merge conflicts |
fredrikdanebjer
previously approved these changes
Feb 1, 2024
Thalley
force-pushed
the
auto_ccc_discover_fix
branch
from
668bbf3 to
d97c605
Compare
|
Fixed build errors in MICP and Broadcast Assistant after rebase |
Thalley
force-pushed
the
auto_ccc_discover_fix
branch
from
d97c605 to
9ca10dd
Compare
kruithofa
previously approved these changes
Feb 2, 2024
fredrikdanebjer
previously approved these changes
Feb 2, 2024
|
@Thalley please rebase |
…ribe Several places the LE Audio clients called bt_gatt_subscribe without checking the return value, which could cause some issues in the worst case, and in the best case, cause some unexpected behavior. Some implementations had a bit more updating to handle the new behavior. Signed-off-by: Emil Gydesen <[email protected]>
The LE Audio implementations do not really support bonding yet, and removing subs on disconnect is the most effective (and correct) way of ensuring that we do not subscribe more than once when we re-discover after reconnection. The broadcast assistant and the media control client does not support multiple connections as of this commit, so they needed special treatment. In the case that we do discovery on multiple ACL connections, it is important that the existing subscriptions are removed correctly by calling bt_gatt_unsubscribe. In order to implement this change properly on some of the clients, thet had no proper connection references or support for clearing the data on disconnects, they had to be updated as well. The csip_notify.sh test has been disabled, as that expected a notification in the client, but since this commit removes that (until bonding is properly supported in the clients), then the test will fail. Signed-off-by: Emil Gydesen <[email protected]>
Expand the babblesim tests for LE audio to verify that all the discovery functions can be called multiple times without error. HAS is an exception as that has an existing separate check that disallows discovery multiple times. Signed-off-by: Emil Gydesen <[email protected]>
Thalley
dismissed stale reviews from fredrikdanebjer and kruithofa
via
bb70533
Thalley
force-pushed
the
auto_ccc_discover_fix
branch
from
9ca10dd to
bb70533
Compare
|
Rebased to solve merge conflicts |
fredrikdanebjer
approved these changes
Feb 2, 2024
kruithofa
approved these changes
Feb 2, 2024
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.
The way that bt_gatt_subscribe is used by the LE Audio GATT clients were subpar. They were built on the assumption that we are not using bonding. None of the clients really support proper bonding, and thus the subscriptions should not be considered working after a disconnection.
For this
BT_GATT_SUBSCRIBE_FLAG_VOLATILEhas been used, until we support bonding.The discovery functions have been modified to better support this as well, except for the broadcast assistant and media control client that have some other fundamental issue with multiple connections.
One BSIM test was disabled as that expected bonding support on the client.
Multiple BSIM tests were updated to verify that we can at least do multiple discovery calls for a single connection, but should be updated to also verify that discovery and other features work after a reconnect, but that is a TODO for now.
fixes #67434