-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
extmod/nimble: Automatically notify subscribed connections on gatts_write(). #6848
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
Conversation
fac8509
to
f905c23
Compare
hi @andrewleech -- I think if I were designing this from scratch I'd do it this way, but unfortunately this is a breaking change to existing code that already explicitly calls gatts_notify after gatts_write (leading to duplicate notifications). So I think if we're going to merge this we need to make this behavior opt-in, either by an additional argument to gatts_write, or an extra flag on the characteristic. |
@jimmo yes the compatibility issue is a significant one. This auto notification also makes it harder to know when an indication will be sent / should be listened for. The other big issue with this change is I can't find any matching function for btstack - leaving me with a situation where I'd need to manually C code a similar functionality for btstack.... or we can just ditch this and handle it all in aioble instead :-) |
I propose we add a optional argument to gatts_write Thinking about what event to raise for this... in the case where there are no subscribers or all subscribers are notify-only, there's no event to raise. For indications, NimBLE does have considerable logic to ensure delivery of the indication, even if a subscribed peer is disconnected it will be sent on next connection, and the pending bit will only be cleared on ack. So this means that the concept of a "update-indication-complete" event is a bit unclear. i.e. in many cases you don't want to be waiting for this to happen. So we'll still raise the _IRQ_GATTS_INDICATE_DONE event when it succeeds, but I don't think it's appropriate for e.g. aioble to be able to On btstack, we manage the CCCD ourselves, so we can implement similar functionality to ble_gatts_chr_updated (although it won't be particularly simple). |
Just a further thought -- from testing (and reading the NimBLE code), the gatts_notify and gatts_indicate methods completely ignore the subscription state (and do not set the pending bit in the CCCD). So they should stay unchanged, and can be used for the "sending a notification/indication to a known connected peer, with immediate ack event for indicate". |
See #7564 for adding |
Closed in preference for #7564 |
…n-main Translations update from Hosted Weblate
This change means that when a central has subscribed to notifications on a characteristic on the peripheral running micropython, any writes to that characteristic are automatically notified to the connected central.
This is the same as the accidentally-closed #6305
I know this change has some side-effects in the tests that have not yet been resolved.