ukBaz / python-bluezero

A simple Python interface to Bluez
MIT License
387 stars 112 forks source link

Should Characteristic Automatically Resolve GATT? #370

Closed chadrockey closed 2 years ago

chadrockey commented 2 years ago

https://github.com/ukBaz/python-bluezero/blob/6f46b6cb7102831c1427f12e65b727fc6b54774a/bluezero/GATT.py#L100

The Central service automatically calls load_gatt in the connect() function.

The Service automatically calls resolve_gatt in its own constructor: https://github.com/ukBaz/python-bluezero/blob/main/bluezero/GATT.py#L32

When I create a characteristic like: char = central.add_characteristic(SERVICE, UUID)

It feels unintuitive to have to follow up with char.resolve_gatt()

It's also a more difficult pattern because some calls fail if resolve_gatt not performed. For instance: char.flags throws an exception if called before resolve_gatt

print(char.flags) File "/home/root/./onboardlibs/bluezero/GATT.py", line 175, in flags return self.characteristic_props.Get( AttributeError: 'NoneType' object has no attribute 'Get'

ukBaz commented 2 years ago

The situation is that a look up needs to happen to find the D-Bus object path for a given GATT UUID. The D-Bus object path only exists once the remote device has been connected to and the services resolved. That is what resolve_gatt does and gets called automatically by Central once it has been connected. If you are using the central there shouldn't be a need for you to do char.resolve_gatt() as this is taken care of once the connect has happened.

There are a couple of ways at coming at this problem. In my later BlueZ central only library (BLE GATT) there is no upfront specifying the characteristic UUID's. In that library the read/writes require the UUID to be specified and the look-up happens at that point.

There are trade offs between the those two approaches. I'm always open to suggestions if you have ideas you would like to share.

On to the specific error you got, I am not certain what you are saying there. The library has three different levels of complexity and people should try to use the level of complexity they feel comfortable with. Using the 100 level files does require the resolve GATT to be done after the device connects. The level 10 Central does it.automatically after connection. Please let me know if I have misunderstood this issue and there is something specific for me to look at.

chadrockey commented 2 years ago

Thanks for the clarification.

Do you have an intended usage pattern for Central? My use case is: Connect WriteToCharacteristic Disconnect

Do you have a suggested API for this? I didn't see how to write from a characteristic object inside the Central class. Am I missing a function that automatically creates the characteristics based on the gatt profiles?

This is the only place I see characteristics in the central source file:

def add_characteristic(self, srv_uuid, chrc_uuid)

Here is how I stumbled into using the library:

central = central.Central(device_addr=address)

central.connect()

# Setup characteristics now
char1 = central.add_characteristic(SERVICE_UUID, CHAR1_UUID)
char1.resolve_gatt()

led_char = central.add_characteristic(SERVICE_UUID, LED_UUID)
led_char.resolve_gatt()

Here is the error I get when I call WRITE without having called resolve_gatt() on the characteristic:

2022-05-16 07:16:46,396 - bluezero.GATT - ERROR - Service: XXXX-UUID-XXXX with Characteristic: Service: XXXX-UUID-XXXX not defined onon device: MAC:MAC:MAC:MAC. Cannot write_value

From this line:

led_char.write_value(buf)

Separately, as per my original report, even with central.connect being called, I get an error if I do not call characteristic.resolve_gatt()

It seems that service class in this file calls resolve_gatt in the constructors, but the characteristic class in this same file does not resolve_gatt in the constructor: https://github.com/ukBaz/python-bluezero/blob/main/bluezero/GATT.py#L30


Finally, while we're chatting a bit, I wanted to thank you for this project. I've done a few projects before using the BlueZ DBus layer python APIs and it's always been a bad time. I've also used bleak for a few proofs-of-concept, but it's central only and to do any real work it has to wrap your main and sometimes it's difficult to integrate asyncio with the rest of an existing program.

ukBaz commented 2 years ago

I have noticed before that I don't have an example of using Central in the examples directroy. This is maybe something I should resolve but previously I've pointed people to microbit.py and that has generally been enough for them.

I'm not in a position to test this right now but to try to answer this in a timely manner, here is my initial thought on what you have shown... You are setting up the characteristic after the connection and this might be where your issue is. Bluezero is trying to resolve the characteric when it connects but you haven't told it about the characteristic when it is doing it.

from bluezero import central

# define device information
remote_device = central.Central(device_addr=address)
led_char = central.add_characteristic(SERVICE_UUID, LED_UUID)

# Connect
remote_device.connect()

# Write to characteristic
led_char.value = buf

# Disconnect
remote_device.disconnect()

buf will need to be a list of byte values to be written. e.g [1] or [0xff, 0x00, 0, 255, 0b10101]

Let me know if that doesn't work and I'll try to find some time to test it.

chadrockey commented 2 years ago

I think I see the issue! Thank you for the explanations.

Central won't resolve characteristics AFTER connection, it will only resolve what it's its been told to look for before.

I'll test and confirm.

I believe this is similar to how the Service in that file is written. May be worth putting a guard on characteristic after connection or potentially a pattern in the constructor similar to:

__init__()
.....
if connected and not gatt_resolved:
     gatt_resolve()

If both usage patterns are supportable, it could be a big to others in supporting both. For me, the issue comes from when you make a server or peripheral, you often do the following: configure functionality configure characteristics configure connection settings start main_loop()

and the corresponding clients typically unwind that order in reverse: start main connection get connection settings get characteristics use functionality


I'll make an attempt to repay your effort by adding a central basic example later this week.

I think the typical HeartRateService matching central and peripheral would be a nice intro for usage of this project. I might have identified some issues with the abbreviated UUIDs (0x180D), so I might make special attention to have the server and client use them interchangeably/unpredictably.

The microbit code is very nice, but I found that it did too much to be a clean intro and wrapped the central class too cleanly. Very good for making a library as a class, but not so quick to unwrap it into a script.

chadrockey commented 2 years ago

And I think I should clarity that the usage pattern:

connect char = .... char.resolve_gatt()

Works perfectly well and I see no issues with write_value(buf). I'll double check if the assignment operator is functionally equivalent and potentially switch over because it looks cool.

I went with write_value(buf) because it looks much more compatible with bleak (where I was porting code from) and I think supporting the same data formatting (and easier) would be a good thing.

ukBaz commented 2 years ago

I think this has been resolved so closing