Add support for pairing callbacks #1864
Conversation
dlech
left a comment
dlech
left a comment
There was a problem hiding this comment.
Thanks for getting this moving again. I've made some preliminary comments. It will take me some time to go back and look at the previous discussions to refresh my memory on all of the details that have been looked at before. And I'll need some time to put something together for testing.
In the meantime, it looks like there is perhaps some preliminary refactoring that could be split out into a separate pull request to keep things moving.
| # we can add a handler for them) and also ensures that the pairing | ||
| # agent will only handle handle requests for this specific connection. | ||
| self._bus: MessageBus = MessageBus( | ||
| bus_type=BusType.SYSTEM, negotiate_unix_fd=True |
There was a problem hiding this comment.
| bus_type=BusType.SYSTEM, negotiate_unix_fd=True | |
| bus_type=BusType.SYSTEM, negotiate_unix_fd=True, auth=get_dbus_authenticator() |
| negotiate_unix_fd=True, | ||
| auth=get_dbus_authenticator(), | ||
| ).connect() | ||
| if not self._bus.connected: |
There was a problem hiding this comment.
By not creating a new MessageBus here, it changes the behavoir when when people disconnect and reconnect without stopping notifications first.
I'm not sure we can do this without breaking existing users.
| member="Pair", | ||
| ) | ||
| ) | ||
| reply = await self._pair(self._pairing_callbacks) |
There was a problem hiding this comment.
IIRC, there was some slight difference between the pairing here and in self.pair(). I'm guessing that is why self._pair() is introduced.
Can we pull this change out into a separate pull request to make it easier to look at on it's own?
|
|
||
| return services | ||
|
|
||
| def get_device_props(self, device_path: str) -> Device1: |
There was a problem hiding this comment.
I would prefer to stick with having methods to get individual properties using the _get_device_property() so that we get a more helpful error message when things go wrong.
|
|
||
| if kwargs.get("pairing_callbacks"): | ||
| logger.warn( | ||
| "Pairing is not available in WinRT.", |
There was a problem hiding this comment.
This one should be "not implemented" rather than "not available".
| """ | ||
| Async version of the builtin input function. | ||
| """ | ||
| return input(msg) |
There was a problem hiding this comment.
input() is a blocking function, so will break asyncio. We should replace this with something else like aioconsole.
| def __init__(self) -> None: | ||
| super().__init__() |
There was a problem hiding this comment.
| def __init__(self) -> None: | |
| super().__init__() |
This should not be needed.
|
|
||
| print("scanning...") | ||
|
|
||
| device = await BleakScanner.find_device_by_address(addr) |
There was a problem hiding this comment.
Would be nice to have the possibility to find by name as well like other examples.
|
@tgagneret-embedded Thanks for picking this back up. We have a lot of Home Assistant users who are really excited about having this feature. As soon as this gets going, I'll work on adding support to ESPHome as well. |
3 participants
Add support for pairing callbacks.
This is based on: