diff --git a/goodwe_controller/DECISIONS/TEL-10-publishing-the-integrator.md b/goodwe_controller/DECISIONS/TEL-10-publishing-the-integrator.md new file mode 100644 index 0000000..8dc1df6 --- /dev/null +++ b/goodwe_controller/DECISIONS/TEL-10-publishing-the-integrator.md @@ -0,0 +1,182 @@ +# TEL-10 — the loop's integrator is published, as three site rows + +**Status:** decided and implemented on branch `TEL-10`. +**Card:** TEL-10 — *"`self.i_w` is carried between cycles and published nowhere"*. +**Nothing here reached the inverter, and nothing here can.** `app/control.py` +is not touched by this branch; `compute()`, its returned `Decision` and every +constant it reads are byte-identical to `release/1.0`. The diff is a controller +recording three values it already had, and a publisher announcing three rows. + +--- + +## 0. The three things a reader should take away + +1. **The row that matters is the one that is usually indistinguishable from + `Setpoint`.** With nothing binding, the command *is* the integrator rounded + to 10 W, so on a quiet day publishing the wrong one looks correct. §2 is + the fixture that separates them by kilowatts, and §4 is the mutation that + proves the separation is what the check reads. +2. **`Loop reason` is a genuinely new sentence, not a second copy of one.** The + status page's `Why` row and `sensor.goodwe_controller_status` both carry the + *arbiter's* line (`loop -> -5000 W`), which prints the winning claim's NAME. + `compute()`'s own reason has never left the process, and `slew-limited` — the + value SAFETY-48 is about — appears in neither of the other two. §3. +3. ⚠️ **Three entities are three entities.** This is an addition to a live + install's entity list, which TEL-07 refused to make for four per-unit rows + at a roster of one. The argument for making it here is in §5, and so is the + argument that keeps it to three rather than to one per inverter. + +--- + +## 1. Why this is a card at all + +`self.i_w` has been a separate quantity from the command since SAFETY-04, which +split the accumulator out of the velocity form precisely so the two could be +bounded apart. Since then it has been: + +* carried between cycles by `Controller.cycle()`, +* read by nothing outside `compute()`, +* and written to no entity, no MQTT topic, no attribute and no log line. + +The consequence is not theoretical. The investigation behind SAFETY-48 needed to +know what the integrator was doing during the export episodes and could only +**reconstruct** it, from the grid trace plus a replay of the real `compute()`. +The reconstruction reproduced about a third of the engagements it was checked +against and was **discarded entirely** — so the card it was gathered for had to +be argued from a measurement nobody could stand behind. + +⚠️ **And the reconstruction could not have been made to work.** `i_w` at a given +instant depends on every cycle before it, including every one in which the +arbiter overrode the loop and `main.py` re-seeded `self.i_w = self.target`. That +re-seed is invisible in the grid trace. A replay can only recover the integrator +on a run where the loop won every arbitration, which is not the run anybody is +investigating. + +## 2. What is published, and against what + +Three rows, on the SITE device, appended after `release/1.0`'s eight sensors: + +| key | entity | what it is | +|---|---|---| +| `loop_i` | `sensor.goodwe_loop_integrator` | `Decision.i_w` — the accumulator AFTER this cycle, BEFORE the output clamp, the slew limit and quantisation, which is what its own docstring says it is | +| `loop_lead` | `sensor.goodwe_loop_integrator_lead` | `decision.i_w - prev_w`, where `prev_w` is the command `compute()` was handed | +| `loop_reason` | `sensor.goodwe_loop_reason` | `Decision.reason`, unmodified | + +⚠️ **`loop_lead` IS NOT A SUBTRACTION OF THE OTHER TWO ROWS, WHICH IS WHY IT IS +A ROW.** `Setpoint` is the ARBITER's output, after every other strategy has had +its say, and it is THIS cycle's; the lead is measured against the command the +PREVIOUS cycle left standing. On any cycle where the arbiter overrode the loop, +subtracting the two published rows gives a different number — and that is +exactly the cycle on which `main.py` re-seeds the integrator, i.e. the cycle a +reader most needs the real figure for. + +`cycle()` therefore records all three from the one `compute()` call, and names +the command it passes (`loop_prev`) rather than re-reading `self.target` at +publish time. `self.target` is reassigned later in the same method, so a lead +computed at publish time would subtract a different cycle's command and call the +answer a lead. ⚠️ **At the line as written the two expressions are the same +value, and the mutation that swaps them is a recorded SURVIVOR** — see §4 M2. +The name is kept for the coupling, not for a behaviour it currently buys. + +### The not-running branch + +`cycle()`'s `else` arm — stopped AND shadow mode off — already zeroes +`self.i_w`, because no accumulation may survive the outage. The three rows are +zeroed and `loop_reason` becomes `not-running` on the same branch. A row that +kept repeating the last integrator there would say the loop was winding while +nothing is being controlled. + +⚠️ **`auto: false` ALONE IS NOT THAT BRANCH.** The shipped `shadow_mode: auto` +means a stopped controller still runs the whole decision path and records it +(SAFETY-10), so the loop is live and the integrator is real. The first draft of +the check asserted `not-running` about a loop that was running; it reads the +shadow case as its own claim now. + +## 3. Why `loop_reason` is not the row that already exists + +`self.reason` is `Resolution.explain()`: + +``` +"loop -> -5000 W" # winner, watts +"loop -> -5000 W, limited by soc" # ...and which limit bit +"CONTRADICTION between limits - commanding 0 W (loop)" +``` + +`Claim.set("loop", P_LOOP, decision.target_w, decision.reason, ...)` does carry +the loop's reason INTO the arbiter — and `explain()` prints the winner's +**source name**, never the claim's reason. So the loop's own word has always +been built, handed over, and dropped one line later. + +Measured on the fixture in `test_telemetry.py` part 2, same cycle: + +``` +the arbiter's sentence: 'loop -> -5000 W' +the loop's own word : 'i-clamped' +``` + +## 4. Verified by breaking it + +Nine mutations, run in the CI image on a purged tree, each fingerprinted before, +after, and again after the revert. The table with byte deltas and red counts is +in `test_telemetry.py`'s header; two entries are worth repeating here. + +**M1 — `self.loop_i_w = loop_prev` (−3 B), the card's own named mutation: +publish the COMMAND under the integrator's name.** The first draft of the +acceptance check was *"the published integrator reaches its bound"*, and that +check was **GREEN** under M1 — because the command reaches the bound too, twelve +cycles later. MUTATION.md habit 4 in the check written to close the card. It +asserts the **lead time** now (the integrator is at the bound at least ten +cycles before the command is), and is red on 5 named checks. + +**M2 — `decision.i_w - self.target` (+2 B) is a SURVIVOR, and is reported as +one.** At that line `cycle()` has not yet executed `self.target = +resolution.target_w`, so the two expressions denote the same value and the +mutation changes nothing that runs. The run was purged, which is not the +direction stale bytecode fabricates. M2b (`decision.i_w - decision.target_w`, ++8 B) is the mutation that does change the value and is red. + +## 5. What was rejected + +**Publishing as an MQTT attribute on an existing entity.** There is no +`json_attributes_topic` anywhere in this add-on; adding one would be a second +convention for one card, and HA's recorder keeps long-term statistics for a +state with `state_class`, not for an attribute. The reason this card exists is +that nobody had history to read. + +**A status-page row instead.** The page is not recorded. `Why` already carries +the arbiter's sentence and stays that way; DOCS.md gains the section that says +how to read the three entities. + +**One integrator row per inverter.** §6.2's architecture is ONE integrator on +the P1 meter whose output is then SPLIT. A per-unit copy would invent N of a +thing there is one of, and the two copies would disagree the moment the +allocator gave the units different shares. `test_mqtt.py` part 5 grades this at +every roster length from 1 to 6, and the mutation that moves `loop_i` into +`UNIT_SENSORS` is red on 8 named checks. + +**Adding the rows at N≥2 only, the way TEL-07 gates its per-unit rows.** +TEL-07's gate exists because at a roster of one its four rows carry information +the site rows already carry exactly. These three carry information NOTHING +carries, at every roster length, including one. + +**A `## Unreleased` block in `CHANGELOG.md`.** `test_version_gate.py` part 11b +refuses a changelog whose newest `## ` heading is not `config.yaml`'s +`version:` — PKG-14's rule that the bump and the closed section are one act. +Opening a section here would mean bumping the version, which is a packaging +decision this card does not get to take. The operator-facing account is in +DOCS.md; the release note belongs to whichever PKG ticket cuts the next +version, and this file is what it should be written from. + +## 6. What this card does NOT establish + +* **It says nothing about whether the integrator's behaviour is right.** That is + SAFETY-48. This branch adds an observable and changes no decision; the + 15.7-day replay that grades the behaviour is in that card. +* **The three rows do not exist on the live install until the add-on is + updated**, and this branch does not update it. `tools/rehearsal/announced- + reference.txt` (what the BUILD announces) records them; `live-entities- + recorded.txt` (what the INSTALL has) deliberately does not, because an id the + build announces and the install lacks is a new entity, not an orphan. +* **`loop_lead` is signed and is not a distance.** Where the integrator and the + command have opposite signs it is the sum of two magnitudes. A reader wanting + "how far behind is the command" should take the magnitude and say so. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 9994a32..55e08c9 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -287,8 +287,10 @@ it always has. published — `Setpoint`, `Grid power`, `Battery power`, `Battery SoC`, `Circulating power`, `Maintenance phase`, `Controller status`, `P1 sample age` — keep their device, their `unique_id`s and therefore their **entity ids and their -whole recorded history**. At a roster of one, nothing new is announced at all: -no second device, no renamed entity, not one added topic. +whole recorded history**. At a roster of one, nothing new is announced by the +per-unit machinery at all: no second device, no renamed entity, not one added +topic. (TEL-10's three loop rows, below, are site rows and are added at every +roster length, including one.) Declare a second inverter and **four rows per unit** appear beside them: @@ -321,7 +323,53 @@ arrives, and it fails silently and looks correct. There is one P1 meter and one grid connection; a per-inverter copy would invent two of a thing there is one of, and `P1 sample age` is a safety input. Same for `Circulating power` (it is a property of the *set* of inverters), the controller -status, the maintenance phase and the capacity-tariff peak. +status, the maintenance phase, the capacity-tariff peak and the three loop rows +below. + +### Reading the grid-following loop: `Loop integrator`, `Loop integrator lead`, `Loop reason` + +The control law is a bounded integrator: an accumulator that is **not** the +command. Each cycle it moves by `gain × error`; the command is then derived +from it and passed through the output clamp, the slew limit and a 10 W +quantisation before anything is written. Three entities publish what the +accumulator is doing: + +| entity | unit | what it is | +|---|---|---| +| `Loop integrator` | W | the accumulator **after** this cycle — the value *before* the clamp, the slew limit and quantisation | +| `Loop integrator lead` | W | that value minus **the command it was computed against**, i.e. the command the previous cycle left standing | +| `Loop reason` | — | the law's own word for this cycle | + +`Loop reason` takes one of eight values: + +| value | what happened | +|---|---| +| `tracking` | ordinary regulation | +| `deadband` | the error is inside `deadband_w`; the loop is holding | +| `i-clamped` | the integrator hit `integrator_max_w` (which follows `max_w` by default) | +| `clamped` | the *command* came out over `max_w` — only reachable if `integrator_max_w` is deliberately set above the rail | +| `slew-limited` | the command could not move as far as the integrator asked for; `slew_w` is what stopped it | +| `saturated-freeze` | the inverter is not following, so the command's magnitude may fall but not rise | +| `freeze-released` | the freeze had run out of plant-shaped explanations and the command was let move again | +| `not-running` | control is stopped **and** shadow mode is off, so the loop did not run | + +⚠️ **`Loop reason` is not the `Why` row on the status page, and the two will +often disagree.** `Why` is the *arbiter's* sentence — `loop -> -1200 W` — which +names the strategy that won and the watts it won with. It has never carried the +strategy's own reason, so a loop that is slew-limited or frozen says so **only** +here. + +⚠️ **`Loop integrator` is not `Setpoint`, even though it usually looks like +it.** With nothing binding, the command *is* the integrator rounded to 10 W, so +on a quiet day the two rows track each other and the difference is a rounding +step. They separate exactly when something is wrong: a large disturbance sends +the integrator to its bound in one or two cycles while the command may only +travel `slew_w` per cycle, and `Loop integrator lead` is the gap. A standing, +large lead means the loop has wound up further than the command can follow, and +the number of cycles needed to unwind it is that lead divided by `slew_w`. + +Both numbers are `state_class: measurement`, so Home Assistant's recorder keeps +long-term statistics for them. ⚠️ **Two roster names that differ only in punctuation or case** — `inverter-b` and `Inverter B` — are two names to the validator and one id once slugged. The diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 56acb77..48519d1 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -864,6 +864,13 @@ class Controller: self.target = 0.0 self.sat_count = 0 self.i_w = 0.0 # the loop's integrator, carried between cycles + # TEL-10. What the last cycle's loop did, kept for publication. Seeded + # to the SAME values the "stopped or blind" branch of `cycle()` writes, + # because before the first cycle the loop has not run - which is the + # thing that branch is for. + self.loop_i_w = 0.0 + self.loop_i_lead = 0.0 + self.loop_reason = "not-running" self.loop_w = None # what the loop asked for last cycle, or None # Did the grid-following loop actually win the LAST arbitration? That is # the question SAFETY-02 has to ask about the command standing on the @@ -3054,8 +3061,15 @@ class Controller: # level: the two halves of §6.2's identity break in opposite # directions. See `site_batt_w` and test_alloc.py part 8. site_batt = self.site_batt_w + # TEL-10. ⚠️ ONE NAME FOR THE COMMAND THE LOOP WAS MEASURED + # AGAINST. `self.target` is reassigned further down this method, so + # a `decision.i_w - self.target` written at publish time would + # subtract a DIFFERENT cycle's command and call the answer a lead. + # The published lead is built from the value `compute()` was + # actually handed, on the line that hands it over. + loop_prev = self.target decision = compute( - prev_w=self.target, + prev_w=loop_prev, grid_w=self.grid, actual_w=site_batt, tuning=self.tuning, @@ -3068,6 +3082,41 @@ class Controller: self.sat_count = decision.sat_count self.i_w = decision.i_w self.loop_w = decision.target_w + # --- TEL-10 ------------------------------------------------------ + # ⚠️ THE INTEGRATOR HAS NEVER LEFT THIS PROCESS, AND THAT IS WHAT + # THE CARD IS. `self.i_w` is carried between cycles and published + # nowhere - no entity, no attribute, no log line - so the only way + # anyone has ever asked what it was doing is to RECONSTRUCT it from + # the grid trace and a replay. That reconstruction reproduced about + # a third of the engagements it was checked against and was thrown + # away; a card that has to be argued from a discarded reconstruction + # is a card that cannot be argued. + # + # Three rows, and the third is not derivable from the first two by + # anyone reading the dashboard afterwards: + # + # `loop_i` the integrator AFTER this cycle - `Decision.i_w`, + # which its own docstring says is the value BEFORE + # the output clamp, the slew limit and quantisation. + # It is therefore NOT `setpoint` and must never be + # published from it. + # `loop_lead` how far the integrator stands AHEAD of the command + # it was computed against. This is the quantity + # SAFETY-48 bounds, and `integrator_max_w / slew_w` + # is the number of cycles it takes to unwind. + # `loop_reason` `compute()`'s OWN reason string - `deadband`, + # `i-clamped`, `clamped`, `slew-limited`, + # `saturated-freeze`, `freeze-released`, `tracking`. + # + # ⚠️ AND `loop_reason` IS NOT `self.reason`. `self.reason` is the + # ARBITER's sentence (`resolution.explain()`, e.g. "loop -> 1200 W") + # and it does not carry the loop's own word at all: the Claim filed + # below takes `decision.reason` and `Resolution.explain()` prints the + # winner's NAME, not the winner's reason. The status page's "Why" + # row is the arbiter's sentence and stays that way. + 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`. On a default install # the integrator bound is what bites and the output clamp is @@ -3092,6 +3141,15 @@ class Controller: # Stopped or blind: no accumulation may survive the outage. self.i_w = 0.0 self.loop_w = None + # TEL-10, and it is the same zero the line above writes rather than + # a second opinion about it. ⚠️ `not-running` rather than a stale + # `tracking`: a reason row that keeps repeating the last word the + # loop said while the loop is not running is a row that reads as + # health, which is the failure `publish()`'s own None-dropping note + # is about one field over. + self.loop_i_w = 0.0 + self.loop_i_lead = 0.0 + self.loop_reason = "not-running" # A loop that is not running cannot be winding. Clearing this here # rather than letting the last value stand is what makes the alarm # de-assert when control is stopped. @@ -4288,6 +4346,14 @@ class Controller: # rather than publishing a zero that would read as health. "circulation": (None if self.site_circulating_w is None else round(self.site_circulating_w)), + # TEL-10. The loop's own three, written from the fields `cycle()` + # recorded and not re-derived here - the page and the publisher run + # on their own tasks and `self.target` has moved on by the time + # either of them looks (SAFETY-47's note on `sized_from_meter`, one + # field up, is the same hazard). + "loop_i": round(self.loop_i_w, 1), + "loop_lead": round(self.loop_i_lead, 1), + "loop_reason": self.loop_reason, } # ⚠️ ONLY when P1 ingestion is actually running. The ESP32's stale-input # watchdog subscribes to sensor.goodwe_p1_sample_age - see the entity-id diff --git a/goodwe_controller/app/mqtt.py b/goodwe_controller/app/mqtt.py index c0101f0..a3c1115 100644 --- a/goodwe_controller/app/mqtt.py +++ b/goodwe_controller/app/mqtt.py @@ -85,6 +85,36 @@ SENSORS = [ # sensor's VALUE rather than its arrival, so a dead publisher freezes the # number and reads permanently fresh. Filed as TEL-06. ("p1_age", "p1_sample_age_s", "P1 sample age", "s", "duration", "measurement", None), + # --- TEL-10: the loop's integrator, published for the first time --------- + # + # ⚠️ APPENDED, AND THE POSITION IS THE POINT. `discovery_payloads()` walks + # this list in order and `_announce()` publishes what it returns, so a row + # inserted ABOVE would move every later entity's place in the announcement. + # Nothing existing is renamed, re-typed or re-ordered by these three; they + # are three NEW retained topics after the nine that are already there, and + # test_mqtt.py part 1 grades exactly that claim. + # + # ⚠️ AND THEY ARE SITE ROWS, NOT PER-UNIT ONES. There is one integrator - + # §6.2's architecture is ONE integrator on the P1 meter whose output is then + # SPLIT - so a per-unit copy would invent N of a thing there is one of, the + # same argument `grid` and `p1_age` are site rows for. + # + # `state_class: measurement` on the two numbers is what puts them in the + # recorder's statistics, which is the whole point: the reason this card + # exists is that the last investigation had no history to read. + ("loop_i", "goodwe_loop_integrator", "Loop integrator", "W", "power", "measurement", None), + # ⚠️ NOT DERIVABLE FROM THE TWO ROWS EITHER SIDE OF IT, which is why it is + # its own row rather than a subtraction a dashboard could do. `setpoint` is + # the ARBITER's output after every other strategy has had its say, and it is + # also THIS cycle's; the lead is measured against the command `compute()` + # was handed, which is the PREVIOUS one. Subtracting the published rows + # gives a different number on every cycle the arbiter overrode the loop. + ("loop_lead", "goodwe_loop_integrator_lead", "Loop integrator lead", "W", "power", "measurement", None), + # ⚠️ NOT `sensor.goodwe_controller_status` AND NOT THE "Why" ROW. Both of + # those carry the ARBITER's sentence; this is `compute()`'s own word for + # what the control law did with this cycle - `slew-limited` is the value + # SAFETY-48 is about and it appears in neither of the other two. + ("loop_reason", "goodwe_loop_reason", "Loop reason", None, None, None, "mdi:sigma"), ] BASE = "goodwe_ctl" diff --git a/goodwe_controller/test_mqtt.py b/goodwe_controller/test_mqtt.py index fdd1bcc..5d44b92 100644 --- a/goodwe_controller/test_mqtt.py +++ b/goodwe_controller/test_mqtt.py @@ -151,6 +151,47 @@ BASELINE = [ ('goodwe_ctl/availability', 'online', True), ] +# --- TEL-10's three, and they are DELIBERATELY NOT IN `BASELINE` ------------- +# +# ⚠️ `BASELINE` IS WHAT release/1.0 PUBLISHES AND NOTHING MAY BE ADDED TO IT. +# Every strong claim in this file rests on that list being a record of another +# tree; append a row this branch invented and "byte-identical to release/1.0" +# quietly becomes "identical to itself", which is the defect MUTATION.md's +# habit 4 is about - a check whose data cannot reach the failure it names. +# +# So TEL-10's arrival is TWO claims and gets two checks: the nine release/1.0 +# messages did not move, and EXACTLY these three arrived after them. +# +# ⚠️ AND THIS LIST IS A REGRESSION PIN, NOT EVIDENCE OF CORRECTNESS - the same +# status as part 3's sha256 over the per-unit payloads, for the same reason: +# there is no released version of these bytes to diff against, so all it can +# say is "these three payloads have not changed since the PR was reviewed". +_TEL10_NUM = ('"availability_topic": "goodwe_ctl/availability", "device": ' + _DEV + + ', "unit_of_measurement": "W", "device_class": "power", ' + '"state_class": "measurement"}') +TEL10 = [ + ('homeassistant/sensor/goodwe_ctl_loop_i/config', + '{"name": "Loop integrator", "object_id": "goodwe_loop_integrator", ' + '"unique_id": "goodwe_ctl_loop_i", "state_topic": "goodwe_ctl/loop_i", ' + + _TEL10_NUM, True), + ('homeassistant/sensor/goodwe_ctl_loop_lead/config', + '{"name": "Loop integrator lead", "object_id": "goodwe_loop_integrator_lead", ' + '"unique_id": "goodwe_ctl_loop_lead", "state_topic": "goodwe_ctl/loop_lead", ' + + _TEL10_NUM, True), + ('homeassistant/sensor/goodwe_ctl_loop_reason/config', + '{"name": "Loop reason", "object_id": "goodwe_loop_reason", ' + '"unique_id": "goodwe_ctl_loop_reason", "state_topic": "goodwe_ctl/loop_reason", ' + '"availability_topic": "goodwe_ctl/availability", "device": ' + _DEV + ', ' + '"icon": "mdi:sigma"}', True), +] + +# What THIS branch announces at a roster of one: release/1.0's eight sensor +# configs, then TEL-10's three, then the availability message - which stays +# LAST, because `_announce()` publishes it after every config and a broker +# replaying the retained set must see availability arrive after the entities it +# is the availability OF. +SITE_NOW = BASELINE[:-1] + TEL10 + BASELINE[-1:] + class FakeClient: """Captures what `_announce()` actually hands paho.""" @@ -177,7 +218,7 @@ def announce(units=(), omit=(), module=mqtt): return pub.client.msgs -def diff(got, want=BASELINE): +def diff(got, want=SITE_NOW): """`(missing, changed, added)` between two announcement lists, by topic.""" g, w = dict((t, (p, r)) for t, p, r in got), dict((t, (p, r)) for t, p, r in want) missing = [t for t in w if t not in g] @@ -186,7 +227,7 @@ def diff(got, want=BASELINE): return sorted(missing), sorted(changed), sorted(added) -def report(label, got, want=BASELINE): +def report(label, got, want=SITE_NOW): m, c, a = diff(got, want) print(f" {label}: {len(want)} topics, {len(m)} missing, {len(c)} changed, " f"{len(a)} added") @@ -231,11 +272,33 @@ print("\n1. the recorded baseline, and a roster of one") # entity on a live install. m, c, a = report("roster of one (`inverters:` empty, i.e. every install today)", announce(units=roster_names({}))) -check("⭐ AT THE SHIPPED ROSTER OF ONE THE ANNOUNCEMENT IS BYTE-IDENTICAL TO " - "release/1.0: same topics, same unique_ids, same payload bytes, same " - "retain flags, in the same order - no second device, no renamed entity, " - "no added topic", - (m, c, a) == ([], [], []) and announce(units=roster_names({})) == BASELINE) +# ⚠️ THE HEADLINE SAYS EIGHT, NOT NINE, AND THE SLICE UNDER IT IS WHY. +# release/1.0 announces NINE messages; the ninth is `goodwe_ctl/availability`, +# which `_announce()` publishes LAST and which TEL-10 therefore MOVES - it is +# byte-identical but no longer at index 8. So what is asserted here is the eight +# entity CONFIGS, `[:len(BASELINE) - 1] == BASELINE[:-1]`, and the word "nine" +# above it named an assertion this file does not make. The ninth is not +# unguarded: the `== SITE_NOW` on the next check pins it byte-for-byte in its +# new last position. Two sentences, two checks. +check("⭐ AT THE SHIPPED ROSTER OF ONE EVERY ONE OF release/1.0's EIGHT ENTITY " + "CONFIGS IS STILL BYTE-IDENTICAL AND STILL FIRST: same topics, same " + "unique_ids, same payload bytes, same retain flags, in the same order - " + "no second device, no renamed entity, nothing re-ordered. TEL-10 adds " + "rows AFTER them; it may not move one of them. (release/1.0's NINTH " + "message, `availability`, is byte-identical too and deliberately moves " + "to LAST; the `== SITE_NOW` below is what pins it)", + announce(units=roster_names({}))[:len(BASELINE) - 1] == BASELINE[:-1]) + +# ⚠️ AND THE ADDITION IS ITS OWN CLAIM, WITH A COUNT. "Nothing moved" and +# "exactly three arrived" are different sentences and a single `== SITE_NOW` +# would let a fourth row hide inside whichever of the two a reader had in mind. +_m1, _c1, _a1 = diff(announce(units=roster_names({})), BASELINE) +check("⭐ TEL-10 ADDS EXACTLY THREE TOPICS AND CHANGES NONE: the integrator, " + "the lead and the loop's own reason string, all NEW, all on the SITE " + "device - nothing of release/1.0's is missing and nothing of it changed", + (_m1, _c1) == ([], []) + and sorted(_a1) == sorted(t for t, _p, _r in TEL10) + and announce(units=roster_names({})) == SITE_NOW) check("...and the roster-of-one path is the one every install actually reads: " "an empty `inverters:` names exactly one unit", @@ -244,11 +307,11 @@ check("...and the roster-of-one path is the one every install actually reads: " check("...and a publisher told NOTHING about the roster announces the same " "thing, so a caller that has not been taught about units cannot change " "what a live install sees", - announce(units=()) == BASELINE) + announce(units=()) == SITE_NOW) m, c, a = report("roster of one, P1 off (`omit=('p1_age',)`)", announce(units=roster_names({}), omit=("p1_age",)), - [row for row in BASELINE if "p1_age" not in row[0]]) + [row for row in SITE_NOW if "p1_age" not in row[0]]) check("...and the `omit` path is unchanged too: with P1 ingestion off the " "watchdog input is still not announced, and nothing else moved", (m, c, a) == ([], [], [])) @@ -280,11 +343,18 @@ check("⭐ AC 1: AT A ROSTER OF TWO, EVERY ONE OF A's NINE MESSAGES IS STILL " (m, c) == ([], [])) check("...and the eight additions are all B's and A's own per-unit rows, and " "all of them are NEW topics", len(a) == 8) -check("...and A's nine messages are still the FIRST nine, in the same order, " - "so a broker replaying retained messages sees them unchanged", +check("...and A's eight entity configs are still the FIRST eight, in the same " + "order, so a broker replaying retained messages sees them unchanged " + "(the ninth, `availability`, is byte-identical and last - the check " + "above is the one that covers it, as a diff rather than a prefix)", TWO[:len(BASELINE) - 1] == BASELINE[:-1]) -site_ids = {c_["unique_id"] for c_ in cfgs(BASELINE)} +# ⚠️ THE SITE SET IS `SITE_NOW`, NOT `BASELINE`. Read from `BASELINE` it would +# be short by TEL-10's three, and those three would then be counted as PER-UNIT +# rows by every line below that says "not a site id" - which silently changes +# what the sha256 pin, the uniqueness checks and part 7's fed/announced set are +# taken over. +site_ids = {c_["unique_id"] for c_ in cfgs(SITE_NOW)} unit_cfgs = [c_ for c_ in cfgs(TWO) if c_["unique_id"] not in site_ids] a_cfgs = [c_ for c_ in unit_cfgs if "inverter_a" in c_["unique_id"]] b_cfgs = [c_ for c_ in unit_cfgs if "inverter_b" in c_["unique_id"]] @@ -330,7 +400,7 @@ def field(rows, key): for key in ("unique_id", "object_id", "state_topic"): av, bv = set(field(a_cfgs, key)), set(field(b_cfgs, key)) - sv = set(field(cfgs(BASELINE), key)) + sv = set(field(cfgs(SITE_NOW), key)) check(f"⭐ AC 2: no `{key}` is shared between A's entities and B's, and " f"none of B's collides with a SITE entity that already exists", len(av) == 4 and len(bv) == 4 and not (av & bv) and not (bv & sv) @@ -340,10 +410,10 @@ all_ids = field(cfgs(TWO), "unique_id") check("...and across the WHOLE announcement every unique_id is distinct - an " "HA discovery collision is not an error, it DROPS the second entity, so " "one inverter's telemetry would silently stop existing", - len(all_ids) == len(set(all_ids)) == 16) + len(all_ids) == len(set(all_ids)) == 19) all_topics = [t for t, _, _ in TWO] -check("...and so is every topic", len(all_topics) == len(set(all_topics)) == 17) +check("...and so is every topic", len(all_topics) == len(set(all_topics)) == 20) # ⚠️ THE SLUG COLLISION rule 9(c) DOES NOT REFUSE. `inverter-b` and `Inverter B` # are two different names by the validator's test and one string once slugged. @@ -363,8 +433,14 @@ print("\n5. site-scoped sensors stay site-scoped") # firmware's ingest guard). A second copy of it is a second answer to "is the # meter fresh?", one of which is always the one nobody is looking at. `grid` is # the same argument without the safety edge: there is ONE grid connection. +# ⚠️ TEL-10's THREE ARE IN HERE, AND THE ARGUMENT IS `grid`'s. There is ONE +# integrator - §6.2's architecture is one integrator on the P1 meter whose +# output is then SPLIT - so a per-inverter copy of it would invent N of a thing +# there is one of, and two of them would disagree the moment the allocator gave +# the units different shares. SITE_ONLY = ("goodwe_ctl_grid", "goodwe_ctl_p1_age", "goodwe_ctl_circulation", - "goodwe_ctl_phase", "goodwe_ctl_status") + "goodwe_ctl_phase", "goodwe_ctl_status", + "goodwe_ctl_loop_i", "goodwe_ctl_loop_lead", "goodwe_ctl_loop_reason") rows = [] for n in range(1, 7): names = [f"inverter-{chr(ord('a') + i)}" for i in range(n)] @@ -377,8 +453,8 @@ check("⭐ AC 3: `grid` and `p1_sample_age_s` are announced EXACTLY ONCE at ever "two of a thing there is one of, and would duplicate a safety signal", all(counts == [1] * len(SITE_ONLY) for _n, counts, _t in rows)) check("...and the site set does not grow with the roster while the per-unit " - "set does: 8 site entities always, 4 more per unit past the first", - [t for _n, _c, t in rows] == [8, 16, 20, 24, 28, 32]) + "set does: 11 site entities always, 4 more per unit past the first", + [t for _n, _c, t in rows] == [11, 19, 23, 27, 31, 35]) site_devices = {json.dumps(c_["device"], sort_keys=True) for c_ in cfgs(announce(units=NAMES2)) @@ -420,7 +496,7 @@ for c_, eid in zip(cfgs(TWO), composed): f"name {c_['name']!r}") check("⭐ every composed entity_id at a roster of two is DISTINCT: no per-unit " "entity can land on top of a site entity that already has history", - len(composed) == len(set(composed)) == 16) + len(composed) == len(set(composed)) == 19) base_composed = [compose(c_["device"]["name"], c_["name"]) for c_ in cfgs(BASELINE)] check("...and the eight ids that exist today are unchanged by adding B, which " @@ -620,9 +696,9 @@ def properties(mod): bc = [c_ for c_ in uc if "inverter_b" in c_["unique_id"]] ac = [c_ for c_ in uc if "inverter_a" in c_["unique_id"]] comp = [compose(c_["device"]["name"], c_["name"]) for c_ in tc] - out["A byte-identical at N=1"] = base == BASELINE + out["A byte-identical at N=1"] = base == SITE_NOW out["A byte-identical at N=2"] = diff(two)[:2] == ([], []) - out["N=1 adds no topic"] = len(base) == len(none) == len(BASELINE) + out["N=1 adds no topic"] = len(base) == len(none) == len(SITE_NOW) out["unique_ids all distinct"] = ( len(set(c_["unique_id"] for c_ in tc)) == len(tc)) out["slug-alike names still distinct"] = ( @@ -703,6 +779,25 @@ MUTATIONS = [ ' ("grid", "grid_power", "Grid power", "W", "power", "measurement", None),\n')]), ("B's device drops `via_device`", [(' "via_device": DEVICE["identifiers"][0],\n', "")]), + # --- TEL-10 ------------------------------------------------------------ + ("TEL-10's integrator row is DROPPED (published to a topic nothing " + "announced, which AC 4 forbids in the other direction)", + [(' ("loop_i", "goodwe_loop_integrator", "Loop integrator", "W", "power", "measurement", None),\n', "")]), + ("TEL-10's integrator is announced PER INVERTER - two answers to a " + "question §6.2 says there is one integrator for", + [(' ("sign_latch", "sign_latch", "Sign latch", None, None, None, "mdi:flash-alert"),\n', + ' ("sign_latch", "sign_latch", "Sign latch", None, None, None, "mdi:flash-alert"),\n' + ' ("loop_i", "loop_integrator", "Loop integrator", "W", "power", "measurement", None),\n')]), + # ⚠️ THE ONE THAT CHANGES NO BYTE OF ANY PAYLOAD. Every id, name, unit and + # topic is identical; only the POSITION of the integrator row moves. A + # broker replaying the retained set then hands HA a different sequence, and + # part 1's claim is about order as much as about bytes - which is why it is + # asserted as a LIST and not as a set. + ("TEL-10's integrator row is announced BEFORE release/1.0's nine", + [(' ("loop_i", "goodwe_loop_integrator", "Loop integrator", "W", "power", "measurement", None),\n', ""), + (' ("setpoint", "goodwe_setpoint", "Setpoint", "W", "power", "measurement", None),\n', + ' ("loop_i", "goodwe_loop_integrator", "Loop integrator", "W", "power", "measurement", None),\n' + ' ("setpoint", "goodwe_setpoint", "Setpoint", "W", "power", "measurement", None),\n')]), ] survivors = [] @@ -757,11 +852,13 @@ for line in [ "resolve it, because it renames no device and no entity, and the byte-diff " "protects both spellings equally. SIZE: unresolved, 8 live entity ids; it " "wants one look at the live registry, which this branch must not take.", - "3. THE PER-UNIT PAYLOADS HAVE NO RELEASED BASELINE. A's are diffed against " - "release/1.0; B's can only be diffed against this branch (the sha256 pin in " - "part 3). A wrong `device_class` or unit on a per-unit sensor is therefore " - "caught as a CHANGE from review, not as an error. SIZE: 4 sensors x 4 " - "optional fields.", + "3. THE PER-UNIT PAYLOADS HAVE NO RELEASED BASELINE, AND NEITHER DO " + "TEL-10's THREE. A's are diffed against release/1.0; B's can only be " + "diffed against this branch (the sha256 pin in part 3), and `TEL10` above " + "is the same kind of pin for the three site rows this branch adds. A wrong " + "`device_class` or unit on any of them is therefore caught as a CHANGE " + "from review, not as an error. SIZE: 4 per-unit sensors x 4 optional " + "fields, plus 3 site sensors x 4.", "4. NO CHECK HERE RUNS A BROKER. `_announce()` is graded through a fake " "paho client, so QoS, retain semantics on a real broker, and what HA does " "with a retained config for a device it has never seen are all out of " diff --git a/goodwe_controller/test_telemetry.py b/goodwe_controller/test_telemetry.py new file mode 100644 index 0000000..b93d9ce --- /dev/null +++ b/goodwe_controller/test_telemetry.py @@ -0,0 +1,370 @@ +"""Runnable check for TEL-10: the loop's integrator, published. + +`self.i_w` is carried between cycles of `Controller.cycle()` and, before this +card, left the process nowhere at all - no entity, no MQTT attribute, no log +line. The last investigation into what it does had to RECONSTRUCT it from the +grid trace; the reconstruction reproduced about a third of the engagements it +was checked against and was discarded. + +⚠️ WHAT THIS FILE REFUSES TO DO, SAID FIRST. It does not assert that a row +EXISTS. `publish()` handing a dict with `loop_i` in it is satisfied by +`loop_i: 0.0` forever, by `loop_i: setpoint`, and by a row that reports the +command under the integrator's name - which is exactly the confusion the card +is about, because on a default install the two are within a quantisation step +of each other for most of every day. Every check below therefore drives REAL +CYCLES through the real `Controller.cycle()` until the two quantities are +MEASURABLY DIFFERENT, and grades the published number at a tick where reading +the wrong one gives the wrong answer. MUTATION.md habit 4. + +VERIFIED BY BREAKING IT. Every check below was driven by the mutation that +defeats the claim it names, in the CI image, on a tree purged of `__pycache__` +before each run, with the file's sha256 taken before, after, and again after +the revert (MUTATION.md's reporting rules, and habit 18's unique backup path). + + # mutation bytes reds file + -- ---------------------------------------------- ----- ---- ----------- + M1 `self.loop_i_w = loop_prev` - publish the -3 B 5 this file + COMMAND under the integrator's name + M2 `decision.i_w - self.target` for the lead +2 B 0 SURVIVOR + M2b `decision.i_w - decision.target_w` for the lead +8 B 1 this file + M3 `"loop_reason": self.reason` - the ARBITER's -5 B 5 this file + sentence in the loop's row + M4 delete `self.loop_reason = "not-running"` -45 B 1 this file + M5 delete the not-running branch's two zeroes -67 B 1 this file + M6 `"loop_i": round(self.target, 1)` -2 B 4 this file + M7 the `loop_i` row deleted from `SENSORS` -96 B 10+1 test_mqtt + + this file + M8 `loop_i` announced PER INVERTER +89 B 8 test_mqtt + M9 `loop_i` announced BEFORE release/1.0's nine +0 B 7 test_mqtt + +⚠️ A RED COUNT IS A PROPERTY OF THE SUITE AS MUCH AS OF THE MUTATION, SO EVERY +ONE ABOVE NAMES A TREE - this one, at this commit. A later card that adds a +check, re-baselines one or deletes one moves the number with the mutation +untouched. Re-measure rather than inherit: `tools/mutation_run.sh` on a `cp -a` +of the tree, one file at a time, and say which tree. M7's count was written as +8 and is 10 on a re-run, on both trees - the direction was right and the count +was decoration, which is the half of MUTATION.md's reporting rule 3 that is +easiest to let rot. + +⚠️ M9 IS ZERO BYTES AND CHANGES NO PAYLOAD. Every id, name, unit and topic is +identical; only the ORDER of the retained messages moves. No fingerprint and no +byte delta can see it, which is why part 1 asserts the announcement as an +ordered LIST and not as a set. + +⚠️ M2 IS A REAL SURVIVOR AND IS REPORTED AS ONE. At the line it edits, +`loop_prev` and `self.target` are the same value: `cycle()` does not reassign +`self.target` until `self.target = resolution.target_w`, further down. The +mutation therefore changes nothing that runs, on a purged tree, which is not +the direction stale bytecode fabricates. `loop_prev` is kept anyway - it is the +name the `compute()` call is made with, so the lead cannot be measured against +a different cycle's command by anybody moving a line - and M2b is the mutation +that DOES change the value, aimed at exactly that. + +⚠️ AND THE FIRST DRAFT OF THIS FILE HAD TWO CHECKS OF ITS OWN THAT COULD NOT +REACH THE FAILURE THEY NAMED. Both are MUTATION.md habit 4: + + * "the published integrator reaches its bound" was GREEN under M1, because + the command reaches the bound too - twelve cycles later. The check now + asserts the LEAD TIME, not the value. + * "a stopped controller publishes `not-running`" was GREEN under M4 and M5, + because the fixture STARTED stopped and `__init__` writes the same values. + It stops the controller part-way through a run that has already wound the + integrator to -5000 W, so a row left standing is a WRONG row. + +⚠️ MUTATING THE MODULE THIS FILE GRADES? PURGE `__pycache__` FIRST - or run +`tools/mutation_run.sh`, which does. `python3 -B` alone does NOT save you: it +stops bytecode being WRITTEN, not a stale one being READ, and a size-preserving +edit reverted inside one second is invisible to CPython's staleness test. That +is TEST-15, and it has already turned one live mutation into a false survivor. +Read MUTATION.md before you report a survivor. +""" + +import asyncio +import sys + +from app import mqtt +from app.control import Tuning, slew_limit_w +from app.main import Controller + +fails = [] + +# The live install's shape, and every number in it is measured rather than +# chosen: `max_w` and `slew_w` are read straight off the recorder (the setpoint +# sensor's extremes are exactly +-5000 and 11 226 recorded steps are exactly +# 400 W), `gain` and `target_grid_w` are what the options carry. +LIVE = {"max_w": 5000.0, "gain": 0.6, "slew_w": 400.0, "target_grid_w": -10.0, + "deadband_w": 15, "step_w": 10} +# The integrator's bound on a default install: `integrator_max_w` is None, which +# control.py resolves to `max_w`. Written as a literal on purpose (habit 2) - +# `== c.tuning.max_w` would compare the thing under test against itself. +BOUND_W = 5000.0 +# `slew_w / gain`: the error above which the integrator moves further in one +# cycle than the command is allowed to. 400 / 0.6. +OUTRUN_W = 666.67 + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +class _Store: + def __init__(self): + self.data = {} + + def set(self, *a): + pass + + def get_time(self, *a): + return None + + +class _Pub: + """Captures what `Controller.publish()` hands the publisher, every cycle.""" + + enabled = False + + def __init__(self): + self.frames = [] + + def publish(self, values): + self.frames.append(dict(values)) + + def close(self): + pass + + +class _Hass: + """A scripted meter and an inverter that follows what it is told. + + ⚠️ THE PLANT FOLLOWS EXACTLY AND WITH NO LAG, WHICH IS DELIBERATE AND IS + THE HARDER CASE FOR EVERY CHECK HERE. A lagging inverter trips the + SATURATION freeze (`|prev_w - actual_w| > saturation_w`), and the + saturation freeze already holds the integrator - so a laggy fixture would + let SAFETY-13's mechanism pass for SAFETY-48's. Nothing below may be + explained by saturation, and part 4 asserts `frozen` is False throughout. + """ + + def __init__(self, grid, soc=50.0): + self.grid_seq = grid if callable(grid) else (lambda _n: grid) + self.soc = soc + self.batt = 0.0 + self.n = 0 + self.writes = [] + self.calls = [] + + async def limits(self, entity): + return (-LIVE["max_w"], LIVE["max_w"], 1.0) + + async def number(self, entity, invert=False): + return {"sensor.grid": self.grid_seq(self.n), "sensor.soc": self.soc, + "sensor.batt": self.batt}.get(entity) + + async def set_number(self, entity, value): + self.writes.append(float(value)) + self.batt = float(value) + return True + + async def call(self, domain, service, data): + self.calls.append((domain, service, data)) + return True + + +def run(grid, cycles=30, auto=True, stop_after=None, **over): + """`cycles` real `Controller.cycle()`s, and every published frame. + + `stop_after` presses stop part-way through - which is not the same run as + starting stopped, and is the only one that can tell a row that was WRITTEN + on the not-running branch from a row that was never written at all. + """ + async def go(): + hass = _Hass(grid) + pub = _Pub() + c = Controller(dict( + {"setpoint_entity": "number.sp", "meter_entity": "sensor.grid", + "soc_entity": "sensor.soc", "batt_entity": "sensor.batt", + "meter_source": "off", "heartbeat_s": 10, "stale_input_s": 15}, + **LIVE, **over), + hass, _Store(), pub, stop=asyncio.Event()) + c.auto = auto + c.dev_min, c.dev_max = -LIVE["max_w"], LIVE["max_w"] + for k in range(cycles): + if stop_after is not None and k == stop_after: + c.auto = False + await c.read_inputs() + await c.cycle() + c.publish() + hass.n += 1 + return c, hass, pub.frames + return asyncio.run(go()) + + +# ============================================================================== +print("1. TEL-10: the integrator is published, and it is NOT the command") +# ============================================================================== +# A standing 6 kW export against a 5 kW rail and a 400 W slew step. The +# integrator moves `gain * error` = 3.6 kW on the first cycle and reaches its +# 5000 W bound on the second; the command may only move 400 W a cycle, so it is +# at 800 W when the integrator is already railed and needs twelve more cycles to +# get there. That gap is the fixture: for ten cycles the two numbers differ by +# kilowatts, and a row published from the wrong one is wrong by kilowatts. +ctl, hass, frames = run(-6000.0, cycles=30) +i_row = [f["loop_i"] for f in frames] +sp_row = [f["setpoint"] for f in frames] +lead_row = [f["loop_lead"] for f in frames] +why_row = [f["loop_reason"] for f in frames] +print(" cycle loop_i setpoint loop_lead loop_reason") +for k in range(8): + print(f" {k:<5d}{i_row[k]:8.0f} {sp_row[k]:9.0f} {lead_row[k]:10.0f}" + f" {why_row[k]}") + +first_at_bound = next((k for k, v in enumerate(i_row) if abs(v) >= BOUND_W), None) +first_cmd_at_bound = next((k for k, v in enumerate(sp_row) if abs(v) >= BOUND_W), + len(sp_row)) +print(f" the integrator reaches {-BOUND_W:.0f} W at cycle " + f"{first_at_bound}; the command reaches it at cycle " + f"{first_cmd_at_bound}") + +# ⚠️ THE LEAD TIME IS PART OF THE CLAIM, NOT A NICETY. "The published number +# reaches the bound" is satisfied by publishing the COMMAND too - it gets there +# as well, twelve cycles later - so a check written on the value alone grades +# nothing and the mutation that publishes `prev_w` walks straight through it. +# Measured: with `self.loop_i_w = loop_prev` this line is red and the value-only +# form was green. +check("⭐ THE PUBLISHED INTEGRATOR REACHES ITS BOUND, AND REACHES IT TEN CYCLES " + f"BEFORE THE COMMAND DOES. A 6 kW export winds it to exactly " + f"{BOUND_W:.0f} W - `integrator_max_w` is None, which control.py resolves " + "to `max_w` - while the command is still travelling 400 W a cycle", + first_at_bound is not None and i_row[first_at_bound] == -BOUND_W + and first_cmd_at_bound - first_at_bound >= 10) + +# ⚠️ THE PRECONDITION, AS ITS OWN CHECK (habit 1). If the command happened to be +# at the bound too, the line above would be satisfied by publishing EITHER +# number and would be grading nothing. +check("...and the command is NOWHERE NEAR it at that tick, so the two are " + f"separable: setpoint {sp_row[first_at_bound]:.0f} W against an " + f"integrator of {i_row[first_at_bound]:.0f} W", + abs(sp_row[first_at_bound]) <= BOUND_W / 2) +check("...and the gap is the slew limit and not an accident: the command is " + "still travelling 400 W a cycle while the integrator stands still", + abs(sp_row[first_at_bound + 1] - sp_row[first_at_bound]) + == slew_limit_w(Tuning(**LIVE))) + +# The lead is `i_w - prev_w`, and `prev_w` is the command the LAST cycle left +# standing - which is the setpoint this file captured one frame earlier. Graded +# as that identity rather than as a magnitude, because a magnitude cannot tell +# a lead from the integrator itself. +ident = [k for k in range(1, 12) + if round(lead_row[k], 1) != round(i_row[k] - sp_row[k - 1], 1)] +check("⭐ AND `loop_lead` IS THE INTEGRATOR MINUS THE COMMAND IT WAS COMPUTED " + "AGAINST - the PREVIOUS cycle's setpoint, not this one's. Checked as an " + f"identity over cycles 1..11 ({len(ident)} disagreements)", not ident) +check("...and it is large while the command is catching up, so the row is not " + f"a permanent zero: worst lead {min(lead_row):.0f} W", + min(lead_row) <= -2000.0) + + +# ============================================================================== +print() +print("2. TEL-10: the reason row is compute()'s OWN word, not the arbiter's") +# ============================================================================== +# ⚠️ THE TWO SENTENCES EXIST SIDE BY SIDE AND ONE OF THEM ALREADY HAD A ROW. +# `self.reason` is `Resolution.explain()` - "loop -> -800 W" - and it prints the +# winning claim's NAME, never the claim's reason. So a publisher wired to +# `self.reason` would produce a row that is never wrong-looking and never once +# says `slew-limited`, which is the value SAFETY-48 is about. +print(f" the arbiter's sentence: {ctl.reason!r}") +print(f" the loop's own word : {ctl.loop_reason!r}") +check("⭐ THE LOOP'S REASON REACHES THE WIRE, and on a slew-limited cycle it is " + "the string `slew-limited` - a value the arbiter's sentence does not " + "contain at all", + "slew-limited" in why_row and "slew-limited" not in ctl.reason) +check("...and the two rows are DIFFERENT STRINGS on the same cycle, so one " + "cannot have been published from the other", + frames[-1]["loop_reason"] != ctl.reason) +check("...and every published reason is one `compute()` can actually produce", + set(why_row) <= {"tracking", "deadband", "i-clamped", "clamped", + "slew-limited", "saturated-freeze", "freeze-released", + "not-running"}) + +# A converged site: the error is inside the deadband and the law says so. +calm = run(-10.0, cycles=6)[2] +check("...and a site resting inside the deadband publishes `deadband`, not the " + "last exciting word the loop said", calm[-1]["loop_reason"] == "deadband") + +# ⚠️ THE NOT-RUNNING PATH IS A DIFFERENT BRANCH OF `cycle()` AND IS GRADED AS +# ONE (habit 9). That branch zeroes `self.i_w`, so a row that kept repeating the +# last integrator there would say the loop was winding while nothing is being +# controlled at all. +# +# ⚠️ AND `auto: False` IS NOT THAT BRANCH, WHICH IS WHY BOTH ARE HERE. The +# shipped `shadow_mode: auto` means a STOPPED controller still runs the whole +# decision path and records it - SAFETY-10 - so the loop is live, the +# integrator is real, and a check that assumed otherwise would have been +# asserting `not-running` about a loop that was running. It took `auto: False` +# AND `shadow_mode: off` to reach the arm that zeroes it. +shadowed = run(-6000.0, cycles=6, auto=False)[2] +print(f" stopped, shadow_mode auto: status {shadowed[-1]['status']!r}, " + f"loop_i {shadowed[-1]['loop_i']:+.0f} W, " + f"reason {shadowed[-1]['loop_reason']!r}") +check("a STOPPED controller in shadow mode still publishes a live integrator - " + "the decision path runs, so the row must report it rather than a zero " + "that would read as 'nothing is winding'", + shadowed[-1]["status"] == "shadow" + and shadowed[-1]["loop_i"] < -1000.0 + and shadowed[-1]["loop_reason"] != "not-running") + +# ⚠️ AND IT IS STOPPED PART-WAY, NOT STARTED STOPPED, WHICH IS THE WHOLE +# DIFFERENCE. A controller that never ran carries `__init__`'s values, so a run +# that begins stopped is satisfied by a branch that writes NOTHING - measured: +# deleting `self.loop_reason = "not-running"` from `cycle()` outright was a +# -45 B SURVIVOR of the first draft of this check. The loop has to wind first, +# so that a row left standing is a row that says the wrong thing. +stopped = run(-6000.0, cycles=12, stop_after=6, shadow_mode="off")[2] +print(f" while running : loop_i {stopped[5]['loop_i']:+.0f} W, " + f"reason {stopped[5]['loop_reason']!r}, status {stopped[5]['status']!r}") +print(f" after stopping : loop_i {stopped[-1]['loop_i']:+.0f} W, " + f"reason {stopped[-1]['loop_reason']!r}, status {stopped[-1]['status']!r}") +check("...and the precondition: the loop really was winding before stop was " + "pressed, so a stale row afterwards would be a WRONG row and not merely " + "an unset one", + stopped[5]["loop_i"] <= -1000.0 + and stopped[5]["loop_reason"] != "not-running") +check("⭐ AND ONCE THE DECISION PATH IS GENUINELY OFF THE ROWS SAY SO: " + "`not-running` and a zero integrator - the same zero `cycle()` writes to " + "`self.i_w` on that branch, not the number it was carrying a cycle ago", + stopped[-1]["status"] == "stopped" + and stopped[-1]["loop_reason"] == "not-running" + and stopped[-1]["loop_i"] == 0.0 and stopped[-1]["loop_lead"] == 0.0) + + +# ============================================================================== +print() +print("3. TEL-10: announced and published are the same three topics") +# ============================================================================== +# mqtt.py's `omit` note: an entity that exists and is never fed is a worse +# signal than one that does not exist. This is that claim for TEL-10's rows, +# taken off the SHIPPED announcement and the SHIPPED `publish()`. +announced = {t.rsplit("/", 2)[-2].replace(f"{mqtt.BASE}_", "") + for t, _ in mqtt.discovery_payloads()} +published = set(frames[-1]) +TEL10 = {"loop_i", "loop_lead", "loop_reason"} +print(f" announced keys: {sorted(announced & TEL10)}") +print(f" published keys: {sorted(published & TEL10)}") +check("all three rows are announced AND published - no entity nothing feeds, " + "no topic nothing declared", + TEL10 <= announced and TEL10 <= published) +check("...and they are on the SITE device, not per unit - there is one " + "integrator", + all(c["device"] == mqtt.DEVICE + for t, c in mqtt.discovery_payloads(["inverter-a", "inverter-b"]) + if c["unique_id"].replace(f"{mqtt.BASE}_", "") in TEL10)) + +print() +if fails: + print(f"{len(fails)} FAILED:") + for f in fails: + print(f" - {f}") + sys.exit(1) +print("all checks passed") diff --git a/goodwe_controller/tools/rehearsal/announced-reference.txt b/goodwe_controller/tools/rehearsal/announced-reference.txt index 7e2aa5a..6157e3a 100644 --- a/goodwe_controller/tools/rehearsal/announced-reference.txt +++ b/goodwe_controller/tools/rehearsal/announced-reference.txt @@ -38,6 +38,14 @@ # # Editing this file is how you say "the announcement changed on purpose". Do not # edit it to make a check pass. +# +# ⚠️ TEL-10 ADDED THE LAST THREE ROWS, AND THEY ARE ADDITIONS RATHER THAN A +# RE-COMPOSITION. Nothing above them moved: same entity_ids, same unique_ids, +# same device. They are recorded here - the BUILD's reference - and deliberately +# NOT in `live-entities-recorded.txt`, because the install does not have them +# yet and an id the build announces that the install lacks is a NEW entity, not +# an orphan. `test_mqtt.py` part 1 grades the same two claims on the payload +# bytes. sensor.goodwe_setpoint goodwe_ctl_setpoint goodwe_rs485_controller sensor.goodwe_grid_power goodwe_ctl_grid goodwe_rs485_controller sensor.goodwe_battery_power goodwe_ctl_battery goodwe_rs485_controller @@ -46,3 +54,6 @@ sensor.goodwe_circulating_power goodwe_ctl_circulation goodwe_rs485_controlle sensor.goodwe_maintenance_phase goodwe_ctl_phase goodwe_rs485_controller sensor.goodwe_controller_status goodwe_ctl_status goodwe_rs485_controller sensor.goodwe_p1_sample_age goodwe_ctl_p1_age goodwe_rs485_controller +sensor.goodwe_loop_integrator goodwe_ctl_loop_i goodwe_rs485_controller +sensor.goodwe_loop_integrator_lead goodwe_ctl_loop_lead goodwe_rs485_controller +sensor.goodwe_loop_reason goodwe_ctl_loop_reason goodwe_rs485_controller