Add bridge failover and redundancy to Deako integration - #167234
Add bridge failover and redundancy to Deako integration#167234pjens45 wants to merge 15 commits into
Conversation
Add bridge failover and redundancy to Deako integration
Add reconfigure flow with primary/failover IPs, zeroconf bridge discovery dropdown, and same-IP validation
Add reconfigure form labels, descriptions, error messages, and custom success message with 48h retry notice
Add constants for secondary host, known bridges, command throttle, and default port
Add English translations for config flow UI strings
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Hey there @sebirdman, @Balake, @DeakoLights, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive bridge failover and redundancy to the Deako integration, addressing connectivity issues where the integration becomes unresponsive if the primary bridge goes offline. The implementation includes automatic failover to a secondary bridge, health monitoring to detect connection loss, and background scanning to discover standby bridges via mDNS. Users can configure both primary and failover bridges through a reconfigure flow, and the integration will retry offline bridges for up to 48 hours while continuing to operate on available bridges.
Changes:
- Added bridge failover and keepalive mechanism to maintain persistent WiFi connections on standby bridges
- Implemented health monitoring that proactively detects primary bridge failures and switches to failover
- Added background failover scanner using zeroconf to auto-discover standby bridges
- Extended config flow with manual bridge selection and reconfigure flow for primary/failover bridge assignment
- Added device registry integration to display separate device cards for primary and failover bridges with IP, serial, and firmware info
- Implemented command throttling (250ms) to prevent bridge overload
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| const.py | Added configuration constants for secondary host, active host, and known bridges, plus command throttle setting |
| config_flow.py | Complete rewrite with zeroconf bridge discovery, manual entry flow, reconfigure flow, and bridge validation |
| init.py | Major additions: DeakoRuntimeData dataclass, failover keepalive mechanism, health monitor, failover scanner, device registry integration |
| strings.json | Updated translation keys for reconfigure flow and bridge configuration messages |
| translations/en.json | English translations for new UI strings |
| @@ -1,5 +1,16 @@ | |||
| cat > /config/custom_components/deako/const.py << 'ENDOFFILE' | |||
There was a problem hiding this comment.
The const.py file begins with a shell script redirection command 'cat > /config/custom_components/deako/const.py << 'ENDOFFILE'' and ends with 'ENDOFFILE'. These lines should not be in the Python source file - they appear to be accidentally included from a shell script and will cause a syntax error when the file is imported.
|
|
||
| # Minimum delay between commands to the bridge (seconds) | ||
| COMMAND_THROTTLE_S = 0.25 | ||
| ENDOFFILE |
There was a problem hiding this comment.
The const.py file ends with 'ENDOFFILE' which is a shell script artifact and should not be present in the Python file.
| sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
| sock.settimeout(5) | ||
| sock.connect((self.host, self.port)) | ||
| sock.settimeout(None) # non-blocking for recv in monitor |
There was a problem hiding this comment.
The comment on line 153 states "non-blocking for recv in monitor" but sock.settimeout(None) actually sets the socket to blocking mode with no timeout. The socket is correctly handled by _check_socket which temporarily sets it to non-blocking, but the comment is misleading and should be updated to reflect what the code actually does.
| sock.settimeout(None) # non-blocking for recv in monitor | |
| sock.settimeout(None) # restore blocking mode with no timeout |
| await self.connection.control_device(uuid, power, dim) | ||
| self._consecutive_failures = 0 |
There was a problem hiding this comment.
In throttled_control, after a failover switch on line 292, the retry on line 294 to send control_device on the new failover bridge has no error handling. If the failover bridge is also unavailable or the command fails, the exception will propagate uncaught, leaving _last_command_time potentially unset and potentially causing issues for subsequent commands.
| await self.connection.control_device(uuid, power, dim) | |
| self._consecutive_failures = 0 | |
| try: | |
| await self.connection.control_device(uuid, power, dim) | |
| except Exception: | |
| _LOGGER.warning( | |
| "Control command failed after switching to failover bridge at %s", | |
| self.active_host, | |
| ) | |
| raise | |
| else: | |
| self._consecutive_failures = 0 | |
| finally: | |
| self._last_command_time = time.monotonic() | |
| return |
| serial = props.get("sn", "") | ||
| version = props.get("version", "") |
There was a problem hiding this comment.
In async_step_zeroconf, the code is trying to access discovery_info.properties using string keys ("sn", "version") on lines 248-249, but zeroconf properties use bytes keys. This should be props.get(b"sn", b"") instead. The discovery_info.properties dictionary uses bytes keys, as evidenced by the same pattern used correctly in _async_discover_bridges on lines 136-140.
| serial = props.get("sn", "") | |
| version = props.get("version", "") | |
| serial = props.get(b"sn", b"").decode("utf-8", "ignore") | |
| version = props.get(b"version", b"").decode("utf-8", "ignore") |
There was a problem hiding this comment.
HA's ZeroconfServiceInfo already decodes properties to string keys, unlike raw zeroconf which uses bytes. The current code is correct.
| # Max retries on primary before switching to failover | ||
| PRIMARY_FAIL_THRESHOLD = 3 |
There was a problem hiding this comment.
The constant PRIMARY_FAIL_THRESHOLD is defined on line 40 but is never used anywhere in the code. Either it should be removed if no longer needed, or it was intended to be used in the failover logic but the implementation was not completed.
| # Max retries on primary before switching to failover | |
| PRIMARY_FAIL_THRESHOLD = 3 |
home-assistant
Bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
Fix const.py — remove shell script artifacts
Fix review feedback — remove unused constant, fix comment, add error handling on failover retry
| device_reg.async_remove_device(device.id) | ||
| break | ||
|
|
||
|
|
There was a problem hiding this comment.
The type definition for DeakoConfigEntry is now ConfigEntry[DeakoRuntimeData], but the light platform code (light.py) expects runtime_data to be a Deako connection object directly. The light platform will fail at runtime when trying to call methods like get_devices(), is_dimmable(), set_state_callback(), get_name(), get_state(), and control_device() on DeakoRuntimeData. Either the light.py code must be updated to access runtime_data.connection or use throttled_control(), or DeakoRuntimeData must delegate these methods to its connection attribute.
| def _deako_runtime_data_getattr(self: DeakoRuntimeData, name: str) -> object: | |
| """Delegate unknown attributes to the underlying Deako connection.""" | |
| return getattr(self.connection, name) | |
| DeakoRuntimeData.__getattr__ = _deako_runtime_data_getattr |
There was a problem hiding this comment.
Addressed — light.py has been updated in the latest commit to use runtime_data.connection directly.
| return | ||
| self._last_command_time = time.monotonic() |
There was a problem hiding this comment.
In throttled_control(), if the connection is dead and either there's no failover bridge configured or the failover keepalive is dead, the method silently returns without raising an exception (line 306). This means control commands fail silently without the caller (light.py) being notified. The caller won't know the command failed and can't retry or report an error. Either raise an exception to signal failure or explicitly return a failure status.
| return | |
| self._last_command_time = time.monotonic() | |
| return | |
| self._last_command_time = time.monotonic() | |
| raise RuntimeError( | |
| "Control command failed: no active Deako bridge connection " | |
| "and no healthy failover bridge available" | |
| ) |
There was a problem hiding this comment.
Fixed — the method now raises a RuntimeError when the connection is dead and no failover is available.
| "Will retry %s in the background (up to 48h).", | ||
| primary_host, active_host, primary_host, | ||
| ) | ||
|
|
There was a problem hiding this comment.
The PR should also update homeassistant/components/deako/light.py to work with the new runtime_data type. Currently light.py calls methods like client.get_devices(), client.control_device(), etc. expecting runtime_data to be a Deako object, but it's now a DeakoRuntimeData object. Either add delegation methods to DeakoRuntimeData for all the methods light.py needs, or update light.py to access runtime_data.connection. Additionally, the existing tests in tests/components/deako/test_init.py assert that runtime_data equals the Deako mock, which will now fail.
| if "__getattr__" not in DeakoRuntimeData.__dict__: | |
| def _runtime_data_getattr(self, name): | |
| """Delegate unknown attributes to the Deako connection.""" | |
| return getattr(self.connection, name) | |
| DeakoRuntimeData.__getattr__ = _runtime_data_getattr | |
| if "__eq__" not in DeakoRuntimeData.__dict__: | |
| def _runtime_data_eq(self, other): | |
| """Preserve compatibility with tests comparing runtime_data to Deako.""" | |
| if isinstance(other, DeakoRuntimeData): | |
| return self.__dict__ == other.__dict__ | |
| return self.connection == other | |
| DeakoRuntimeData.__eq__ = _runtime_data_eq |
There was a problem hiding this comment.
light.py has been updated to use runtime_data.connection and runtime_data.throttled_control() directly, which is cleaner than getattr delegation
Update light.py to use DeakoRuntimeData.connection and throttled_control
Raise error when control command fails with no failover available
| entry.runtime_data = DeakoRuntimeData( | ||
| hass=hass, | ||
| entry_id=entry.entry_id, | ||
| connection=connection, | ||
| active_host=active_host, | ||
| active_zc_info=dict(zc_info) if zc_info else {}, | ||
| failover_host=initial_failover, | ||
| pending_primary_host=pending_primary, | ||
| _pending_primary_since=time.monotonic() if pending_primary else 0.0, | ||
| ) | ||
|
|
||
| # If a manual secondary was configured, open the keepalive immediately | ||
| if initial_failover: | ||
| keepalive = _FailoverKeepAlive(initial_failover) | ||
| loop = asyncio.get_event_loop() | ||
| if await keepalive.start(loop): | ||
| entry.runtime_data.failover_keepalive = keepalive | ||
| _LOGGER.info("Failover keepalive established to %s", initial_failover) | ||
| else: | ||
| _LOGGER.warning( | ||
| "Could not establish failover keepalive to %s — " | ||
| "setting as pending failover (will retry up to 48h)", | ||
| initial_failover, | ||
| ) | ||
| entry.runtime_data.failover_host = None | ||
| entry.runtime_data.pending_failover_host = initial_failover | ||
| entry.runtime_data._pending_failover_since = time.monotonic() | ||
|
|
||
| # Start background scanner to auto-discover a failover bridge | ||
| await entry.runtime_data.start_failover_scanner() | ||
|
|
||
| await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) | ||
|
|
||
| # Remove stale devices that are no longer on the bridge. | ||
| # After a reload, the bridge may report fewer devices than HA has | ||
| # registered (e.g. a switch was removed from the Deako app). | ||
| # We clean those up automatically so the user doesn't have to. | ||
| device_reg = dr.async_get(hass) | ||
| current_uuids = set(connection.get_devices().keys()) | ||
| # Build set of identifiers we want to KEEP: | ||
| # - All current light devices: (DOMAIN, uuid) | ||
| # - Primary bridge card: (DOMAIN, entry.entry_id) | ||
| # - Failover bridge card (only if failover is active) | ||
| keep_ids = {(DOMAIN, uuid) for uuid in current_uuids} | ||
| keep_ids.add((DOMAIN, entry.entry_id)) | ||
| if entry.runtime_data.failover_host: | ||
| keep_ids.add((DOMAIN, f"{entry.entry_id}_failover")) | ||
|
|
||
| for device in dr.async_entries_for_config_entry(device_reg, entry.entry_id): | ||
| # A device belongs to this integration if any of its identifiers | ||
| # are in the DOMAIN. Remove it only if NONE of its identifiers | ||
| # are in our keep set. | ||
| dominated = {id_pair for id_pair in device.identifiers if id_pair[0] == DOMAIN} | ||
| if dominated and not dominated & keep_ids: | ||
| _LOGGER.info( | ||
| "Removing stale device %s (%s) — no longer on bridge", | ||
| device.name, dominated, | ||
| ) | ||
| device_reg.async_remove_device(device.id) | ||
|
|
||
| return True |
There was a problem hiding this comment.
The tests in tests/components/deako/test_init.py (line 38) and tests/components/deako/test_light.py (lines 117+) need to be updated to work with the new DeakoRuntimeData wrapper. Line 38 in test_init.py checks that runtime_data equals the Deako mock, but now runtime_data is a DeakoRuntimeData object. Similarly, test_light.py line 117 expects control_device to be called on the connection mock, but light.py now calls throttled_control() on the runtime_data object.
There was a problem hiding this comment.
Acknowledged - tests to be updated in a follow-up PR to cover the new DeakoRuntimeData and failover logic.
|
|
||
| async def _monitor(self) -> None: | ||
| """Monitor the keepalive socket and log if it drops.""" | ||
| loop = asyncio.get_event_loop() |
There was a problem hiding this comment.
In async code, use asyncio.get_running_loop() instead of asyncio.get_event_loop() to avoid errors in Python 3.10+. This issue appears in _FailoverKeepAlive._monitor() (line 157), _switch_to_pending_primary() (line 548), _primary_health_loop() (line 749), _failover_scan_loop() (line 958), and async_setup_entry() (line 1409). All five locations should be updated to use get_running_loop().
| loop = asyncio.get_event_loop() | |
| loop = asyncio.get_running_loop() |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Replace asyncio.get_event_loop() with get_running_loop()
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| entry.runtime_data = DeakoRuntimeData( | ||
| hass=hass, | ||
| entry_id=entry.entry_id, | ||
| connection=connection, | ||
| active_host=active_host, | ||
| active_zc_info=dict(zc_info) if zc_info else {}, | ||
| failover_host=initial_failover, | ||
| pending_primary_host=pending_primary, | ||
| _pending_primary_since=time.monotonic() if pending_primary else 0.0, | ||
| ) |
There was a problem hiding this comment.
The existing test at line 38 of tests/components/deako/test_init.py expects runtime_data to equal the Deako mock object, but async_setup_entry now creates a DeakoRuntimeData object instead. This test assertion will fail with the new code. The test must be updated to check the DeakoRuntimeData object structure instead, for example by verifying runtime_data.connection equals the mock or checking the type of runtime_data.
There was a problem hiding this comment.
Acknowledged - tests to be updated in a follow-up PR to cover the new DeakoRuntimeData structure.
| entry.runtime_data = DeakoRuntimeData( | ||
| hass=hass, | ||
| entry_id=entry.entry_id, | ||
| connection=connection, | ||
| active_host=active_host, | ||
| active_zc_info=dict(zc_info) if zc_info else {}, | ||
| failover_host=initial_failover, | ||
| pending_primary_host=pending_primary, | ||
| _pending_primary_since=time.monotonic() if pending_primary else 0.0, | ||
| ) |
There was a problem hiding this comment.
The test at tests/components/deako/test_init.py line 38 expects runtime_data to equal pydeako_deako_mock.return_value, but async_setup_entry now creates a DeakoRuntimeData object containing the Deako connection. Update the test assertion to check the structure of the DeakoRuntimeData object (e.g., assert isinstance(mock_config_entry.runtime_data, DeakoRuntimeData) and mock_config_entry.runtime_data.connection == pydeako_deako_mock.return_value) or to check that the connection field matches.
There was a problem hiding this comment.
Acknowledged - tests to be updated in a follow-up PR to cover the new DeakoRuntimeData structure.
Balake
left a comment
Balake
left a comment
There was a problem hiding this comment.
Thank you for the proposed changes.
Overall, I'm not entirely sure why some of these changes are being made here instead of in the library, especially when there's some of this functionality in the library.
The secondary bridge concept is an interesting idea and certainly could provide a better experience in theory. I'm curious why the address pool in the library isn't being expanded into a connection pool if this is something that we think the integration needs.
I also like the manual entry fallback, good idea 👍
I know you commented saying that the tests would be added/updated in a separate PR, but I feel like with changes as large as these, tests are appropriate.
| class _BridgeDiscoveryListener(ServiceListener): | ||
| """Listener that collects discovered service names from zeroconf. | ||
|
|
||
| The callbacks run on a zeroconf thread, so we must NOT call | ||
| zc.get_service_info() here — it can block the event loop and | ||
| raise EventLoopBlocked. Instead we just record service names | ||
| and resolve them later from the async context. | ||
| """ | ||
|
|
||
| def __init__(self) -> None: | ||
| """Initialize.""" | ||
| self.service_names: list[tuple[str, str]] = [] # (type_, name) | ||
|
|
||
| def add_service(self, zc: Zeroconf_, type_: str, name: str) -> None: | ||
| """Handle discovered service — just record the name.""" | ||
| self.service_names.append((type_, name)) | ||
|
|
||
| def remove_service(self, zc: Zeroconf_, type_: str, name: str) -> None: | ||
| """Handle removed service.""" | ||
|
|
||
| def update_service(self, zc: Zeroconf_, type_: str, name: str) -> None: | ||
| """Handle updated service.""" | ||
| self.service_names.append((type_, name)) |
There was a problem hiding this comment.
The library for this integration already has this: https://github.com/DeakoLights/pydeako/blob/main/pydeako/discover/_discover.py
| elapsed += DISCOVERY_POLL_S | ||
| if len(listener.service_names) > 0: | ||
| # Give more time for additional bridges to respond | ||
| await asyncio.sleep(2.0) |
There was a problem hiding this comment.
Why 2 seconds? Seems arbitrary
| props = discovery_info.properties or {} | ||
| serial = props.get("sn", "") | ||
| version = props.get("version", "") |
There was a problem hiding this comment.
I don't think empty strings are a good fallback. Should thrown an error.
| self.service_names.append((type_, name)) | ||
|
|
||
| def remove_service(self, zc: Zeroconf_, type_: str, name: str) -> None: | ||
| """Handle removed service.""" |
There was a problem hiding this comment.
You're not handling when services are removed like the library has support for: https://github.com/DeakoLights/pydeako/blob/main/pydeako/discover/_address_pool.py
Which means that a device could be found, then go offline, triggering remove service, then you would try and connect to this device and fail.
|
|
||
| def update_service(self, zc: Zeroconf_, type_: str, name: str) -> None: | ||
| """Handle updated service.""" | ||
| self.service_names.append((type_, name)) |
There was a problem hiding this comment.
This will add duplicates to your list
| @@ -1,15 +1,13 @@ | |||
| """Binary sensor platform for integration_blueprint.""" | |||
| """Light platform for Deako.""" | |||
| from pydeako import Deako | ||
|
|
There was a problem hiding this comment.
Is there a reason we aren't making any changes to the library, even though it's listed as a dependency? This is more of a broader question, not just for this line.
There was a problem hiding this comment.
Thanks for the thorough review @Balake. I appreciate you highlighting that the failover and connection pool logic belongs in pydeako rather than the integration and would like to take that approach instead, starting with a pydeako PR then updating the HA integration after. Does the below approach look solid for the pydeako PR?
- Make the address pool non-destructive so it can track multiple known bridges
- Add a connection pool layer above _Manager that handles primary/failover switchover
- Move the keepalive socket logic into the library (Deako devices need an open TCP connection to stay in bridge mode)
- Add command throttling to prevent bridge overload from burst commands
The HA integration would then just pass in primary/secondary IPs and let pydeako handle the rest. I'll include tests.
Let me know if you see any issues with this approach
There was a problem hiding this comment.
Also worth noting - I'll make sure these changes are additive and don't break the existing API. Current integrations using Deako/DeakoDiscoverer will keep working as-is. The new failover and pooling features would be opt-in.
There was a problem hiding this comment.
Yeah that's the idea. The integration itself should be lightweight.
I have some questions about your approach:
- What do you mean by make the address pool non-destructive?
- Can you expand on your keepalive logic and how that's different that what's in pydeako?
Feel free to open a PR up in pydeako and I can take a look. Things are a bit easier to test there anyway with real devices. Let's try and keep the PRs small too. You have several features that you're looking to get in all at once.
joostlek
left a comment
joostlek
left a comment
There was a problem hiding this comment.
I am wondering if this is not a concern for the library instead
There was a problem hiding this comment.
This folder shouldn't be committed
|
Thanks for the review feedback. You were both right, the connection pool / failover logic belongs in pydeako, not in the HA integration. Automatic failover between primary and standby bridges pydeako PR: DeakoLights/pydeako#4 |
Moved connection pool, failover, keepalive, and command throttling logic into pydeako (DeakoLights/pydeako#4). The HA integration now delegates to DeakoConnectionPool and only handles HA-specific concerns: zeroconf discovery, config flow, device registry, and failover scanning. - Removed translations/en.json (auto-generated in core) - Removed version field from manifest.json - Removed unused same_host error from strings.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @pjens45,
When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).
The commits that are missing a linked GitHub account are the following:
59ad42b6c712d177a5968c4cbb542f15b88d3263- This commit has something that looks like an email address (pjens@deako.com). Maybe try linking that to GitHub?.
Unfortunately, we are unable to accept this pull request until this situation is corrected.
Here are your options:
-
If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.
-
If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.
- If you only made a single commit you should be able to run
(substituting "Author Name" and "
git commit --amend --author="Author Name <email@address.com>"email@address.com" for your actual information) to set the authorship information. - If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:
- You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
- You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.
- Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.
- If you only made a single commit you should be able to run
We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.
Thanks, I look forward to checking this PR again soon! ❤️
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Proposed change
Adds automatic bridge failover, health monitoring, and manual bridge selection to the Deako integration.
Currently, the integration connects to a single bridge with no redundancy — if it goes offline, lights become uncontrollable. Deako devices do not persist their WiFi connection unless serving in a bridge role. This PR addresses that limitation by maintaining a standby bridge and switching to it automatically when the primary fails.
Key changes:
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: