fix(coordinator): don't let the stale MQTT cache clobber fresher HTTP state (upstream #71) - #2
Open
Colorado4Wheeler wants to merge 1 commit into
Conversation
… state The SDK's MQTT cache (mower_sdk.state_manager) stores the last message ever received with no timestamp and no expiry, and never clears it. _async_update_data() re-applied it on every tick gated only on "is not None", so once the mower stops publishing state -- which is exactly what docking does -- that one message was replayed every 30s indefinitely, overwriting the fresher HTTP fallback value on every tick where the hourly fallback was throttled out. The correct value appeared for a single tick, then reverted. Compute MQTT freshness first and only re-apply the cache while it is genuinely fresh. Real-time pushes are unaffected: they arrive via _handle_state -> _update_from_state, which sets the state directly and does not go through this block. Fixes segwaynavimow#71 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Upstream segwaynavimow/NavimowHA#71 reports a stale-cache bug in
NavimowCoordinator._async_update_data(). It has been open since 2026-06-08 and upstream has had no commits since 2026-04-10, so it looks unlikely to be fixed there.This fork carries the bug unchanged (
coordinator.pyatdb092a1is byte-identical to upstream's in this function), so this PR applies the fix here. It touches nothing the fork added.Issues are disabled on this repo, so a PR seemed like the right way to raise it — happy for it to be closed and just taken as a patch if you'd rather commit it yourself.
I hit it on an i110 tonight and captured what I think is fairly tidy evidence, in case it is useful for confirming the mechanism.
Mechanism
Each tick (
UPDATE_INTERVAL = 30) the coordinator:_last_state, gated only onis not None— never on age;MQTT_STALE_SECONDS(300) andHTTP_FALLBACK_MIN_INTERVAL(3600) has elapsed, fetches over HTTP and overwrites.mower_sdk.state_manager.StateManagerstores the last message ever received with no timestamp and no expiry, and never clears it:So once the mower stops publishing state — which is exactly what happens when it docks — step 1 replays that last message every 30 s indefinitely, and the only thing that can correct it is permitted once an hour. The correct value appears for one tick, then reverts.
Observed
Battery, this integration vs. an independent poller against the same account, on one evening (UTC). The mower was mowing until ~22:20, then docked and charged:
They agree while the mower is moving and diverge the moment it docks. Earlier the same evening the integration corrected at 21:07:53 and 22:07:05 — 59 minutes apart, the
HTTP_FALLBACK_MIN_INTERVALcadence — and sat flat at 93 for 47 minutes in between while the true value walked 92 → 88.The mower was lifted once during the session, and the
isLiftedmessage is the one that got stuck:lawn_mower.<mower>heldstate: errorwithmetrics: {'raw_state': 'isLifted'}for nearly an hour after the mower had been cleared and docked. That attribute is also how you can tell the replay from a real fetch —_device_status_to_state()hardcodesmetrics=None, so a state carryingmetricscan only have come from the MQTT cache.Worth noting this fork's own additions make the diagnosis easier and are not implicated: the
/realtimeDate/locationchannel stayed current to the second throughout (pose_time23:08:13, ingested 23:08:14), which rules out a dead MQTT connection and isolates the fault to the state path.The change
Compute freshness first, and only re-apply the cache while it is genuinely fresh. Real-time pushes are unaffected — they arrive via
_handle_state→_update_from_state, which sets the state directly and does not go through this block.This is upstream segwaynavimow#71's proposed fix. Note that lowering
HTTP_FALLBACK_MIN_INTERVALon its own does not fix it — it only makes the correct value flash more often, because the cache still clobbers it on every intervening tick.Residual, separate from the above
With the patch in, the HTTP value sticks instead of being overwritten, but while the mower is docked it still only refreshes hourly, so battery during a charge is correct-but-coarse. If you wanted to address that too, making
HTTP_FALLBACK_MIN_INTERVALconfigurable via an options flow (also suggested in upstream segwaynavimow#70) would cover it without changing the default for anyone.Thanks for the position/zone work, the dock auto-learn and map card are genuinely good additions.
Environment
v1.1.0-position.5(manifest1.1.0+position.3), installed via HACSnavimow-sdk:0.1.2