Skip to content

Bluetooth: Audio: BASS server shall set BIS_Sync_State to be 0x0 afte… #89890

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pin-zephyr
Copy link
Collaborator

…r stopping synchronization

BASS server need to notify that both BIS and PA are no longer synced by setting all required fields to 0 after client stops BIS by sending a BIG control PDU with a BIG TERMINATED indication.

meanwhile BIG_Encryption in the notification should also be reset to be BT_BAP_BIG_ENC_STATE_NO_ENC when BIS is not synced anymore.

BASS server need to notify that both BIS and PA are no longer synced by
setting all required fields to 0 after client stops BIS by sending a
BIG control PDU with a BIG TERMINATED indication.

meanwhile BIG_Encryption in the notification should also be reset to be
BT_BAP_BIG_ENC_STATE_NO_ENC when BIS is not synced anymore.

Signed-off-by: Ping Wang <[email protected]>
@pin-zephyr pin-zephyr force-pushed the bass_notification_bis_reset branch from 02b1778 to 8192eae Compare May 14, 2025 06:58
Comment on lines +173 to 180
} else if (reason == BT_HCI_ERR_CONN_TIMEOUT ||
reason == BT_HCI_ERR_LOCALHOST_TERM_CONN ||
reason == BT_HCI_ERR_REMOTE_USER_TERM_CONN) {
/* BIS is not synced anymore */
mod_src_param.encrypt_state = BT_BAP_BIG_ENC_STATE_NO_ENC;
} else {
mod_src_param.encrypt_state = recv_state->encrypt_state;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the else here just be mod_src_param.encrypt_state = BT_BAP_BIG_ENC_STATE_NO_ENC;?

If update_recv_state_big_cleared is called and it isn't failing due to BT_HCI_ERR_TERM_DUE_TO_MIC_FAIL when we are attempting to decrypt, I guess the encrypt state should just always be set to BT_BAP_BIG_ENC_STATE_NO_ENC

} else {
mod_src_param.encrypt_state = recv_state->encrypt_state;
}

if (reason != BT_HCI_ERR_LOCALHOST_TERM_CONN) {
if (reason != BT_HCI_ERR_CONN_TIMEOUT &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can BT_HCI_ERR_CONN_TIMEOUT happen when attempting to sync to the BIG? Or only after a successfull sync?

I'm wondering if the logic should be:
If mod_src_param.subgroups[i].bis_sync is set for any subgroup, then we consider it synced and set it to 0.
If mod_src_param.subgroups[i].bis_sync is 0, then we consider it non-synced and set it to BT_BAP_BIS_SYNC_FAILED.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I understand, BT_HCI_ERR_CONN_TIMEOUT(e.g link loss) can happen at any time, I am not so sure if the logic you mentioned can cause some potential issue :-)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that it's not particular explicit in the core spec what termination reason is sent when. The point of my comment was to achieve the same solution but in a way that won't be affected by controllers that may have different behavior

For example, if the BIG Sync fails, then some controller may perhaps send BT_HCI_ERR_CONN_TIMEOUT as the reason, in which case BT_BAP_BIS_SYNC_FAILED should be set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

2 participants