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

Code hangs when BLE client disconnects first #219

Closed
billwirz opened this issue Jun 16, 2020 · 6 comments
Closed

Code hangs when BLE client disconnects first #219

billwirz opened this issue Jun 16, 2020 · 6 comments
Assignees
Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working
Milestone

Comments

@billwirz
Copy link

billwirz commented Jun 16, 2020

  • bleak version: 0.6.5a3 (develop)
  • Python version: 3.7.3
  • Operating System: Raspbian GNU/Linux 10 (buster), kernel: 4.19.118-v7+
  • BlueZ version (bluetoothctl -v) in case of Linux: 5.50

Description

When connecting to a BLE device, if the client disconnects first, bleak seems to hang. This issue was fixed by modifying bluezdbus\client.py.

What I Did

I ran the notifications example "enable_notifications.py", connecting to a custom-built BLE device. This device was guaranteed to disconnect from the client before the 5 second asyncio.sleep had completed. I only modified it by commenting out the stop_notify line (no longer needed) and by adding a logging line after the BleakClient section, which never ran.

"""
Notifications
-------------
Example showing how to add notifications to a characteristic and handle the responses.
Updated on 2019-07-03 by hbldh <henrik.blidh@gmail.com>
""" 
async def run(address, loop, debug=False):
    if debug:
        import sys

        # loop.set_debug(True)
        l = logging.getLogger("asyncio")
        l.setLevel(logging.DEBUG)
        h = logging.StreamHandler(sys.stdout)
        h.setLevel(logging.DEBUG)
        l.addHandler(h)
        logger.addHandler(h)

    async with BleakClient(address, loop=loop) as client:
        x = await client.is_connected()
        logger.info("Connected: {0}".format(x))

        await client.start_notify(CHARACTERISTIC_UUID, notification_handler)
        await asyncio.sleep(5.0, loop=loop)
        #await client.stop_notify(CHARACTERISTIC_UUID)
    logger.info("BleakClient exited successfully")

In order to debug this problem, logger statements were added to bluezdbus\client.py, which made it clear that the code was hanging on BleakClient.disconnect when called by BleakClient.aexit, during the following section:

# Try to disconnect the actual device/peripheral
try:
	await self._bus.callRemote(
		self._device_path,
		"Disconnect",
		interface=defs.DEVICE_INTERFACE,
		destination=defs.BLUEZ_SERVICE,
	).asFuture(self.loop)
except Exception as e:
	logger.error("Attempt to disconnect device failed: {0}".format(e))

This was fixed by making the following changes to bluezdbus\client.py:

...
def __init__(self, address, loop=None, **kwargs):
   ...
        # BillMicro: device disconnection fix
        self._device_disconnected = False
...
async def disconnect(self) -> bool:
	"""Disconnect from the specified GATT server.

	Returns:
		Boolean representing if device is disconnected.

	"""
	# BillMicro: device disconnection fix
	if not self._device_disconnected:
		logger.debug("Disconnecting from BLE device...")

		# Remove all residual notifications.
		await self._cleanup_notifications()
                ...
        else:
            is_disconnected = True
...
def _properties_changed_callback(self, message):
	...
				task = self.loop.create_task(self._cleanup_all())
				# BillMicro: device disconnection fix
				self._device_disconnected = True
				...

However, this is a non-satisfactory solution. It can be be found here: 8794eac

@Ladvien
Copy link

Ladvien commented Jun 18, 2020

I'm digging in on this one.

@billwirz
Copy link
Author

Hi @Ladvien

Thanks for your suggestion - I agree that there must be a better way to fix this than what I did...

I tried using the ConnectionDone exception, as well as a number of others, but the code still hangs, and no exception is raised. I will keep trying to see if I can find a better solution or at least get a better handle on what's going on.

@Ladvien
Copy link

Ladvien commented Jun 19, 2020

Hello @BillMicro,

Well, I've dug pretty deep. I'm not sure, but it seems like something is happening in the txdbus library and the deferred which is returned, then converted to an asyncio.future, is not completing or throwing an error. I got lost trying to trace it--new to all this, but I'll dig in a little more tomorrow.

@billwirz
Copy link
Author

Hi @Ladvien

I'll take a look at that, as well as trying to understand the basics of how txdbus and twisted work. I found a version of bleak that doesn't do this - it's the latest proglove branch of the https://github.com/workaroundgmbh/bleak.git fork, so maybe I'll take a look at how that is different as well.

billwirz pushed a commit to billwirz/bleak that referenced this issue Jun 19, 2020
When server (BLE device) disconnects before bleak does, the code hangs when BleakClientBlueZDBus.__aexit__() calls BleakClientBlueZDBus.disconnect(). This is temporarily fixed here with a disconnection state variable.
@Ladvien
Copy link

Ladvien commented Jun 19, 2020

Oh nice! Thank you for sharing.

It looks like the Proglove folk are using bleak in testing or production. That's great. It's a huge project and would feel bad if Henrik had to maintain it on his own.

I took a look at their additions, they mention a "leaky event loop" on POSIX systems. I'm guessing that's what we were seeing. Glad there are smarter, more experienced people out there than me. :)

@hbldh hbldh self-assigned this Jun 24, 2020
@hbldh hbldh added Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working labels Jun 24, 2020
@hbldh
Copy link
Owner

hbldh commented Jun 24, 2020

This is troublesome indeed. An exception is desired, not a complete hanging...

This will be looked at in a later release though. Other issues have precendence right now.

@hbldh hbldh added this to the Version 0.7.0 milestone Jun 30, 2020
@hbldh hbldh mentioned this issue Jun 30, 2020
@hbldh hbldh closed this as completed in c267f9a Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend: BlueZ Issues and PRs relating to the BlueZ backend bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants