Files
goodwe-addon/goodwe_controller/app/store.py
T
glenn schrooyenandClaude Opus 5 cc4e88490b SAFETY-02 review: a latch that a restart clears is not a latch
BLOCKER. `auto` was persisted and `sign_wd.tripped` was not, so reconstructing
the Controller against the same store - which is what a container restart does -
brought it back ARMED, into the same fault, unattended: tripped=False, banner
back to NOT READY, and it resumed commanding -1000 then -2000 W before
re-tripping 180 s later. "Restart the add-on" is the first thing an operator
looking at a FAILSAFE banner and a 0 W inverter will try, and under a supervisor
that restarts a crashed add-on it is a loop of three-minute wrong-direction
bursts. DOCS.md's "it does not clear itself" was false across a restart.

`sign_tripped` and `sign_reason` are Store keys now, beside the maintenance
state and for the reason store.py's own docstring gives. Written the moment the
trip happens - reason first, flag second, so a crash between the two can never
persist a latch with no explanation - restored in Controller.__init__, cleared
only by the operator's start. The regression is covered: reverting the restore
fails seven checks in test_watchdog.py.

Review follow-ups, no behaviour change:

- The arming gate is tested against the REAL arbiter. It is the decision the
  ticket turns on and it was asserted in three comment blocks and verified by
  nothing - every case passed `following` in as a literal. Twelve claim
  configurations now derive it from resolve(), including the redirect case
  `winner == "loop"` alone would have got wrong and the `lo` limit that raises
  magnitude without flipping sign.
- The MQTT `status: "failsafe"` value is tested. The check that claimed to cover
  it restated the line above it and never touched the payload.
- The worked example described a config that does not ship: at the defaults
  maintenance_charge_w 2500 against max_w 2000 is an arbiter contradiction and
  0 W, not a -2500 W charge. Rewritten at 1500 W. The contradiction is a real
  pre-existing bug - a default cheap-window maintenance charge silently never
  happens - filed separately and deliberately not fixed here.
- Forward hazard recorded at the gate: `winner == "loop"` disarms the watchdog
  under any higher-priority SET, and a peak-shaving strategy at P_PEAK IS
  grid-following in sign, so it would blind this for every peak event.
- disagrees() is public so the replay harness stops carrying its own copy of the
  predicate it grades.
- held_s takes the clock instead of reading one.
- The two limits are now each stated in both places rather than one each.
- The no-false-positive argument is tied to control.py's freeze permitting the
  command to cross zero; under the shrink-only encoding control.py records as
  rejected, this would false-trip on a load step.
- Measured figures corrected: 93.4 % / 4212 of 17280, per-cluster
  0.3/0.9/10.5/0.0, and the near-circularity of those figures is now stated
  rather than leaned on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
2026-08-28 22:33:15 +02:00

108 lines
4.6 KiB
Python

"""Persistent state, in /data (the add-on's only durable volume).
The maintenance cycle is month-scale state: it MUST survive add-on restarts,
HA restarts and power cuts, or the battery quietly stops being maintained and
nothing says so.
Times are stored as ISO-8601 with an explicit UTC offset and parsed back to
aware datetimes. That is not fussiness: the YAML implementation this replaces
was disabled for hours by exactly one naive-vs-aware subtraction, and it failed
silently - the automation never even recorded itself as triggered.
"""
import json
import logging
import os
import tempfile
from datetime import datetime, timezone
_LOG = logging.getLogger("goodwe.store")
# ⚠️ "auto" is deliberately NOT in here. Controller falls back to the add-on's
# `auto_start` option only when the key is absent - if a default supplied False,
# the fallback could never fire and auto_start would silently do nothing on
# every fresh install.
#
# ⚠️ `sign_tripped` IS in here, and the difference from `auto` is the whole
# reason both decisions are written down. `auto` has an option to fall back to
# (`auto_start`), so a stored default would mask a fresh install. A latched
# failsafe has no such fallback: absent means "never tripped", which is exactly
# what a default of False says.
#
# It is persisted for the reason this module's docstring gives for maintenance
# state, applied to something with a much shorter fuse. SAFETY-02's latch is the
# only thing standing between a controller that has been caught driving the wrong
# way and its own restart, and "restart the add-on" is the FIRST thing an operator
# looking at a FAILSAFE banner will try. Held only in memory it came back armed,
# into the fault, unattended - measured: after reconstructing Controller against
# the same store, `tripped` was False, the banner read NOT READY, and it resumed
# commanding -1000, -2000 W before re-tripping 180 s later. Under a supervisor
# that restarts a crashed add-on that is a loop, three minutes of wrong-direction
# power at a time, forever.
DEFAULTS = {
"phase": "idle",
"phase_started": None,
"last_completed": None,
"last_start_attempt": None,
"sign_tripped": False,
"sign_reason": "",
}
class Store:
def __init__(self, path: str = "/data/state.json"):
self.path = path
self.data = dict(DEFAULTS)
self.load()
def load(self) -> None:
try:
with open(self.path, "r", encoding="utf-8") as fh:
loaded = json.load(fh)
if isinstance(loaded, dict):
self.data = {**DEFAULTS, **loaded}
_LOG.info("state restored: phase=%s last_completed=%s "
"sign_tripped=%s",
self.data.get("phase"), self.data.get("last_completed"),
self.data.get("sign_tripped"))
except FileNotFoundError:
_LOG.info("no saved state, starting fresh")
except (json.JSONDecodeError, OSError) as err:
# A corrupt state file must not stop the controller: losing the
# maintenance history is recoverable, refusing to run is not.
_LOG.warning("state unreadable (%s) - starting fresh", err)
def save(self) -> None:
# Atomic replace: a power cut mid-write must not leave a truncated file
# that reads as "no maintenance ever ran".
try:
os.makedirs(os.path.dirname(self.path), exist_ok=True)
fd, tmp = tempfile.mkstemp(dir=os.path.dirname(self.path))
with os.fdopen(fd, "w", encoding="utf-8") as fh:
json.dump(self.data, fh, indent=1)
os.replace(tmp, self.path)
except OSError as err:
_LOG.error("could not persist state: %s", err)
# -- typed helpers ------------------------------------------------------
def get_time(self, key: str):
raw = self.data.get(key)
if not raw:
return None
try:
parsed = datetime.fromisoformat(raw)
except (TypeError, ValueError):
_LOG.warning("unparseable timestamp for %s: %r", key, raw)
return None
# Anything that ever escaped without an offset is treated as UTC rather
# than raising later in an arithmetic expression.
return parsed if parsed.tzinfo else parsed.replace(tzinfo=timezone.utc)
def set_time(self, key: str, when: datetime | None) -> None:
self.data[key] = when.astimezone(timezone.utc).isoformat() if when else None
self.save()
def set(self, key: str, value) -> None:
self.data[key] = value
self.save()