-
Notifications
You must be signed in to change notification settings - Fork 73
fix: Ensure immediate local connection is attempted #640
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -29,6 +29,7 @@ | |||
| MIN_BACKOFF_INTERVAL = datetime.timedelta(seconds=10) | ||||
| MAX_BACKOFF_INTERVAL = datetime.timedelta(minutes=30) | ||||
| BACKOFF_MULTIPLIER = 1.5 | ||||
| START_ATTEMPT_TIMEOUT = datetime.timedelta(seconds=5) | ||||
|
|
||||
|
|
||||
| class RoborockDevice(ABC, TraitsMixin): | ||||
|
|
@@ -107,25 +108,32 @@ def is_local_connected(self) -> bool: | |||
| """ | ||||
| return self._channel.is_local_connected | ||||
|
|
||||
| def start_connect(self) -> None: | ||||
| async def start_connect(self) -> None: | ||||
| """Start a background task to connect to the device. | ||||
|
|
||||
| This will attempt to connect to the device using the appropriate protocol | ||||
| channel. If the connection fails, it will retry with exponential backoff. | ||||
| This will give a moment for the first connection attempt to start so | ||||
| that the device will have connections established -- however, this will | ||||
| never directly fail. | ||||
|
|
||||
| If the connection fails, it will retry in the background with | ||||
| exponential backoff. | ||||
|
|
||||
| Once connected, the device will remain connected until `close()` is | ||||
| called. The device will automatically attempt to reconnect if the connection | ||||
| is lost. | ||||
| """ | ||||
| start_attempt: asyncio.Event = asyncio.Event() | ||||
|
|
||||
| async def connect_loop() -> None: | ||||
| backoff = MIN_BACKOFF_INTERVAL | ||||
| try: | ||||
| while True: | ||||
| try: | ||||
| await self.connect() | ||||
| start_attempt.set() | ||||
| return | ||||
| except RoborockException as e: | ||||
| start_attempt.set() | ||||
| _LOGGER.info("Failed to connect to device %s: %s", self.name, e) | ||||
| _LOGGER.info( | ||||
| "Retrying connection to device %s in %s seconds", self.name, backoff.total_seconds() | ||||
|
|
@@ -136,8 +144,16 @@ async def connect_loop() -> None: | |||
| _LOGGER.info("connect_loop for device %s was cancelled", self.name) | ||||
| # Clean exit on cancellation | ||||
| return | ||||
| finally: | ||||
| start_attempt.set() | ||||
|
|
||||
| self._connect_task = asyncio.create_task(connect_loop()) | ||||
|
|
||||
|
||||
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace should be removed from this blank line.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -82,14 +82,16 @@ async def discover_devices(self) -> list[RoborockDevice]: | |||||
|
|
||||||
| # These are connected serially to avoid overwhelming the MQTT broker | ||||||
|
||||||
| # These are connected serially to avoid overwhelming the MQTT broker |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The asyncio.gather(*start_tasks) call should include return_exceptions=True to prevent one device's connection failure from blocking the discovery of other devices. Without this parameter, if any start_connect() raises an exception that escapes the timeout, the entire discover_devices() operation will fail.
Consider changing this to:
await asyncio.gather(*start_tasks, return_exceptions=True)| await asyncio.gather(*start_tasks) | |
| await asyncio.gather(*start_tasks, return_exceptions=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace should be removed from this blank line.