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

Handle a device already being connected with the BlueZ Dbus backend #994

Merged
merged 5 commits into from
Sep 10, 2022

Conversation

bdraco
Copy link
Contributor

@bdraco bdraco commented Sep 10, 2022

Fixes #992

Copy link
Collaborator

@dlech dlech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a changelog entry too.

bleak/backends/bluezdbus/client.py Show resolved Hide resolved
@dlech
Copy link
Collaborator

dlech commented Sep 10, 2022

For the record, there are a couple of pitfalls with this:

  1. This will now allow disconnecting devices that we don't "own" the connection to. If the device is used by the OS (e.g. a keyboard or mouse), this could be problematic.
  2. Many devices that are already connected won't be advertising, so may not be discoverable with BleakScanner so additional changes like those suggested in Device may already be connected when calling connect with BlueZ #992 (comment) may be needed to fully make use of this change.

In spite of these, this seems like a step in the right direction.

@bdraco
Copy link
Contributor Author

bdraco commented Sep 10, 2022

2. Many devices that are already connected won't be advertising, so may not be discoverable with BleakScanner so additional changes like those suggested in Device may already be connected when calling connect with BlueZ #992 (comment) may be needed to fully make use of this change.

The workaround I'm using for that is https://github.com/Bluetooth-Devices/bleak-retry-connector/blob/e8bb72257a4e58a55a839525b2762d7db8dcab0c/src/bleak_retry_connector/__init__.py#L246

Note: its not so pretty and I'd like to get rid of it

@bdraco
Copy link
Contributor Author

bdraco commented Sep 10, 2022

This also need to handle the device not being in the history anymore

2022-09-10 13:07:42.089 ERROR (MainThread) [aiohomekit.utils] Failure running background task: Task-10942
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/utils.py", line 29, in _handle_task_result
    task.result()
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/controller/ble/pairing.py", line 411, in _process_disconnected_events
    results = await self.get_characteristics(list(self.subscriptions))
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/controller/ble/client.py", line 75, in _async_wrap
    return await func(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/controller/ble/pairing.py", line 707, in get_characteristics
    return await self._get_characteristics_without_retry(characteristics)
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/controller/ble/pairing.py", line 111, in _async_wrap
    return await func(self, *args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/controller/ble/pairing.py", line 714, in _get_characteristics_without_retry
    await self._populate_accessories_and_characteristics()
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/controller/ble/pairing.py", line 583, in _populate_accessories_and_characteristics
    await self._ensure_connected()
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/controller/ble/pairing.py", line 323, in _ensure_connected
    self.client = await establish_connection(
  File "/usr/local/lib/python3.10/site-packages/aiohomekit/controller/ble/connection.py", line 48, in establish_connection
    return await retry_establish_connection(
  File "/usr/local/lib/python3.10/site-packages/bleak_retry_connector/__init__.py", line 410, in establish_connection
    await client.connect(
  File "/usr/local/lib/python3.10/site-packages/bleak_retry_connector/__init__.py", line 138, in connect
    connected = await super().connect(
  File "/usr/local/lib/python3.10/site-packages/bleak/backends/bluezdbus/client.py", line 158, in connect
    if not manager.is_connected(self._device_path):
  File "/usr/local/lib/python3.10/site-packages/bleak/backends/bluezdbus/manager.py", line 610, in is_connected
    return self._properties[device_path][defs.DEVICE_INTERFACE]["Connected"]
KeyError: 'org.bluez.Device1'

@bdraco
Copy link
Contributor Author

bdraco commented Sep 10, 2022

Still doing testing. Will do another turn once done testing

@bdraco bdraco changed the title Handle a device already being connected Handle a device already being connected with the BlueZ Dbus backend Sep 10, 2022
@bdraco
Copy link
Contributor Author

bdraco commented Sep 10, 2022

Additional testing looks good

@bdraco bdraco marked this pull request as ready for review September 10, 2022 20:31
Co-authored-by: David Lechner <david@lechnology.com>
CHANGELOG.rst Outdated Show resolved Hide resolved
@dlech dlech merged commit 34895fc into hbldh:develop Sep 10, 2022
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.

Device may already be connected when calling connect with BlueZ
2 participants