fix(gecloud): re-read EMS battery inverter settings and warn on a slot 1 override

[springfall2008]

This is an automated draft PR generated from issue #5103 — a maintainer should review it before merging.

Fixes #5103

Summary

Under GE Cloud EMS auto-config, finding an EMS sets polling_mode = False, and the 10-minute settings refresh then gated each battery inverter's /settings re-read on seconds == 0. The AC3s' registers were therefore snapshotted exactly once per process start and published stale thereafter, so a DC discharge slot 1 changed behind Predbat's back overrode the EMS and stopped the plant discharging every evening with nothing in the log or the status.

This implements the reporter's options (1) and (2), which the triage comment picked out as the cheapest useful pair:

• Re-read on a slow cadence. Battery inverters under EMS control are now re-read hourly (SETTINGS_SLOW_REFRESH_SECONDS) rather than once at startup, so the published entities and the control ledger track reality between restarts. That is 1 extra /settings read per inverter per hour — deliberately slow, given the EMS API volume concerns in Excessive GE EMS writes? #4232. Nothing changes for non-EMS plants: polling_mode short-circuits the gate there exactly as before.
• Warn on a slot 1 override. After each refresh, any battery inverter whose own charge or DC discharge slot 1 is not 00:00-23:59 — the window Predbat - Givenergy EMS - GivTCP #3781 asks for precisely so the inverter never overrides the EMS — is reported to the log and to the Predbat status, naming the inverter and the offending value. Raised once per episode (following the auth-denied precedent a few lines up) so a standing misconfiguration does not inflate error_count every hour, and re-armed once a clean read comes back.

Option (3), writing slot 1 back to 00:00-23:59, is deliberately not implemented — see Notes.

One extra fix on the same line: the refresh gate now keys off first rather than seconds == 0. first stays True across startup backoff retries while seconds keeps climbing, so on a plant whose first successful run() landed on a retry the battery inverters were previously never read at all, not even once.

Testing

coverage/run_pre_commit (pre-commit hooks + ./run_all --quick): all hooks pass, all tests pass. Spelling verified separately with npx cspell against the en-gb dictionary (the local hook is a no-op without it) — 0 issues.

./run_all --test ge_cloud: 96 passed, 0 failed, including the two new sub-tests ems_slot_overrides and ems_settings_reread.

The new test fails without the fix, in two stages:

• Stashing the whole source change fails the module at import (ImportError: cannot import name 'SETTINGS_SLOW_REFRESH_SECONDS'), which proves the red run but says nothing about the assertions.
• So I also ran a targeted red: with the helpers left importable, I reverted only the run() gate back to seconds == 0 and dropped the check call. Result: 95 passed, 1 failed, failing on exactly the reported defect — ERROR: at the slow refresh cadence the battery inverter should be re-read, got ['ems001']. Restoring the gate returns it to 96/96.

What the tests cover: the hourly-vs-10-minute cadence under EMS; the 19:00 end time from the field report being detected, logged and raised as a status; a zeroed 00:00-00:00 slot 1 also counting as an override (Predbat enables the per-inverter discharge switch under EMS, so that window bars discharge entirely); HH:MM:SS and unpadded times normalising rather than false-positiving; null/unknown/non-time register values being ignored; the once-per-episode status and its re-arming after a clean read; and that neither the EMS device itself nor a non-EMS plant is ever subject to the check.

Notes

• Option (3) not implemented. Writing slot 1 back to 00:00-23:59 adds writes and, as the issue says, could fight a third-party controller that changes those slots on purpose. enable_default_options() is where it would naturally live (it already zeroes slots 2–10 every 24h and deliberately leaves slot 1 alone), gated behind a config switch. Left for a maintainer decision rather than taken unilaterally.
• Duplicated-ish logic, deliberately not shared. enable_default_options() scans the same time registers for slots 2–10, but it writes and compares raw values, while this path reports and normalises first. Folding the new normalise_register_time() into that comparison would change write behaviour on values like 0:00, which is outside what this ticket asks for, so the two are left separate.
• Status message length. record_status() clamps the sensor state to 255 characters. One inverter with one bad slot is well under; an inverter with all four slot 1 registers wrong would be clamped in the sensor state, with the full text still in the log. That is the documented behaviour of that clamp, not new here.
• Blast radius established by search, not GitNexus. The GitNexus MCP tools were not available in this session, so impact() could not be run. By search: the only modified existing symbol is GECloudDirect.run, whose sole production caller is ComponentBase.start (component_base.py:264) and whose signature is unchanged — only the internal refresh cadence moved. Every new symbol (SETTINGS_SLOW_REFRESH_SECONDS, EMS_SLOT_FULL_DAY_START/END, EMS_SLOT_FAMILIES, normalise_register_time, find_ems_slot_overrides, check_ems_inverter_slots, ems_slot_warned) appears only in gecloud.py and test_ge_cloud.py, with no name collisions elsewhere in the tree.
• tools/debug-journal.md's GE Cloud row informed the approach — in particular that the settings list is fetched once per process start and that the mock in test_ge_cloud.py does not mirror the real initialize() unless kept in step, which is why ems_slot_warned was added to MockGECloudDirect alongside the real one.
• The issue's separate observation that forced export also failed on this plant before the slot changed is not addressed here — as the reporter says, this defect does not explain that part.

🤖 Generated with Claude Code