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

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

Closed
NbrTDB opened this issue Sep 28, 2020 · 7 comments
Assignees
Labels
Backend: pythonnet Issues or PRs relating to the .NET/pythonnet backend bug Something isn't working

Comments

@NbrTDB
Copy link

NbrTDB commented Sep 28, 2020

  • bleak version: 0.8.0
  • Python version: 3.8.5
  • Operating System: Windows 10 2004, build 19041.508

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:

  • Spawn a new client and connect to a device
  • Cause the device to disconnect on its end (e.g. turn off Bluetooth)
  • Discard the client
  • Reconnect to the device using a new client
  • Attempt to disconnect; first attempt appears to do nothing and device does not disconnect. Any callbacks set on disconnect do not activate and the device does not indicate a disconnection has occurred.
  • Second attempt throws AttributeError on the await client.disconnect()

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'
@dlech dlech added bug Something isn't working Backend: pythonnet Issues or PRs relating to the .NET/pythonnet backend labels Sep 28, 2020
@hbldh
Copy link
Owner

hbldh commented Sep 28, 2020

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...

@hbldh hbldh assigned hbldh and unassigned hbldh Sep 28, 2020
@NbrTDB
Copy link
Author

NbrTDB commented Sep 28, 2020

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:

  • if I always requested a disconnect on Bleak's end, I would be able to repeatedly connect/disconnect reliably and without any problems; at least, to me in testing it seemed really solid with the couple different devices I had to test. Disconnection callbacks also reliably fired and seemed to coincide closely with the moment the devices in question indicated the connection had ended.
  • if the remote device disconnects itself (however that occurred), then after reconnecting, a subsequent client.disconnect() on Bleak's end seems to silently fail, no disconnect callbacks run, no errors thrown, and the device shows it is still connected even after waiting for quite some time. Then, as discussed, any further attempts to use client.disconnect() result in AttributeErrors.

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
Copy link
Collaborator

dlech commented Oct 7, 2020

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
Copy link
Collaborator

dlech commented Oct 7, 2020

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

@dlech
Copy link
Collaborator

dlech commented Oct 7, 2020

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.

dlech added a commit that referenced this issue Oct 8, 2020
This adds an event waiter so that `BleakClientDotNet.disconnect()`
will actually await the disconnection event before returning.

This is a partial fix for #313.
@hbldh
Copy link
Owner

hbldh commented Oct 9, 2020

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!

@dlech dlech self-assigned this Oct 10, 2020
@hbldh hbldh mentioned this issue Oct 20, 2020
@hbldh
Copy link
Owner

hbldh commented Oct 20, 2020

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

@hbldh hbldh closed this as completed Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend: pythonnet Issues or PRs relating to the .NET/pythonnet backend bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants