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

DEV-02: per-instance inverter objects, so nothing is graded against a sibling
checks / unit (pull_request) Successful in 4m15s
Details

Browse Source 
Every piece of per-device state in the Controller was a scalar, and several of
them are safety inputs. app/inverter.py introduces an `Inverter` that owns its
entities, its rating, its device range, its SignWatchdog, its last_write and
its last_beat, under store keys namespaced by unit name. The Controller holds a
list of them and keeps only what is genuinely site-level: the P1 reading, the
site integrator, and the tariff quarter.

WHAT THE SCALARS WERE SILENTLY ASSERTING

  dev_min/dev_max   one pair, read from one setpoint_entity. A second
                    inverter's rating was NEVER READ AT ALL, so its clamp, its
                    freeze rail and the peak guard's achievable_w were the
                    first one's numbers.
  soc               one reading into the maintenance FSM, SAFETY-24's pack
                    floor and arbitrage's bounds. An average lets the emptier
                    pack be driven below its own floor.
  sign_wd           ONE SignWatchdog, whose own docstring says "one instance
                    per actuator", latched under FLAT store keys - so a trip
                    caused by B latched A across restarts and one operator
                    press cleared both.
  last_beat         one deadline and one serialised hass.call(), so a hung
                    write to A ate B's 30 s layer-1 budget.

actual_w BECAME SITE LEVEL WHILE THE READINGS BECAME PER-INSTANCE, and those
are opposite directions on purpose. compute()'s anti-windup test is
`abs(prev_w - actual_w) > saturation_w` with prev_w the SITE target, so feeding
it one inverter's power compares a site number against a unit number, trips the
saturation freeze permanently and leaves a standing grid error. Measured on
admin/goodwe's two-master rig (tools/alloc_sim.py --splits, 2 kW load, 40
cycles), worst |delta| between splits at the meter:

    gain    one unit's power      the site sum
    0.2           510 W                0 W
    0.4           250 W                0 W
    0.6            50 W                0 W

That was ALLOC-01's disclosed finding; test_alloc.py part 8 kept its stimulus
and now grades the other row.

THE STORE MIGRATION IS GRADED, NOT MERELY PERFORMED. sign_tripped/sign_reason
move to unit/<name>/... once, verified by READING BACK what was written, and
the version marker is set only when nothing that carried state was lost. The
flat keys stay in the file so a rollback still finds its latch. And __init__
compares the pre-migration flat latch against what unit A actually came up
with: if the latch did not survive it is RE-ARMED from the legacy value and
shouted about. A latch that silently fails to migrate is a disarmed safety
guard on an install whose state file says it is protected - which is strictly
worse than never persisting it, because now there is evidence saying it is
fine. test_inverter.py part 2 breaks the migration and watches the guard fire.

HEARTBEATS ARE INDEPENDENT AND THE BUDGET IS DERIVED. Each inverter owns its
deadline, the refreshes go out concurrently, and each write is bounded by
WRITE_BUDGET_S = 4 s. The inequality is graded rather than asserted: 4 +
heartbeat_s's schema maximum of 25 < 30, validate.FIRMWARE_INGEST_VETO_S.
Verified by injecting a transport that accepts and never answers.

live_units() ANSWERS HONESTLY. A unit whose every write has been rejected for a
whole layer-1 window is not commandable; the allocator drops it, the survivors
carry its share, the action log names it, and the dropped unit is commanded
0 W rather than left standing on its last share. A roster entry with NO
setpoint_entity is deliberately not handled there - that is a configuration
that cannot be delivered, and it reaches main.py's refusal branch, which is how
that branch keeps a live trigger after this card removed its old one.

AT N=1 NOTHING MOVES. The removed scalars survive as read/write properties of
the sole inverter, so every existing replay runs unmodified - and they REFUSE
at N>=2 rather than answering for the first inverter, which is the tripwire on
a consumer that was never made per-instance.

EVIDENCE. 19 files green. test_inverter.py is new (82 checks). Eighteen
mutations were applied to the shipped modules on a purged tree via
tools/mutation_run.sh and each was observed RED. Two of them SURVIVED first
time and the checks were changed rather than the table: the serialised dispatch
and the shared last_beat both passed while part 6 gathered its own writes and
computed its own `due` list. send_heartbeats() was extracted from run_control()
so the shipped dispatch is reachable by a check at all.

