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

fix(client): Ensure that the bus is connected before pairing #1713

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

pmessan
Copy link

@pmessan pmessan commented Jan 17, 2025

If pairing is attempted before the bus is connected, the pairing process fails due to an exception being raised.

CHANGELOG.rst Outdated Show resolved Hide resolved
@pmessan pmessan force-pushed the fix/pairing/init_bus branch from 044b022 to 8aea8fb Compare January 17, 2025 15:24
@dlech
Copy link
Collaborator

dlech commented Jan 17, 2025

Can you please describe how to reproduce the problem that this is fixing?

@pmessan
Copy link
Author

pmessan commented Jan 17, 2025

On a Linux system:

  1. Enable discovery (have a device ready to pair / connect)
  2. When device is discovered, in line with the documentation of BleakClientBlueZDBus.pair(), trust device, then pair, then connect.
  3. Calling BleakClientBlueZDBus.pair() on a Linux-based central will cause an error similar to Object None has no attribute 'call'.
  4. This is because, in BleakClientBlueZDBus.pair() we use the bus connection without checking if it exists whereas for example in BleakClientBlueZDBus.connect(), (here), the bus connecion is initialized the before proceeding with the BLE connection. This means if you call pair() before you call connect(), an exception will be raised.

@dlech
Copy link
Collaborator

dlech commented Jan 17, 2025

OK, so I think what we really want here is to solve #309 where we can just call pair() instead of connect().

Or perhaps even better, add a pair=True paramter to the connect() method and also the BleakClient() constructor.

@pmessan
Copy link
Author

pmessan commented Jan 17, 2025

OK, so I think what we really want here is to solve #309 where we can just call pair() instead of connect().

Or perhaps even better, add a pair=True paramter to the connect() method and also the BleakClient() constructor.

Has anyone attempted to solve this already?
Also, since the pair method is currently broken and does not work without a workaround, would it make sense to fix that then tackle the enhancement as described? Currently with this workaround on my local system I can call pair()(to enable secure connections) then connect() with success.

@dlech
Copy link
Collaborator

dlech commented Jan 17, 2025

Also, since the pair method is currently broken and does not work without a workaround, would it make sense to fix that then tackle the enhancement as described

I don't think it is broken. Everyone so far connects first, then pairs.

@pmessan
Copy link
Author

pmessan commented Jan 17, 2025

Also, since the pair method is currently broken and does not work without a workaround, would it make sense to fix that then tackle the enhancement as described

I don't think it is broken. Everyone so far connects first, then pairs.

But that's in direct contrast to the documentation of that method 😄

@dlech
Copy link
Collaborator

dlech commented Jan 17, 2025

But that's in direct contrast to the documentation of that method 😄

Can you share a link? I'm not finding that.

@pmessan
Copy link
Author

pmessan commented Jan 17, 2025

Additionally, per my testing, calling pair() which calls the Pair method on the BlueZ device, triggers a connection attempt from the Linux host to the remote host by itself anyways.

@pmessan
Copy link
Author

pmessan commented Jan 17, 2025

But that's in direct contrast to the documentation of that method 😄

Can you share a link? I'm not finding that.

Else you need to StartDiscovery, Trust, Pair and Connect in sequence.

@pmessan pmessan force-pushed the fix/pairing/init_bus branch from 8aea8fb to 42a6a3c Compare January 20, 2025 09:06
If pairing is attempted before the bus is connected, the pairing process fails due to an exception being raised.
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.

3 participants