diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index c6f91fb..854a322 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -60,6 +60,7 @@ phase having charged nothing. | `step_w` | 10 | Quantisation | | `saturation_w` | 500 | Divergence that counts as "the inverter is at a limit" | | `saturation_cycles` | 3 | How many consecutive cycles before freezing. **Do not set to 1** | +| `integrator_max_w` | 3000 | Bound on the loop's accumulator, separate from `max_w`. Caps how much stale error can be waiting to unwind when the sign flips. **Keep it above `max_w`, and do not set it equal to `max_w`** | | `heartbeat_s` | 10 | Refresh interval; must stay well under the firmware watchdog | | `stale_input_s` | 15 | How long inputs may be missing before commanding 0 W | | `auto_start` | false | Start controlling on boot (only after commissioning) | diff --git a/goodwe_controller/app/control.py b/goodwe_controller/app/control.py index 0830b60..f12ae7d 100644 --- a/goodwe_controller/app/control.py +++ b/goodwe_controller/app/control.py @@ -28,6 +28,16 @@ class Tuning: step_w: int = 10 saturation_w: float = 500.0 saturation_cycles: int = 3 + # ⚠️ The integrator's OWN bound, and deliberately not max_w. A commercial + # controller on this same site clamped only its output and still reported + # 14 768 W: with the inverter switched off its integrator climbed ~130 W + # every 4 s past 10 kW while the output sat on the 5 kW rail, so the moment + # the error flipped there were minutes of accumulated wind to burn off + # before the command moved at all. Bounding the accumulator is what makes + # recovery time finite; bounding the output only hides it. + # Headroom above max_w is wanted (a legitimate large error must not be + # truncated at the rail), headroom without limit is the bug. + integrator_max_w: float = 3000.0 # What the meter should rest at, in W. Negative = a slight export. # ⚠️ The deadband is a one-way ratchet: any resting point inside it holds # forever, and the meter's IMPORT register counts every positive one with @@ -44,6 +54,10 @@ class Decision: sat_count: int frozen: bool reason: str + # The integrator AFTER this cycle, pre-clamp-to-max_w. Carry it back in as + # `i_w` next cycle; that is what keeps it a separate quantity from the + # command, which is the whole point of the bound above. + i_w: float = 0.0 def compute( @@ -52,12 +66,14 @@ def compute( actual_w: float, tuning: Tuning, sat_count: int = 0, + i_w: float | None = None, ) -> Decision: """One control cycle. A cycle is one meter update (~5 s on a HomeWizard P1). `prev_w` what we last commanded `grid_w` net grid power, + = importing `actual_w` what the inverter reports it is doing, + = discharging + `i_w` the carried integrator, or None to seed it from `prev_w` """ reason = "tracking" @@ -71,6 +87,16 @@ def compute( # on EVERY large correction, because the plant itself needs 3-6 s to settle # while a cycle is ~5 s. Requiring N consecutive saturated cycles is what # lets slew be larger than saturation_w. + # + # The spec states this window twice and differently: "> 10 s" (§11.2) and + # "3 samples" (§10.3). Cycles are authoritative here because this function + # has no clock - it is driven one cycle per meter update by run_control(), + # which only calls cycle() when the meter value changes. At the ~5 s + # HomeWizard P1 cadence the default 3 cycles is ~15 s, i.e. the stricter + # reading of the two. On a faster meter it is not, so saturation_cycles is + # configurable and must be raised to keep the window over 10 s. + # ponytail: a seconds-based window would mean plumbing wall-clock or dt + # into a pure function whose whole value is that it has neither. saturated_now = abs(prev_w - actual_w) > tuning.saturation_w sat_count = min(sat_count + 1, 10) if saturated_now else 0 frozen = sat_count >= tuning.saturation_cycles @@ -83,11 +109,36 @@ def compute( # 70 s" is how a healthy loop is recognised at a glance, and a command # frozen where it should not be is how two real bugs were caught. error = grid_w - tuning.target_grid_w + + # --- the integrator ---------------------------------------------------- + # This loop is in velocity form: the accumulator IS the commanded power, so + # for years "the integrator" and "the output" were one variable and could + # not be bounded apart. `i_w` is that accumulator made explicit. A caller + # that passes nothing gets the old behaviour exactly - seeded from the last + # command every cycle - and main.py carries it instead, which is what turns + # the two clamps below into two independent limits. + if i_w is None: + i_w = float(prev_w) + if abs(error) < tuning.deadband_w: - want = prev_w reason = "deadband" else: - want = prev_w + tuning.gain * error + step_i = tuning.gain * error + # ⚠️ Freeze means "may not wind FURTHER", not "may not move". A strict + # freeze would strand the command at whatever it had reached until the + # inverter started tracking again - and the inverter is not tracking, + # that is what saturation means, so nothing would ever release it. The + # unwind direction is the escape route and stays open; the same rule is + # applied again to the output below. + if not frozen or abs(i_w + step_i) < abs(i_w): + i_w = i_w + step_i + + # ⚠️ Applied EVERY cycle, frozen or not, and before the output clamp: the + # freeze is conditional, this bound is not. Order matters only in that the + # command below is derived from the already-bounded integrator, so no + # accumulated value can reach the wire even once. + i_w = max(-tuning.integrator_max_w, min(tuning.integrator_max_w, i_w)) + want = i_w # ⚠️ Maintenance shaping (charge-only, cheap-window floor) used to live # here. It now belongs to arbiter.py as limit claims, so that precedence @@ -123,7 +174,7 @@ def compute( step = max(1, int(tuning.step_w)) target = round(target / step) * step - return Decision(float(target), sat_count, frozen, reason) + return Decision(float(target), sat_count, frozen, reason, float(i_w)) def maintenance_charge_floor( diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 1e2750e..e9128fc 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -70,6 +70,7 @@ class Controller: step_w=int(opts.get("step_w", 10)), saturation_w=float(opts.get("saturation_w", 500)), saturation_cycles=int(opts.get("saturation_cycles", 3)), + integrator_max_w=float(opts.get("integrator_max_w", 3000)), ) self.maint = Maintenance( MaintConfig( @@ -89,6 +90,8 @@ class Controller: self.auto = bool(store.data.get("auto", opts.get("auto_start", False))) self.target = 0.0 self.sat_count = 0 + self.i_w = 0.0 # the loop's integrator, carried between cycles + self.loop_w = None # what the loop asked for last cycle, or None self.reason = "starting" self.grid = self.soc = self.batt = None self.peak_fc = None @@ -231,17 +234,36 @@ class Controller: # wish. If something outranked the loop, that is what the hardware # actually did, and the controller must track reality or it jumps # the moment it regains control. + # + # ⚠️ 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. + # Re-seed from what the hardware was actually told. This is the + # failsafe case too - every layer-1 stop resolves to 0 W, so + # entering failsafe re-seeds the integrator to zero and the first + # cycle after release starts from zero instead of dumping the whole + # stale period as power. + if self.loop_w is None or self.target != self.loop_w: + self.i_w = self.target decision = compute( prev_w=self.target, grid_w=self.grid, actual_w=self.batt, tuning=self.tuning, sat_count=self.sat_count, + i_w=self.i_w, ) if decision.frozen and self.sat_count < self.tuning.saturation_cycles: self.log_event(f"saturation freeze ({self.target:.0f} W vs {self.batt:.0f} W)") self.sat_count = decision.sat_count + self.i_w = decision.i_w + self.loop_w = decision.target_w claims.append(Claim.set("loop", P_LOOP, decision.target_w, decision.reason)) + else: + # Stopped or blind: no accumulation may survive the outage. + self.i_w = 0.0 + self.loop_w = None resolution = resolve(claims) if resolution.contradiction: diff --git a/goodwe_controller/config.yaml b/goodwe_controller/config.yaml index 30892ec..e50a5fc 100644 --- a/goodwe_controller/config.yaml +++ b/goodwe_controller/config.yaml @@ -48,6 +48,7 @@ options: step_w: 10 saturation_w: 500 saturation_cycles: 3 + integrator_max_w: 3000 heartbeat_s: 10 stale_input_s: 15 auto_start: false @@ -88,6 +89,7 @@ schema: step_w: int(1,100) saturation_w: int(100,2000) saturation_cycles: int(1,10) + integrator_max_w: int(100,15000) heartbeat_s: int(2,25) stale_input_s: int(5,120) auto_start: bool diff --git a/goodwe_controller/test_control.py b/goodwe_controller/test_control.py index 478629b..3a85516 100644 --- a/goodwe_controller/test_control.py +++ b/goodwe_controller/test_control.py @@ -73,6 +73,70 @@ check("freeze still allows magnitude to fall", d.target_w < 2000) d = compute(prev_w=0, grid_w=7, actual_w=0, tuning=Tuning(deadband_w=1, step_w=10)) check("quantised to step_w", d.target_w % 10 == 0) +print("SAFETY-04: the integrator is bounded apart from the output") + +# The historical runaway, with its real numbers. A commercial controller on +# this site, with the inverter switched OFF, wound ~130 W every 4 s past 10 kW +# and reported 14 768 W while its output clamp sat at 5 kW. At gain 0.6 that +# rate is a standing error of 130/0.6 ≈ 217 W that never resolves, because the +# inverter is not there to resolve it. 150 cycles is past the ~113 it took to +# reach 14 768 W at that rate. +RUNAWAY_ERROR = 130.0 / 0.6 +RUNAWAY_CYCLES = 150 +HISTORICAL_W = 14768.0 + + +def runaway(tuning): + """Inverter off: it reports 0 W forever, the error never clears.""" + prev, i_w, sat = 0.0, 0.0, 0 + worst_i, worst_cmd = 0.0, 0.0 + for _ in range(RUNAWAY_CYCLES): + d = compute(prev_w=prev, grid_w=RUNAWAY_ERROR, actual_w=0.0, + tuning=tuning, sat_count=sat, i_w=i_w) + prev, i_w, sat = d.target_w, d.i_w, d.sat_count + worst_i = max(worst_i, abs(i_w)) + worst_cmd = max(worst_cmd, abs(prev)) + return worst_i, worst_cmd + + +TR = Tuning(max_w=2000, integrator_max_w=3000) +wi, wc = runaway(TR) +check(f"runaway: integrator plateaus at {wi:.0f} W (<= 3000)", wi <= TR.integrator_max_w) +check(f"runaway: emitted command peaks at {wc:.0f} W (<= 2000)", wc <= TR.max_w) +check("runaway: nowhere near the historical 14 768 W", wc < HISTORICAL_W / 4) + +# ...and with the saturation detector deliberately defeated, so that only the +# clamp is holding. This is the AC that says the two mechanisms are +# independent: kill one, the other still bounds it. +TD = Tuning(max_w=2000, integrator_max_w=3000, saturation_w=1e9) +wi, wc = runaway(TD) +check(f"runaway with the detector defeated: integrator still <= 3000 ({wi:.0f} W)", + wi <= TD.integrator_max_w) +check("runaway with the detector defeated: command still <= max_w", wc <= TD.max_w) + +# The bound is not max_w. If someone "simplifies" them into one key this fails. +d = compute(prev_w=0, grid_w=6000, actual_w=0, + tuning=Tuning(max_w=2000, integrator_max_w=3000, slew_w=5000)) +check("integrator bound is separate from the output clamp", + d.i_w == 3000 and d.target_w == 2000) + +# Freeze = does not accumulate. Same input twice; the integrator must not move. +TF = Tuning(saturation_w=500, saturation_cycles=3) +f1 = compute(prev_w=2000, grid_w=800, actual_w=0, tuning=TF, sat_count=3, i_w=2000.0) +check("frozen: integration does not accumulate", f1.i_w == 2000.0 and f1.frozen) +f2 = compute(prev_w=2000, grid_w=-800, actual_w=0, tuning=TF, sat_count=3, i_w=2000.0) +check("frozen: unwinding is still allowed", f2.i_w < 2000.0) + +# False-positive guard: a normal 2 kW load step must not trip the detector, +# because the plant needs several cycles to catch up on every one of them. +prev, actual, sat, i_w, froze = 0.0, 0.0, 0, 0.0, False +for _ in range(12): + d = compute(prev, 2000.0 - actual, actual, T, sat, i_w) + prev, sat, i_w = d.target_w, d.sat_count, d.i_w + actual = actual + 0.94 * (prev - actual) + froze = froze or d.frozen +check("a normal 2 kW load step does not trip the saturation freeze", not froze) + print("capacity tariff") check("no forecast means no cap", maintenance_charge_floor(2500, None, 3500) == 2500) check("headroom caps the charge", maintenance_charge_floor(2500, 2000, 3500) == 1500)