goodwe-addon/goodwe_controller/DECISIONS/TEL-11-pv-production.md

# TEL-11 — the PV production channel

*What was decided, what it was decided against, and what the evidence was.*

## 0. The problem, in OPT-10's numbers

Over sixteen days at the reference site the pack reached 90 % SoC on **two** of
them, and **20.5 kWh of surplus left the site while the pack still had room** —
**12.6 kWh** of it with the battery **not charging at all**. That single
measurement is upstream of most of what else was found that week: the nightly
floor, SAFETY-37's misdiagnosis, the SAFETY-57 deadlock, a 2 h 47 min saturation
freeze.

It cannot be **attributed**, because **PV production is recorded nowhere in this
system**. Three causes need three different fixes:

| cause | fix |
|---|---|
| there was no irradiance | none — the site behaved correctly |
| production was there, charge power was limited | a limit, somewhere |
| production was there, the pack had room, the optimiser declined | **the only software defect** |

Guessing between them is how an optimiser gets tuned to solve a cloud problem.
**This card adds the channel and does not do the attribution.** Ship it, let it
record a week, OPT-10 picks it up with data.

## 1. It is an HA entity, not a register — and that is a product decision

The RS485 bus this add-on speaks carries the **battery** inverter it drives. On
nearly every site the PV inverter is a different box from a different vendor,
reporting into Home Assistant through its own integration; it is **not** on this
bus and there is no reason it would be. So the signal comes from an entity:
`pv_entity` takes whatever `sensor.*` the site's PV integration provides, any
brand HA can read.

Reading a register would have been cheaper on one house and wrong for a fleet —
the trade [[product-not-one-house]] names.

## 2. The sign convention: **positive is production**, and there is no `pv_invert`

> Reversed by TEL-14 (flag and negative caution): `DECISIONS/TEL-14-pv-sign.md`.

### The convention is derived, not assumed

Two sign rules already ship, and both are recorded in `DOCS.md`'s *Sources*
table rather than inferred here:

* `meter_entity` — **positive must mean importing** (`meter_invert` flips it);
* `batt_entity` — **`+` = discharging** (`batt_invert` flips it).

Both are *"positive = power arriving at the house's AC bus"*. Production arrives
there too, so **positive = producing** is the continuation of the shipped
convention rather than a fresh choice.

### Why an invert flag would be over-building

The question is not "could a sensor be reversed" — it could. It is what a flag
would **buy**, and the answer is nothing, for two reasons that do not apply to
the two keys it would sit beside:

