Feature/windows pairing #640
Conversation
Signed-off-by: Bojan Potočnik <[email protected]>
This agent currently does nothing, it just logs method calls to check if it is properly registered. Signed-off-by: Bojan Potočnik <[email protected]>
Signed-off-by: Bojan Potočnik <[email protected]>
Also pairingAgent is moved to BleakClient class for easier access Signed-off-by: Bojan Potočnik <[email protected]>
Also modify example to use this callback and retrieve passkey from the user. Signed-off-by: Bojan Potočnik <[email protected]>
Signed-off-by: Bojan Potočnik <[email protected]>
Ensure that if no callback is provided, pairing works as before, meaning that Just Works pairing requests are confirmed. Signed-off-by: Bojan Potočnik <[email protected]>
Signed-off-by: Bojan Potočnik <[email protected]>
…' into feature/clientUpate
Only use the protection level if it's set to anything
Pairing code had to be modified to meet the pythonic winrt module in order to complete the pairing process without installing the dotnet libraries (pythonnet and the .net core sdk)
Builds on PR hbldh#523 to include pairing for Windowns with both dotnet and winrt backends. Updated timeout in connect to use the same timeout as provided to the connect function.
…into feature/windowsPairing
Added windows pairing to .NET and WinRT backends, following the process started in PR hbldh#523.
…into feature/windowsPairing
|
Thanks for this. I currently don't have any devices to test this with. Does anyone have any suggestions of something I could get (or wants to send me a spare)? |
I think some suggested a device in another thread. Let me see what I can do. Edit: Please send me an email, to the one listed on my account. |
| DBusObject = "o" | ||
| DBusString = "s" | ||
| DBusUInt16 = "q" | ||
| DBusUInt32 = "u" |
There was a problem hiding this comment.
These aliases have lead to wrong usage. They are only meant for bindings of D-Bus methods, however they are used below where actual Python types should be used (e.g. str, int). So I think we should remove them to avoid the confusion.
| io_capabilities (`Capability`): I/O capabilities of this device, used for determining pairing method. | ||
| """ | ||
|
|
||
| def __init__(self, io_capabilities: IOCapability = IOCapability.KEYBOARD_DISPLAY): |
There was a problem hiding this comment.
I'm wondering if it makes sense to have a default here or if we should require users to specify this along with their callback function.
There was a problem hiding this comment.
IOCapability could also be part of the proposed callback class. In that case, it should be an enum.IntFlags and have the same values as this so it can be used directly in the WinRT backend (these values are defined by Bluetooth spec and not specific to Windows, IIRC - should probably research more before doing exactly as I've suggested) and converted to the appropriate strings in the BlueZ backend.
There was a problem hiding this comment.
| Implemented by using the `BlueZ DBUS Agent API <https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/agent-api.txt>`_. | ||
|
|
||
| Args: | ||
| io_capabilities (`Capability`): I/O capabilities of this device, used for determining pairing method. |
There was a problem hiding this comment.
Since we are using type hints, we can leave out the type from the doc strings. If we want to keep them anyway, the ` needs to be removed.
| # D-Bus message bus | ||
| self._bus: Optional[MessageBus] = None | ||
| # Path can be anything as long as it is unique | ||
| self._path = f"/org/bluez/agent{time.time() * 1000:.0f}" |
There was a problem hiding this comment.
it would be nice to include bleak in the path.
| self._io_capabilities = io_capabilities | ||
|
|
||
| # Callback for every device (single agent handles all pairing requests) | ||
| self._callbacks: Dict[DBusObject, PairingCallback] = {} |
There was a problem hiding this comment.
| self._callbacks: Dict[DBusObject, PairingCallback] = {} | |
| self._callbacks: Dict[str, PairingCallback] = {} |
| print("\t\tValue: ", await client.read_gatt_char(char)) | ||
|
|
||
|
|
||
| if platform.system() == "Darwin": |
There was a problem hiding this comment.
It would probably be better to leave out this check and just let the example throw whatever error the pair method actually throws on Mac.
| print("Paired") | ||
|
|
||
| services = await client.get_services() | ||
| print(services) |
There was a problem hiding this comment.
Seems unnecessary since we are enumerating the services below.
| @@ -295,6 +298,11 @@ async def pair(self, protection_level: int = None, **kwargs) -> bool: | |||
| 2: Encryption - Pair the device using encryption. | |||
| 3: EncryptionAndAuthentication - Pair the device using | |||
| encryption and authentication. (This will not work in Bleak...) | |||
| callback (`PairingCallback`): callback to be called to provide or confirm pairing pin | |||
There was a problem hiding this comment.
We can omit the type here.
| callback (`PairingCallback`): callback to be called to provide or confirm pairing pin | |
| callback: callback to be called to provide or confirm pairing pin |
| print(service) | ||
| for char in service.characteristics: | ||
| print("\t", char) | ||
| print("\t\tValue: ", await client.read_gatt_char(char)) |
There was a problem hiding this comment.
| print("\t\tValue: ", await client.read_gatt_char(char)) | |
| print("\t\tValue:", await client.read_gatt_char(char)) |
|
|
||
| async def main(mac_addr: str): | ||
| # Remove this device if it is already paired (from previous runs) | ||
| if await BleakClient.remove_device(mac_addr): |
There was a problem hiding this comment.
Doesn't this method only work on BlueZ? Maybe we should connect first, then unpair if paired, then pair again instead.
|
There are still some conflicts to address. This is a lot of commits to rebase on develop, but try to resolve the last conflicts and we can see if this is mergable after that! |
|
I would really like to get this merged in to the main code base, but I am very busy with work at the moment (and have been since the PR was reviewed). If anyone would like to assist, I do not mind granting access for them. |
|
I'm pretty interested in looking into this. If I'm understanding correctly, it is functionally complete and is just a matter of cleanup from PR comments and more testing? |
Yes, that's correct. |
OK cool well I'll at least try to get this working on Ubuntu and Windows and go from there. |
let me know if you need/want access to make changes to the branch. |
|
Is there any schedule for release this feature? |
|
I want to bring this topic back to live. I already ported the WinRT part to bleak version 0.15.1 and using it successfully for a few weeks now in my environment. |
|
Splitting it into smaller pieces is always a good thing. I can't find it now, but someone recently mentioned implementing this as an async context manager, which I think is a good idea (it's been too long, I don't remember if it is done this way already or not). So it would be used like this: async with BleakPairingAgent(callback):
await client.pair()Also, since #982 was just merged, I would like to rework this to follow the pattern used by BleakClient and BleakScanner there where we have a common top-level class with a platform specific backend (depedency-injection-like). |
|
FYI, I've made some attempt to move this along in #1100. It still needs a bit of work and there are still some unresolved issues, so feedback from anyone who wants to use this feature would be appreciated. |
|
After a too long time, I found some time (and need) to get back to this feature. However, I see that in the meantime In my opinion, this PR could be closed - however major part of this PR is also done by @jpeters-ml, which should provide his opinion. I'll base my fork on the |
|
Considering that these changes are being moved with #1100 I don't mind closing this as I haven't had the time to update. |
|
Closing in favor of #1100 |
Code derived from #523.
Added Windows pairing to backends .NET and WinRT. Updated pairing example to allow Windows.