Skip to content
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

Bluetooth: tbs: change notifications #68367

Closed

Conversation

piotrnarajowski
Copy link
Contributor

@piotrnarajowski piotrnarajowski commented Jan 31, 2024

Change notifications for tbs functions. This is needed to reproduce this issue.
work in progress

@piotrnarajowski
Copy link
Contributor Author

@Thalley This is an RFC, could you please take a look? Came across this issue while going through TBS tests that require updating characteristic values with value len > ATT_MTU-3. Without this workaround, bt_gatt_notify_uuid doesn't work and returns -ENOMEM

@Thalley
Copy link
Collaborator

Thalley commented Feb 2, 2024

@Thalley This is an RFC, could you please take a look? Came across this issue while going through TBS tests that require updating characteristic values with value len > ATT_MTU-3. Without this workaround, bt_gatt_notify_uuid doesn't work and returns -ENOMEM

The approach is fine, but needs a bit more love to make it more readable. Effectively this is truncating the notifications to fit in the MTU.

I would suggest to make a generic notify function so you only need to have the truncating code once, and then properly describe the magic numbers used. See e.g. https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/bluetooth/audio/ascs.c#L177-L218 for inspiration, as that effectively does the same.

Change notifications for tbs functions
work in progress

Signed-off-by: Piotr Narajowski <[email protected]>
@piotrnarajowski
Copy link
Contributor Author

@Thalley I can't realy find the way to make one generic ntf function...

@Thalley
Copy link
Collaborator

Thalley commented Feb 6, 2024

@Thalley I can't realy find the way to make one generic ntf function...

I don't have a line-by-line solution either, but possible the way that MCS implements it could be useful as inspiration: https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/bluetooth/audio/mcs.c#L965-L995

Copy link

github-actions bot commented Apr 7, 2024

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Apr 7, 2024
@Thalley
Copy link
Collaborator

Thalley commented Apr 7, 2024

As discussed with @sjanc the right solution may be to implement/fix #64175

@github-actions github-actions bot removed the Stale label Apr 8, 2024
Copy link

github-actions bot commented Jun 7, 2024

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jun 7, 2024
@github-actions github-actions bot closed this Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants