Report local devices as disconnected when they stop responding - #81
Merged
Conversation
IntesisHomeLocal only ever wrote _connected in connect() and stop(), so
is_connected meant "connect() succeeded once" rather than "the device is
reachable". _request_values() absorbed every connection and auth error
and returned {}, and _run_updater() then iterated zero values, fired the
update callback anyway, and looped. A unit that stopped responding after
setup kept reporting its last-known state indefinitely, with no signal a
consumer could act on.
_connected could not simply be cleared on failure, because it was also
the updater loop's own condition - the loop would have exited on the
first outage and never returned, leaving the device permanently
unavailable even after it recovered.
- The loop is now governed by a separate _running flag, cleared by
stop(), so _connected is free to be a pure reachability signal.
- _connected goes False once _unavailable_after (300s) elapses with no
successful poll, and True again on the next success, so a device that
recovers does so without the consumer rebuilding the controller. The
grace period is elapsed time rather than a failure count because a
failed request takes anywhere from milliseconds to ~20s, so a count
maps to no fixed duration.
- Polling backs off while failing (6s doubling to a 30s ceiling). These
units are not powerful and polling a struggling one at the full rate
can make matters worse. The failure counter decays instead of resetting
so a unit that alternates timeouts and successes still gets the easing
off, rather than being held at the full rate.
- _request_values() no longer swallows errors; the updater classifies
them. Rejected credentials are not transient, so they stop the updater
rather than being retried forever, mirroring the cloud reconnect loop.
- New last_successful_update property, populated for local from the
updater and for the socket-based controllers from the shared read loop,
giving consumers a staleness signal independent of the boolean.
Fixes #80
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016LhhoeQLuyYkRmgfUgL57x
Adds the last_successful_update property, and changes is_connected for IntesisHomeLocal from "connect() succeeded" to a live reachability signal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016LhhoeQLuyYkRmgfUgL57x
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #80. Bumps the version to 2.2.0.
The problem
IntesisHomeLocalonly ever wrote_connectedinconnect()andstop(), sois_connectedmeant "connect()succeeded once andstop()hasn't been called" rather than "the device is reachable"._request_values()absorbed every connection and auth error and returned{};_run_updater()then iterated zero values, fired the update callback anyway, and looped. A unit that stopped responding after setup kept reporting its last-known state indefinitely, and a consumer received a byte-identical signal in the healthy and dead cases._connectedcouldn't simply be cleared on failure, because it was also the updater loop's own condition — the loop would have exited on the first outage and never returned, leaving the device permanently unavailable even after it recovered.The change
Separated the two jobs
_connectedwas doing. The loop is now governed by a new_runningflag, cleared bystop(), which frees_connectedto be a pure reachability signal: it goesFalseonce_unavailable_after(300s) elapses with no successful poll, andTrueagain on the next success. A device that recovers does so on its own, without the consumer rebuilding the controller.The grace period is elapsed time, not a failure count. A failed request takes anywhere from milliseconds (connection refused) to ~20s (
_requestuses a 10s timeout and retries twice internally), so a fixed count maps to no fixed duration. With the production constants the flip lands at 312s for instant failures and 322s for timeouts — a count of 50 would have spanned 300s to 1300s across the same two cases.Polling backs off while failing — 6s doubling to a 30s ceiling. These units aren't powerful and polling a struggling one at the full rate can make matters worse. The failure counter decays rather than resetting on success, so a unit alternating timeouts and successes still gets the easing off instead of being pinned at the full rate for the whole period it's struggling.
Failures are classified rather than absorbed.
_request_values()no longer swallows exceptions, so the updater can distinguish them. Rejected credentials aren't transient, so they stop the updater instead of being retried every 6s forever — mirroring_reconnect_loopon the cloud side, which also gives up onIHAuthenticationError. This path became reachable in 2.1.0, when_authenticate()started raising on rejection. Previously the oldexcept IHConnectionErrorinside the loop body was dead code, since_request_valuescaught everything first.New
last_successful_updateproperty (UTC datetime,Noneif never). Populated for local from the updater, and for the socket-based controllers from the shared read loop in_data_received, so it's meaningful for cloud and IntesisBox too. It gives consumers a staleness signal on a tighter schedule than the boolean — worth having because the cloud has the same class of blind spot:readuntilhas no timeout and the keepalive is fire-and-forget, so a blackholed socket can sit_connected = Trueuntil the kernel gives up retransmitting.Note the local 300s grace and the cloud's
_reconnect_delay_max = 300are numerically identical but semantically unrelated — one is a grace period, the other a ceiling on reconnect attempt spacing.Why local doesn't get a reconnect loop
The cloud needs
_should_reconnect/_reconnect_taskbecause a dead socket must be rebuilt and re-authenticated. HTTP is stateless and_requestalready re-authenticates on error codes 1 and 5, so the local updater heals in place. The deeper asymmetry: local polls, so silence is diagnostic; the cloud pushes only on change, so silence is the normal idle case and can't be used to infer anything.Tests
Four new cases, all failing against the pre-fix code:
test_local_reports_disconnected_then_recovers— device stops answering,is_connectedflipsFalse, then recovers when it starts answering again. Also asserts the updater task survived the outage, which is what the naive fix breaks.test_local_updates_last_successful_update— advances while healthy, freezes during an outage.test_local_stops_updater_when_credentials_rejected— updater exits,error_messageset.test_local_backs_off_while_failing— interval growth and the ceiling.Failure injection is a mutable switch in
tests/__init__.pythatlocal_api_callbackconsults, so a test can make the mocked device die or start rejecting credentials part way through a run. Afast_local_controllerfixture compresses the timings beforeconnect(), so the suite runs in the same time as before.67 passed;
pylint10.00/10;ruff checkandruff format --checkclean.Behavioural note for consumers
For
IntesisHomeLocal,is_connectedcan now returnFalseon a live controller. That's the point of the change, and hass-intesishome's existingavailablelogic already assumes it — but any consumer treatingis_connectedas "was set up successfully" will see newFalsevalues. A device that reboots or an AP that reassociates will surface as a brief unavailable period if it takes more than 300s to answer again.