1. **Those two quantities are read by the control path; this one is read by
   nothing.** `inv.batt` feeds SAFETY-30's per-unit sign check and SAFETY-31's
   circulating-power sensor (`_read_unit()`'s own docstring says so), so a
   reversed battery sensor hands the site a plausible number it then **acts**
   on — a latch that trips when it should not, or does not trip when it should.
   `pv_w` reaches no decision at all (§4), so a reversed PV sensor cannot
   mislead one.
2. **Production is unipolar, so the error is self-announcing.** Battery power
   and grid power are legitimately negative half the time, which is exactly why
   a flipped sign is invisible in them. A PV array does not consume: a reversed
   sensor reads **−4 kW at noon**, in the first day of the very history this card
   exists to create. An invert flag would let an installer *hide* that by
   flipping it to something that looks right, and a second place for the sign to
   be wrong is a second thing to get wrong.

So: the convention is documented in `config.yaml`, both translations, `DOCS.md`
and `app/pv.py`, the value is published **exactly as the sensor reports it**,
and there is no flag. `test_pv.py` part 1 pins `-8 W` through unchanged.

### And no negative-value alarm either

The tempting middle path — warn when the reading is negative — needs a
threshold, because a PV inverter's own standby draw makes a small negative
reading at night entirely ordinary. Any threshold here would be a number nobody
measured, in a card whose whole subject is not fabricating numbers. The
published row is the detector.

## 3. Absence is never zero, and it is not "dropped" either

This is the constraint that shaped the publication, and the obvious
implementation fails it.

`MqttPublisher.publish()` **skips** a `None` value. That is right for
`circulation` — its note in `main.py` says a missing per-unit reading should
leave the last figure standing rather than publish a zero that reads as health —
and it is **exactly wrong here**: a skipped row leaves Home Assistant holding
the last number it was told, *forever*, and the recorder then draws a **flat
line at that value through the outage**. Read back a week later that is
indistinguishable from a real measurement, and if the last value happened to be
small it manufactures precisely the *"there was no surplus to store"* conclusion
the card exists to **test**.

So TEL-11 publishes the absence, using Home Assistant's own mechanism:

> `homeassistant/components/mqtt/const.py`: `PAYLOAD_NONE = "None"`.
> `homeassistant/components/mqtt/sensor.py`, in the state-message handler:
> `if payload == PAYLOAD_NONE: self._attr_native_value = None; return` — and it
> is tested **before** the numeric branch, so it holds for a row carrying
> `device_class: power`.
> (github.com/home-assistant/core, branch `dev`, retrieved 2026-09-24. The
> shipped MQTT-sensor documentation says the same thing in its `state_topic`
> entry: *"a 'None' value will set the sensor to an unknown state"*.)

The entity therefore goes to **`unknown`** and the recorder stores a **gap**.
`mqtt.ABSENT_AS_UNKNOWN` is a one-key set and `test_pv.py` part 3 asserts both
that the pv row gets this treatment **and that `circulation` still does not** —
widening the rule silently would be a behaviour change to another card's row.

⚠️ **What was NOT built, and why.** A per-entity `availability` block
(`availability_mode: all`, an extra retained topic) would also produce a
distinguishable state — `unavailable` rather than `unknown` — but it needs a
second topic, a second retained message, a change to `_payload()`'s
byte-order-sensitive key construction, and it survives restarts only if the
retention is right. The payload route is one branch in `publish()` and one
constant, and it is graded as **bytes on the topic** rather than as a claim
about HA's availability handling.

### Three states, not two

| state | frame | wire | page |
|---|---|---|---|
| no source (`pv_entity` unset) | no `pv` key | no topic, no entity | no row |
| no reading | `pv: None` | `goodwe_ctl/pv` = `None` | `PV —` + a caution naming the reason |
| zero watts | `pv: 0.0` | `goodwe_ctl/pv` = `0.0` | `PV 0 W` |

A failed read also **clears** the last good value (`_read_pv()` assigns on every
call), so a reading never outlives its own sensor. `test_pv.py` part 2 drives a
good read followed by a dead one, because a fixture that answers the same thing
forever cannot reach that failure — MUTATION.md habit 9's shape.

## 4. It must not become a control input, and that is graded twice

### In the suite

`test_pv.py` part 6 drives twelve cycles of the real `Controller.cycle()`
against a 6 kW export and asserts the commands written to the inverter are
**bit-identical** across `unset` / `no reading` / `0 W` / `8 kW` of production,
with a precondition check that the stimulus commands something at all (habit 1).
`inputs_ok` is asserted not to move either: a broken PV integration is **not** a
missing input, because nothing computes with it.

A computed census (habit 12 — the scope is derived from the tree, not typed)
then reports which of the 33 modules under `app/` mention the material in any
spelling, comments included. It is **three**: `pv.py`, `main.py`, `mqtt.py`. The
scanner's own red direction is exercised on every run.

### Across trees, by hash

`tools/safety56_behaviour_trace.py` is the pattern; `tools/tel11_pv_trace.py` is
this card's version of it, and **its control clause had to be different**, which
is worth stating because the difference is the interesting part.

SAFETY-56 changed wording and had to show behaviour did not move, so its
`--compare` **refuses** unless the `sentences` halves **differ** — otherwise the
two runs might be one tree twice. TEL-11's headline claim is the opposite shape:
with the option unset, **nothing** differs. "The documents are identical" is the
*result*, so it cannot also be the control. The two things proved instead are:

* the two runs came from **different trees** (`app/*.py` sha256 + byte count,
  compared, and an identity is **refused**);
* **exactly one** side reacts to `pv_entity` — that side is the HEAD, the other
  is a tree that does not have the option. Neither, or both, is refused.

It drives 41 stimulus cells (5 grid worlds × 8 PV schedules, plus a run stopped
part-way) and prints two documents per setting: `control` — every command
written, every `self.target`, `inputs_ok`, the loop's three fields, the
arbiter's resolution, the maintenance phase, tick by tick, **with no TEL-11
material in it** — and `telemetry`, which is everything the card is allowed to
move.

Measured, in the CI image, on purged trees:

```
  HEAD is B; baseline is A (it does not react to `pv_entity`).
  tree      A 73296909170ec789237d6e704cce9ab0d9f83b3df1495a14d2497285c3041a12
  tree      B d5011b6a18e2e3803dc09cac0214b0378fa4c7388fc0ad8ad89b832a5c92af48
  control   A unset 846a10a5b89e321086d535825925721e2f791012e8405948d4fd38d3f9d0be5f
  control   B unset 846a10a5b89e321086d535825925721e2f791012e8405948d4fd38d3f9d0be5f
  control   B set   846a10a5b89e321086d535825925721e2f791012e8405948d4fd38d3f9d0be5f
  telemetry A unset a443a6ceaffe072b44bcd5f2ad946c1ed6c9e01b69e278eca631f7f4d7892164
  telemetry B unset a443a6ceaffe072b44bcd5f2ad946c1ed6c9e01b69e278eca631f7f4d7892164
  telemetry B set   4e6d1cb289a957223ad0205f732a6fec6110ce1f2032c98ad60694e67625274f
OK  1/3  UNSET: control AND telemetry are byte-identical to the baseline tree.
OK  2/3  SET: the control document is byte-identical with the channel switched on.
OK  3/3  CONTROL OF THE CONTROL: the telemetry document DOES move when the
         option is set (42 stimulus cells differ).
```

Note what clause 1 claims and clause 2 does not: **unset** is identical in
`control` *and* `telemetry`, which is stronger than "the control path is
untouched" — an install that does not set the option sees no change of any kind,
including its announcement and its status rows. **Set** is identical in
`control` only, which is the whole of "instrumentation, not an input".

## 5. Genuinely optional — and not a second `price_now_entity`

`price_now_entity` / `price_avg_entity` are legal **only** while
`price_provider` is `off`, and the add-on **refuses to start** otherwise. That
is the trap this card was told not to rebuild, and it is avoided structurally
rather than by intention:

* `validate.py` does not mention `pv_entity` at all. It cannot conflict with
  anything, because nothing else reads it.
* the MQTT row lives in `OPTIONAL_SENSORS`, not `SENSORS`, and
  `discovery_payloads()`'s new `include` argument **defaults to empty**. So a
  caller that has never heard of `pv_entity` — every fixture in `test_mqtt.py`,
  `tools/rehearse_roster.py`, anything written next year — announces exactly
  what it announced before. `p1_age` solves the same problem the other way
  round, with `omit`, because it is announced by default and withdrawn when the
  meter is off; getting that direction wrong here would have left most installs
  with an entity nothing feeds, which is the hazard `omit`'s own note names.
* the read is gated on the option, so an unset install makes **no state call at
  all** — not a read that fails quietly.
* there is **no status row** when it is unset. `peak_forecast_entity` nags when
  it is unset because an unset forecast leaves a money guard without an input;
  this is instrumentation, and a permanent caution on every install that never
  asked for a PV channel is the warning spam the card forbids.
* when it **is** set and the read fails, the row is `ok=False, warn=True` — a
  **caution**. `status_payload()` builds `NOT READY` out of rows that are
  neither `ok` nor `warn`, so a telemetry channel can never put a battery site
  into `NOT READY`.

## 6. The unit is read off the entity, and an unreadable one is refused

`read_inputs()` guesses kW-or-W for `peak_forecast_entity` with
`abs(raw) < 50`, and that is safe there: nobody's quarter-hour demand forecast
is 50 W. It is **not** safe here — 30 W of real production at dusk is an
ordinary reading, and that heuristic would publish it as 30 kW.

So `pv.watts()` reads the entity's own `unit_of_measurement` and converts
against Home Assistant's own `UnitOfPower` table (`mW`, `W`, `kW`, `MW`, `GW`,
`TW`; every factor an exact power of ten). A sensor with **no** unit, or a unit
outside that table, is **refused by name** and the status page says which. The
refusal is loud; a guess would be silent, and a factor of 1000 in the one
channel that exists to be believed a week later is the worse failure.

⚠️ `BTU/h` is in HA's enum and is deliberately **not** in the table: its factor
is a measured conversion rather than a decimal shift, no PV integration
publishes in it, and a fabricated constant is what this repository's whole
process exists to prevent. It lands in the refusal, by name, and `test_pv.py`
part 1 pins that.

## 7. The mutation battery

Every check this card adds was driven by the mutation that defeats the claim it
names. Method: `cp -a` of the tree per mutation into a **unique** directory,
the edit applied there, `__pycache__` purged, the suites run in the CI image
with the tree bind-mounted `:ro`, one file at a time. **The shipped tree is
never edited**, so habit 18's revert hazard — SAFETY-37's harness killed between
the edit and the `cp` that put the file back, and two harnesses sharing one
backup path — is closed by construction rather than by a verified revert. Each
row below records the byte delta, the file's sha256 before and after, and that
the mutated line was **read back off disk** before the run started.

| # | mutation | file | bytes | sha256 (first 16) before → after | red on |
|---|---|---|---|---|---|
| M1 | the `pv` row moved into `SENSORS`, i.e. announced on every install | `mqtt.py` | −79 B | `328d249b9d698ef6` → `532d3954c023bf3d` | 3 named in `test_mqtt.py`, 1 in `test_pv.py` |
| M2 | `publish()` drops a `None` `pv` row again, so HA keeps the last number | `mqtt.py` | −98 B | `328d249b9d698ef6` → `dd54be458f446475` | 1 named (the acceptance criterion itself) |
| M3 | `ABSENT_AS_UNKNOWN` widened past the one key it was written for | `mqtt.py` | +23 B | `328d249b9d698ef6` → `8fe571ebff3dacf4` | 1 named (`circulation` must still be dropped) |
| M4 | a dead sensor reported as **0 W** instead of an absence | `pv.py` | −1 B | `55ba7471e9995436` → `9eecc34152b7ac12` | 4 named |
| M5 | `_read_pv()` assigns only on a good read — a value outlives its sensor | `main.py` | +46 B | `c4d2cab34e50b5fd` → `189802aa69ce166d` | 2 named |
| M6 | the unit is read and then ignored — a kW sensor published as watts | `pv.py` | −18 B | `55ba7471e9995436` → `ab19ef66c52f9379` | 2 named |
| **M7** | **`self.grid = self.grid − self.pv_w` in `read_inputs()` — PV wired into the control path** | `main.py` | +106 B | `c4d2cab34e50b5fd` → `c38216edb7c5c8db` | **2 named, plus `tel11_pv_trace.py` clause 2/3 on 26 of the 41 stimulus cells** |
| M8 | `optional_keys()` returns `("pv",)` unconditionally | `mqtt.py` | −52 B | `328d249b9d698ef6` → `86ff23efe3088ad8` | 3 named |
| M9 | the PV caution made a `bad` row, i.e. a dead sensor is a `NOT READY` | `main.py` | +1 B | `c4d2cab34e50b5fd` → `45c74cab89943564` | 1 named |
| M10 | the read gate removed — an unset option is read once a second forever | `main.py` | −10 B | `c4d2cab34e50b5fd` → `b67baa8aba3019e8` | 2 named, **plus two crashes** |

`on_disk = True` for all ten: the mutated line was grepped back out of the file
after the edit and before the run (MUTATION.md reporting rule 2), and no
mutation is size-preserving, so rule 1's `+0 B` caveat does not arise.

⚠️ **M10's two extra reds are CRASHES, not named checks, and that is recorded
rather than tidied away.** Every `Hass` double in `test_inverter.py` and
`test_telemetry.py` predates this read and has no `state()` method, so an
unconditional PV read is an `AttributeError` rather than a failed assertion —
habit 20's shape, a red with no name attached. Fixing those fixtures would be a
change to what *their* checks report and belongs to whoever next touches them;
this card's own fixture has `state()` and grades the property by name.

⚠️ **M7's trace limb is where the two halves meet.** The mutant moves the
control document on 26 of 41 cells — every cell whose PV schedule produces a
READING. The five `none` cells and the five `zero` cells do **not** move, which
is correct and is worth seeing: subtracting nothing, and subtracting zero,
change nothing. A battery that only drove a dead sensor would have reported this
mutation as a survivor for the third time.

The clean tree those `before` hashes belong to: `app/main.py`
`c4d2cab34e50b5fd`, `app/mqtt.py` `328d249b9d698ef6`, `app/pv.py`
`55ba7471e9995436`.

### ⚠️ M7 was a SURVIVOR on the first battery, and the fixture was the reason

The card's **central** mutation — `self.grid = self.grid - self.pv_w` in
`read_inputs()`, i.e. exactly the defect the whole card forbids — came back
**green** on a six-file run at **+106 B**, on disk, purged, genuinely running.

The property did not hold; the fixture could not reach it. `test_pv.py` part 6
drove a **6 kW export over 12 cycles**, which puts the loop **against its
rail**: the command moves `slew_w` every cycle and −6000 W and −14 000 W of
apparent error produce the **same ramp, tick for tick**. MUTATION.md habit 23,
with a saturated actuator in place of a coinciding value — and the habit's list
of no-op-prone constructs is all *expressions*, so this is worth its own entry,
which it now has.

The fixture now drives a **600 W import the loop settles on**, with a
responsive meter (`grid = base − batt`) so the error actually closes, asserts
the resting target is well inside the rail, and asks habit 23's own question as
its own check: **shift the meter by the same 8 kW and confirm the commands
really would have moved.** On the first fixture that question answers *no*, in
one line. Every number in the table above is from the battery re-run against
the fixed tree.

⚠️ **A red count is a property of the suite as much as of the mutation.** These
name **this** tree at this commit; a later card that adds or re-bases a check
moves the number with the mutation untouched. Re-measure rather than inherit.

## 8. What none of this covers, with shapes

* **Home Assistant itself.** That `None` renders as `unknown` and that the
  recorder keeps a gap is read out of HA's source and quoted in §3; no HA runs
  in this suite. What is graded here is the bytes this add-on publishes.
* **Whether the operator's sensor tells the truth.** A sensor wired backwards,
  or reporting DC where AC was meant, or a string instead of the array, is
  published faithfully. §2 is the argument for why that is the right failure
  mode; it is not a claim that it cannot happen.
* **A roster of more than one.** The row is a **site** row — there is one PV
  array as far as this add-on is concerned — so nothing per-unit exists to
  separate, and every fixture is the shipped roster of one.
* **The attribution.** This card records production. Whether the 20.5 kWh was
  cloud, a limit or the optimiser is OPT-10's, and it needs a week of this
  channel's history before it can be asked.
* **`tools/`, the firmware and the ingress HTML** are outside part 6's census.
* **The identity is only as wide as the stimulus.** `tel11_pv_trace.py` prints
  its 41 cells with the documents for exactly that reason.

## 9. No `CHANGELOG.md` entry

`test_version_gate.py` 11b refuses a newest `## ` heading that is not
`config.yaml`'s `version:`, and opening an `## Unreleased` section would mean
taking a packaging decision this card does not get to take. The operator account
is in `DOCS.md`; the release note belongs to the next PKG ticket, which should
say that **0.10.1 → next** adds one optional key, announces no new entity on an
install that does not set it, and changes no control behaviour.