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

Simplify bluetooth API #69

Closed
geeksville opened this issue Mar 29, 2020 · 4 comments
Closed

Simplify bluetooth API #69

geeksville opened this issue Mar 29, 2020 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@geeksville
Copy link
Member

geeksville commented Mar 29, 2020

In chat @girtsf asked (wrt the python API library/tool he's developing):

what would be the tradeoffs for grouping most things into a single bluetooth characteristic and having a top-level proto with a oneof?

Yah, some of this is an accident of history. That was my original implementation - you can see some notes here in the inline docs for ToRadio and FromRadio:

https://github.com/meshtastic/Meshtastic-protobufs/blob/master/mesh.proto#L440

But I switched relatively early to the sixish ble characteristics you see there currently. There were two reasons (which kinda made since then but less so now):

  • With multiple characteristics I didn't need to build a state machine on the device side to provide complex behavior for when the phone reads FromRadio. i.e. I woudn't need to go "the phone has sent WantNodes to me, so I should send one node from the DB at a time until done with that and then go back to providing received messages from the mesh. In fact, thinking now about it I'm not sure such a statemachine is needed on the device side, it might be better to just pass through any RX mesh packets we have and only if none are waiting, send the node db records? (then let the phone be smart enough to not process those messages until it has a nodedb (which it needs to intelligently provide those messages to higher application layers)

  • I initially didn't have a working phone app (or trusted/working device bluetooth code), so it was super handy for me to be able to use a tool like NRFConnect to do initial debugging of the device side (and that tool doesn't understand protobufs).

Now that things are a bit more mature, I think it might be a super good idea to go back to just having only the two FromRadio and ToRadio characteristics. This could be done incrementally without breaking the android app by:

  1. initially develop your python API by using the current device code (unchanged) but only implement FromRadio handling (if a packet arrives there print it to your debug console) and ToRadio handling (just have a test function to send a test packet to the broadcast address of a "hello world" text message). This would allow you to get the basic device/app plumbing all working before having to change any device code.

At startup read from FromRadio until you get back an empty packet. From then on just subscribe to NOTIFY ble messages for FromRadio. (This is similar to the android app)

Don't implement/use any of the other characteristics.

  1. Uncomment the extra lines you see commented out in FromRadio and ToRadio. You'll also need to add a couple of other Protobufs in that 'API' for things like 'WantConfig', 'WantMyNodeInfo' etc... Change the device code that handles ToRadio writes to properly handle these new operations. Either you can do this, or if you don't feel comfortable yet mucking with that device code tell me you want me to do it.

  2. Finish your python API using just these two characteristics. (Separately I'll change the android app to only use these two characteristics)

  3. Delete the 4ish unneeded characteristics from the docs and the device code.

(cc @jeksys because useful and relevant to you)

@geeksville geeksville added the enhancement New feature or request label Mar 29, 2020
@geeksville
Copy link
Member Author

another nice benefit of this change: someday exposing the same API over serial or a UDP socket becomes trivial.

@geeksville
Copy link
Member Author

Hey @girtsf - I think I will do this now (I'll keep the old API working though). Doing it now will prevent me from wasting some work in the NRF52 port.

@geeksville geeksville assigned geeksville and unassigned girtsf Apr 20, 2020
@geeksville
Copy link
Member Author

geeksville commented Apr 20, 2020

(also - I need a way to tunnel debug output on that platform (rather than serial) so I think I'm going to make an adapter to (if the app asks) to squirt debug messages from the MCU over protobufs in FromRadio. This will also allow bug reports to include device debug putput)

also - I just found a nice mac and linux friendly python api: https://github.com/adafruit/Adafruit_Python_BluefruitLE

geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Apr 21, 2020
geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Apr 22, 2020
@geeksville
Copy link
Member Author

@girtsf Did you ever make much progress on the python client idea? (no pressure if not) I'm almost finished with these changes (changing the android app to use them later tonight?). But after that just to test that this new PhoneAPI class really is detached from bluetooth, I'm thinking an easy way to do that is to make a SerialPhoneAPI subclass, which can instantiate it on a serial port. To test that idea I'm interested in making a small python client.

geeksville added a commit to geeksville/Meshtastic-Android that referenced this issue Apr 23, 2020
geeksville added a commit to geeksville/Meshtastic-Android that referenced this issue Apr 23, 2020
geeksville added a commit to geeksville/Meshtastic-Android that referenced this issue Apr 23, 2020
mc-hamster added a commit that referenced this issue Feb 17, 2021
mc-hamster added a commit that referenced this issue Feb 17, 2021
Merge pull request #69 from mc-hamster/master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants