From 47e99a37bc2dedfb4e36b95d4a80e1a2cf1156ee Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Sun, 8 Nov 2026 18:06:00 +0100 Subject: [PATCH] MAINT-20 part 2 r2: app/main.py comments, lines 4001-4500 (_shadow_settle .. check_output_clamp) Co-Authored-By: Claude --- goodwe_controller/app/main.py | 197 ++++++++++++++++++++++------------ 1 file changed, 127 insertions(+), 70 deletions(-) diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 37cea67..7e43d71 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -4027,8 +4027,9 @@ class Controller: if len(self.inverters) > 1: probe = any(_probing.values()) # ⚠️ `actual_w` IS THE SITE SUM AND THAT IS DEV-02's OTHER HALF. - # The anti-windup test inside compute() is `abs(prev_w - actual_w) - # > saturation_w`, and `prev_w` is the SITE target. + # The anti-windup test in compute() is `abs(prev_w - actual_w) > + # saturation_w` and `prev_w` is the SITE target; one inverter's + # power at N=2 would trip the saturation freeze permanently. # (DECISIONS/MAIN-orchestration-archive.md) site_batt = self.site_batt_w # SAFETY-103: what each battery can be doing (its reading may be @@ -4085,14 +4086,20 @@ class Controller: if len(self.inverters) == 1: self._refusal_update(decision, loop_prev, site_batt, probe, rejudged) - # TEL-10 ⚠️ THE INTEGRATOR HAS NEVER LEFT THIS PROCESS, AND THAT - # IS WHAT THE CARD IS. - # AND `loop_reason` IS NOT `self.reason`. + # TEL-10. ⚠️ THE INTEGRATOR HAS NEVER LEFT THIS PROCESS: `loop_i` + # is `Decision.i_w`, the value BEFORE the clamp, slew and + # quantisation, so it is NOT `setpoint` and must not be published + # from it; `loop_lead` is how far it stands ahead of the command; + # `loop_reason` is `compute()`'s own word and NOT `self.reason`. # (DECISIONS/MAIN-orchestration-archive.md) self.loop_i_w = decision.i_w self.loop_i_lead = decision.i_w - loop_prev self.loop_reason = decision.reason - # SAFETY-03 ⚠️ READ THE BOOLEANS, NOT `decision.reason`. + # SAFETY-03. ⚠️ READ THE BOOLEANS, NOT `decision.reason`: `reason` + # is overwritten by slew-limiting or the saturation freeze after + # the clamp, so a string test is silent on the cases a winding loop + # most often presents as. SAFETY-40: every number comes from THIS + # decision, at clamp time. # (DECISIONS/MAIN-orchestration-archive.md) self.loop_clamped = ( decision.i_clamped or decision.clamped, @@ -4133,8 +4140,11 @@ class Controller: # stale from this line onwards. Incremented BEFORE resolve() so that the # record resolve() builds is stamped with the tick it is for. self.alloc_tick += 1 - # ⚠️ `weights=` IS OPT-09, AND ITS ABSENCE WAS THE CARD. - # A METHOD, NOT A DICT. + # ⚠️ `weights=` IS OPT-09, AND ITS ABSENCE WAS THE CARD. A METHOD, NOT + # A DICT: the weighting is by ENERGY HEADROOM, so it depends on the + # direction (the sign of the site target, chosen inside `resolve()`; + # see `pack_weights()`). ⚠️ A PREFERENCE, NOT A PERMISSION: a weight + # cannot stop a unit being commanded. # (DECISIONS/MAIN-orchestration-archive.md) _live = self.live_units() while True: @@ -4142,8 +4152,9 @@ class Controller: weights=self.pack_weights, tick=self.alloc_tick, reserve=self._held_back()) # SAFETY-73: a probe pin stands only while the resolved site target - # asks at least PROBE_W its way. - # (DECISIONS/MAIN-orchestration-archive.md) + # asks at least PROBE_W its way; below that a sibling would carry + # the opposite direction (circulation), so the pin goes back to the + # closed limit. (DECISIONS/MAIN-orchestration-archive.md) _void = [k for k, c in enumerate(claims) if c.source == "refused" and c.kind == "set" and c.value * resolution.target_w < _ctl.PROBE_W ** 2] @@ -4157,10 +4168,10 @@ class Controller: self.allocation = resolution.allocation if self.modes_on: self._mode_fault_log(resolution) - # THE TWO REFUSALS, AND THEY ARE NOT THE SAME REFUSAL Both do the same - # thing - REPLACE the resolution with a 0 W failsafe which is then - # written like any other cycle at `write_setpoint(self.target)` below. - # AND NEITHER MAY BE LEFT UNGRADED. + # THE TWO REFUSALS ARE NOT THE SAME REFUSAL: both REPLACE the + # resolution with a 0 W failsafe, written like any other cycle; they + # differ in WHO refused, hence two branches and two messages. ⚠️ + # NEITHER MAY BE LEFT UNGRADED (test_peakguard.py part 10h). # (DECISIONS/MAIN-orchestration-archive.md) if not resolution.allocated: self.log_event("ARBITER: " + resolution.allocation.explain(), @@ -4172,9 +4183,12 @@ class Controller: scope_errors=resolution.scope_errors, allocation=resolution.allocation, winner_priority=resolution.winner_priority) - # 2. THE ALLOCATOR SUCCEEDED AND THIS CONTROLLER CANNOT DELIVER IT. - # ⚠️ RE-AIMED BY DEV-02, NOT DELETED, AND THE DIFFERENCE MATTERS. - # (DECISIONS/MAIN-orchestration-archive.md) + # 2. THE ALLOCATOR SUCCEEDED AND THIS CONTROLLER CANNOT DELIVER IT. ⚠️ + # RE-AIMED BY DEV-02, NOT DELETED: a roster entry with no writer would + # have the site target written to the others (over-command by its + # share); "nothing" here means 0 W. Not folded into `live_units()`: a + # dead link is dropped and announced, an undeliverable roster stops the + # site. (DECISIONS/MAIN-orchestration-archive.md) elif {u for u in resolution.shares if not self._can_write(u)}: undeliverable = ", ".join( sorted(u for u in resolution.shares if not self._can_write(u))) @@ -4184,7 +4198,10 @@ class Controller: f"setpoint entity for {undeliverable} - refusing to command a " f"partial site", hint=messages.hint("no_setpoint")) # ⚠️ `shares={}`, NOT THE SHARES IT REFUSED TO DELIVER, AND SINCE - # DEV-02 THAT IS LOAD-BEARING RATHER THAN TIDY. + # DEV-02 THAT IS LOAD-BEARING: the write loop commands + # `shares.get(name, 0.0)`, so carrying the split through would + # write the numbers this branch declined, or nothing at all when + # they equal what stands (an unchanged value is not re-sent). # (DECISIONS/MAIN-orchestration-archive.md) resolution = Resolution(0.0, "failsafe", None, False, resolution.considered, @@ -4194,8 +4211,9 @@ class Controller: allocation=resolution.allocation, winner_priority=resolution.winner_priority) # 3. AC 6: A DROPPED INVERTER IS NEVER SILENT. The survivors carry its - # share, which is a real change in what the site can deliver, and an - # operator who is not told reads a working site. + # share, a real change in what the site can deliver, and an operator + # who is not told reads a working site. Edge-triggered on the SET of + # dropped units (the action log is a log of events, §15). # (DECISIONS/MAIN-orchestration-archive.md) _dropped = tuple(u for u, _why in (self.allocation.dropped if self.allocation else ())) @@ -4210,7 +4228,10 @@ class Controller: hint=messages.hint("units_back")) self._dropped_last = _dropped # 3b. OPT-09 / IF-04: WATTS NOBODY COULD TAKE ARE NEVER SILENT EITHER. - # `unmet_w` is the site target the envelopes could not absorb. + # `unmet_w` is the site target the envelopes could not absorb; not a + # broken allocation (fusing it with `ok` would let one saturated + # inverter park the site), but the site failing to deliver what the + # loop asked. Edge-triggered on the VALUE. # (DECISIONS/MAIN-orchestration-archive.md) _unmet = int(self.allocation.unmet_w if self.allocation else 0) if _unmet != self._unmet_last: @@ -4225,24 +4246,27 @@ class Controller: self._unmet_last = _unmet if resolution.scope_errors: # A claim reached the arbiter on an actuator its source's scope - # does not permit. (DECISIONS/MAIN-orchestration-archive.md) + # does not permit: a BUG in the filing site, not a state to survive + # (a per-inverter constraint in the site list is the `lo > hi` this + # card removes). Loud, every cycle; the resolution is unchanged. + # (DECISIONS/MAIN-orchestration-archive.md) _LOG.error("A limit was filed on the wrong inverter, which is a " "software fault. %s.", " / ".join(resolution.scope_errors)) if resolution.contradiction: self.log_event("ARBITER CONTRADICTION - commanding 0 W, see the log", hint=messages.hint("clash")) - # SAFETY-02's arming gate Was the number about to go on the wire - # chosen by the grid-following loop, or by a strategy that commands - # against the meter on purpose? - # THE EXPRESSION LIVES IN watchdog.py, NOT HERE, and that is not - # tidiness. - # (DECISIONS/MAIN-orchestration-archive.md) + # SAFETY-02's arming gate: was the number chosen by the grid-following + # loop, or by a strategy that commands against the meter on purpose? + # Only the former is expected to agree in sign with P1. ⚠️ THE + # EXPRESSION LIVES IN watchdog.py, NOT HERE: read loop_is_following() + # before touching the gate. (DECISIONS/MAIN-orchestration-archive.md) self.following = loop_is_following(resolution, self.loop_w) self.target = resolution.target_w self.reason = resolution.explain() - # UX-08: the same answer in plain words, for the page's "Why" line. + # UX-08: the same answer in plain words, for the page's "Why" line; # `reason` stays the technical record the log and the checks read. - # (DECISIONS/MAIN-orchestration-archive.md) + # UX-10: every key `why_key()` can return has words in + # messages.WHY_WINNER. (DECISIONS/MAIN-orchestration-archive.md) _why = why_key(resolution.winner, claims) if _why == "inputs missing" and not resolution.contradiction: # UX-11: past the hold, say which reading is missing. @@ -4253,10 +4277,12 @@ class Controller: _why, resolution.target_w, resolution.bound_by, contradiction=resolution.contradiction, raised=why_raised(resolution, claims)) - # DEV-02: EACH INVERTER IS TOLD ITS OWN SHARE ⚠️ ITS OWN SHARE, NEVER - # THE SITE TARGET. - # AT N=1 THIS IS BYTE-IDENTICAL. - # (DECISIONS/MAIN-orchestration-archive.md) + # DEV-02: EACH INVERTER IS TOLD ITS OWN SHARE, NEVER THE SITE TARGET + # (handing each unit `self.target` is the ARCH-01 fault: each supplies + # it in full and the gain at the meter becomes N times the per-unit + # gain). At N=1 this is byte-identical. ⚠️ A unit not in `shares` is + # commanded 0 W, not left standing: an inverter holds its last command + # for ever. (DECISIONS/MAIN-orchestration-archive.md) self._loop_alarm_input(resolution) writes = [] for inv in self.inverters: @@ -4264,13 +4290,19 @@ class Controller: writes.append(self.write_setpoint(inv.target, unit=inv)) await asyncio.gather(*writes) - # SAFETY-03, and its boundary is the WRITE ⚠️ NOT THE SAME BOUNDARY AS - # SAFETY-18's BELOW, THOUGH THE CALLS ABUT. + # SAFETY-03, and its boundary is the WRITE. ⚠️ NOT THE SAME BOUNDARY AS + # SAFETY-18's BELOW, THOUGH THE CALLS ABUT: this one reads + # `self.clamp_last`, None until write_setpoint() has run, so it must + # follow the write (test_clamp.py part 6). Two guards, two orderings: + # do not "tidy" them into one rule. # (DECISIONS/MAIN-orchestration-archive.md) await self.check_output_clamp() - # SAFETY-18, and its boundary is the ASSIGNMENT ⚠️ THE BOUNDARY IS - # `self.target = resolution.target_w` ABOVE, NOT write_setpoint(). + # SAFETY-18, and its boundary is the ASSIGNMENT. ⚠️ THE BOUNDARY IS + # `self.target = resolution.target_w` ABOVE, NOT write_setpoint(): both + # terms of the conjunction must describe the SAME moment, so moving it + # above the assignment grades the previous cycle's command against this + # cycle's meter (test_freeze.py part 5). # (DECISIONS/MAIN-orchestration-archive.md) await self.check_meter_frozen() @@ -4280,17 +4312,17 @@ class Controller: await self.check_price_gap() await self.check_fixed_note() - # SAFETY-31's alarm, and its boundary is the LATCH, not a number - # Unconstrained where the two above are not: it grades - # `self.circ_wd.tripped`, which was advanced near the top of this - # method beside SAFETY-02 and which nothing since has touched. + # SAFETY-31's alarm, and its boundary is the LATCH, not a number: it + # grades `self.circ_wd.tripped`, advanced near the top of this method. + # Called on EVERY cycle, not only the one that trips, because its + # second job is re-raising the alarm after a restart. # (DECISIONS/MAIN-orchestration-archive.md) await self.check_circulating() - # SAFETY-42's alarm, and its boundary is the LATCH, like SAFETY-31's It - # grades `inv.delivery_wd.tripped`, which was advanced near the top of - # this method beside SAFETY-02 and SAFETY-31 and which nothing since - # has touched. (DECISIONS/MAIN-orchestration-archive.md) + # SAFETY-42's alarm, and its boundary is the LATCH, like SAFETY-31's: + # it grades `inv.delivery_wd.tripped`, and is called on EVERY cycle + # because its second job is re-raising the alarm after a restart. + # (DECISIONS/MAIN-orchestration-archive.md) await self.check_not_delivering() # --- SAFETY-07's alarm, and it is NOT the engagement ------------------ @@ -4298,31 +4330,41 @@ class Controller: # method assigned above and nothing since has touched. await self.check_peak_saturated() - # SAFETY-24's alarm, and it is the OTHER way the money gets spent Same - # boundary argument as check_peak_saturated(): it grades - # `self.peak_guard`, which cycle() assigned above and nothing since has - # touched. (DECISIONS/MAIN-orchestration-archive.md) + # SAFETY-24's alarm, and it is the OTHER way the money gets spent. Same + # boundary argument as check_peak_saturated(); kept as its own method + # because the two states are mutually exclusive and want OPPOSITE + # operator actions (saturated: shed load; this: the pack is at its + # floor and the quarter is not being shaved). + # (DECISIONS/MAIN-orchestration-archive.md) await self.check_peak_floor_limited() - # TARIFF-01, and it is LAST on purpose Banking a quarter is - # bookkeeping about a moment that has already happened; a slow SD card - # must never sit between the meter and the setpoint. - # (DECISIONS/MAIN-orchestration-archive.md) + # TARIFF-01, and it is LAST on purpose: banking a quarter is + # bookkeeping about a moment that has passed, and a slow SD card must + # never sit between the meter and the setpoint. It grades neither + # `target` nor `standing`. (DECISIONS/MAIN-orchestration-archive.md) self.peak.sync(self.p1) - # SAFETY-10, AND IT IS LAST FOR A REASON THAT IS NOT TIDINESS Every - # guard above has now read `self.target` - the resolved intent - and - # SAFETY-18 in particular grades exactly that. + # SAFETY-10, AND IT IS LAST FOR A REASON THAT IS NOT TIDINESS: every + # guard above has read `self.target`, the resolved intent (SAFETY-18 + # grades exactly that); re-seeding before them would hand each a 0 W no + # decision produced. See `_shadow_settle()`. # (DECISIONS/MAIN-orchestration-archive.md) self._shadow_settle(resolution) def _shadow_settle(self, resolution) -> None: - """SAFETY-10. Record the decision, then re-seed the carried command. - WHY THE RE-SEED IS A SAFETY PROPERTY AND NOT BOOKKEEPING. + """SAFETY-10. Record the decision, then re-seed the carried command + (`self.target`, each `inv.target`) to `shadow.WIRE_W`, in this order + because the second destroys the first's evidence. ⚠️ THE RE-SEED IS A + SAFETY PROPERTY: compute's slew base, the sign watchdogs + (`standing_w()`) and SAFETY-42 read the carried command as a statement + about the HARDWARE; a shadow decision there would latch a trip that + survives into live operation. One log line per CHANGE of decision. (DECISIONS/MAIN-orchestration-archive.md)""" engaged = self.shadow if engaged != self._shadow_was: - # ⚠️ `first` EXISTS SO A LIVE INSTALL'S FIRST CYCLE IS SILENT. + # ⚠️ `first` EXISTS SO A LIVE INSTALL'S FIRST CYCLE IS SILENT (a + # "shadow mode off" line at every start pushed a clamp episode out + # of the 40-row log); coming up IN shadow mode always gets a line. # (DECISIONS/MAIN-orchestration-archive.md) first = self._shadow_was is None self._shadow_was = engaged @@ -4352,11 +4394,16 @@ class Controller: self._shadow_winner = None return self.shadow_would_w = float(resolution.target_w) - # SAFETY-47. ⚠️ NO LOGIC HERE, DELIBERATELY. + # SAFETY-47. ⚠️ NO LOGIC HERE, DELIBERATELY: all three rungs live in + # `arbitrage.sized_from_meter()`; a rung written at a call site is one + # no check can reach with the other two clear (TEST-16). # (DECISIONS/MAIN-orchestration-archive.md) self.shadow_sized_from_meter = arbitrage.sized_from_meter( self.arb, self.arb_headroom_w, winner=resolution.winner) - # ⚠️ THE WINNER AND THE ROUNDED WATTS, NOT THE WATTS ALONE. + # ⚠️ THE WINNER AND THE ROUNDED WATTS, NOT THE WATTS ALONE (a loop + # target that that wanders by a watt would be the whole log). ⚠️ + # SAFETY-47 ADDS THE CAVEAT FLAG TO THE EDGE: the same winner at the + # same watts with and without the caveat are two different claims. # (DECISIONS/MAIN-orchestration-archive.md) edge = (resolution.winner, round(float(resolution.target_w)), self.shadow_sized_from_meter) @@ -4373,7 +4420,9 @@ class Controller: def _loop_alarm_input(self, resolution) -> None: """SAFETY-85. The loop's clamp counts only on a cycle the loop decided - (a rating may cut it). (DECISIONS/MAIN-orchestration-archive.md)""" + (a rating may cut it). A battery another owner set to discharge counts + when it gave less than it was told for `saturation_cycles` cycles in a + row. DECISIONS/SAFETY-85-no-false-not-keeping-up.md""" d = self._loop_decision if d is None: self._short_run = 0 @@ -4416,8 +4465,11 @@ class Controller: self.loop_clamp_facts = () def _settling(self, owns, now) -> bool: - """SAFETY-119: does the loop's bound on this cycle count? - (DECISIONS/MAIN-orchestration-archive.md)""" + """SAFETY-119: does the loop's bound on this cycle count? Not within a + settle (HANDBACK_CYCLES cycles and SHORT_SPAN_S) after another owner + handed the wire back; across all hand-backs the bound is hidden on at + most ROLLING_BOUND_CYCLES cycles in any ROLLING_S. + DECISIONS/SAFETY-121-bound-every-quarter.md""" def aged(t0, span): return now is None or t0 is None or now - t0 >= span @@ -4444,11 +4496,16 @@ class Controller: return True async def check_output_clamp(self) -> None: - """SAFETY-03. Alarm whenever a clamp actually engaged this cycle. - PUBLISHING IS NOT IN THE WRITE PATH. - (DECISIONS/MAIN-orchestration-archive.md)""" - # ⚠️ THE EVIDENCE IS PER UNIT AND THE ALARM IS NOT. - # (DECISIONS/MAIN-orchestration-archive.md) + """SAFETY-03. Alarm whenever a clamp actually engaged this cycle. Runs + AFTER write_setpoint() (the clamp is arithmetic done inside the write). + ⚠️ PUBLISHING IS NOT IN THE WRITE PATH: a stalled supervisor must never + delay or drop a frame. ⚠️ THIS IS NOT A TRIP: no claim, no latch, 0 W + is not commanded (§11.2: clamp and alarm); turning it into one is a + deliberate change. (DECISIONS/MAIN-orchestration-archive.md)""" + # ⚠️ THE EVIDENCE IS PER UNIT AND THE ALARM IS NOT: `clamp_last` is on + # the Inverter, `ClampWatchdog` is one per controller (an anti-flood + # counter), so any unit that clamped is an engagement and the reason + # names it. (DECISIONS/MAIN-orchestration-archive.md) clamped_units = [inv for inv in self.inverters if inv.clamp_last is not None and inv.clamp_last.engaged] engaged_write = bool(clamped_units)