Skip to content

extmod/nimble: Provide subscription information on subscribtion update. #7555

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

extmod/nimble: Provide subscription information on subscribtion update. #7555

wants to merge 1 commit into from

Conversation

MarceauFillon
Copy link

@MarceauFillon MarceauFillon commented Jul 21, 2021 •
edited
Loading

This change transmits subscription info when a subscribe event is fired.
This event initiates a new MP event and provides this information:

  • Connection handle of the connection subscribing/unsubscribing
  • Value handle of the characteristic being subscribed/unsubscribed to
  • Current subscription status to notifications (binary format 0 or 1)
  • Current subscription status to indications (binary format 0 or 1)

This implementation is inspired from another solution.
This solution uses such events to keep a list of subscribed connections.
See: https://github.com/h2zero/esp-nimble-cpp/search?q=setSubscribe

@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label Jul 21, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2021 •
edited
Loading

Codecov Report

Merging #7555 (8a49dff) into master (d934f8c) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7555      +/-   ##
==========================================
- Coverage   98.29%   98.27%   -0.02%     
==========================================
  Files         154      154              
  Lines       19990    19992       +2     
==========================================
- Hits        19649    19647       -2     
- Misses        341      345       +4
Impacted Files Coverage Δ
py/nativeglue.c 94.44% <0.00%> (-1.12%) ⬇️
py/obj.c 96.03% <0.00%> (-0.40%) ⬇️
py/runtime.c 99.24% <0.00%> (-0.30%) ⬇️
py/vm.c 99.03% <0.00%> (-0.01%) ⬇️
py/emitnative.c 99.26% <0.00%> (-0.01%) ⬇️
py/obj.h 100.00% <0.00%> (ø)
py/modsys.c 100.00% <0.00%> (ø)
py/objtype.c 100.00% <0.00%> (ø)
py/objenumerate.c 100.00% <0.00%> (ø)
py/objgenerator.c 100.00% <0.00%> (ø)
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d934f8c...8a49dff. Read the comment docs.

extmod/nimble: Provide subscription information on subscribtion update.
8a49dff 
This change transmits subscription info when a subscribe event is fired.
This event initiates a new MP event and provides this information:

- Connection handle of the connection subscribing/unsubscribing
- Value handle of the characteristic being subscribed/unsubscribed to
- Current subscription status to notifications (binary format 0 or 1)
- Current subscription status to indications (binary format 0 or 1)

This implementation is inspired from another solution.
This solution uses such events to keep a list of subscribed connections.
See: https://github.com/h2zero/esp-nimble-cpp/search?q=setSubscribe
@MarceauFillon MarceauFillon force-pushed the nimble_lib_on_subscription_update branch from 60b7dd4 to 8a49dff Compare July 21, 2021 23:53
MarceauFillon pushed a commit to MarceauFillon/micropython-lib that referenced this pull request Jul 22, 2021
micropython/bluetooth/aioble: save subscribed connections
3d20816 
This change handles a new micropython event.
This event is fired upon susbcription update.
A characteristic now saves a list of subscribed connections.
This change depends on this PR: micropython/micropython#7555
@MarceauFillon MarceauFillon mentioned this pull request Jul 22, 2021
Closed
@jimmo
Copy link
Member

jimmo commented Jul 22, 2021

I'm happy to add support for the subscription status event, but I'm not sure I want to make management of subscription part of the application's responsibility (or aioble) as it should be possible to keep this entirely internal to the stack. See comments at #6848 (comment) -- is it sufficient to just be able to set update=True on your writes and have NimBLE send the notification/indication to the subscribed peers?

@jimmo jimmo mentioned this pull request Jul 23, 2021
Closed
@jimmo
Copy link
Member

jimmo commented Jul 23, 2021

See #7564 for adding send_update=True to gatts_write.

@MarceauFillon
Copy link
Author

@jimmo the fix implemented in #7564 leaving NimBLE to deal with notification/indication to the subscribed peers is sufficient for our needs thanks. I will close this Pull Request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
extmod Relates to extmod/ directory in source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MarceauFillon @codecov-commenter @jimmo @dpgeorge