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

Add new backend: Silicon Labs BGAPI #1194

Draft
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

karlp
Copy link
Contributor

@karlp karlp commented Jan 6, 2023

This supports the (very nice) bleak API for any device that uses the Silicon Labs BGAPI protocol. This is mostly a Silabs EFR32 type device running "NCP" firmware, like so:
Block diagram of NCP

This is "cross platform" in an entirely different sense :)

I've implemented this in a couple of commits, to make it a little easier to review, but I'm happy to go back and squash/split further if desired. It's largely a) scanner, b) client basics, c) client remainder, d) docs, but I've left a "quirk" commit standalone so it stands out what it's doing. I only found the poetry reformatting late in the game, so I've left it standalone for now.

I'm certain there are holes or areas that I've not discovered/considered so far, but it's already ~equivalent to the bluez backend, only with less surprises and hidden quirks, wrt my own usecases :)

@dlech
Copy link
Collaborator

dlech commented Jan 6, 2023

This seems like a nice option to work around OS quirks for sure.

I'm a bit hesitant about picking this up from a maintenance point of view. At a minimum, I would need somebody to send me one of these adapters so I could test it. If it doesn't add much time to the manual checks I do before releases, I will consider it. But if it just takes too much time and isn't used by many, I'm afraid it will end up broken and not well supported like the Android backend.

@karlp
Copy link
Contributor Author

karlp commented Jan 6, 2023

I completely understand the maintenance question of an extra backend like this :) I just felt it was at a stage that it was worth sharing and seeing what the feelings on this were. It's definitely still got bugs, and I'll keep pushing and rebasing this as best I can while I work through them. If nothing else, it can live in our fork as long as it needs.

I'm more than happy to just ship you a couple of adapters though, if that's all it would take :)
I've been using https://eu.mouser.com/ProductDetail/Silicon-Labs/XG24-EK2703A and https://eu.mouser.com/ProductDetail/Silicon-Labs/SLTB010A locally, as they're "easy" but you'd probably want some pre-programmed? I'd be happy to ship you flashed versions if desired :)

Copy link
Collaborator

@dlech dlech left a comment

Choose a reason for hiding this comment

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

Feel free to reach out via email about sending some adapters. I can't make a 100% promise that if you send them I will have time for this, but it does look fairly simple at first glance.

It would also be nice to do #1021 first so we don't have to duplicate those classes here.

Comment on lines 31 to 41
backend=bleak.backends.bgapi.client.BleakClientBGAPI,
bgapi="/home/karlp/SimplicityStudio/SDKs/gecko-4.2.0/protocol/bluetooth/api/sl_bt.xapi",
adapter="/dev/ttyACM1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of requiring this, it would be nice to use environment variables to pass this information. Then any Bleak-based app could be run using that backend without having to modify any code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, this seemed like a reasonable method to start, but I can do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You'd still expect the user to provide the backend explicitly though right? I could modify the selector right now to auto select the backend if an enviironmen variable pointing to the bgapi file exists? is that what you mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that way we can run all of the examples without having to modify the code.

@karlp
Copy link
Contributor Author

karlp commented Jan 8, 2023

#1021 does look good, I definitely felt some of those were laregely copy/pasted

This implements the bleak backend api using Silicon Labs BGAPI, via
their own provided pybgapi library.

See: https://pypi.org/project/pybgapi/

At this stage, it implements the Bleak Scanner API.
No docs or anything yet, it's still unclear if this is just insanity or
not.

Signed-off-by: Karl Palsson <[email protected]>
Just remove them.

Signed-off-by: Karl Palsson <[email protected]>
This appears to be enough to correctly iterate the GATT DB, based on
what appears to be most common parts of all the other backends.

Signed-off-by: Karl Palsson <[email protected]>
Added a few user friendly helpers to auto switch to
write/write_without_response where appropriate, rather than simply
failing.  The BGAPI would simply send the other style as requested, and
the remote device would simply ignore it.

For "start_notify", the "force_indicate" kwarg has been used, to support
both notify and indicate, as in the WinRT backend,
see also hbldh#526 for more information

Signed-off-by: Karl Palsson <[email protected]>
Reformatting for poetry style only.

Signed-off-by: Karl Palsson <[email protected]>
Adds a feature to the changelog and some initial notes on the specifics
of this backend.

Signed-off-by: Karl Palsson <[email protected]>
This allows examples to be switched to the alternate backend by simply
specifiying an environment variable.

Signed-off-by: Karl Palsson <[email protected]>
After adding environment variable support, update the docs to match.

Signed-off-by: Karl Palsson <[email protected]>
karlp and others added 10 commits January 12, 2023 13:39
Decode advertising data type 0x1b "LE BLUETOOTH DEVICE ADDRESS"
We don't have anything to _do_ with this right now, perhaps eventually
the bleak advertising type could be expanded to more graciously handle
these lesser used types, but for now, just decode them and leave them
dangling.

Ref: Bluetooth Core Specification Supplement (CSS), A.1.16
Move the serial connector into a "singleton" style, so that you can have
multiple clients at the same time.  The real goal is to have both
scanners and clients using the same connection, but start with multiple
scanner support to get things working.

Note, despite no bleak apis being touched, this code doesn't actually
work with bluez-dbus at least!

Needs cleanup of the dangling code and logging!

Signed-off-by: Karl Palsson <[email protected]>
Properly handle start/stop pairing with better separation between
backend and frontend.

loggers are now per instance, rather than per module, to help identify
individual scanners.

Signed-off-by: Karl Palsson <[email protected]>
Sourced: Assigned-numbers.pdf rev 2022­12­12
Signed-off-by: Karl Palsson <[email protected]>
bleak's internal advertisement data object doesn't have anywhere to put
this, so just decode it to avoid the warnings for now.

Signed-off-by: Karl Palsson <[email protected]>
This uses the new style, allowing multiple clients in the same
application as desired.

Some care is required to ensure that we don't hang forever if we are
disconnected while attempting long lived operations, and also that we
don't attempt to operate on invalid connections.

Signed-off-by: Karl Palsson <[email protected]>
Without this, I was occasionally seeing apparent bugs in the NCP
firmware, where it would start handing out connection handle 0, without
failure.  The api docs imply:

"If a connection can't be established, for example, the remote device
has gone out of range, has entered into deep sleep, or is not
advertising, the stack will try to connect forever. In this case, the
application will not get an event related to the connection request. To
recover from this situation, the application can implement a timeout and
call sl_bt_connection_close to cancel the connection request."

that we should be explicitly calling close therefore, and adding this
eliminated these errors in testing.

Signed-off-by: Karl Palsson <[email protected]>
Don't use a carbon copy of the bluez implmentation.

Signed-off-by: Karl Palsson <[email protected]>
Instead of trying to handle unexpected disconnects at every stage, only
handle them where we expect.  Single operations like read_char still
need it, but they are not nested.  For get_services, instead of
attempting to handle on every iteration, just wrap the entire task, so
it naturally cancels on disconnection, without needing to check further.

Signed-off-by: Karl Palsson <[email protected]>
@dlech
Copy link
Collaborator

dlech commented Jul 18, 2023

I changed this to draft status since it is still a work in progress. Happy to take a look again if there is still interest in moving forward here.

@karlp
Copy link
Contributor Author

karlp commented Jul 18, 2023

That seems perfectly reasonable. The corporate backing behind this has dried up, and while I've personally got all the parts and still think it's worthwhile, I'm not sure how much time I would have going forwards with it. I've flagged it locally though to make sure I push my last changes here, I did make follow ups to make it even better that don't appear to have made it up yet!

@karlp
Copy link
Contributor Author

karlp commented Jul 18, 2023

Ok, this is the latest and greatest. It was overhauled to use a singelton around the serial port, to allow a single python application to have multiple concurrent clients and scanners, and various field discovered bugs and quirks of the silabs stack squashed. The latter commits could perhaps be cleaned/squished more, which is why this wasn't pushed up earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants