MAINT-04: refuse a maintenance charge the ceiling cannot deliver #10

Merged
admin merged 2 commits from MAINT-04 into release/1.0 2026-08-30 03:01:15 +02:00
4 changed files with 109 additions and 5 deletions
Showing only changes of commit ffb8ec6373 - Show all commits
+31
View File
@@ -43,6 +43,37 @@ faster maintenance cycle — raise it first, then these. `test_validate.py` now
reads `config.yaml` itself and asserts the shipped numbers pass the rule, so a
default that the validator refuses cannot ship again.
### ⚠️ Upgrading an existing default install: the add-on may refuse to start
An install still carrying `maintenance_charge_w: 2500` against `max_w: 2000`
now meets rule 6 and **exits 78 with the refusal banner** instead of starting.
That is the intended behaviour — the configuration was already broken, it was
just silent about it — but an add-on that will not come back after an update
needs the fix stated here rather than only in the log.
**The fix, either way: set `maintenance_charge_w` to `2000` or lower, OR raise
`max_w` to `2500` or higher** (Settings → Add-ons → GoodWe RS485 Controller →
Configuration), then start it. The banner in the log says the same thing and
names both numbers.
Whether you hit this at all depends on whether the Supervisor kept the old
value, and **both cases are covered above because we did not verify which one
applies here.** The Supervisor merges an add-on's *stored* options over the
defaults in `config.yaml`, so:
- **A key the operator never edited** re-derives from the new `config.yaml` and
becomes 2000 on upgrade. Non-event, nothing to do.
- **A key the operator did set** persists at 2500 and the add-on refuses to
start until it is changed. Note this includes anyone who used the
Configuration page's YAML editor, which writes the whole options block back —
so "I never touched that field" is not a guarantee the old value is gone.
⚠️ **This is reasoned from the Supervisor's documented merge behaviour, not
measured on the live install.** The reference install's stored options could not
be read (`/api/hassio/` refuses a long-lived token) and probing it was out of
scope — it regulates a real battery. It runs `max_w: 5000` and is unaffected
either way.
⚠️ **Known, deliberately left, and worth a follow-up:** `main.py` and
`maintenance.py` still fall back to **2500** when the key is absent from the
options dict, while `config.yaml` and `validate.py` now say 2000. Unreachable on
+4 -3
View File
@@ -28,9 +28,10 @@ _LOG = logging.getLogger("goodwe.maint")
IDLE, DRAIN, CHARGE, HOLD = "idle", "drain", "charge", "hold"
# A drain of a 13.5 kWh usable pack at 2500 W is ~5 h even with no house load
# helping. 12 h means the battery is not following commands - a fault, not a
# slow day. The charge phase deliberately waits for sun and for cheap hours, so
# A drain of a 13.8 kWh usable pack (300 Ah x 51.2 V, DoD 90) at the shipped
# 2000 W is ~6.9 h even with no house load helping - and less in practice, since
# house load drains alongside the export and soc_floor stops short of empty.
# 12 h means the battery is not following commands - a fault, not a slow day. The charge phase deliberately waits for sun and for cheap hours, so
# it is expected to span a night; 36 h still bounds it.
DRAIN_TIMEOUT_MIN = 720
CHARGE_TIMEOUT_MIN = 2160
+6 -2
View File
@@ -327,8 +327,12 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str]
# rule at 2000 and then built by main.py at 2500 - the contradiction this
# rule exists to catch, in the one case it cannot see. It is unreachable on
# a Supervisor install and those files are another ticket's right now.
# THE REAL FIX is one source of truth for option defaults; until then, if
# config.yaml's value moves, move this and main.py's together.
# THE REAL FIX is one source of truth for option defaults; until then the
# divergence is PINNED BY A CHECK rather than by this comment - see
# test_validate.py, "the KNOWN cross-file default divergence". It asserts
# the mismatch as it stands and goes red if EITHER side moves, including
# when somebody closes it in main.py, at which point this block and that
# check both get deleted.
charge_w = _number(opts, "maintenance_charge_w", 2000.0)
if charge_w is None:
problems.append(
+68
View File
@@ -253,6 +253,74 @@ check("...and the shipped charge rate is at or under the shipped ceiling",
check("...and the shipped discharge rate too, so it warns about nothing",
_shipped["maintenance_discharge_w"] <= _shipped["max_w"])
# --------------------------------------------------------------------------- #
print("the KNOWN cross-file default divergence, pinned rather than commented")
# ⚠️ WHAT THIS GUARDS, AND WHY IT ASSERTS THE BUG RATHER THAN THE FIX.
# config.yaml and validate.py now default maintenance_charge_w to 2000, while
# main.py's two `opts.get(..., 2500)` fallbacks and MaintConfig's dataclass
# default still say 2500. So an options dict with the key ABSENT is ACCEPTED by
# rule 6 at 2000 and then built by main.py at 2500 - which is a contradiction:
#
# validate_startup_config -> [] (accepted)
# MaintConfig().charge_w -> 2500.0
# arbiter -> 0.0 W, contradiction=True
#
# That is the bug rule 6 exists to catch, passing rule 6. It is unreachable on a
# Supervisor install (every key is always supplied from config.yaml) but fully
# reachable from sim/inject.py and from tests - which is exactly where anyone
# debugging maintenance works.
#
# The fix is ONE SOURCE OF TRUTH for option defaults. That is a bigger change
# than this ticket and it lands in main.py, which another ticket is live in - so
# the divergence is RECORDED here instead of closed. The check therefore passes
# on the mismatch and FAILS when either side moves, in EITHER direction:
#
# - config.yaml moves -> FAIL: re-decide what the fallback should be
# - main.py is fixed to match -> FAIL: the divergence is gone, DELETE this
# check and the ⚠️ block in validate.py
#
# A comment cannot do that, and "a comment where a check belonged" is this
# repo's most repeated defect.
_APP = os.path.join(os.path.dirname(os.path.abspath(__file__)), "app")
def _read(*parts):
with open(os.path.join(*parts), encoding="utf-8") as fh:
return fh.read()
# Every literal fallback for this key in main.py, whatever the call shape.
_main_falls = [float(m) for m in re.findall(
r'\.get\(\s*"maintenance_charge_w"\s*,\s*(-?\d+(?:\.\d+)?)\s*\)',
_read(_APP, "main.py"))]
# ...and MaintConfig's own dataclass default, which is the one that bites when
# main.py is bypassed entirely (a Maintenance built straight from defaults).
_dc_falls = [float(m) for m in re.findall(
r"^\s*charge_w:\s*float\s*=\s*(-?\d+(?:\.\d+)?)\s*$",
_read(_APP, "maintenance.py"), re.M)]
check(f"main.py's fallbacks are still found and still 2500 {_main_falls}",
len(_main_falls) == 2 and set(_main_falls) == {2500.0})
check(f"MaintConfig's default is still found and still 2500 {_dc_falls}",
len(_dc_falls) == 1 and set(_dc_falls) == {2500.0})
check(f"...and config.yaml still ships 2000, so the divergence is UNCHANGED "
f"(shipped {_shipped['maintenance_charge_w']:.0f}, code 2500)",
_shipped["maintenance_charge_w"] == 2000.0)
# ⚠️ And the consequence itself, through the REAL code rather than through the
# numbers above - so that if someone "fixes" this by changing a literal without
# understanding it, the thing that goes red is the behaviour, not the transcript.
from app.maintenance import MaintConfig # noqa: E402
_absent = {k: v for k, v in base().items() if k != "maintenance_charge_w"}
_built = MaintConfig().charge_w
check("with the key ABSENT the validator accepts...",
validate_startup_config(_absent, -5000, 5000) == [])
check(f"...while the code would build {_built:.0f} W against a "
f"{_absent['max_w']:.0f} W ceiling...", _built > _absent["max_w"])
_r = _charge(_absent["max_w"], _built)
check("...which the arbiter resolves to 0 W as a contradiction - the hole, "
"asserted so it cannot widen unnoticed",
_r.target_w == 0.0 and _r.contradiction)
# --------------------------------------------------------------------------- #
print("max_w must be a positive magnitude")
# Schema-blocked on a real install, but validate_startup_config() is also