-
Notifications
You must be signed in to change notification settings - Fork 7.3k
samples: Bluetooth: Audio: Update the BAP Broadcast assistant samples #88985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
samples: Bluetooth: Audio: Update the BAP Broadcast assistant samples #88985
Conversation
Reset internal state of the sample and clear the bonding information. Signed-off-by: Graham Wacey <[email protected]>
Hello @gWacey, and thank you very much for your first pull request to the Zephyr project! |
@@ -568,7 +576,6 @@ BT_CONN_CB_DEFINE(conn_callbacks) = { | |||
int main(void) | |||
{ | |||
int err; | |||
struct bt_bap_broadcast_assistant_add_src_param param = { 0 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this moved from here to the global scope? Seems like it would be reset in either case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was moved to be consistent with the other data areas.
Where is it reset when it is only in the main?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. It should probably have been moved to the loop so that it's 0-initialized for each iteration
@@ -543,11 +545,17 @@ static void reset(void) | |||
{ | |||
printk("\n\nReset...\n\n"); | |||
|
|||
bt_le_per_adv_sync_delete(pa_sync); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pa_sync
may be NULL here; suggest to check for NULL before calling this, as well as logging any errors that could occur
@@ -480,7 +481,8 @@ static void disconnected(struct bt_conn *conn, uint8_t reason) | |||
|
|||
printk("Disconnected: %s, reason 0x%02x %s\n", addr, reason, bt_hci_err_to_str(reason)); | |||
|
|||
bt_conn_unref(broadcast_sink_conn); | |||
bt_unpair(BT_ID_DEFAULT, NULL); | |||
bt_conn_unref(conn); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should only ever be a single connection in this sample, right? Then the change to unref
seems unnecessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we discuss?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure - For visibility sake it's often better to do it here on Github so others can read the comments later :)
@@ -480,7 +481,8 @@ static void disconnected(struct bt_conn *conn, uint8_t reason) | |||
|
|||
printk("Disconnected: %s, reason 0x%02x %s\n", addr, reason, bt_hci_err_to_str(reason)); | |||
|
|||
bt_conn_unref(broadcast_sink_conn); | |||
bt_unpair(BT_ID_DEFAULT, NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it makes sense to support bonding and then unpair on disconnect.
Either we support bonding, or we don't :) (and it's required to support for LE Audio)
If you are experiencing issues with the current bonding support, we should look into alternatives such as overwriting bonds when there's a new bond only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we discuss?
Reset internal state of the sample and clear the bonding information.