From d298f8aa4fc6f475ed62ec960a34d2e63e59f728 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 3501-4000 (module level) Co-Authored-By: Claude --- goodwe_controller/app/main.py | 216 +++++++++++++++++++++------------- 1 file changed, 133 insertions(+), 83 deletions(-) diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 994f6b7..37cea67 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -3520,7 +3520,9 @@ class Controller: and abs(float(standing[inv.name])) <= _ctl.PROBE_W and abs(float(inv.batt) - inv.idle_w) < _ctl.REFUSED_BELOW_W), - # ⚠️ THIS UNIT'S OWN CUT-OUT. + # ⚠️ THIS UNIT'S OWN CUT-OUT (`bms_floor`): not the shipped + # 10 % and not the site's `peak_soc_floor`, which bounds a + # money-saving discharge only. # (DECISIONS/MAIN-orchestration-archive.md) soc=inv.soc, bms_floor=inv.bms_soc_floor)) for inv in self.inverters], @@ -3529,17 +3531,14 @@ class Controller: was_short = inv.delivery_wd.tripped if inv.delivery_wd.evaluate(_delinquent.get(inv.name), now) \ and not was_short: - # REASON FIRST, FLAG SECOND, and each `set` writes the file - - # the same atomicity ordering the three trips above use: a crash - # between the two must not persist a latch with no explanation - # for the operator who has to decide about clearing it. - # - # ⚠️ AND THERE IS NO FLAT MIRROR OF THIS PAIR, WHERE SAFETY-02 - # HAS ONE. store.py's rule 2 keeps the flat keys "so a rollback - # still finds its latch", and the version being rolled back TO - # drives exactly ONE actuator - a roster at which this check is - # inert by construction. A flat mirror would hand a downgraded - # add-on a latch taken on evidence about a roster it cannot have. + # REASON FIRST, FLAG SECOND, and each `set` writes the file (as + # the three trips above): a crash between must not persist a + # latch with no explanation. ⚠️ NO FLAT MIRROR OF THIS PAIR, + # unlike SAFETY-02: store.py's rule 2 keeps flat keys so a + # rollback finds its latch, but the version rolled back TO + # drives one actuator, where this check is inert; a mirror + # would hand it a latch about a roster it cannot have. + # DECISIONS/MAIN-orchestration.md. inv.store_set("delivery_reason", inv.delivery_wd.reason) inv.store_set("delivery_tripped", True) self._delivery_announced.discard(inv.name) @@ -3565,7 +3564,9 @@ class Controller: # ⚠️ `any`, NOT the sole unit's. The hold exists so a single missed # poll does not become a fault; a LATCHED inverter must still reach the # claim list below, because its `failsafe` SET is what parks it. - # (DECISIONS/MAIN-orchestration-archive.md) + # SAFETY-30's site latch takes the roster past the hold too (its + # `sign_site` SET parks every inverter): the trip would stop acting the + # moment a poll was missed. (DECISIONS/MAIN-orchestration-archive.md) if not self.inputs_ok and not any(i.sign_wd.tripped for i in self.inverters) \ and not (n_units > 1 and self.site_sign_wd.tripped): @@ -3589,9 +3590,11 @@ class Controller: claims = [] - # safety limits: these bind EVERY strategy, always Defence in depth: - # write_setpoint() clamps to the device range too. - # (DECISIONS/MAIN-orchestration-archive.md) + # safety limits: these bind EVERY strategy, always. Defence in depth: + # write_setpoint() clamps to the device range too, and the duplication + # is deliberate. ⚠️ PER-INVERTER, ONE PAIR PER UNIT from THAT unit's + # own min/max and supervised cap: there is no site rating + # (arbiter.CLAIM_SCOPE). (DECISIONS/MAIN-orchestration-archive.md) for inv in self.inverters: if inv.dev_min is not None and inv.dev_max is not None: claims.append(Claim.limit("device", P_SAFETY, inv.dev_min, inv.dev_max, "rating", @@ -3600,10 +3603,12 @@ class Controller: # setpoint entity (§11.3), not a bound on the grid connection. claims.append(Claim.limit("supervised", P_SAFETY, -inv.max_w, inv.max_w, "max_w", actuator=self.actuator_of("supervised", inv))) - # OPT-09: WHAT THIS PACK'S STATE OF CHARGE FORBIDS ⚠️ THE POINT OF - # THE CARD, AND IT IS AN ENVELOPE RATHER THAN A WEIGHT. - # AT A ROSTER OF ONE THIS IS NEVER FILED, AND THAT IS THE SAME - # IDENTITY `sign_site` IS GUARDED BY RATHER THAN A SPECIAL CASE. + # OPT-09: WHAT THIS PACK'S STATE OF CHARGE FORBIDS. ⚠️ AN ENVELOPE, + # NOT A WEIGHT: a zero weight does not stop `allocator._fill()` + # commanding a pack (at or under its BMS cut-out `hi = 0`, at 100 % + # `lo = 0`). ⚠️ NEVER FILED AT N=1 (it would bound the site target: + # SAFETY-25 Option A), so there nothing here stops the loop + # discharging into the BMS cut-out; the BMS does. # (DECISIONS/MAIN-orchestration-archive.md) if n_units > 1: _pe = packshare.envelope(pack_energy_of(inv)) @@ -3615,9 +3620,11 @@ class Controller: # sum: what this site can deliver, not what one of its inverters can. max_w = self.site_max_w - # safety stops ⚠️ SAFETY-02 is claimed FIRST and unconditionally while - # latched. - # AND IT IS PER-INVERTER, THOUGH ITS EVIDENCE IS SITE-WIDE. + # safety stops. ⚠️ SAFETY-02 is claimed FIRST and unconditionally while + # latched, a SET at P_SAFETY, and re-filed EVERY cycle (claims are not + # sticky). ⚠️ AND IT IS PER-INVERTER, THOUGH ITS EVIDENCE IS SITE-WIDE: + # at N>=2 the evidence cannot be pinned on a sibling this controller + # never commanded (arbiter.CLAIM_SCOPE). # (DECISIONS/MAIN-orchestration-archive.md) for inv in self.inverters: if inv.sign_wd.tripped: @@ -3625,29 +3632,35 @@ class Controller: "sign disagreement", actuator=self.actuator_of("failsafe", inv))) - # SAFETY-30: THE SITE SIGN TRIP, AND IT IS A SITE SET ⚠️ THE TWIN OF - # THE LOOP ABOVE, NOT A DUPLICATE OF IT. - # AND THE `n_units > 1` GUARD IS NOT REDUNDANT, EVEN THOUGH `cycle()` - # never arms this detector at a roster of one. - # (DECISIONS/MAIN-orchestration-archive.md) + # SAFETY-30: THE SITE SIGN TRIP, A SITE SET: the SUM of every standing + # command against the meter, which no single inverter owns; parking one + # unit on it would leave the rest pushing the same way. ⚠️ The `n_units + # > 1` guard is not redundant: the latch is restored from the store, + # and an install reconfigured back to one inverter must come up + # driving. (DECISIONS/MAIN-orchestration-archive.md) if n_units > 1 and self.site_sign_wd.tripped: claims.append(Claim.set("sign_site", P_SAFETY, 0.0, "site sign disagreement", actuator=self.actuator_of("sign_site"))) - # SAFETY-31: WHAT "ACTED ON" MEANS, AND WHAT IT HONESTLY CANNOT ⚠️ A - # SITE SET AT P_SAFETY: STOP COMMANDING, EVERYWHERE. - # THE OBJECTION. + # SAFETY-31: ⚠️ A SITE SET AT P_SAFETY: STOP COMMANDING, EVERYWHERE. A + # circulating loop needs two participants and one is this controller: + # stopping it halves the loop and makes the residue visible at P1. It + # does NOT stop a wedged inverter (HW-01); the remaining actuator is a + # human at the isolator. A site claim: circulating power is zero for + # any one unit (arbiter.CLAIM_SCOPE). # (DECISIONS/MAIN-orchestration-archive.md) if self.circ_wd.tripped: claims.append(Claim.set("circulation", P_SAFETY, 0.0, "circulating power", actuator=self.actuator_of("circulation"))) - # SITE, both of them, and the two reasons are why. "stopped" is the - # operator switching control off for the whole site; "inputs missing" - # is the SITE's own telemetry gone - the meter this controller - # regulates against. (DECISIONS/MAIN-orchestration-archive.md) + # SITE, both of them: "stopped" (the operator switching control off) + # and "inputs missing" (the meter gone) are not about a pack. ⚠️ + # SAFETY-10: `and not self.shadow` is the one failsafe this card + # changes (a stop claim would record "stopped" on every shadow cycle); + # the wire gate in `_write_unit()` is stronger. + # (DECISIONS/MAIN-orchestration-archive.md) if not self.auto and not self.shadow: claims.append(Claim.set("safety", P_SAFETY, 0.0, "stopped", actuator=self.actuator_of("safety"))) @@ -3661,11 +3674,13 @@ class Controller: claims.append(Claim.set("safety", P_SAFETY, 0.0, "readings late", actuator=self.actuator_of("safety"))) - # SAFETY-07: the quarter-peak projection guard ONE `lo` limit at - # P_PEAK, and peakguard.py carries the whole argument. - # `frozen` IS EVALUATED ON THIS CYCLE'S STANDING COMMAND, NOT ON - # `self.freeze_wd.active`. - # (DECISIONS/MAIN-orchestration-archive.md) + # SAFETY-07: the quarter-peak projection guard: ONE `lo` limit at + # P_PEAK; peakguard.py carries the argument. ⚠️ `frozen` is evaluated + # on THIS cycle's standing command, not `self.freeze_wd.active` (a + # cycle behind). ⚠️ `achievable_w` keeps the floor from filing `lo > + # hi` (0 W). ⚠️ Filed EVERY cycle (claims are not sticky). ⚠️ NOT RUN + # WITHOUT P1: it would be blind, and blind means "no charging"; the + # report is NOT COVERED. (DECISIONS/MAIN-orchestration-archive.md) self._peak_proj = (None if not self.p1_enabled else self.peak.project(self.p1.averager, now, roll=self.modes_on)) @@ -3698,16 +3713,27 @@ class Controller: # (DECISIONS/MAIN-orchestration-archive.md) achievable_w=self.site_rating_w, # ⚠️ SAFETY-24. `pack_floor_soc` IS A MEASURED READING AND MAY BE - # None, and it is handed over as None rather than defaulted here. + # None, and it is handed over as None rather than defaulted (a + # defaulted 100 % is a full battery, the value that disarms this + # floor); `decide()` treats None as floor-limited. # (DECISIONS/MAIN-orchestration-archive.md) soc=self.pack_floor_soc, # ⚠️ SAFETY-32. THE SAME OPTION, REDUCED OVER THE PACKS' OWN - # CUT-OUTS. (DECISIONS/MAIN-orchestration-archive.md) + # CUT-OUTS: `binding_soc_floor()` takes the MAXIMUM with each + # pack's `bms_soc_floor`, pairing with the MINIMUM in + # `pack_floor_soc` (both move towards refusing a shave). + # "Unreachable" is a claim about rule ordering, and sim/inject.py + # does not run rule 8: asserted over the whole schema range in + # test_peakguard.py part 12. + # (DECISIONS/MAIN-orchestration-archive.md) soc_floor=peakguard.binding_soc_floor(self.packs, float(self.o.get( "peak_soc_floor", peakguard.PEAK_OPTION_DEFAULTS["peak_soc_floor"]))), max_hold_s=self.p1.averager.max_hold_s, - # ⚠️ SAFETY-20. (DECISIONS/MAIN-orchestration-archive.md) + # ⚠️ SAFETY-20. The device range goes in TOO: `standing` is + # `last_write`, the value AFTER `clamp_to_rating()`, so a rail from + # bare `max_w` is a ceiling the device cannot reach. + # (DECISIONS/MAIN-orchestration-archive.md) frozen=looks_frozen(getattr(self.p1_source, "unchanged_s", None), site_standing, max_w, self.freeze_wd.cfg, dev_min=self.site_dev_bound("dev_min"), @@ -3721,13 +3747,18 @@ class Controller: self.peak_guard = self._peak_landing(self.peak_guard) self.peak_guard = peakguard.cap_floor( # SAFETY-128 self.peak_guard, self.answer.peak_cap(site_standing)) - # SITE, and peakguard.claim() says so on the object it builds. - # (DECISIONS/MAIN-orchestration-archive.md) + # SITE, and peakguard.claim() says so on the object it builds: the + # quantity is the 15-minute average OFFTAKE AT THE GRID CONNECTION, + # which the capacity tariff bills (one quarter-hour peak per site, not + # per pack). ⚠️ It is also the `lo` half of SAFETY-25 §0c's table: a + # site `lo` that the deferred loop reserve's per-inverter `hi` must + # never cross. (DECISIONS/MAIN-orchestration-archive.md) peak_claim = (peakguard.claim(self.peak_guard) if self.peak_guard is not None else None) if peak_claim is not None: claims.append(peak_claim) - # ⚠️ EDGE-LOGGED, not level-logged. + # ⚠️ EDGE-LOGGED, not level-logged: the action log holds 40 lines, and + # a line every 15 s for a quarter would push the cause off the end. # (DECISIONS/MAIN-orchestration-archive.md) engaged_now = self.peak_guard is not None and self.peak_guard.engaged if engaged_now != self._peak_engaged: @@ -3741,9 +3772,13 @@ class Controller: "peak_reserve" if _pg.floor_limited else "peak_saturated" if _pg.saturated else "peak_on")) - # OPT-04: price arbitrage (§10.1 priority 3) ONE `set` claim at - # P_ARBITRAGE, and app/arbitrage.py carries the whole argument. - # IT IS PLACED AFTER THE PEAK BLOCK BECAUSE IT READS ITS CONCLUSION. + # OPT-04: price arbitrage (§10.1 priority 3): ONE `set` claim at + # P_ARBITRAGE; arbitrage.py carries the argument. ⚠️ AFTER THE PEAK + # BLOCK BECAUSE IT READS ITS CONCLUSION (`engaged_now`). ⚠️ The + # three-state collapse lives in arbitrage.guard_state(), NOT here (None + # = the guard never ran, without P1). ⚠️ NO FETCH ON THIS PATH. ⚠️ The + # uncertainty set is NOT empty (no weather forecast is built). + # TARIFF-11: `arb_at` keeps the status row honest. # (DECISIONS/MAIN-orchestration-archive.md) self.arb, self.arb_at = None, now self.arb_headroom_w = None @@ -3753,7 +3788,10 @@ class Controller: # same headroom), and this site-wide trade is not run. site_trade = arbitrage.enabled(self.o) and not self.modes_on if site_trade or self.modes_on: - # SAFETY-47. HOISTED OUT OF THE CALL, NOT COPIED. + # SAFETY-47. HOISTED OUT OF THE CALL, NOT COPIED: one evaluation + # feeds both the sizing below and `arbitrage.sized_from_meter()`, + # so the caveat on the recorded watts is never answered from a + # different cycle's guard. # (DECISIONS/MAIN-orchestration-archive.md) self.arb_headroom_w = peakguard.charge_room_w(self.peak_guard, self._peak_proj) @@ -3764,7 +3802,10 @@ class Controller: self.fresh_tariff_plan(now), now, peak_guard_active=arbitrage.guard_state(self.peak_guard), # SAFETY-45. THE SAME GUARD'S SECOND ANSWER, AND THE CONTINUOUS - # ONE. (DECISIONS/MAIN-orchestration-archive.md) + # ONE: the watts the billed quarter can still afford clamp the + # buy leg (a switch alone gave 28 charge onsets an hour on + # TEST-23). ⚠️ It reads the peak block's conclusion, so it must + # stay after it. (DECISIONS/MAIN-orchestration-archive.md) charge_headroom_w=self.arb_headroom_w, # The emptiest pack again, and for the sell side that is the # right reduction. ⚠️ THE BUY SIDE WANTS THE FULLEST, and there @@ -3775,8 +3816,12 @@ class Controller: soc=self.pack_floor_soc, min_spread_eur_kwh=arbitrage.threshold_eur_kwh(self.o), uncertainty=frozenset({Uncertainty.NO_WEATHER}), - # OPT-09. PER-PACK SIZING. - # (DECISIONS/MAIN-orchestration-archive.md) + # OPT-09. PER-PACK SIZING, not `max_w`: the trade is sized to + # the packs whose own state of charge permits that direction + # (`trade_power_w()`). ⚠️ At N=1 both sides are `max_w` + # exactly. ⚠️ On the sell side this is a no-op today (`soc=` + # below is the EMPTIEST pack); test_packshare.py part 9 grades + # it. (DECISIONS/MAIN-orchestration-archive.md) charge_w=charge_w, discharge_w=discharge_w, soc_floor_pct=floor_pct, soc_ceiling_pct=ceiling_pct) # SITE, and arbitrage.claim() says so on the object it builds. It @@ -3801,25 +3846,25 @@ class Controller: # STAYS SHORT. (DECISIONS/MAIN-orchestration-archive.md) for inv, result in zip(self.inverters, maint_results): if result.owns_setpoint: - # PER-INVERTER. §12's monthly recalibration is a PACK procedure - # - drain THIS pack to its floor, charge THIS pack full, - # recalibrate THIS pack's coulomb counter. Two packs - # recalibrate on two schedules. - # FILED ON THE PACK WHOSE STATE MACHINE PRODUCED IT - `inv`, - # the roster entry this `result` was ticked from - and that is - # the whole of MAINT-05 at this line. + # PER-INVERTER: §12's recalibration is a PACK procedure, filed + # on the pack whose state machine produced it (`inv`, + # MAINT-05). ⚠️ IT STAYS `SCOPE_UNIT`, never a site claim + # (ARCH-01: per-inverter limits filed site-wide are the + # cross-inverter `lo > hi`). # (DECISIONS/MAIN-orchestration-archive.md) claims.append(Claim.set("maintenance", P_MAINTENANCE, float(result.setpoint_w or 0.0), result.phase, actuator=self.actuator_of("maintenance", inv))) self.sat_count = 0 elif result.charge_only: - # Money outranks the maintenance schedule: while the - # quarter-hour projection is over the cap, the charge-only - # shaping is simply not claimed, so the loop can discharge and - # shave the peak. - # THE CONJUNCTION LIVES IN peakguard.py, NOT HERE, and that is - # not tidiness. + # Money outranks the maintenance schedule: while the projection + # is over the cap the charge-only shaping is not claimed, so + # the loop can shave. ⚠️ Shaping is `hi <= 0` and the peak + # claim `lo >= 0`; both filed give `lo > hi`, which resolves to + # 0 W in the quarter that sets the bill, so the higher claimant + # SUPPRESSES the lower. ⚠️ THE CONJUNCTION LIVES IN + # peakguard.shaping_suspended(), NOT HERE. One suspension flag + # per SITE (`arbitrate()` allows one pack in CHARGE). # (DECISIONS/MAIN-orchestration-archive.md) if peakguard.shaping_suspended( self.peak_guard, self.peak_fc, self.peak_aim_w()): @@ -3838,7 +3883,9 @@ class Controller: reason = "charge-only" if self.cheap: # ⚠️ CFG-03. `self.maint.cfg.charge_w`, NOT A SECOND - # READ OF `self.o`. + # READ OF `self.o`: a literal fallback here let the + # shaping and the state machine use different rates + # from the same install. # (DECISIONS/MAIN-orchestration-archive.md) floor = maintenance_charge_floor( self.maint.cfg.charge_w, @@ -3853,10 +3900,12 @@ class Controller: actuator=self.actuator_of("maintenance", inv))) - # OPT-11: each battery's mode, a soft unit limit at P_MODE Filed only + # OPT-11: each battery's mode, a soft unit limit at P_MODE. Filed only # with battery modes on, and never on a battery in any maintenance - # phase: maintenance at 60/40 outranks the mode. - # (DECISIONS/MAIN-orchestration-archive.md) + # phase (maintenance outranks the mode). The range always holds 0 W + # (packmode.mode_range()) and gives way to the peak floor only as far + # as needed, so the two never meet in `lo > hi` + # (arbiter._unit_modes()). (DECISIONS/MAIN-orchestration-archive.md) if self.modes_on: tick = self._mode_roles(now, maint_results) for inv, result in zip(self.inverters, maint_results): @@ -3870,8 +3919,11 @@ class Controller: actuator=self.actuator_of("mode", inv))) - # the grid-following loop ⚠️ SAFETY-10: `or self.shadow`. - # (DECISIONS/MAIN-orchestration-archive.md) + # the grid-following loop. ⚠️ SAFETY-10: `or self.shadow`: the loop is + # the leg a shadow run most needs; it is safe because the loop's + # `prev_w` is `self.target`, re-seeded to what is on the wire. + # SAFETY-52: a refusal learned while control ran does not survive a + # stop or a start. (DECISIONS/MAIN-orchestration-archive.md) if bool(self.auto) != self._refusal_auto: self._refusal_auto = bool(self.auto) self._refusal_end("control started" if self.auto @@ -3886,10 +3938,11 @@ class Controller: self._unit_refusals() _probing = {inv.name: self._unit_probe(inv) for inv in self.inverters if inv.refused} - # SAFETY-73: each refusing unit's own P_SAFETY claim A hard unit limit - # that always intersects (OPT-11's P_MODE layer sits inside it): closed - # in the refused direction, so the allocator hands the share to the - # others within their own limits. + # SAFETY-73: each refusing unit's own P_SAFETY claim: a hard unit limit + # that always intersects (OPT-11's P_MODE sits inside it), closed in + # the refused direction, so the allocator hands the share to the others + # within their own limits; a pin at PROBE_W during its probe. Filed + # before the loop so OPT-11's loop bound sees it. # (DECISIONS/MAIN-orchestration-archive.md) for inv in self.inverters: if not inv.refused: @@ -3942,13 +3995,10 @@ class Controller: _live96 = set(self.live_units()) self._loop_start = {} if (self.auto or self.shadow) and self.inputs_ok: - # ⚠️ prev is the ARBITER's last output, not the loop's own last - # wish. - # The integrator has to track the same reality, and it is no longer - # prev_w, so it needs saying out loud: if the arbiter did not give - # the loop what it asked for last cycle, the loop's accumulated - # error belongs to a command that never happened. - # (DECISIONS/MAIN-orchestration-archive.md) + # ⚠️ prev is the ARBITER's last output, not the loop's own wish: if + # something outranked the loop, that is what the hardware did. The + # integrator re-seeds from the same (entering failsafe re-seeds it + # to zero). (DECISIONS/MAIN-orchestration-archive.md) moved = self.loop_w is None or self.target != self.loop_w if moved: self.i_w = self.target