INVERTER B STILL DOES NOT EXIST. build_roster() returns exactly one inverter
from the flat options and is the one function CFG-04 changes; every two-unit
figure here is architecture, and the second rating in the checks is marked a
hypothesis.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
glenn schrooyen
2026-09-02 14:50:43 +02:00
co-authored by Claude Opus 5
parent 3030f72b9e
commit e306c74cb8
10 changed files with 3027 additions and 325 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/inverter.py
+334
View File
@@ -0,0 +1,334 @@
"""One inverter, as an OBJECT rather than as a set of Controller scalars.
WHAT THIS FIXES, in one sentence: every piece of per-device state in this
add-on used to be a single scalar on the Controller, and several of those
scalars are SAFETY INPUTS - so a second inverter would have been graded against
the first one's numbers.
The scalars, and what each one was silently asserting:
`dev_min` / `dev_max` ONE pair, read from ONE `setpoint_entity`. A second
inverter's rating was never read at all, so its clamp,
its freeze rail and the peak guard's `achievable_w`
were all inverter A's.
`soc` ONE state of charge into the maintenance FSM,
SAFETY-24's pack floor and arbitrage's bounds. An
average lets the emptier pack be driven below its own
floor; a maximum parks the fuller one.
`batt` ONE battery power into the anti-windup saturation
detector - see the SITE/UNIT note at the bottom of
this docstring, because this one goes the OTHER WAY.
`sign_wd` ONE SignWatchdog, whose own docstring says "one
instance per actuator", latched under FLAT store keys.
A trip caused by B latched A across restarts and one
operator action cleared both.
`last_write` ONE write bookkeeping, and ONE serialised `hass.call()`
behind it: a hung write to A ate B's 30 s layer-1
budget.
⚠️ WHAT IS DELIBERATELY *NOT* HERE, AND IT IS THE HALF THAT IS EASY TO GET
BACKWARDS. THERE IS NO `Tuning` ON THIS OBJECT AND THERE IS NO INTEGRATOR.
§6.2 and ALLOC-01's stability argument are ONE integrator on P1, ABOVE the
split: the site target S is divided into per-unit shares, so the gain seen at
the meter is a CONVEX COMBINATION of the per-unit gains and a bad split cannot
destabilise the site. Give each inverter its own `Tuning` and somebody will
eventually call `compute()` with it, and the moment two integrators regulate
one meter the whole argument is gone - that is the two-writers failure the
arbiter exists to prevent, one level up.
The one field of `Tuning` that genuinely IS per-device is `max_w`, the
supervised cap on what may be WRITTEN to one setpoint entity (§11.3), and it is
here as `max_w`. The SITE `Tuning` the loop runs on takes its bound from the
roster - `sum(inv.max_w)` - so at N=1 it is byte-identical to the shipped
number and at N>=2 it is what the site can actually deliver. That is the whole
of "the rating stops being a scalar"; a per-unit Tuning would be three more
numbers nobody reads and one more place a limit has to be kept in step (CFG-03
already tracks three).
------------------------------------------------------------------------------
SITE OR UNIT? THE TWO HALVES OF THE N=1 IDENTITY, AND THEY MOVE IN OPPOSITE
DIRECTIONS
At N=1 §6.2's identity holds: the fabricated grid reading and the battery
setpoint are the same number, so "the site" and "the inverter" are the same
thing and every scalar above was RIGHT. Splitting them is not one change, it is
two, and they point opposite ways:
COMMANDING becomes per-unit. One setpoint entity per inverter, one rating,
one clamp, one watchdog, one heartbeat.
SENSING, for the loop, becomes SITE. `compute()` regulates the METER. Its
`actual_w` is the anti-windup input to
`abs(prev_w - actual_w) > saturation_w`, where
`prev_w` is the SITE target. Feed it ONE unit's
power at a roster of two and that test compares
a site number against a unit number, trips the
freeze permanently and leaves a standing grid
error.
That second half is MEASURED, not argued. On admin/goodwe's two-master rig
(`tools/alloc_sim.py --splits --feedback unit`), with the loop fed one
inverter's power the aggregate trace at the meter becomes a function of the
split - up to 510 W apart at gain 0.2, 250 W at gain 0.4, 50 W at gain 0.6 -
while the allocator's shares sum to S in every one of those runs. The
counterfactual is the proof: change that ONE input to the site total and every
split is bit-identical at every gain the schema accepts. ALLOC-01's review
isolated it and test_alloc.py part 8 graded it as a disclosed defect waiting on
this card.
So this object owns `batt` - THIS inverter's own power, which is what a
per-unit consumer needs - and the Controller sums them into the site figure the
loop is fed. Getting that backwards reintroduces the divergence.
------------------------------------------------------------------------------
LIVENESS, AND WHY IT IS A METHOD WITH A REASON
`Controller.live_units()` decides which inverters may be commanded this tick,
and `allocator.plan()` DROPS what it does not list - the survivors then carry
the whole site target. A wrong answer there is a real change in what the site
can deliver, so `is_live()` answers one question and says which: CAN THIS
CONTROLLER PUT A NUMBER ON THIS INVERTER RIGHT NOW.
ONE way it cannot, and exactly one: every write has been REJECTED for a whole
`LINK_STALE_S` window. Not one rejection - `hass.set_number()` returns False for
a transient 400 as readily as for a device that has gone - but a continuous run
of them with no acceptance in between.
⚠️ AND NOT "IT HAS NO `setpoint_entity`", THOUGH THAT IS ALSO A UNIT THIS
CONTROLLER CANNOT COMMAND. The two look the same and want opposite answers. A
link that WAS working and stopped is an event: the survivors carry its share,
the record says so, and the site goes on running at reduced strength, which is
the best available outcome. A roster entry with no writer configured at all is a
CONFIGURATION that cannot be delivered - it was never reachable, nothing has
gone wrong on the wire, and quietly running the site at part strength on a
roster somebody wrote down wrong is how a half-driven site reads as healthy on
every dashboard. That one belongs to main.py's refusal branch, which commands
0 W and NAMES the inverter; putting it here would absorb it into the ordinary
drop and delete the last trigger that branch has. See `Controller.cycle()`.
⚠️ AND NOT "ITS SENSORS ARE MISSING" EITHER. A pack whose SoC did not answer is
a third fault with a third remedy: `inputs_ok` already stops the whole site at
P_SAFETY for that, and dropping the unit from the SPLIT instead would hand its
share to a sibling while the site kept regulating - the opposite of what a
missing measurement should cause. Liveness is about the WRITE PATH, and only
about a write path that has stopped answering.
"""
import math
from datetime import datetime
from .clamp import ClampResult
from .store import unit_key
from .watchdog import SignWatchdog, SignWatchdogConfig
# How long every write to one inverter must have been REJECTED before this
# controller stops claiming it can command it.
#
# ⚠️ IT IS LAYER 1'S OWN WINDOW, NOT A NEW OPINION ABOUT LINKS. The ESP32
# commands 0 W and keeps commanding it after ~30 s without a refresh
# (`validate.FIRMWARE_INGEST_VETO_S`), so a link this controller has not been
# able to write for a whole 30 s is a link the hardware is about to take away
# from it anyway. Claiming it as commandable past that point is the add-on
# telling the allocator it can deliver watts that the firmware has already
# decided it will not. No config key: CFG-03 tracks three places a default has
# to be kept in step and this does not earn a fourth - it is derived from a
# constant the firmware and validate.py already share.
LINK_STALE_S = 30.0
# The one inverter this add-on drives today. ⚠️ A NAME, NOT A CONFIGURATION -
# it exists so that a per-inverter claim has an actuator to be filed ON, which
# is what makes the site/unit split something a check can grade rather than a
# comment. It lives here rather than in main.py now that the roster does.
UNIT_A = "inverter-a"
# The per-unit keys this object keeps in the Store, namespaced by unit name.
# ⚠️ THE VALUES ARE THE PRE-DEV-02 FLAT KEY NAMES, and that is what makes the
# migration mechanical rather than transcribed: `store.migrate_unit_keys()`
# reads this same mapping, so a key renamed here cannot silently stop being
# migrated. See store.py.
UNIT_STORE_KEYS = ("sign_tripped", "sign_reason")
class Inverter:
"""ONE inverter: its entities, its rating, its watchdogs, its store keys.
Constructed once per configured inverter and held in `Controller.inverters`.
Everything on it is about THIS device; nothing on it is about the site.
⚠️ `hass` IS NOT A CONSTRUCTOR ARGUMENT. This object is built in
`Controller.__init__`, which runs before any I/O and, in four existing
checks, with `hass=None` entirely. The device range is read LATER, by
`Controller.run_control()`, from THIS unit's own `setpoint_entity` - which
is the whole of "each instance's rating is read from its own entity".
"""
def __init__(self, name, *, setpoint_entity="", soc_entity="",
batt_entity="", batt_invert=False, max_w=2000.0,
store=None, sign_cfg=None):
self.name = str(name)
self.setpoint_entity = str(setpoint_entity or "")
self.soc_entity = str(soc_entity or "")
self.batt_entity = str(batt_entity or "")
self.batt_invert = bool(batt_invert)
# The supervised cap on what may be written to THIS setpoint entity.
# See the module docstring on why this is a number and not a `Tuning`.
self.max_w = float(max_w)
# --- the device's own reported range, read from its own entity -------
# None until `hass.limits()` has answered. NOT guessed at: ESPHome is
# often not up when the add-on boots, and (None, None) is the honest
# report that the range checks were skipped rather than invented.
self.dev_min = None
self.dev_max = None
# --- this unit's own measurements ------------------------------------
self.soc = None
self.batt = None
# --- what this unit was last told, and what it last accepted ---------
# `target` is THIS unit's share of the site target from the last
# resolution - the number its heartbeat re-emits and the number
# SAFETY-02 grades against the meter. At N=1 it IS the site target,
# because the allocator is a pass-through there.
self.target = 0.0
self.last_write = None
self.last_write_at = None
self.last_write_ok = None
# ⚠️ TWO TIMESTAMPS, NOT ONE FLAG. `last_write_ok` is the last write's
# verdict and it flips on a single transient 400; liveness needs a RUN
# of rejections, so `rejected_since` records when the current run
# started and is cleared by any acceptance.
self.rejected_since = None
# The heartbeat's own deadline, on the event loop's monotonic clock.
# ⚠️ PER UNIT, WHICH IS THE WHOLE OF AC 6. One shared `last_beat` and
# one serialised write meant a hung call to A pushed B's refresh past
# layer 1's ~30 s veto - i.e. one inverter's transport fault zeroing a
# healthy sibling. Starts at 0.0, which is what the single scalar did.
self.last_beat = 0.0
# --- SAFETY-03's per-write evidence ----------------------------------
# What THIS unit's last write clamped, or None before it has run. The
# ALARM (`ClampWatchdog`) stays one per controller - clamp.py says so in
# its own docstring, and it is an anti-flood counter over an event bus
# rather than a property of a device - but the EVIDENCE is per unit,
# because a clamp on B says nothing about A.
self.clamp_last: ClampResult | None = None
# --- SAFETY-02, one instance per actuator, under NAMESPACED keys -----
# ⚠️ THE WATCHDOG'S OWN DOCSTRING SAID "one instance per actuator" WHILE
# THE CONTROLLER HELD EXACTLY ONE. Restored from THIS unit's own keys,
# so a trip caused by B latches B and leaves A commanding - and an
# operator clearing one does not silently clear the other.
self.store = store
data = getattr(store, "data", {}) if store is not None else {}
self.sign_wd = SignWatchdog(
sign_cfg or SignWatchdogConfig(),
tripped=bool(data.get(self.key("sign_tripped"), False)),
reason=str(data.get(self.key("sign_reason"), "") or ""))
# -- store ---------------------------------------------------------------
def key(self, name: str) -> str:
"""This unit's namespaced Store key for `name`. See store.unit_key()."""
return unit_key(self.name, name)
def store_set(self, name: str, value) -> None:
if self.store is not None:
self.store.set(self.key(name), value)
# -- the numbers other modules ask this object for -----------------------
def rating_w(self) -> float:
"""The largest magnitude this unit can be commanded, in W.
`min(max_w, |dev_min|, |dev_max|)` - the supervised cap and whatever the
device itself reports, whichever is smaller. It is the same conjunction
`freeze.rail_ceiling_w()` computes for the freeze rail and the same one
main.py hands the peak guard as `achievable_w`, which is why it is
written once here: three transcriptions of a rating is three chances for
inverter B to be graded against inverter A's number.
A bound that could not be read is SKIPPED, not treated as zero: (None,
None) means ESPHome was not up, and answering 0 W there would say this
inverter can do nothing.
"""
out = abs(float(self.max_w))
for bound in (self.dev_min, self.dev_max):
if bound is not None and math.isfinite(float(bound)):
out = min(out, abs(float(bound)))
return out
def has_writer(self) -> bool:
return bool(self.setpoint_entity)
# -- liveness ------------------------------------------------------------
def note_write(self, ok: bool, now: datetime) -> None:
"""Record one write's verdict. The only thing that moves liveness."""
self.last_write_ok = bool(ok)
if ok:
self.rejected_since = None
elif self.rejected_since is None:
self.rejected_since = now
def is_live(self, now: datetime, stale_s: float = LINK_STALE_S):
"""`(live, why_not)` - may this controller command this unit THIS TICK?
Read the module docstring before widening this - in particular on why a
unit with NO `setpoint_entity` is deliberately still "live" here and is
refused one layer up instead. The allocator DROPS what `live_units()`
does not list and the survivors carry the whole site target, so a false
"not live" is a site target divided among fewer inverters than exist -
and at a roster of one it is the allocator refusing outright and the
site commanding 0 W.
⚠️ ANSWERED PER TICK, NEVER CACHED, for `allocator.plan()`'s reason: a
link goes stale between one cycle and the next.
"""
if self.rejected_since is not None:
held = (now - self.rejected_since).total_seconds()
if held >= float(stale_s):
return False, (f"every write rejected for {held:.0f} s "
f"(limit {float(stale_s):.0f} s)")
return True, ""
def link_note(self, now: datetime) -> str:
"""One line for the status page. Empty when there is nothing to say."""
if not self.has_writer():
return (f"{self.name}: NO SETPOINT ENTITY - this controller cannot "
f"command it, and the site refuses rather than running at "
f"part strength")
live, why = self.is_live(now)
if not live:
return f"{self.name}: NOT COMMANDABLE - {why}"
if self.rejected_since is not None:
held = (now - self.rejected_since).total_seconds()
return (f"{self.name}: writes rejected for {held:.0f} s - still "
f"commandable until {LINK_STALE_S:.0f} s")
return ""
def __repr__(self) -> str:
return (f"<Inverter {self.name} sp={self.setpoint_entity!r} "
f"max_w={self.max_w:.0f} dev=({self.dev_min}, {self.dev_max})>")
def build_roster(opts: dict, store, sign_cfg=None) -> list:
"""The site's inverters, in roster order. TODAY THAT IS EXACTLY ONE.
⚠️ INVERTER B DOES NOT EXIST YET (~2 weeks out) AND THIS FUNCTION MUST NOT
INVENT IT. There is no `inverters:` list in config.yaml, no second
`setpoint_entity`, and no measured rating, ramp or lag for a second device -
SPIKE-04 measures those and CFG-04 shapes the option that carries them. What
this card delivers is that the CODE no longer assumes there is one; adding
the second entry is a config change to THIS FUNCTION and to nothing else,
which is the point.
So: unit A is built from the flat options, exactly as the shipped scalars
were, and the flat options keep their names. An install that upgrades reads
identically - same entities, same rating, same behaviour - which is AC 8.
"""
return [Inverter(
UNIT_A,
setpoint_entity=opts.get("setpoint_entity", ""),
soc_entity=opts.get("soc_entity", ""),
batt_entity=opts.get("batt_entity", ""),
batt_invert=bool(opts.get("batt_invert")),
max_w=float(opts.get("max_w", 2000)),
store=store, sign_cfg=sign_cfg)]
goodwe_controller/app/main.py
+920 -206
View File
File diff suppressed because it is too large Load Diff
goodwe_controller/app/store.py
+174
View File
@@ -39,6 +39,13 @@ _LOG = logging.getLogger("goodwe.store")
# 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.
#
# ⚠️ AND SINCE DEV-02 THE TWO SAFETY-02 KEYS ARE THE *LEGACY* SPELLING. They are
# FLAT - one `sign_tripped` for a controller that now holds a LIST of inverters -
# so a trip caused by inverter B would latch inverter A across restarts, and one
# operator action would clear both. They live on here as the pre-DEV-02 layout
# that `migrate_unit_keys()` reads ONCE and never reads again. Nothing in the
# add-on writes them any more; see SCHEMA_VERSION.
DEFAULTS = {
"phase": "idle",
"phase_started": None,
@@ -48,6 +55,173 @@ DEFAULTS = {
"sign_reason": "",
}
# --------------------------------------------------------------------------- #
# DEV-02: PER-INVERTER KEYS, AND THE MIGRATION IS THE RISKY HALF
#
# ⚠️ A LATCH THAT SILENTLY FAILS TO MIGRATE IS A DISARMED SAFETY GUARD ON AN
# INSTALL THAT BELIEVES IT IS PROTECTED. SAFETY-02's whole reason for being
# persisted is that "restart the add-on" is the FIRST thing an operator staring
# at a FAILSAFE banner tries, and a latch held only in memory comes back armed,
# into the fault, unattended. A latch that is written down under a key nothing
# reads any more is exactly the same failure with a longer paper trail: the file
# still says `sign_tripped: true`, the banner is green, and the controller
# resumes driving the wrong way.
#
# So this is a MIGRATION WITH A POSTCONDITION, not a copy. `migrate_unit_keys()`
# reports what it did AND reads the result back; `Controller.__init__` grades
# that report and refuses to come up quietly if the latch did not survive. The
# test that grades it BREAKS the migration and watches the guard fire.
# --------------------------------------------------------------------------- #
# 0 = the flat, one-inverter layout that shipped before DEV-02.
# 1 = per-inverter keys under `unit/<name>/...`.
SCHEMA_VERSION = 1
SCHEMA_KEY = "schema_version"
# Which flat keys belong to ONE inverter, and therefore move into its namespace.
# ⚠️ THE MAINTENANCE KEYS ARE NOT IN HERE AND THAT IS DELIBERATE. `phase`,
# `phase_started`, `last_completed` and `last_start_attempt` belong to the
# maintenance state machine, of which this add-on still holds exactly ONE - a
# monthly recalibration is a pack procedure and per-pack SCHEDULING is a card of
# its own, not something to infer from a key rename. Moving them here would
# quietly re-home an install's whole maintenance history under a unit name; they
# stay where they are, untouched, which is the other half of "migrates without
# losing its latch OR ITS HISTORY".
LEGACY_UNIT_KEYS = ("sign_tripped", "sign_reason")
def unit_key(unit: str, key: str) -> str:
"""The namespaced Store key for `key` on inverter `unit`.
⚠️ ONE SPELLING, IN ONE PLACE. `Inverter.key()` calls this and so does the
migration, so a check can assert that what was migrated is what is read.
Two transcriptions of a key name is one copy plus a future disagreement
about which one holds the latch - and the copy that drifts is always the one
that reads.
"""
return f"unit/{unit}/{key}"
class MigrationReport:
"""What `migrate_unit_keys()` did, and whether it can be believed.
⚠️ IT CARRIES `verified`, WHICH IS READ BACK FROM THE STORE RATHER THAN
ASSUMED FROM THE ASSIGNMENT. That is the difference between this and a
two-line copy: the caller is able to ask "is the latch that was in the file
before now under the key the watchdog reads", and a `False` there is a
disarmed guard, which is an ERROR and not a log line nobody sees.
"""
def __init__(self):
self.ran = False # did anything need moving
self.from_version = 0
self.moved = {} # new key -> value written
self.kept = [] # keys already namespaced, left alone
self.verified = True # every move read back equal
self.problems = [] # human-readable, one per failed move
# ⚠️ THE SUBSET THAT MATTERS, AND THE DISTINCTION IS NOT PEDANTRY. A
# store that drops a write of `sign_tripped=False` has lost the
# DEFAULT - the same statement "absent" already makes, so nothing is
# gone. One that drops a write of `True` has lost A LATCH. Only the
# second is a disarmed safety guard, and only the second may be allowed
# to shout: an alarm that fires on the harmless case is an alarm an
# operator learns to dismiss before the real one arrives.
self.lost = []
@property
def ok(self) -> bool:
"""Nothing that CARRIED STATE failed to land."""
return not self.lost
def explain(self) -> str:
if not self.ran:
return (f"store already at schema {self.from_version}; no "
f"per-inverter migration needed")
out = (f"migrated {len(self.moved)} flat key(s) to inverter "
f"namespaces: " + ", ".join(sorted(self.moved)))
if self.kept:
out += " (left alone, already namespaced: " + ", ".join(sorted(self.kept)) + ")"
if self.problems:
out += " - PROBLEMS: " + "; ".join(self.problems)
if self.lost:
out += " - STATE LOST: " + "; ".join(self.lost)
return out
def migrate_unit_keys(store, unit: str) -> MigrationReport:
"""Move the pre-DEV-02 flat per-inverter keys into `unit`'s namespace.
`unit` is the FIRST inverter in the roster, which on every install that can
possibly have a flat key is the only one it ever had. A store written by a
version that knew about more than one inverter is already at
SCHEMA_VERSION and is not touched.
THE THREE RULES, each one a case that would otherwise be a wrong answer:
1. IT NEVER OVERWRITES AN EXISTING NAMESPACED VALUE. A store that has both
spellings has been written by this version, and the namespaced one is the
live latch - the flat one is the fossil. Preferring the fossil would
resurrect a trip the operator has already cleared.
2. THE FLAT KEYS ARE LEFT IN THE FILE. Deleting them buys tidiness and costs
a rollback: an operator who downgrades the add-on after a migration would
find no latch at all, which is the disarmed guard again in the other
direction. Left in place, a downgrade finds the trip it had. They go
stale from this moment - nothing writes them again - and the direction
that staleness fails in is a SPURIOUS latch (a 0 W hold an operator
clears), never a missing one.
3. THE VERSION MARKER IS WRITTEN LAST. A crash between the copies and the
marker leaves the migration to run again, and rule 1 makes running it
again a no-op. A marker written first would leave a half-migrated store
looking finished.
"""
rep = MigrationReport()
data = getattr(store, "data", {})
rep.from_version = int(data.get(SCHEMA_KEY, 0) or 0)
if rep.from_version >= SCHEMA_VERSION:
return rep
rep.ran = True
for legacy in LEGACY_UNIT_KEYS:
target = unit_key(unit, legacy)
if target in data:
rep.kept.append(target)
continue
# ⚠️ `DEFAULTS[legacy]` RATHER THAN A LITERAL. An install that never
# tripped has no `sign_tripped` in its file at all, and the value it
# then lands on has to be the one `Store.load()` would have supplied -
# otherwise "absent" and "never tripped" stop being the same statement,
# which is the distinction store.py's own header is about.
value = data.get(legacy, DEFAULTS.get(legacy))
store.set(target, value)
rep.moved[target] = value
# THE POSTCONDITION, AND IT IS A READ-BACK RATHER THAN AN ASSUMPTION. This
# is the difference between this function and a two-line copy: the caller
# can ask "is the latch that was in the file before now under the key the
# watchdog reads", through the same `unit_key()` the watchdog's constructor
# reads through, and a `False` there is a guard that is off on an install
# whose state file says it is on.
for target, value in rep.moved.items():
got = getattr(store, "data", {}).get(target, "<<missing>>")
if got == value:
continue
rep.verified = False
note = f"{target} was written as {value!r} and reads back as {got!r}"
rep.problems.append(note)
legacy = target.rsplit("/", 1)[-1]
if value != DEFAULTS.get(legacy):
rep.lost.append(note)
if rep.ok:
store.set(SCHEMA_KEY, SCHEMA_VERSION)
else:
# ⚠️ NOT MARKED DONE. A migration that lost state must run again on the
# next start rather than be recorded as finished - the one thing worse
# than a failed migration is a failed migration a later version skips.
_LOG.error("per-inverter store migration LOST STATE and has not been "
"marked done: %s", rep.explain())
return rep
class Store:
def __init__(self, path: str = "/data/state.json"):