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

Want to use BleakClient within bleak package #582

Closed
jackjansen opened this issue Jun 27, 2021 · 5 comments · Fixed by #982
Closed

Want to use BleakClient within bleak package #582

jackjansen opened this issue Jun 27, 2021 · 5 comments · Fixed by #982
Assignees
Labels
Opinions Appreciated Please add an opinion on your desired resolution on this issue!

Comments

@jackjansen
Copy link
Contributor

I would like to have at least BleakClient available within bleak, in other words: move the platform-dependent imports from __init__.py to a new module (say api.py). __init__.py would then import them from there, but so could other modules within bleak.

If you're willing accept a pull request for this let me know.

The reason I want this is that I'm working on a Pythonic interface to get and put values, using 2904 descriptors to marshal and unmarshal values, plus accessor classes for services and characteristics. Hopefully you'll eventually be able to do things like

async with PythonicBleakClient(address, timeout=10) as client:
    batteryLevel = await client.services['Battery Service'].characteristics['Battery Level'].get()

But PythonicBleakClient and the accessor classes are backend-independent, and I don't want to put all the functionality in BaseBleakClient.

@hbldh hbldh self-assigned this Jun 28, 2021
@hbldh
Copy link
Owner

hbldh commented Jun 28, 2021

I can understand your desire for this; I would also have liked to make Bleak more Pythonic and generate Service, Characteristic and Descriptor instances in runtime to match what the connected peripheral responds that it has. I failed when encountering multiple backends and I retreated to a lower level API instead, in order to get a cross-platform API working before moving on. I never got around to doing that.

One main design criteria of Bleak is that it should always be pip-installable and importable right off the bat, and I also want it to be as simple as from bleak import BleakClient and that the user should not be required to learn how to any other class in order to get something working. I am fine with moving the platform-dependent parts to another file, but I think that having a fully functional client class at root level is the best option here.

And, given that your PythonicBleakClient is truly backend-independent, then it could be implemented either in the BaseBleakClient or in a new inheriting class in between the BaseBleakClient and the backend implementations. Also, given that Bleak should always be importable in any backend, having platform dependent imports in the __init__.py should not pose any problem either.

Somewhere down the line you will have to get dirty with the specific implementations anyway, so I cannot see the real world value of an independent import of the BaseBleakClient. I am not trying to be difficult here, but I do not see a way forward to achieve both what you want and what I want Bleak to be. If you find a path that I do not see I would gladly accept that PR.

A sidenote: there is an add-on package to Bleak that adds a rather large XML with specifications of services and characteristics. This might fit better in with that? Or being a separate package in a similar fashion?

@jackjansen
Copy link
Contributor Author

What I'm suggesting here is purely a slight structural change, with no change to the external API:

  1. Move the sequence of platform-dependent if statements and import as statements from __init__.py to a new api.pi (or even _api.py).
  2. Replace the code in __init__.py with from bleak._api import BleakClient, BleakScanner.

On adding the code to BaseBleakClient (or between BaseBleakClient and the implementation classes): I think that putting it into a subclass is better structurally, because anyone not interested in the Pythonic interface doesn't pay for it, and moreover any silly bugs in my new code will be much less likely to bother people who don't use it:-)

(And, on bleak-sigspec: I wasn't aware of it, it looks interesting. I think what I'm trying to do and that package are orthogonal. Bleak-sigspec is another source for the information in 2904 descriptors if the device doesn't provide them. But I have to experiment: I don't quite understand how bleak-sigspec finds the correct XML file based on the UUID or the BleakGattCharacteristic....)

@Carglglz
Copy link
Contributor

@jackjansen I'm the author of bleak-sigspec

I don't quite understand how bleak-sigspec finds the correct XML file based on the UUID or the BleakGattCharacteristic....)

See get_xml_char

It uses the name of the characteristic or gets the name from characteristics uuid with bleak.uuids.uuidstr_to_str:

if isinstance(characteristic, BleakGATTCharacteristic):
        characteristic = uuidstr_to_str(characteristic.uuid)

Then it parses the XML file with the same name, and unpacks data based on its specifications. (which I found to be harder than expected since some characteristics are compounds of another set of characteristics, forcing me to deal with recursion). Also I ended up defining I custom Struct class to unpack value types not defined in python e.g (ieee11073)

So if you need any help/ have any question I'm glad to help 👍

@dlech dlech added the Opinions Appreciated Please add an opinion on your desired resolution on this issue! label Aug 6, 2021
@dlech
Copy link
Collaborator

dlech commented Feb 5, 2022

I would like to have at least BleakClient available within bleak, in other words: move the platform-dependent imports from __init__.py to a new module (say api.py). __init__.py would then import them from there, but so could other modules within bleak.

I am in favor of this from a documentation point of view. If we do this, we can document all of the os-specific quirks in one place instead of having 5 copies of the documentation for each method (base, windows, mac, linux, android). I am also in favor of this from an intellesense point of view. The MS VS Code Python tools always pick up the Windows backend no matter which OS is actually being used.

@jackjansen
Copy link
Contributor Author

jackjansen commented Jul 25, 2022

I didn't have time to work on Bleak for more than a year (which was quite a bit longer than anticipated:-), but I'm going to pick this one up again. I hope to have a pull request shortly (presuming real work doesn't interfere again:-)

Plan is to create the _api.py with abstract classes like AbstractBleakClient, then import the machine-dependent classes as MachdepBleakClient, then have a class BleakClient inheriting from those two. The AbstractBleakClient could then be annotated with the types and documentation, I have the feeling that would address #822 and #833 too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Opinions Appreciated Please add an opinion on your desired resolution on this issue!
Projects
None yet
4 participants