hbldh / bleak

A cross platform Bluetooth Low Energy Client for Python using asyncio
MIT License
1.82k stars 301 forks source link

AttributeError: 'NoneType' object has no attribute 'Dispose' when disconnecting devices that have previously disconnected due to range/power off/etc #313

Closed NbrTDB closed 4 years ago

NbrTDB commented 4 years ago

Description

I've found a repeatable scenario wherein Bleak throws an AttributeError on attempting to await client.disconnect() a device which had previously disconnected itself and subsequently been reconnected to. I am not certain if this is a genuine issue or just a misuse of Bleak. In any case, catching the AttributeError still does not result in the ability to disconnect the device (it will just throw the same error each time).

In short:

What I Did

I put together a snippet that reproduces the problem for me; it requires a device where the BLE connection can quickly be severed on the device side and said device be ready to reconnect again quickly (other devices should work if the timings are adjusted to suit)

    async def __attributeerror_test(self, mac):
        print("Connecting to mac: {0}".format(mac), flush=True)
        client = BleakClient(mac)
        await client.connect()
        print("Poking services...", flush=True)
        await client.get_services()
        # now disconnect        
        print("Disconnect on device end now...", flush=True)
        for x in range(0,5):
            print("{}...".format(x))
            await asyncio.sleep(1)
        print("Waiting for device to indicate disconnection... ensure it is ready to reconnect", flush=True)
        for x in range(0,15):
            print("{}...".format(x))
            await asyncio.sleep(1)
        # if a disconnection callback had been set, it should have fired by now
        # must make a new client for a new connection
        del client
        print("Reconnecting...", flush=True)        
        client = BleakClient(mac)
        await client.connect()
        print("Poking services...", flush=True)
        await client.get_services()
        print("Disconnecting...", flush=True)
        await client.disconnect()
        print("Waiting for device to indicate disconnection...", flush=True)
        for x in range(0, 10):
            print("{}...".format(x))
            await asyncio.sleep(1)
        print("At this point, nothing has happened and the device still shows as connected... trying again", flush=True)
        print("Disconnecting...", flush=True)
        await client.disconnect() # this will throw an AttributeError
Connecting to mac: [mac here]
Poking services...
Disconnect on device end now...
0...
1...
2...
3...
4...
Waiting for device to indicate disconnection... ensure it is ready to reconnect
0...
1...
2...
3...
4...
5...
6...
7...
8...
9...
10...
11...
12...
13...
14...
Reconnecting...
Poking services...
Disconnecting...
Waiting for device to indicate disconnection...
0...
1...
2...
3...
4...
5...
6...
7...
8...
9...
At this point, nothing has happened and the device still shows as connected... trying again
Disconnecting...
Traceback (most recent call last):
  File "c:/Users/user/Documents/ble/test.py", line 338, in <module>
    test.start()
  File "c:/Users/user/Documents/ble/test.py", line 330, in start
    self.loop.run_until_complete(self.__loop())
  File "C:\Python38\lib\asyncio\base_events.py", line 616, in run_until_complete
    return future.result()
  File "c:/Users/user/Documents/ble/test.py", line 323, in __loop
    await self.__stdin_msg_handler(msgs)
  File "c:/Users/user/Documents/ble/test.py", line 306, in __stdin_msg_handler
    await self.__attributeerror_test(mac)
  File "c:/Users/user/Documents/ble/test.py", line 197, in __attributeerror_test
    await client.disconnect() # this will throw an AttributeError
  File "C:\Python38\lib\site-packages\bleak\backends\dotnet\client.py", line 230, in disconnect
    self._requester.Dispose()
AttributeError: 'NoneType' object has no attribute 'Dispose'
hbldh commented 4 years ago

Gah... this is complex....

The Windows backend API has no dedicated disconnect function, so the only way I have found to facilitate a disconnect is by Dispose:ing of the .NET client. I have not tested that repeatedly though... Even with the Dispose it takes quite some time, since the garbage collection in .NET is not guaranteed to happen when you request it to happen.

But in your example you call disconnect on client you have already called disconnect on. It is wrong behaviour, and it is reasonable that an error is thrown. It is not the right one, granted, but still. This SO post describes the situation pretty well. I am uncertain of why it disconnects the first time and not the second though...

NbrTDB commented 4 years ago

I agree, an explosion of some sort would be expected for trying to disconnect from something twice in a row, so I guess the real conundrum is why the first client.disconnect() request is rendered ineffective on a device has previously disconnected itself in an older connection session.

In the test case shown here, the problem only arises after a disconnect occurred remotely, so:

After using client.disconnect() on a device that had previously disconnected itself, I haven't tried forcing the device to disconnect itself a second time, maybe I should see what happens then?

dlech commented 4 years ago

If Microsoft implemented the Dispose pattern correctly, it should disconnect without having to wait for the garbage collector. I have had the same experience that there is some delay after calling Dispose though, so I think we can conclude that it is not a synchronous operation. (In fact, there is a new DisposeAsync pattern being introduced in .NET because of issues like this.)

Anyway, we could wait a while for a ConnectionStatusChanged event before returning from the async disconnect method to help ensure that the device had a chance to disconnect before allowing the program to move on.

dlech commented 4 years ago

Hmm... I've been trying this with a different device and it disconnects right away when calling Dispose().

dlech commented 4 years ago

I will also suggest that multiple calls to disconnect() should be allowed. There is precedence for this, e.g. Python allows calling close() multiple times.

Furthermore, async with BleakClient():, will implicitly call disconnect() at exit. So it would also raise an error if the device was disconnected inside the with block.

hbldh commented 4 years ago

Yes, I agree. Multiple calls to disconnect should all yield the same True response, without error.

I have not experienced problems with disconnecting on Windows since I added the Dispose call on disconnect. It takes some time, granted, but it hasn't failed to disconnect. The PR is a good improvement though!

hbldh commented 4 years ago

Additional disconnect calls are now allowed in the released version 0.9.0. Will close this issue for now.