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

Create event loop only when necessary #58

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

rytilahti
Copy link
Owner

Calling new_event_loop() created a second event loop on python <3.10, and asyncio.wait_for() created a task on the wrong loop, apparently.

This PR checks if a loop exists already, and creates one only if none is available. Tested to work both using the dockerfile in the linked issue, and when it gets used by homeassistant.

Fixes #57

@TechHummel
Copy link
Contributor

Reporting back from the HA issue:

Logger: homeassistant.components.climate
Source: components/eq3btsmart/climate.py:219
Integration: Klima (documentation, issues)
First occurred: 20:57:19 (1 occurrences)
Last logged: 20:57:19

eq3btsmart: Error on device update!
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 477, in _async_add_entity
    await entity.async_device_update(warning=False)
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 702, in async_device_update
    await task
  File "/usr/local/lib/python3.10/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/src/homeassistant/homeassistant/components/eq3btsmart/climate.py", line 219, in update
    self._thermostat.update()
  File "/usr/local/lib/python3.10/site-packages/eq3bt/eq3btsmart.py", line 217, in update
    self._conn.make_request(PROP_WRITE_HANDLE, value)
  File "/usr/local/lib/python3.10/site-packages/eq3bt/bleakconnection.py", line 111, in make_request
    with self:
  File "/usr/local/lib/python3.10/site-packages/eq3bt/bleakconnection.py", line 56, in __enter__
    self._loop.run_until_complete(self._conn.connect())
  File "/usr/local/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
    return future.result()
  File "/usr/local/lib/python3.10/site-packages/bleak/backends/bluezdbus/client.py", line 107, in connect
    device = await BleakScannerBlueZDBus.find_device_by_address(
  File "/usr/local/lib/python3.10/site-packages/bleak/backends/scanner.py", line 221, in find_device_by_address
    return await cls.find_device_by_filter(
  File "/usr/local/lib/python3.10/site-packages/bleak/backends/scanner.py", line 250, in find_device_by_filter
    async with cls(detection_callback=apply_filter, **kwargs):
  File "/usr/local/lib/python3.10/site-packages/bleak/backends/scanner.py", line 96, in __aenter__
    await self.start()
  File "/usr/local/lib/python3.10/site-packages/bleak/backends/bluezdbus/scanner.py", line 137, in start
    self._stop = await manager.active_scan(
  File "/usr/local/lib/python3.10/site-packages/bleak/backends/bluezdbus/manager.py", line 376, in active_scan
    reply = await self._bus.call(
  File "/usr/local/lib/python3.10/site-packages/dbus_next/aio/message_bus.py", line 305, in call
    await future
RuntimeError: Task <Task pending name='Task-2163' coro=<BleakClientBlueZDBus.connect() running at /usr/local/lib/python3.10/site-packages/bleak/backends/bluezdbus/client.py:107> cb=[_run_until_complete_cb() at /usr/local/lib/python3.10/asyncio/base_events.py:184]> got Future <Future pending> attached to a different loop

As you suspected, still doesn't work.

@rytilahti
Copy link
Owner Author

So I asked about this from homeassistant developers, and they mentioned that the are patching bleak internally which is why this approach won't work. Two potential solutions were offered: use the homeassistant-provided BLEDevice (as it's already in homeassistant's event loop), another one (which may or may not be simpler) would be allowing to pass an eventloop to BleakConnection and then pass the homeassistant one for it to use.

Both of these will require some not so light refactoring, so it might be just easier to create a separate asyncio thermostat interface for homeassistant to use (#59).

@rytilahti
Copy link
Owner Author

I'll merge this as it is, afaik, the correct thing to do even if it doesn't fix the homeassistant issue it was aimed to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash under python <3.10: got Future <Future pending> attached to a different loop
2 participants