-
Notifications
You must be signed in to change notification settings - Fork 300
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
base: develop
Are you sure you want to change the base?
Conversation
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. |
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 :) |
There was a problem hiding this 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.
docs/backends/bgapi.rst
Outdated
backend=bleak.backends.bgapi.client.BleakClientBGAPI, | ||
bgapi="/home/karlp/SimplicityStudio/SDKs/gecko-4.2.0/protocol/bluetooth/api/sl_bt.xapi", | ||
adapter="/dev/ttyACM1", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
#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]>
1c2ea6f
to
0053587
Compare
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]>
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 20221212 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]>
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. |
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! |
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. |
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:
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 :)