Skip to content

fix(coordinator): don't let the stale MQTT cache clobber fresher HTTP state (upstream #71) - #2

Open
Colorado4Wheeler wants to merge 1 commit into
pgoutsos:mainfrom
Colorado4Wheeler:fix/stale-mqtt-cache-issue-71
Open

fix(coordinator): don't let the stale MQTT cache clobber fresher HTTP state (upstream #71)#2
Colorado4Wheeler wants to merge 1 commit into
pgoutsos:mainfrom
Colorado4Wheeler:fix/stale-mqtt-cache-issue-71

Conversation

@Colorado4Wheeler

Copy link
Copy Markdown

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.py at db092a1 is 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:

  1. copies the SDK's cached MQTT state into _last_state, gated only on is not None — never on age;
  2. then, only if MQTT has been quiet for MQTT_STALE_SECONDS (300) and HTTP_FALLBACK_MIN_INTERVAL (3600) has elapsed, fetches over HTTP and overwrites.

mower_sdk.state_manager.StateManager stores the last message ever received with no timestamp and no expiry, and never clears it:

async def state(self, state: DeviceStateMessage) -> None:
    self._last_state = state
    await self.state_callback.data_event(state)

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:

time this integration independent poller
22:05 – 22:15 88 → 87 88 → 87
22:25 91 91
22:26 87 — reverted one tick later
22:30 87 (frozen) 96
22:35 87 (frozen) 100
23:18 87 (frozen) 100

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_INTERVAL cadence — 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 isLifted message is the one that got stuck: lawn_mower.<mower> held state: error with metrics: {'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() hardcodes metrics=None, so a state carrying metrics can only have come from the MQTT cache.

Worth noting this fork's own additions make the diagnosis easier and are not implicated: the /realtimeDate/location channel stayed current to the second throughout (pose_time 23: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.

@@ -138,8 +138,12 @@
         except ConfigEntryAuthFailed:
             raise
 
+        now = time.monotonic()
+        mqtt_fresh = (
+            self._last_mqtt_update is not None
+            and now - self._last_mqtt_update <= MQTT_STALE_SECONDS
+        )
+
         cached_state = self.sdk.get_cached_state(self.device.id)
-        if cached_state is not None:
+        if cached_state is not None and mqtt_fresh:
             self._last_state = cached_state
             self._last_data_source = "mqtt_cache"
 
@@ -147,11 +151,7 @@
         if cached_attrs is not None:
             self._last_attributes = cached_attrs
 
-        now = time.monotonic()
-        is_mqtt_stale = (
-            self._last_mqtt_update is None
-            or now - self._last_mqtt_update > MQTT_STALE_SECONDS
-        )
+        is_mqtt_stale = not mqtt_fresh
         can_http_fetch = (
             self._last_http_fetch is None
             or now - self._last_http_fetch > HTTP_FALLBACK_MIN_INTERVAL

This is upstream segwaynavimow#71's proposed fix. Note that lowering HTTP_FALLBACK_MIN_INTERVAL on 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_INTERVAL configurable 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

  • Fork version: v1.1.0-position.5 (manifest 1.1.0+position.3), installed via HACS
  • navimow-sdk: 0.1.2
  • Home Assistant Core: 2026.7.4 (HAOS 18.1)
  • Mower: Navimow i110

… 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant