ukBaz / python-bluezero

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

Expose on_connect and on_disconnect callbacks for the Peripheral #325

Closed a-krawciw closed 3 years ago

a-krawciw commented 3 years ago

Based on #324 . I added properties into peripheral so that the connection callbacks are more accessible.

I also modified dbus_tools.py to account for an issue where the address used in the bluez path does not match the public address of the device. In this case nothing could happen and an exception was thrown.

ukBaz commented 3 years ago

I've taken a look at this pull request and I am seeing exceptions being thrown such as:

ERROR:dbus.connection:Exception in handler for D-Bus signal:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 232, in maybe_handle_message
    self._handler(*args, **kwargs)
  File "/home/thinkabit1/PycharmProjects/python-bluezero/bluezero/adapter.py", line 279, in _properties_changed
    new_dev = device.Device(
  File "/home/thinkabit1/PycharmProjects/python-bluezero/bluezero/device.py", line 53, in __init__
    raise ValueError("Cannot find a device: " + device_addr +
ValueError: Cannot find a device: xx:xx:xx:xx:xx:xx using adapter: yy:yy:yy:yy:yy:yy
ERROR:dbus.connection:Exception in handler for D-Bus signal:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 232, in maybe_handle_message
    self._handler(*args, **kwargs)
  File "/home/thinkabit1/PycharmProjects/python-bluezero/bluezero/adapter.py", line 279, in _properties_changed
    new_dev = device.Device(
  File "/home/thinkabit1/PycharmProjects/python-bluezero/bluezero/device.py", line 53, in __init__
    raise ValueError("Cannot find a device: " + device_addr +
ValueError: Cannot find a device: xx:xx:xx:xx:xx:xx using adapter: yy:yy:yy:yy:yy:yy
ERROR:dbus.connection:Exception in handler for D-Bus signal:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/dbus/connection.py", line 232, in maybe_handle_message
    self._handler(*args, **kwargs)
  File "/home/thinkabit1/PycharmProjects/python-bluezero/bluezero/adapter.py", line 279, in _properties_changed
    new_dev = device.Device(
  File "/home/thinkabit1/PycharmProjects/python-bluezero/bluezero/device.py", line 53, in __init__
    raise ValueError("Cannot find a device: " + device_addr +
ValueError: Cannot find a device: xx:xx:xx:xx:xx:xx using adapter: yy:yy:yy:yy:yy:yy

I think the logic around on_connect and on_disconnect needs to be thought about a little bit more.

I think there are a number of variables such as:

When I was looking at Peripheral role with a BLE random address the states were:

I was also seeing things being triggered multiple times for the same event. This seems like it might be slightly more gnarly that I first thought. Especially not to break the beacon scanning or Central role.

I'll try to take a look at this again soon.

ukBaz commented 3 years ago

@a-krawciw I have pushed an update to this to rectify the issues I was seeing. Please can you take a look and see if they are satisfactory.

One of the areas of improvement is around general improvement in the logic of when the callbacks get called to support beacon discovery, central role and peripheral role. With the on_connect and on_disconnect they now support 0, 1, or 2 parameters. The issue was that when we are building a peripheral the device interface disappears at the same time as the client/device disconnects. If the callback has 1 parameter then it is expecting a Bluezero Device object. If there are two parameters then it is expecting an adapter address and a device address. You can also have 0 parameters. When on_disconnect is used with Central then 1 parameter seems best, but for peripheral then it can't use the device object because it gets deleted before the callback can use it.

a-krawciw commented 3 years ago

Looks good, and it is still running nicely on my end. I renamed the class to be a bit more specific and updated the docstring for the variable callback parameters.

ukBaz commented 3 years ago

Thanks @a-krawciw for your help with this. I think it is a good addition to Bluezero