ukBaz / python-bluezero

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

Receiving old data when re enabling notifications on a Characteristic #342

Closed Owenber123 closed 3 years ago

Owenber123 commented 3 years ago

Hello,

I am having an issue with notifications when I attempt to stop_notify and then start_notify again. It seems like the second time around I am receiving the data that I received the first time in my new callback function. I have validated that my device is not sending the data twice the second time. Here is my flow of execution:

main:

self.imageData = []

def imageData_cb(self, imgPacket):
    self.imageData.extend(imgPacket)

device.startListenImageTX(userCallback=imageData_cb)
...
device.stopListenImageTX()
# Here we can see that the length of data transmitted was around 200,000 bytes

device.startListenImageTX(userCallback=imageData_cb)
# Length of imageData at this point is 0 but very quickly grows to 200,000
device.retryTransmission = 1 # This informs the peripheral to re transmit the data

device.stopListenImageTX()
# Here we can see that the length of data transmitted was around 400,000 bytes, exactly twice the size of the initial transmission

device:


def imageTx_cb(self, iface, changed_props, invalidated_props):
    """
    Callback function for receiving a notify for image transfer
    """
    # Ensure this is a characteristic notification
    if iface != constants.GATT_CHRC_IFACE:
        return
    if 'Value' in changed_props:
        data = bytes(changed_props['Value'])
        # Give data to user via user callback
        self.char_user_callback(data)

def startListenImageTX(self, userCallback):
  char = self.resolved_characteristics["char"]
  self.char_user_callback = userCallBack
  char.add_characteristic_cb(self.imageTx_cb)
  char.start_notify()

def stopListenImageTX(self):
  char = self.resolved_characteristics["char"]
  char.stop_notify()
  self.char_user_callback = None
  char.add_characteristic_cb(None)

Here are the first 100 bytes of data for both transmissions to prove its an overlap and not one after the other, seems to be every 32 bytes, I'm assuming that's just how changed_props is constructed:

First Transmission:
[0, 188, 2, 0, 235, 164, 11, 23, 4, 5, 6, 4, 5, 4, 4, 4, 4, 4, 4, 6, 4, 5, 5, 4, 4, 5, 4, 6, 4, 4, 4, 6, 4, 5, 5, 4, 5, 4, 5, 5, 4, 5, 4, 4, 4, 4, 4, 4, 4, 5, 5, 4, 4, 5, 4, 5, 4, 4, 4, 4, 5, 5, 4, 4, 4, 5, 5, 6, 4, 4, 4, 5, 4, 4, 6, 5, 4, 4, 4, 5, 6, 4, 4, 4, 4, 5, 4, 4, 4, 4, 4, 5, 6, 4, 4, 4, 4, 5, 4, 6]
Second Transmission:
[0, 188, 2, 0, 0, 188, 2, 0, 235, 164, 11, 23, 235, 164, 11, 23, 4, 5, 6, 4, 5, 4, 4, 4, 4, 4, 4, 6, 4, 5, 5, 4, 4, 5, 4, 6, 4, 4, 4, 6, 4, 5, 5, 4, 5, 4, 5, 5, 4, 5, 4, 4, 4, 4, 4, 4, 4, 5, 5, 4, 4, 5, 4, 5, 4, 4, 4, 4, 5, 5, 4, 4, 4, 5, 5, 6, 4, 4, 4, 5, 4, 4, 6, 5, 4, 4, 4, 5, 6, 4, 4, 4, 4, 5, 4, 4, 4, 4, 4, 5]

I am convinced that the char.stop_notify() handles the clean up required to re enable notifications during the same connection and I have tried almost everything to prove that Python is not reusing the same image collection helper instance. It seems like the data is being send through the callback twice or there are two callbacks linked to the characteristic. Any ideas on my wrongdoing or additional debug advice would be greatly appreciated. Thanks in advance.

ukBaz commented 3 years ago

Looking at the Bluezero code for adding a characteristic callback:

https://github.com/ukBaz/python-bluezero/blob/main/bluezero/GATT.py#L250-L251

        self.characteristic_props.connect_to_signal('PropertiesChanged',
                                                    callback)

There doesn't appear to be anything that removes a callback from a characteristic so when you add a callback a second time it connects another one to the PropertiesChanged signal. I would expect that if you keep adding callbacks the number notifications would go up.

The quick fix for this is that your code could not add a callback every time it calls startListenImageTX. When it is doing a restart, it only does a start_notify()

I'll also investigate if there is a better way of checking if a duplicate callback is being added.

ukBaz commented 3 years ago

I've created a new branch with an experiment. The commit is at:

https://github.com/ukBaz/python-bluezero/commit/6028928567343aecdc2e0fa4d4de761335891380

When calling add_characteristic_cb with None or no value then it will remove the callback from the signal. My quick experiment seemed to work but would be interested in feedback from you on using it in your situation.

Also, open to any feedback if this is the best way of doing it.

ukBaz commented 3 years ago

@owenber123 Did you get chance to look at this to see if it worked for you?