admin/goodwe-addon
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

MAINT-05: one recalibration per pack, and only one pack moves power at a time
checks / unit (pull_request) Successful in 4m48s
Details

Browse Source 
§12's monthly cycle drains a pack to empty and refills it to true its coulomb
counter. There was exactly one `Maintenance` and one un-namespaced history, so
at two packs only the first was ever recalibrated - and nothing prevented the
cycle draining pack A while the loop charged pack B, which is two power-moving
phases on one grid connection pulling against each other.

* ONE `Maintenance` PER ROSTER ENTRY, each with its own schedule, phase and
  history under `unit/<name>/...`. `store.migrate_maint_keys()` moves an
  install's flat history into the first pack's namespace under
  `migrate_unit_keys()`'s three rules - shared in code via `_move_flat_keys()`
  and `_verify_moves()`, not restated - on its own marker, because unlike
  DEV-02's it runs only where the roster has more than one pack. The flat keys
  are kept LIVE as the rollback view.
* ONE MAINTENANCE SLOT PER SITE (`maintenance.arbitrate()`). At most one pack
  in a power-moving phase - drain, charge AND hold. The holder is DERIVED from
  the packs' own stored phases, never persisted, so a restart mid-drain
  re-derives it. Several packs due in the same hour: the slot goes to the one
  that has gone longest without a completed cycle; roster order breaks a
  numeric tie and nothing else. Nothing preempts a cycle in flight.
* THE DEFERRED PACK IS NOT STARVED AND IS NOT A FAULT. A deferral does not
  stamp `last_start_attempt` and does not move the phase, so neither the 12 h
  drain nor the 36 h charge timeout is reachable by waiting.
* The claim is filed on the pack whose state machine produced it, and stays
  SCOPE_UNIT - ARCH-01's rule.
* `validate.py` rule 6 grades `maintenance_charge_w` against the WEAKEST cap on
  the roster now, as its own comment said it must once `maint_unit` stopped
  being `inverters[0]`.
* At a roster of one nothing changes, and it is asserted: no new store key, the
  migration does not run, the four cycle events are the shipped strings, the
  banner is the bare phase.
* RECORDED NOT FIXED: a drain pins its pack while the loop's target is a site
  claim, so below the pin the allocator refuses the split and the site parks at
  0 W. Pre-existing at 2d025ee. Pinned as a running check.

