feat: Connect to devices asynchronously#588
feat: Connect to devices asynchronously#588allenporter merged 7 commits intoPython-roborock:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the device manager to connect to devices asynchronously in the background instead of blocking on connection, allowing the application to continue operating while devices connect. Connection failures are automatically retried with exponential backoff.
- Changed
device_manager.pyto callstart_connect()instead ofawait connect()when discovering devices - Added
start_connect()method with exponential backoff retry logic indevice.py - Added test coverage for connection failure and retry behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| roborock/devices/device_manager.py | Changed from blocking await connect() to non-blocking start_connect() call |
| roborock/devices/device.py | Added start_connect() method with retry loop, exponential backoff constants, and task cancellation in close() |
| tests/devices/test_device_manager.py | Added test fixtures and test case for connection failure scenarios with retry behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
e849123 to
723ae10
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| if self._connect_task: | ||
| self._connect_task.cancel() | ||
| try: | ||
| await self._connect_task |
There was a problem hiding this comment.
Maybe I'm sleep deprived but can you double check this is the right functionality? Won't this reopen the connection on close()
There was a problem hiding this comment.
The self._connect_task is a task started in start_connect. That start event happens once there, and gets kicked off in the background. Then in close, we cancel the task. This line will wait for the task to complete -- either because it already completed or because its waiting for cancellation to finish.
This has an example https://docs.python.org/3/library/asyncio-task.html#asyncio.Task.cancel and what we're doing here is similar to how the main example looks.
There was a problem hiding this comment.
Ah yes that makes sense. Looking at it again. Thank you for the explanation.
Update the device manager to not block on connecting to devices immediately, but instead connect to them asynchronously in the background. Once they connect, the devices will reconnect on their own (e.g. using the mqtt session or local channel behavior).
Add a test that exercises the behavior of an unsuccessful connect with retries in the background.