checks / unit (pull_request) Successful in 3m44s
Option defaults for the maintenance cycle were declared in three places that
could disagree, and MAINT-04 proved they already did. config.yaml shipped
`maintenance_charge_w: 2000`; main.py carried TWO `opts.get(..., 2500)`
fallbacks; MaintConfig's dataclass carried a third `2500.0`. So an options dict
with the key ABSENT was ACCEPTED by validate.py's rule 6 - which read 2000 -
and then built by main.py at 2500, against a 2000 W ceiling:
options dict with maintenance_charge_w ABSENT, max_w=2000
validate_startup_config -> [] (accepted, no problems)
MaintConfig().charge_w -> 2500.0
arbiter.resolve() -> 0.0 W, contradiction=True
That is the bug rule 6 exists to refuse, passing rule 6. It is unreachable on a
Supervisor install, which always supplies every key from config.yaml, and fully
reachable from sim/inject.py and from the test suite - which is where anyone
debugging maintenance actually works.
MAINT-04 shipped a TRIPWIRE and said so: test_validate.py pinned all three
literals and asserted the consequence through the real arbiter, so the
divergence could not silently widen or be half-closed. Its PASSING CONDITION
WAS THE BUG. Both it and validate.py's comment said to delete each other when
the divergence was closed. It is closed, so both are deleted.
THE DIVERGENCE WAS WIDER THAN ONE KEY. `maintenance_discharge_w` had the
identical three-way split (config.yaml 2000, MaintConfig 2500) and the card
named only the charge side. Closing one and leaving its twin would have
satisfied the numbers and left three sources of truth one line down, which is
the state that produced the bug. Both are closed, and so is the whole family:
all eight `maintenance_*` keys go through the same mechanism.
WHAT THE SINGLE SOURCE IS. config.yaml's `options:` block - it is what the
Supervisor reads and what the operator edits, so it wins by definition. There
is now exactly one in-code copy, `maintenance.MAINT_OPTION_DEFAULTS`, which is
TARIFF-02's `PRICE_OPTION_DEFAULTS` and SAFETY-24's `PEAK_OPTION_DEFAULTS`
pattern applied to the family that named the card - validate.py's rule 8 comment
already called that "CFG-03 read as a rule rather than as a complaint", so this
makes the rule hold where it was first broken rather than inventing a fourth
mechanism for one option family.
A MIRROR RATHER THAN A RUNTIME READ, and the reason is mechanical: the
Dockerfile copies `app/` and `run.sh` into the image and NOT config.yaml. The
running add-on cannot open the file it is configured by, so there is nothing to
read at 03:00 on a client's Pi, and a YAML-subset parser that could fail at
import is a new way for the add-on not to start. What makes the copy safe is
that test_maintenance.py pins every entry by READING the shipped file - drift is
a CI failure, not a comment.
EVERY OTHER SITE NOW DERIVES FROM IT OR HAS CEASED TO EXIST:
* MaintConfig's eight dataclass defaults are indexed out of the mirror. No
numeric literal survives in the class that held the third opinion.
* `MaintConfig.from_options()` is the ONLY place in the add-on that reads a
`maintenance_*` option. main.py's eight inline `opts.get(key, <literal>)`
calls are gone: moving the read next to the mirror means the fallback cannot
be anything but the mirror, because there is no literal in sight of it to
disagree with. It uses `key in opts` rather than truthiness, so a shipped
`false` or `0` is honoured instead of being quietly replaced by the default
- this card's own bug in a different costume.
* main.py's SECOND fallback site - the cheap-window charge floor at the
P_MAINT_SHAPE claim - reads `self.maint.cfg.charge_w` instead of re-opening
the options dict with its own literal. The Controller had already parsed
this once in __init__; asking the object that owns it cannot drift from it,
and the rate the shaping asks for can no longer be a different number from
the rate the state machine drains at on the same install.
* validate.py's rule 6 and its `maintenance_discharge_w` warning take their
fallbacks from the mirror.
WHERE THE RANGE LIVES, STATED AND CHECKED. config.yaml's `schema:` block carries
`maintenance_charge_w: int(500,5000)` - a fourth declaration site of a different
kind. It is a BOUND, not a default, so it is not this bug; but leaving the
question open is how a fifth site arrives. The decision recorded in config.yaml
and in maintenance.py is: bounds live in the schema block ONLY, because the
Supervisor is what enforces them and the Supervisor reads only that file, so a
Python copy of `int(500,5000)` would be a place to disagree and nothing else.
test_maintenance.py checks both halves of that: every mirrored key must have a
schema entry - without one the Supervisor drops the option and the add-on
silently runs on the mirror instead of the operator's value - and every shipped
default must lie INSIDE its own range, since a default the schema rejects is a
config that cannot be re-entered once it is edited. What the schema grammar
cannot express is a relation BETWEEN keys (`maintenance_charge_w` against
`max_w`), which is exactly why rule 6 is written in Python and why it does not
belong in the mirror either.
THE REPLACEMENT CHECKS, AND HOW THEY WERE VERIFIED. test_validate.py's new
section exercises the same four consumers the defect ran through, the same way:
the validator ACCEPTS a dict with every maintenance key missing; a REAL
Controller built from that same dict holds 2000 W (through main.py, not through
a regex over its source); `MaintConfig()` answers 2000 W with no dict at all;
and the real arbiter emits `-2000 W, limited by supervised(max_w)` instead of
`0 W, contradiction=True`. test_maintenance.py pins the mirror to config.yaml in
both directions - equal values, and full coverage of every `maintenance_*` key
the file ships, so a ninth key cannot arrive unmirrored - and asserts the
consumers move as a RELATION rather than against numbers, reading MaintConfig's
defaults back out of `dataclasses.fields()`.
Each new check was verified by BREAKING the thing it watches and observing red,
this project's rule:
1. config.yaml -> 1500, mirror left at 2000: test_maintenance 1 FAILED,
test_validate 4 FAILED.
2. config.yaml AND the mirror -> 1500: green again, and every consumer moved
together - MaintConfig() 1500 W, the Controller 1500 W, and the arbiter
`-1500 W, limited by maintenance(cheap-window charge)` where it had said
`-2000 W, limited by supervised(max_w)`. That is AC4: change the single
source and watch the consumers follow.
3. `charge_w: float = 2500.0` retyped into the dataclass: 2 FAILED in each of
the two files.
4. `opts.get("maintenance_charge_w", 2500)` restored in main.py: 1 FAILED,
naming the file and the literal it found.
5. `maintenance_hold_min` deleted from config.yaml's options block: 3 FAILED.
Full suite 17/17 in goodwe-addon-ci, twice, once under parallel load.
Stale prose that asserted the now-closed divergence as current is corrected in
app/watchdog.py, test_watchdog.py and test_peakguard.py - watchdog.py's example
still uses 1500 W, but because the point of the example is a charge strictly
inside the ceiling, not because 2500 is what ships.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
232 lines
10 KiB
Python
232 lines
10 KiB
Python
"""Runnable check for the maintenance state machine. `python3 test_maintenance.py`
|
|
|
|
Walks a whole cycle in simulated time, which is the test the YAML version never
|
|
had - and it is exactly the phase transitions that a real cycle takes ten hours
|
|
to exercise once a month.
|
|
|
|
⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run
|
|
`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it
|
|
stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving
|
|
edit reverted inside one second is invisible to CPython's staleness test. That
|
|
is TEST-15, and it has already turned one live mutation into a false survivor.
|
|
Read MUTATION.md before you report a survivor.
|
|
"""
|
|
|
|
import dataclasses
|
|
import re
|
|
import sys
|
|
from datetime import datetime, timedelta, timezone
|
|
from pathlib import Path
|
|
|
|
from app.maintenance import (
|
|
CHARGE, DRAIN, HOLD, IDLE, MAINT_OPTION_DEFAULTS, MaintConfig, Maintenance,
|
|
)
|
|
|
|
fails = []
|
|
|
|
|
|
def check(name, cond):
|
|
print(f" {'ok ' if cond else 'FAIL'} {name}")
|
|
if not cond:
|
|
fails.append(name)
|
|
|
|
|
|
class FakeStore:
|
|
"""Same surface as Store, no disk."""
|
|
|
|
def __init__(self):
|
|
self.data = {"phase": IDLE, "phase_started": None,
|
|
"last_completed": None, "last_start_attempt": None}
|
|
|
|
def set(self, k, v):
|
|
self.data[k] = v
|
|
|
|
def set_time(self, k, when):
|
|
self.data[k] = when
|
|
|
|
def get_time(self, k):
|
|
return self.data.get(k)
|
|
|
|
def save(self):
|
|
pass
|
|
|
|
|
|
cfg = MaintConfig(enabled=True, interval_days=28, start_hour=10,
|
|
discharge_w=2500, charge_w=2500,
|
|
soc_floor=11, soc_target=99, hold_min=120)
|
|
|
|
t0 = datetime(2026, 8, 23, 10, 0, tzinfo=timezone.utc)
|
|
|
|
print("scheduling")
|
|
m = Maintenance(cfg, FakeStore())
|
|
check("never run before is due", m.due(t0))
|
|
r = m.tick(t0, soc=80)
|
|
check("starts at the start hour when due", r.phase == DRAIN and r.owns_setpoint)
|
|
check("drain commands the discharge rate", r.setpoint_w == 2500)
|
|
|
|
m2 = Maintenance(cfg, FakeStore())
|
|
r = m2.tick(t0.replace(hour=11), soc=80)
|
|
check("does not start outside the start hour", r.phase == IDLE)
|
|
|
|
m3 = Maintenance(cfg, FakeStore())
|
|
m3.store.set_time("last_completed", t0 - timedelta(days=3))
|
|
check("not due three days after a cycle", not m3.due(t0))
|
|
check("due 29 days after a cycle",
|
|
Maintenance(cfg, FakeStore()).due(t0) and
|
|
(t0 - (t0 - timedelta(days=29))) >= timedelta(days=28))
|
|
|
|
print("ownership per phase")
|
|
m = Maintenance(cfg, FakeStore())
|
|
m.force_start(t0)
|
|
r = m.tick(t0, soc=80)
|
|
check("drain: maintenance owns the setpoint", r.owns_setpoint and not r.charge_only)
|
|
r = m.tick(t0 + timedelta(hours=3), soc=10.5)
|
|
check("drain exits at the floor", r.phase == CHARGE)
|
|
check("charge: the LOOP owns the setpoint", not r.owns_setpoint and r.charge_only)
|
|
r = m.tick(t0 + timedelta(hours=20), soc=99)
|
|
check("charge exits at the target", r.phase == HOLD)
|
|
check("hold: maintenance owns and commands zero",
|
|
r.owns_setpoint and r.setpoint_w == 0.0)
|
|
r = m.tick(t0 + timedelta(hours=21), soc=100)
|
|
check("hold keeps holding before hold_min", r.phase == HOLD)
|
|
r = m.tick(t0 + timedelta(hours=22, minutes=5), soc=100)
|
|
check("hold completes after hold_min", r.phase == IDLE)
|
|
check("completion is recorded", m.store.get_time("last_completed") is not None)
|
|
check("no longer due right after completing", not m.due(t0 + timedelta(hours=22)))
|
|
|
|
print("failure handling")
|
|
m = Maintenance(cfg, FakeStore())
|
|
m.force_start(t0)
|
|
r = m.tick(t0 + timedelta(hours=13), soc=60)
|
|
check("drain aborts on timeout", r.phase == IDLE)
|
|
check("abort commands zero", r.setpoint_w == 0.0)
|
|
|
|
m = Maintenance(cfg, FakeStore())
|
|
m.force_start(t0)
|
|
r = m.tick(t0 + timedelta(minutes=5), soc=None)
|
|
check("no SoC: holds the phase rather than guessing", r.phase == DRAIN)
|
|
check("no SoC: still commands the drain rate", r.setpoint_w == 2500)
|
|
|
|
m = Maintenance(cfg, FakeStore())
|
|
m.store.set("phase", "banana")
|
|
r = m.tick(t0, soc=50)
|
|
check("unknown phase recovers to idle at 0 W", r.phase == IDLE and r.setpoint_w == 0.0)
|
|
|
|
print("disabled schedule")
|
|
m = Maintenance(MaintConfig(enabled=False), FakeStore())
|
|
check("disabled never starts", m.tick(t0, soc=80).phase == IDLE)
|
|
|
|
print("CFG-03: config.yaml is the source, this module is the only mirror")
|
|
# ⚠️ WHY THIS IS PINNED BY READING THE FILE. Before CFG-03 each maintenance
|
|
# default was written down three times - config.yaml's `options:` block,
|
|
# `opts.get(key, <literal>)` in main.py, and MaintConfig's dataclass defaults -
|
|
# and MAINT-04 proved they had already drifted (2000 shipped, 2500 in code). The
|
|
# fix is one source and one mirror; the mirror is only worth anything if drift
|
|
# from it is a CI failure rather than a comment, and a transcription of
|
|
# config.yaml into this file would just be a fourth place to disagree. So the
|
|
# shipped values are READ out of the file the Supervisor reads.
|
|
#
|
|
# Regex rather than PyYAML on purpose, for test_validate.py's reason: the add-on
|
|
# image installs py3-aiohttp and py3-paho-mqtt only, and a check that skips on
|
|
# ImportError is not a check.
|
|
HERE = Path(__file__).resolve().parent
|
|
_CFG_TEXT = (HERE / "config.yaml").read_text(encoding="utf-8")
|
|
_SCHEMA_AT = _CFG_TEXT.index("\nschema:")
|
|
|
|
|
|
def shipped_default(key):
|
|
"""The value under `options:` - the FIRST match, before `schema:`."""
|
|
m = re.search(rf"^ {key}: *(.+?) *$", _CFG_TEXT[:_SCHEMA_AT], re.M)
|
|
return None if m is None else m.group(1)
|
|
|
|
|
|
def schema_range(key):
|
|
"""`int(lo,hi)` under `schema:` -> (lo, hi); None if the key is absent."""
|
|
m = re.search(rf"^ {key}: *\w+\((-?[\d.]+),(-?[\d.]+)\) *$",
|
|
_CFG_TEXT[_SCHEMA_AT:], re.M)
|
|
return None if m is None else (float(m.group(1)), float(m.group(2)))
|
|
|
|
|
|
def as_number(text):
|
|
if text in ("true", "false"):
|
|
return 1.0 if text == "true" else 0.0
|
|
return float(text)
|
|
|
|
|
|
_ship = {k: shipped_default(k) for k in MAINT_OPTION_DEFAULTS}
|
|
print(f" config.yaml ships {_ship}")
|
|
check("every mirrored key is actually shipped in config.yaml's options: block",
|
|
all(v is not None for v in _ship.values()))
|
|
check("...and MAINT_OPTION_DEFAULTS reproduces each one exactly, so the mirror "
|
|
"is a copy of the source and not an opinion about it",
|
|
all(str(MAINT_OPTION_DEFAULTS[k]).lower() == _ship[k]
|
|
for k in MAINT_OPTION_DEFAULTS))
|
|
# ⚠️ THE OTHER DIRECTION MATTERS TOO. Equality alone would still pass if the
|
|
# mirror quietly stopped covering a key someone added to config.yaml, and the
|
|
# key nobody mirrors is exactly the key that gets a literal typed for it.
|
|
_shipped_keys = set(re.findall(r"^ (maintenance_[a-z_]+):", _CFG_TEXT[:_SCHEMA_AT],
|
|
re.M))
|
|
check(f"...and it covers EVERY maintenance_* option the file ships, so a ninth "
|
|
f"key cannot arrive unmirrored {sorted(_shipped_keys)}",
|
|
_shipped_keys == set(MAINT_OPTION_DEFAULTS))
|
|
|
|
# ⚠️ WHERE THE RANGE LIVES, ASSERTED RATHER THAN COMMENTED. The PM proofread on
|
|
# this card named a FOURTH declaration site of a different kind:
|
|
# `maintenance_charge_w: int(500,5000)` in the schema block. It is a BOUND, not
|
|
# a default, and the decision this card records is that bounds live in the
|
|
# schema block ONLY - the Supervisor enforces them and reads only that file, so
|
|
# a Python copy would be a place to disagree and nothing else. What that
|
|
# decision needs from a check is (a) every mirrored key really does have a
|
|
# schema entry, or the Supervisor drops the option and the add-on silently runs
|
|
# on the mirror instead of the operator's value, and (b) the shipped default
|
|
# lies INSIDE its own range, because a default the schema rejects is a config
|
|
# that cannot be re-entered once it is edited.
|
|
_ranged = {k: schema_range(k) for k in MAINT_OPTION_DEFAULTS}
|
|
check("every mirrored key is in the schema: block too - `bool` has no range, so "
|
|
"it is the one exception and it is named",
|
|
all(v is not None for k, v in _ranged.items()
|
|
if k != "maintenance_enabled")
|
|
and _ranged["maintenance_enabled"] is None)
|
|
_outside = {k: (as_number(_ship[k]), rng) for k, rng in _ranged.items()
|
|
if rng is not None and not rng[0] <= as_number(_ship[k]) <= rng[1]}
|
|
check(f"...and every shipped default sits inside its own schema range, so the "
|
|
f"Supervisor accepts the file it ships with {_outside or 'all inside'}",
|
|
not _outside)
|
|
|
|
# ⚠️ AND THE CONSUMERS MOVE WITH IT - checked as a RELATION, not against
|
|
# numbers. AC4 asks for the replacement to be broken and every consumer observed
|
|
# moving together. A number typed here would pass whether or not the derivation
|
|
# survived; `dataclasses.fields()` reads what the class actually defaulted to,
|
|
# so this can only pass while the class is still indexing the mirror.
|
|
_fields = {f.name: f.default for f in dataclasses.fields(MaintConfig)}
|
|
check(f"MaintConfig's field names are the option names minus the prefix, so a "
|
|
f"new field cannot miss the mirror by being spelt differently {sorted(_fields)}",
|
|
{f"maintenance_{n}" for n in _fields} == set(MAINT_OPTION_DEFAULTS))
|
|
check("...and every dataclass default IS the mirror's value - no literal "
|
|
"survives in the class that used to hold the third opinion",
|
|
all(float(d) == float(MAINT_OPTION_DEFAULTS[f"maintenance_{n}"])
|
|
for n, d in _fields.items()))
|
|
# from_options() is the only reader, so it is the only thing that can put a
|
|
# different number in front of main.py.
|
|
_from_empty = MaintConfig.from_options({})
|
|
check("...and from_options({}) - the sim/inject.py and test-suite path, the one "
|
|
"the Supervisor never takes and the only one the divergence was reachable "
|
|
"from - equals MaintConfig() exactly",
|
|
_from_empty == MaintConfig())
|
|
check("...while a supplied value still wins, or the mirror would be a ceiling "
|
|
"rather than a default",
|
|
MaintConfig.from_options({"maintenance_charge_w": 1234}).charge_w == 1234.0)
|
|
# ⚠️ `key in opts`, not truthiness: `false` and `0` are values an operator can
|
|
# set, and a truthy read would silently replace both with the default - this
|
|
# card's own bug in a different costume.
|
|
check("...and an explicit falsey value is honoured rather than defaulted away",
|
|
MaintConfig.from_options({"maintenance_start_hour": 0}).start_hour == 0
|
|
and MaintConfig.from_options({"maintenance_enabled": False}).enabled is False)
|
|
|
|
print()
|
|
print()
|
|
if fails:
|
|
print(f"{len(fails)} FAILED: {', '.join(fails)}")
|
|
sys.exit(1)
|
|
print("all checks passed")
|