Verified: 19/19 suites green in the CI image; 17 mutations run through
tools/mutation_run.sh (bytecode purged), every one observed red.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
admin
2026-09-04 13:38:29 +02:00
co-authored by Claude Opus 5
parent 2d025eebf7
commit 1aef19e995
9 changed files with 1980 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/maintenance.py
+309 -16
View File
@@ -18,16 +18,47 @@ OWNERSHIP, not priority. Exactly one thing writes the setpoint at any moment:
The loop is never "disabled" - it yields. The heartbeat to the inverter keeps
running in every phase, so the hardware watchdog stays fed and a crash here
still ends with the inverter at 0 W rather than latched.
ONE PACK AT A TIME (MAINT-05). There is now one `Maintenance` per roster entry,
each with its own schedule, its own phase and its own history under its own
Store keys - and therefore, for the first time, two of them able to move power
on ONE grid connection at once. Read `arbitrate()` before touching any of it:
the site holds exactly ONE maintenance slot, a pack that is not IDLE holds it,
and every other pack is held at IDLE under the loop until it is released. That
is not a scheduling nicety. A drain of pack A against a charge of pack B is two
power-moving phases pulling opposite ways across the meter - the circulating
power SAFETY-31 exists to DETECT, arriving here as something this module can
simply not do.
"""
import logging
import math
from dataclasses import dataclass, field
from datetime import datetime, timedelta, timezone
from .store import MAINT_UNIT_KEYS, unit_key
_LOG = logging.getLogger("goodwe.maint")
IDLE, DRAIN, CHARGE, HOLD = "idle", "drain", "charge", "hold"
# ⚠️ THE PHASES THAT MOVE POWER, WHICH IS NOT THE SAME LIST AS "owns the
# setpoint". The table above is about WHO WRITES the number; this is about
# whether the pack is being pushed. DRAIN and HOLD own the setpoint. CHARGE does
# NOT - the loop owns it - and CHARGE is in here anyway, because the charge-only
# clamp plus the cheap-window floor is what pulls a second pack full while the
# first is being emptied. Leaving CHARGE out is precisely the interlock hole
# this card is about: pack A charging from the grid at 2000 W while pack B
# drains 2000 W to export is 4 kW going round in a circle with the meter at
# zero, and both packs believing they are behaving.
#
# HOLD commands 0 W and moves nothing, and it is in here for the other half of
# the sentence: it OWNS the actuator - the pack is parked at the top for the BMS
# and is deliberately not available to the loop - so a second cycle starting
# under it would still be two recalibrations at once, which is the thing whose
# value is a clean uninterrupted coulomb count.
POWER_MOVING = (DRAIN, CHARGE, HOLD)
# 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.
@@ -153,17 +184,102 @@ class MaintResult:
setpoint_w: float | None = None
charge_only: bool = False
events: list = field(default_factory=list)
# ⚠️ WHY A DEFERRAL IS A FIELD AND NOT JUST A LOG LINE. A pack that is due
# and is not starting looks, from every surface an operator has, exactly
# like a pack whose schedule has quietly stopped firing - §12's opening
# sentence, the failure this whole module exists because of. It is carried
# so the status page can SAY "waiting for the other pack" rather than
# showing an idle machine and a due date in the past. Empty on every path
# that is not deferring, so a roster of one never sees it.
deferred: str = ""
class UnitHistory:
"""One pack's maintenance history in the Store, under one spelling.
⚠️ THE NAMESPACE IS A CONSTRUCTOR ARGUMENT AND `None` IS A REAL VALUE, NOT
AN OMISSION. `unit=None` means the FLAT keys - `phase`, `phase_started`,
`last_completed`, `last_start_attempt` - which is what every install has
written since the add-on shipped and what a one-inverter site keeps writing
after MAINT-05. §6.2's identity: with one pack there is nothing for a
namespace to distinguish it FROM, and adding one would change the layout of
a live state file to say something it already said. `arbiter.actuator_for()`
makes the same move for the same reason and its docstring argues it.
⚠️ `mirror_flat` IS THE ROLLBACK VIEW AND IT IS ONLY EVER THE FIRST PACK'S.
At N>=2 pack A's history has been migrated into its namespace and is written
there; the flat keys are ALSO kept in step, so an operator who rolls the
add-on back to a single-FSM version finds a CURRENT history rather than one
dated the day the second inverter was declared. store.migrate_maint_keys()
carries the argument, and it is DEV-02's rule 2 applied to history instead
of to a latch: "left in the file" is not the same promise as "still true".
Pack B has no flat spelling to mirror to - the flat keys mean pack A on
every install that has them - so it is constructed with mirroring off, and
a mirror from B would OVERWRITE A's rollback view with B's phase.
"""
def __init__(self, store, unit: str | None = None, mirror_flat: bool = False):
self.store = store
self.unit = unit
self.mirror_flat = bool(mirror_flat)
def key(self, name: str) -> str:
"""The Store key this pack's `name` lives under."""
return name if self.unit is None else unit_key(self.unit, name)
def get(self, name: str, default=None):
return getattr(self.store, "data", {}).get(self.key(name), default)
def set(self, name: str, value) -> None:
self.store.set(self.key(name), value)
if self.mirror_flat:
self.store.set(name, value)
def get_time(self, name: str):
return self.store.get_time(self.key(name))
def set_time(self, name: str, when) -> None:
self.store.set_time(self.key(name), when)
if self.mirror_flat:
self.store.set_time(name, when)
class Maintenance:
def __init__(self, cfg: MaintConfig, store):
"""ONE pack's recalibration cycle: its schedule, its phase, its history.
⚠️ ONE PER ROSTER ENTRY SINCE MAINT-05, AND THE SLOT IS NOT ITS OWN. This
object decides WHEN ITS PACK would like to start; `arbitrate()` decides
which of them may, because the constraint is a property of the grid
connection and no single FSM can see it. Never call `tick()` in a loop
without passing the `blocked_by` that `arbitrate()` returned for it: that is
the two-packs-at-once bug with the interlock present but not wired.
"""
def __init__(self, cfg: MaintConfig, store, unit: str | None = None,
name: str = "", mirror_flat: bool = False):
self.cfg = cfg
self.store = store
# ⚠️ `self.store` IS THE VIEW, NOT THE Store. Everything in this class
# names an UNQUALIFIED key - "phase", "last_completed" - and the view is
# the one place that turns it into the key this pack owns. A raw
# `store.set("phase", ...)` anywhere below would write pack A's history
# from pack B's state machine, which is the shared-state bug this card
# is about, and there would be nothing to see at a roster of one.
self.store = UnitHistory(store, unit, mirror_flat)
self.unit = unit
# What an event or a refusal calls this pack. At a roster of one it is
# never rendered - a single machine cannot be deferred - so the shipped
# one-inverter reason strings are untouched.
self.name = str(name or unit or "the pack")
# Edge-memo for the deferral note. The action log holds 40 lines and a
# deferral lasts as long as the other pack's cycle - hours - so a line
# per cycle would flush the very context that explains it. Same pattern
# as `Controller._arb_action` and `_maint_shape_suspended`.
self._deferred = ""
# -- state ---------------------------------------------------------------
@property
def phase(self) -> str:
return self.store.data.get("phase", IDLE)
return self.store.get("phase", IDLE)
def _enter(self, phase: str, now: datetime, events: list, msg: str) -> None:
self.store.set("phase", phase)
@@ -185,14 +301,60 @@ class Maintenance:
last = self.store.get_time("last_completed")
return None if last is None else last + timedelta(days=self.cfg.interval_days)
def waited_days(self, now: datetime) -> float:
"""Days since this pack last COMPLETED a cycle; `inf` if it never has.
⚠️ THE ARBITRATION KEY, AND IT IS DELIBERATELY NOT `due()`. `due()` is a
yes/no and two packs that are both due are indistinguishable by it -
which is how a roster-order tie-break gets written and how the first
entry then wins every contested month forever while the second drifts.
This is an ORDERING: the pack whose coulomb counter has gone longest
without a recalibration goes first. `inf` for never-recalibrated is the
same statement `due()` makes about an absent `last_completed`, kept as
one so a fresh pack cannot lose to one that ran last month.
"""
last = self.store.get_time("last_completed")
return math.inf if last is None else (now - last).total_seconds() / 86400.0
def _cooled(self, now: datetime) -> bool:
"""Is the post-abort cooldown over?"""
last_try = self.store.get_time("last_start_attempt")
return (last_try is None
or (now - last_try) >= timedelta(minutes=RESTART_COOLDOWN_MIN))
def wants_start(self, now: datetime) -> bool:
"""Would this pack begin a cycle on this tick if the site let it?
⚠️ THE SAME PREDICATE `tick()` USES, NOT A COPY OF IT. `arbitrate()` has
to know who is asking before anybody moves, and a second transcription
of "enabled, in the start hour, due and cooled down" is a second opinion
about who was asking - so the IDLE branch of `tick()` calls this one.
"""
return (self.phase == IDLE and self.cfg.enabled
and now.hour == self.cfg.start_hour and self.due(now)
and self._cooled(now))
# -- operations ----------------------------------------------------------
def force_start(self, now: datetime) -> list:
def force_start(self, now: datetime, blocked_by: str = "") -> list:
"""Start a cycle regardless of the calendar.
Commissioning uses this: a cycle must be proven end to end before a site
is signed off, and nobody waits a month to find out the schedule never
fires.
⚠️ REGARDLESS OF THE CALENDAR, NOT REGARDLESS OF THE INTERLOCK. The
calendar is a preference and an operator may overrule it; "one pack
moves power at a time" is a property of the grid connection and an
operator pressing a button does not add a second one. A forced start
against a busy site is REFUSED, by name, and the phase machine does not
move - the refusal is returned as an event so the operator reads it in
the action log rather than watching nothing happen.
"""
if blocked_by:
msg = (f"maintenance: forced start REFUSED on {self.name} - "
f"{blocked_by}")
_LOG.warning("%s", msg)
return [msg]
events = []
self.store.set_time("last_start_attempt", now)
self._enter(DRAIN, now, events, "maintenance: forced start -> drain")
@@ -203,23 +365,80 @@ class Maintenance:
self._enter(IDLE, now, events, f"maintenance: ABORT ({why})")
return events
def _defer_note(self, why: str, events: list) -> list:
"""Say once, not every second, that this pack is waiting its turn.
⚠️ THE WORDING IS DELIBERATELY NOT AN ABORT AND NOT A TIMEOUT. §12's
notes are explicit that a 12 h phase timeout means "the battery is not
following commands - a FAULT, not a slow day", and the one thing that
must not happen here is an operator reading a deferral as that. This
line says the pack is WAITING, names the pack it is waiting for, and
says it will run next - none of which any failure path in this module
says.
"""
if self._deferred != why:
self._deferred = why
msg = (f"maintenance: {self.name} is due and is WAITING its turn - "
f"{why}. It is idle under the control loop and will "
f"recalibrate at the next start hour once the slot is free; "
f"this is not a stalled phase.")
_LOG.info("%s", msg)
events.append(msg)
return events
# -- the machine ---------------------------------------------------------
def tick(self, now: datetime, soc: float | None) -> MaintResult:
def tick(self, now: datetime, soc: float | None,
blocked_by: str = "") -> MaintResult:
"""One cycle of this pack's state machine.
`blocked_by` is `arbitrate()`'s answer for THIS pack: empty when the
site's one maintenance slot is available to it, otherwise the reason it
is not. It gates two things and nothing else - a start, and a phase that
is already moving power on a pack that does not hold the slot.
"""
events: list = []
phase = self.phase
# ⚠️ THE SLOT IS CHECKED BEFORE THE PHASE, AND THIS BRANCH IS A RECOVERY
# RATHER THAN AN EXPECTED PATH. `arbitrate()` grants the slot to the
# pack that is already mid-cycle, so a pack that is BOTH moving power
# and blocked means two histories on disk say "drain" - a hand-edited
# state file, or a store carried across a version that had no interlock.
# The fail-safe direction is unambiguous: two packs pulling against each
# other on one connection is the fault SAFETY-31 latches the whole site
# for. Stand this one down to 0 W and say why; the holder keeps its
# cycle, which is the outcome that loses no coulomb count at all.
if blocked_by and phase in POWER_MOVING:
events += self.abort(now, f"interlock - {blocked_by}")
return MaintResult(IDLE, True, 0.0, False, events)
if phase == IDLE:
if self.cfg.enabled and now.hour == self.cfg.start_hour and self.due(now):
last_try = self.store.get_time("last_start_attempt")
cooled = (
last_try is None
or (now - last_try) >= timedelta(minutes=RESTART_COOLDOWN_MIN)
)
if cooled:
self.store.set_time("last_start_attempt", now)
self._enter(DRAIN, now, events,
"maintenance: due -> drain (exports the surplus)")
return MaintResult(DRAIN, True, self.cfg.discharge_w, False, events)
if self.wants_start(now):
if blocked_by:
# ⚠️ `last_start_attempt` IS NOT STAMPED HERE, AND THAT IS
# THE STARVATION FIX RATHER THAN AN OMISSION. That key is
# the post-ABORT cooldown: it exists so a cycle that failed
# does not re-trigger inside the same start hour. A deferral
# is not an attempt - nothing was tried and nothing failed -
# and stamping it would put this pack under a 90 minute
# cooldown for a start hour that is 60 minutes long, so the
# pack that lost the arbitration would still be locked out
# at the moment the winner released the slot.
#
# ⚠️ AND THE PHASE DOES NOT MOVE, WHICH IS THE OTHER HALF.
# `phase_started` is untouched, so the 12 h DRAIN and 36 h
# CHARGE timeouts - which mean "the battery is not following
# commands, a fault" - cannot be reached by waiting. A
# deferred pack is IDLE under the loop, not a stuck one.
return MaintResult(IDLE, False, None, False,
self._defer_note(blocked_by, events),
blocked_by)
self._deferred = ""
self.store.set_time("last_start_attempt", now)
self._enter(DRAIN, now, events,
"maintenance: due -> drain (exports the surplus)")
return MaintResult(DRAIN, True, self.cfg.discharge_w, False, events)
self._deferred = ""
return MaintResult(IDLE, False, None, False, events)
# ⚠️ Fail toward inaction. Without a state-of-charge reading none of the
@@ -273,3 +492,77 @@ class Maintenance:
_LOG.error("maintenance: unknown phase %r, forcing idle", phase)
events += self.abort(now, f"unknown phase {phase!r}")
return MaintResult(IDLE, True, 0.0, False, events)
def _days(n: float) -> str:
return "never recalibrated" if n == math.inf else f"{n:.0f} d since its last cycle"
def arbitrate(machines, now: datetime) -> list:
"""Grant the site's ONE maintenance slot. Returns a list parallel to
`machines`: "" for the pack that may move power, otherwise the reason the
pack may not.
⚠️ THE SLOT IS A PROPERTY OF THE GRID CONNECTION, WHICH IS WHY IT IS NOT ON
`Maintenance`. Each FSM can see its own pack's schedule and nothing else; the
quantity being rationed - "how many phases are moving power across one
meter" - is a sum over the roster, exactly like `sum(|B_i|) - |sum(B_i)|` in
SAFETY-31 and for the same reason. A per-pack answer to a site-level
question is the shape of defect this repository keeps finding.
⚠️ IT IS DERIVED, NOT PERSISTED, AND THAT IS DELIBERATE. The holder is
"whichever pack's stored phase is not IDLE" - a fact already on disk, in the
per-pack history the packs themselves write. A separate `maint_slot_holder`
key would be a second opinion about the same thing, and the two would
disagree the first time the add-on was killed between writing the phase and
writing the holder. Restart-safe by construction: a controller that comes
back up mid-drain reads the drain and re-derives the holder from it.
TWO CLAUSES, IN ORDER:
1. A PACK ALREADY MOVING POWER KEEPS THE SLOT. Nothing preempts a cycle in
flight, because a recalibration interrupted halfway is worth less than
nothing - the coulomb counter is graded on ONE uninterrupted drain and
refill, so a cycle stopped at 40 % has spent the energy and bought no
calibration. If more than one pack somehow comes up moving power, the
FIRST in roster order holds and `tick()` stands the others down; see the
recovery branch there.
2. OTHERWISE THE LONGEST-WAITING CANDIDATE STARTS. `wants_start()` is asked
of every pack, and among those that say yes the slot goes to the one with
the largest `waited_days()`. NOT roster order: two packs on one monthly
schedule are due in the same hour every month, and a roster-order tie
would hand the slot to `inverters[0]` every single time while the second
pack's coulomb counter drifted forever - which is, in a different
costume, the exact defect this card was opened for. Roster order breaks a
genuine numeric tie and nothing else.
⚠️ AT A ROSTER OF ONE THIS IS THE IDENTITY FUNCTION. One machine is never
another machine's blocker: clause 1 finds at most itself and clause 2 has at
most one candidate, so it returns `[""]` on every tick and no reason string
it can build is ever rendered. That is what makes the one-inverter behaviour
byte-identical rather than merely similar.
"""
blocks = [""] * len(machines)
busy = [(i, m) for i, m in enumerate(machines) if m.phase in POWER_MOVING]
if busy:
held, holder = busy[0]
for i, m in enumerate(machines):
if i != held:
blocks[i] = (f"{holder.name} is mid-cycle ({holder.phase}) and "
f"holds the site's one maintenance slot")
return blocks
wants = [(i, m) for i, m in enumerate(machines) if m.wants_start(now)]
if len(wants) <= 1:
return blocks
# Largest waited_days wins; `-index` makes a numeric tie fall to roster
# order rather than to whatever `max()` happens to see first.
won, winner = max(wants, key=lambda p: (p[1].waited_days(now), -p[0]))
for i, m in wants:
if i != won:
blocks[i] = (f"{winner.name} has waited longer "
f"({_days(winner.waited_days(now))} against "
f"{_days(m.waited_days(now))}) and takes the site's "
f"one maintenance slot first")
return blocks