fix(gecloud): keep live data flowing during slow settings refreshes; stale reads no longer overwrite newer writes

[mgazza]

Problem

1. A slow settings refresh holds up live data. GECloudDirect.run() waits for the whole periodic settings refresh before it returns. ComponentBase.start() calls run() one cycle at a time, so a slow refresh delays the next status and meter update.

Settings are read one register at a time. A register the inverter lists but cannot serve uses up its whole retry budget on every refresh. One example is AC charge slots 8/9, setting ids 120–125, on a three-phase GivEnergy inverter. In production we saw a 656 s gap between telemetry updates while those ids failed repeatedly.

2. A stale settings read can overwrite a newer confirmed write. Suppose a bulk settings read is in flight and switch_event writes the same register. The read then finishes with the older value, and that older value replaces the confirmed write in self.settings, even though pending_writes still holds the write. This was reproduced on current main using the real async_get_inverter_settings, async_read_inverter_setting, async_write_inverter_setting and switch_event.

There is a second form of the same race. A write can still be in flight when the refresh stores its results. The event handler then updates the register object that the refresh has just replaced, so the cache and the published entity keep the old value.

Fix

Isolation

• The periodic refresh (seconds % 600) now runs as its own task. run() waits up to SETTINGS_REFRESH_WAIT_SECONDS (20 s) for it, then carries on. A fast refresh therefore behaves exactly as before.
• No overlap. While a refresh is still running, a newly due one is skipped and logged. The devices that were due are remembered and read by the next refresh, so the hourly re-read of EMS member inverters (GE Cloud EMS: battery inverter settings are never re-read after startup, so a changed AC3 slot silently stops evening discharge #5103) is not lost.
• Startup and EMS behaviour are unchanged. At startup and on a device-list change, settings are still read inline, because automatic config is built from them. The device selection is the same as before. Any background refresh is cancelled and awaited first.
• Defaults run on fresh settings. The default options pass and the AC charge gate re-check (GH#5269) always run on fresh settings. If the refresh outlasts the wait, the refresh runs them itself when it completes, instead of the cycle using the old snapshot.
• Shutdown. final() cancels the task and waits for it. A cancelled settings read cancels and waits for its own ReadSetting-* tasks.
• Errors are still counted. A background refresh that raises logs a traceback and fails the next run(). It is counted as an error, as it was when the refresh ran inline.
• Departed devices. A device removed while its settings are being read is not put back into settings, the published entities, register_entity_map, register_list or the backoff state.

Write vs read

• Each confirmed write gets a sequence number.
  • When the refresh stores its results, a write wins only if it was confirmed after that register's read was issued.
  • A read issued after the write keeps the inverter's own value, so a later change in the GivEnergy app is not masked.
  • pending_writes is still cleared every cycle, as before.
• async_write_inverter_setting also updates the register currently held in self.settings. This covers a refresh that replaced the register while the write was in flight.
• If a setting is written twice in one cycle, the pending-write read now returns the newest value.

Honest readbacks and backoff

• A failed read keeps the last known value, marked read_failed and published with a read_failed attribute. Each refresh logs which ids failed.
• On periodic refreshes, a register that has failed 3 refreshes in a row is backed off. It is not read for 15, then 30, then at most 60 minutes, and then tried again. The backoff is time-based, so no register goes unread for more than an hour whatever the refresh cadence.
  • A failure only counts when the device answered other reads in the same refresh. A refresh where nothing answers (an inverter or GE Cloud outage) clears the backoff, so every register is read again as soon as the device is back.
  • A single good read clears the backoff.
  • Startup and new-device reads always read every register.
  • Extra settings reads are not turned off, because they are how external changes and control state are seen.

Tests

unit_test.py -t ge_cloud: 144 passed (138 existing and 6 new). I ran the first five new tests against main's gecloud.py and all five fail there. The sixth tests behaviour added in the review round. I also ran mutation checks: removing each mechanism (the write-wins merge and its issue-sequence check, the outage gate, the owed devices, the deferred defaults, error reporting, the write-path cache update and the shutdown cancel) fails at least one test.

Test	What it checks	On main
settings_refresh_live_data	Drives the real ComponentBase.start() serial loop. While a refresh is stuck on a read, live status keeps being fetched and no overlapping refresh starts. The held refresh then runs to completion and stores the new value it read, while a write made during it still wins.	The loop stalls at 600 s
settings_refresh_cancelled_on_stop	When the component stops, the held refresh and its read are cancelled and awaited, and no tasks are left.	The loop stalls
settings_read_vs_write	Write lands during the read; write lands after the merge; double-write readback; an earlier write does not mask a later external change.	All three fail
settings_read_backoff	The backoff pattern, read_failed kept and published, recovery, no backoff on full reads, and no backoff from a device-wide outage.	No backoff support
settings_refresh_follow_ups	Owed EMS re-read after a skipped slot, default options deferred to fresh settings, background failure reported.	(new behaviour)
settings_refresh_departed_device	A departed device is not brought back by a held refresh. A device-list change cancels the held refresh and reads settings inline.	run(600) blocks

I also ran the rest of the --quick suite and it passes. The exception is two Prometheus metrics tests (data_age_metrics, control_conflicts_metrics), which also fail on unmodified main in my environment because prometheus_client is not installed.

Reproduction proofs (outside the repo, synthetic IO, no real inverter)

• Poll isolation: real GECloudDirect with run() callbacks awaited one after another, and the read of id 120 held from 600 s.
  • main: callbacks completed [0], status publishes 2, held read not cancelled on stop. FAIL
  • this branch: callbacks [0, 600, 720, 840, 1200], 5 status publishes, one held read, read cancelled on stop. PASS
• Write/read race: the 4 production methods compiled unchanged from the AST, with a bulk read held across switch_event.
  • main: cache value after the older read returns is False. FAIL
  • this branch: True, the pending write is kept. PASS

🤖 Generated with Claude Code

https://claude.ai/code/session_018BaJjLpuMEtQYfvmTRcFJf

[springfall2008]

Thanks, this is a careful piece of work. I'd merge it as is. The three points below are small and could go in this PR or a follow-up.

I ran unit_test.py --test ge_cloud on fb81519b: 144 passed.

What I checked

• Shutdown. ComponentBase.start() awaits final() on stop. GECloudDirect didn't define final() before, and CarChargerControl doesn't define one either, so the new one doesn't hide any existing cleanup.
• Write callers. All six callers of async_write_inverter_setting set setting["value"] themselves after the write: switch_event, number_event, select_event, enable_default_options, ensure_ac_charge_gate and test_gecloud_direct. Updating the cache inside the write doesn't conflict with any of them. It only makes the callers' own update redundant, and it fixes the case where the refresh replaced the register mid-write.
• Write-wins merge. issue_seq is taken when each read is actually issued. Skipped registers fall back to refresh_seq. An older write left in recent_writes can't beat a later refresh. There is no await between the merge and self.settings[device] = results, so a write can't land in that gap.
• No overlap. The task counts as running until apply_settings_defaults finishes, so the defaults pass can't overlap the next refresh. Cancellation also waits for the ReadSetting-* tasks.
• Test isolation. The new tests build their own MockGECloudDirect and leave the shared fixture alone, so they can't cause order-dependent failures in later tests.

Nits

1. A backed-off register is logged as a failed read on every refresh. A skipped register has no value, so it is marked read_failed=True (L2859). refresh_settings then logs Warn: ... could not read setting ids [...] (L2608) every 10 minutes while the backoff lasts.

The read_failed attribute is fine. The log should still separate skipped (backed off) from read and failed, because someone triaging a log will otherwise conclude the device is still timing out on those ids.

2. device_answered also counts readbacks that never reach the device. device_answered (L2831) counts any result that has a value. async_read_inverter_setting answers a register with a pending write from pending_writes without touching the device (L2675, context: "predbat").

So one confirmed write in the same cycle can make a real outage look as if the device answered. The failing registers then count towards backoff instead of the backoff being cleared. This is unlikely, because a write normally fails during an outage too. Excluding context == "predbat" results would make the check exact.

3. Cancelling during the defaults pass can drop the 24-hour pass for some devices. apply_settings_defaults sets default_options_stamp (L2548) before it loops over the devices (L2555).

A device-list change can cancel a detached refresh while it is part way through that loop. Every device the loop hasn't reached then misses its default options until the next 24-hour pass. This couldn't happen when the refresh ran inline. Setting the stamp after the loop, or tracking which devices have been done, would close it.

Observations (no change needed)

• Pending-write readback window. The pending_writes readback now protects a write only until the end of the current cycle, about 20 seconds later. Before, it covered the whole inline refresh. Reads issued after that go to the inverter. The PR description says this is intended ("a read issued after the write keeps the inverter's own value"). It is fine provided GE Cloud's setting read is live rather than eventually consistent.
• Startup reads ignore the backoff. The inline startup and device-change reads run with backoff=False, so a register the inverter can't serve still slows those paths down. The PR documents this, and it is reasonable because automatic config needs the full snapshot.