GoodWe RS485 Controller
-
Field installation & commissioning guide — v0.1.0
+
Field installation & commissioning guide — v0.1.0, outdated: where it disagrees with DOCS.md, DOCS.md wins
Installer copy
@@ -503,7 +503,7 @@ figcaption { font-size:9pt; color:#5b6169; margin-top:1.2mm; }
| Setting | Value | Reason |
|---|
gain | 0.6 | Inverter needs 3–6 s to settle (1.4 s dead time, 94 % by 3.3 s) against a ~5 s cycle. At the limit — do not raise |
- slew_w | 1000 | Most of a correction in the first cycle without running far ahead of the hardware |
+ slew_w | 0 | 0 follows the rail (max_w / 9). A faster fixed value is refused at startup (SAFETY-39) — see DOCS.md |
deadband_w | 15 | Measured residual: mean 15.4 W, max 27 W. At 10 W the command never rests |
saturation_w / cycles | 500 / 3 | Divergence = inverter at a limit → magnitude may fall, never rise. The 3-cycle term is essential: tested instantly it fires on every large correction |
step_w | 10 | Register is 1 W but response lands on a ~17.6 W ladder. Finer is meaningless |
diff --git a/docs/field-guide.html b/docs/field-guide.html
index ddbb264..77570b2 100644
--- a/docs/field-guide.html
+++ b/docs/field-guide.html
@@ -60,7 +60,7 @@ figcaption { font-size:9pt; color:#5b6169; margin-top:1.2mm; }
GoodWe RS485 Controller
-
Field installation & commissioning guide — v0.1.0
+
Field installation & commissioning guide — v0.1.0, outdated: where it disagrees with DOCS.md, DOCS.md wins
Installer copy
@@ -468,7 +468,7 @@ figcaption { font-size:9pt; color:#5b6169; margin-top:1.2mm; }
| Setting | Value | Reason |
|---|
gain | 0.6 | Inverter needs 3–6 s to settle (1.4 s dead time, 94 % by 3.3 s) against a ~5 s cycle. At the limit — do not raise |
- slew_w | 1000 | Most of a correction in the first cycle without running far ahead of the hardware |
+ slew_w | 0 | 0 follows the rail (max_w / 9). A faster fixed value is refused at startup (SAFETY-39) — see DOCS.md |
deadband_w | 15 | Measured residual: mean 15.4 W, max 27 W. At 10 W the command never rests |
saturation_w / cycles | 500 / 3 | Divergence = inverter at a limit → magnitude may fall, never rise. The 3-cycle term is essential: tested instantly it fires on every large correction |
step_w | 10 | Register is 1 W but response lands on a ~17.6 W ladder. Finer is meaningless |
diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md
index 0ca2edb..837742b 100644
--- a/goodwe_controller/DOCS.md
+++ b/goodwe_controller/DOCS.md
@@ -4,8 +4,8 @@ Drives a GoodWe ES/BP battery inverter over its RS485 meter bus: holds net grid
exchange at zero, and runs a monthly battery maintenance cycle so the BMS can
balance cells and recalibrate its coulomb counter.
-Installers: read `FIELD-GUIDE.md` in the repository. It is not optional reading —
-it contains the commissioning gates and the failure modes.
+Installers: the printed field guide (`docs/GoodWe-RS485-Field-Guide.pdf`) is
+v0.1.0 and outdated; where it disagrees with this page, this page wins.
## Before you start
@@ -165,12 +165,14 @@ inverters:
soc_entity: sensor.goodwe_a_soc
batt_entity: sensor.goodwe_a_battery_power
max_w: 5000
+ bms_soc_floor: 10
capacity_kwh: 15.36
- name: inverter-b
setpoint_entity: number.goodwe_b_setpoint_w
soc_entity: sensor.goodwe_b_soc
batt_entity: sensor.goodwe_b_battery_power
max_w: 4600
+ bms_soc_floor: 10
capacity_kwh: 20.48
```
@@ -190,16 +192,18 @@ inverter's own **AC** power. If it is DC the AC-side ceiling is nearer 4324 W.
| `name` | yes | Short identifier. It namespaces this unit's SAFETY-02 latch, heartbeat and write bookkeeping in the add-on's state file, so **two units may not share one** |
| `setpoint_entity` | yes | This inverter's own ESPHome `number.*_goodwe_setpoint_w` |
| `max_w` | yes | The supervised cap on what may be written to **this** inverter. The site's ceiling is the **sum** |
+| `bms_soc_floor` | yes | This pack's own BMS cut-out percent (SAFETY-32). See [`bms_soc_floor`](#tariff-all-optional) |
| `soc_entity` | | This pack's state of charge |
| `batt_entity` | | This unit's battery power, `+` = discharging |
| `batt_invert` | | Flip if needed |
+| `capacity_kwh` | | This pack's usable capacity. State it on every entry or on none — see [OPT-09](#how-the-site-target-is-divided-between-the-packs-opt-09) |
⚠️ **Nothing is inherited.** An entry that leaves out `soc_entity` has **no SoC
sensor** — it does *not* fall back to the flat one. That is deliberate: every
per-device value became per-instance in 0.5.x precisely so that no inverter is
ever graded against a sibling's numbers, and a silent fallback would rebuild
-that in YAML. `name`, `setpoint_entity` and `max_w` are required of every entry
-for the same reason: there is no rating to borrow.
+that in YAML. `name`, `setpoint_entity`, `max_w` and `bms_soc_floor` are required
+of every entry for the same reason: there is no rating or cut-out to borrow.
⚠️ **Clear the flat `setpoint_entity` when you declare a roster,** or the add-on
refuses to start and says so. It is the single-inverter form and it is not read
@@ -600,7 +604,7 @@ than in YAML nobody reviews underneath a safety input.
> `ha_signed`.** It reads the same meter and produces the same numbers, but it
> polls the meter directly instead of watching a Home Assistant entity — and
> that is the difference between an age sensor a watchdog can threshold and one
-> it cannot. See "`sensor.p1_sample_age_s`" below; `ha_signed` remains for
+> it cannot. See "`sensor.goodwe_p1_sample_age`" below; `ha_signed` remains for
> installs where the meter is only reachable through Home Assistant.
> ⚠️ **`ha_dsmr` and `mqtt_p1` have never processed a telegram from real
@@ -755,7 +759,7 @@ connection-level reading is still accepted on its own.
**Why this mode exists:** every HTTP response is an *arrival*. The meter
answered, now, with its current reading — whether or not the number moved. That
-is the signal `sensor.p1_sample_age_s` needs and the one Home Assistant cannot
+is the signal `sensor.goodwe_p1_sample_age` needs and the one Home Assistant cannot
give it at all (see the note below). It is also simply fewer moving parts: the
five-second cadence is the meter's own, rather than an integration's polling of
it re-published as a state change.
@@ -792,12 +796,13 @@ as `… 4 rejected, measurement unchanged for 312 s`. On a house drawing real
power that figure stays in the seconds; minutes of it while the load is clearly
not near zero is the meter to go and look at.
-#### `sensor.p1_sample_age_s`
+#### `sensor.goodwe_p1_sample_age`
Published over MQTT discovery whenever a broker is available: **seconds since the
newest accepted telegram**, refreshed every second rather than only when a
telegram lands. The ESP32's stale-input watchdog subscribes to this exact entity
-id, so do not rename it.
+id; do not rename the `GoodWe` device or this entity, since HA composes the id
+from both (see [What appears in Home Assistant at two inverters](#what-appears-in-home-assistant-at-two-inverters)).
The reason it is recomputed against the clock is that Home Assistant only pushes
a state when the state *changes*. A meter sitting at a genuinely constant reading
@@ -833,7 +838,7 @@ behind it would trip the firmware watchdog on a system that is working fine.
> went **42.2 s and 97.0 s** between changes, and 23 Aug peaks at 29.1 s — all
> past the default `meter_max_age_s` of 30.
>
-> So `sensor.p1_sample_age_s` on `ha_signed` is safe to *read*, and it is
+> So `sensor.goodwe_p1_sample_age` on `ha_signed` is safe to *read*, and it is
> correct whenever the value is moving, but it must not yet be thresholded by
> the ESP32 stale-input watchdog: a quiet house would trip the battery to 0 W.
> Raising `meter_max_age_s` is **not** the fix — the two conditions produce an
diff --git a/goodwe_controller/config.yaml b/goodwe_controller/config.yaml
index 2e301b3..d3a9493 100644
--- a/goodwe_controller/config.yaml
+++ b/goodwe_controller/config.yaml
@@ -93,7 +93,7 @@ options:
# against a sibling's numbers; an entry silently inheriting `soc_entity` would
# rebuild that defect in YAML. `name`, `setpoint_entity`, `max_w` and
# `bms_soc_floor` are therefore REQUIRED of every entry (the schema below enforces it, and
- # validate.py rule 9 enforces it again for the callers the Supervisor never
+ # validate.py rules 8 and 9 enforce it again for the callers the Supervisor never
# sees). An omitted `soc_entity`/`batt_entity` means THIS UNIT HAS NO SUCH
# SENSOR - `inputs_ok` already stops the site for that - and never means
# "use the other one's".
@@ -224,7 +224,7 @@ options:
# ha_signed reads ONE signed HA entity (+ import / - export), which is what a
# HomeWizard P1 publishes and what ha_dsmr cannot consume.
# homewizard_local skips Home Assistant entirely and polls the meter's own
- # local API. ⚠️ It is the ONLY mode whose sensor.p1_sample_age_s measures when
+ # local API. ⚠️ It is the ONLY mode whose sensor.goodwe_p1_sample_age measures when
# the meter REPORTED rather than when the value last CHANGED - HA emits
# nothing at all for a repeated reading - so it is the only mode a firmware
# watchdog may threshold. See DOCS.md.
@@ -916,12 +916,13 @@ schema:
# batteries, whereas an absent capacity has an honest answer - no opinion
# about the split, every unit weighs 1.0, which is what shipped.
#
- # ⚠️ THREE KEYS ARE REQUIRED PER ENTRY AND THAT IS DELIBERATE. An entry with
+ # ⚠️ FOUR KEYS ARE REQUIRED PER ENTRY AND THAT IS DELIBERATE. An entry with
# no `name` has no Store namespace and would share another unit's SAFETY-02
# latch; one with no `setpoint_entity` is a unit nothing can command; one with
# no `max_w` would have to borrow a rating, which is the sibling-grading
- # defect DEV-02 removed. The optional pair is the sensors, whose absence
- # honestly means "this unit reports none".
+ # defect DEV-02 removed; `bms_soc_floor` is SAFETY-32's (options block above).
+ # The optional keys are the sensors, whose absence honestly means "this unit
+ # reports none", and `capacity_kwh`.
#
# ⚠️ THIS BLOCK IS DECLARED BEFORE `max_w:` ON PURPOSE. `max_w` now appears
# TWICE under `schema:` - once nested here, once flat below - and a reader
diff --git a/goodwe_controller/test_docs.py b/goodwe_controller/test_docs.py
new file mode 100644
index 0000000..43fe729
--- /dev/null
+++ b/goodwe_controller/test_docs.py
@@ -0,0 +1,210 @@
+"""Runnable check that the operator docs say what the code does. `python3 test_docs.py`
+
+DOC-1. Two things the docs got wrong and an installer would have copied:
+
+1. The `inverters:` example in DOCS.md must be ACCEPTED by the shipped
+ validator (`app/validate.py`) and carry every key config.yaml's schema
+ requires per entry. It omitted `bms_soc_floor` and would not start.
+2. The P1 age entity the ESP32 watchdog subscribes to must be documented under
+ the id Home Assistant composes from what `app/mqtt.py` announces, and the
+ old wrong id must appear in no shipped doc.
+
+Each part also runs its own mutation (habit 1 of MUTATION.md): the spoilt input
+must turn it red, or the green above it means nothing.
+"""
+
+import os
+import re
+import sys
+
+from app import mqtt
+from app.validate import validate_startup_config
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+REPO = os.path.dirname(HERE)
+
+fails = []
+
+
+def check(name, cond):
+ print(f" {'ok ' if cond else 'FAIL'} {name}")
+ if not cond:
+ fails.append(name)
+
+
+def read(*parts):
+ with open(os.path.join(*parts), encoding="utf-8") as fh:
+ return fh.read()
+
+
+DOCS = read(HERE, "DOCS.md")
+CFG = read(HERE, "config.yaml")
+
+
+# --------------------------------------------------------------------------- #
+# A reader for the two YAML shapes used here. No PyYAML in the checks image
+# (test_defaults_pin.py says why), so this is by hand, and part 1a checks it
+# actually found something.
+def scalar(raw):
+ raw = raw.split("#", 1)[0].strip()
+ if raw[:1] in "\"'" and raw[-1:] == raw[:1] and len(raw) >= 2:
+ return raw[1:-1]
+ if raw in ("true", "false"):
+ return raw == "true"
+ if raw == "[]":
+ return []
+ for cast in (int, float):
+ try:
+ return cast(raw)
+ except ValueError:
+ pass
+ return raw
+
+
+def parse_example(text):
+ """Top-level `key: value` lines plus one list of flat mappings."""
+ out, cur_list, cur = {}, None, None
+ for line in text.splitlines():
+ if not line.split("#", 1)[0].strip():
+ continue
+ m = re.match(r"^([a-z0-9_]+):\s*(.*)$", line)
+ if m:
+ if m.group(2).split("#", 1)[0].strip():
+ out[m.group(1)] = scalar(m.group(2))
+ cur_list = None
+ else:
+ cur_list = out[m.group(1)] = []
+ continue
+ m = re.match(r"^\s+-\s+([a-z0-9_]+):\s*(.*)$", line)
+ if m and cur_list is not None:
+ cur = {m.group(1): scalar(m.group(2))}
+ cur_list.append(cur)
+ continue
+ m = re.match(r"^\s+([a-z0-9_]+):\s*(.*)$", line)
+ if m and cur is not None:
+ cur[m.group(1)] = scalar(m.group(2))
+ continue
+ raise ValueError(f"unparsed line in the DOCS.md example: {line!r}")
+ return out
+
+
+# --------------------------------------------------------------------------- #
+print("DOC-1 - the docs say what the code does\n")
+print("1. the `inverters:` example in DOCS.md starts")
+
+section = DOCS[DOCS.index("### More than one inverter (`inverters`)"):]
+section = section[:section.index("\n### ", 1)]
+block = re.search(r"```yaml\n(.*?)```", section, re.S).group(1)
+example = parse_example(block)
+entries = example.get("inverters") or []
+
+# The options a fresh install is given, from config.yaml itself: the example is
+# pasted on top of these, exactly as an installer does.
+opts_block = CFG[CFG.index("\noptions:"):CFG.index("\nschema:")]
+shipped = {m.group(1): scalar(m.group(2))
+ for m in re.finditer(r"^ ([a-z0-9_]+): *(.*)$", opts_block, re.M)}
+
+# config.yaml's per-entry schema: required keys are the ones without `?`.
+schema_block = CFG[CFG.index("\nschema:"):]
+inv_schema = re.search(r"^ inverters:\n((?: .*\n)+)", schema_block, re.M).group(1)
+schema_keys = dict(re.findall(r"^ -? ?\s*([a-z0-9_]+): *(\S+)", inv_schema, re.M))
+required = sorted(k for k, t in schema_keys.items() if not t.endswith("?"))
+
+check(f"1a. the reader found the example: {len(entries)} entries, "
+ f"{len(shipped)} shipped options, schema keys {sorted(schema_keys)}",
+ len(entries) == 2 and all(len(e) >= 4 for e in entries)
+ and len(shipped) > 50 and "bms_soc_floor" in required)
+
+missing = [(e.get("name"), k) for e in entries for k in required if k not in e]
+check(f"1b. every entry carries every key the schema requires {required} "
+ f"(missing: {missing})", not missing)
+
+unknown = [(e.get("name"), k) for e in entries for k in e if k not in schema_keys]
+check(f"1c. no entry uses a key the schema does not declare (found: {unknown})",
+ not unknown)
+
+
+def with_example(ex):
+ opts = dict(shipped)
+ opts.update(ex)
+ return opts
+
+
+problems = validate_startup_config(with_example(example))
+for p in problems:
+ print(f" refused: {p[:160]}")
+check("1d. the shipped validator accepts the example as documented", problems == [])
+
+# Precondition: the example clears the flat setpoint_entity, so without its
+# roster there is no actuator. If this passed, 1d graded the flat keys, not the
+# roster.
+flat_only = {k: v for k, v in example.items() if k != "inverters"}
+check("1e. ...and it is the ROSTER that 1d graded (the example without it is "
+ "refused)", validate_startup_config(with_example(flat_only)) != [])
+
+# The mutation this card is about: the key the docs used to leave out.
+spoilt = dict(example, inverters=[
+ {k: v for k, v in e.items() if k != "bms_soc_floor"} for e in entries])
+sp = validate_startup_config(with_example(spoilt))
+check("1f. MUTATION: the example with `bms_soc_floor` removed is refused, "
+ "and the refusal names it",
+ sp != [] and any("bms_soc_floor" in p for p in sp))
+
+# --------------------------------------------------------------------------- #
+print("\n2. the P1 age entity is documented under its real id")
+
+
+def compose(device_name, entity_name):
+ """HA's entity_id from device name + entity name (mqtt.py's note on
+ DEVICE). Written here, not borrowed from mqtt._slug, so a change to that
+ helper cannot move both sides of the comparison."""
+ s = re.sub(r"[^a-z0-9]+", "_", f"{device_name} {entity_name}".lower())
+ return "sensor." + s.strip("_")
+
+
+p1_rows = [r for r in mqtt.SENSORS if r[0] == "p1_age"]
+check("2a. mqtt.py announces exactly one `p1_age` sensor", len(p1_rows) == 1)
+real_id = compose(mqtt.DEVICE["name"], p1_rows[0][2]) if p1_rows else ""
+print(f" composed from mqtt.py: {real_id}")
+
+# Anchor outside mqtt.py: the id recorded from the live install.
+recorded = read(HERE, "tools", "rehearsal", "announced-reference.txt")
+check("2b. ...and that composes to the id recorded from the live install "
+ "(tools/rehearsal/announced-reference.txt)",
+ re.search(rf"^{re.escape(real_id)}\s+goodwe_ctl_p1_age\b", recorded, re.M)
+ is not None)
+
+m = re.search(r"^#### `(sensor\.[a-z0-9_]+)`\n\n(?:.*\n){0,6}?.*stale-input watchdog",
+ DOCS, re.M)
+documented = m.group(1) if m else None
+check(f"2c. DOCS.md's watchdog-entity section is headed with that id "
+ f"(found {documented!r})", documented == real_id)
+
+# The wrong id, anywhere an installer reads. CHANGELOG.md and DECISIONS/ are
+# history and keep what was said at the time.
+SHIPPED_DOCS = [
+ os.path.join(HERE, "DOCS.md"), os.path.join(HERE, "config.yaml"),
+ os.path.join(REPO, "README.md"), os.path.join(REPO, "FIELD-GUIDE.md"),
+ os.path.join(REPO, "docs", "build_guide.py"),
+ os.path.join(REPO, "docs", "field-guide.html"),
+]
+WRONG = "p1_sample_age_s"
+hits = []
+for path in SHIPPED_DOCS:
+ for n, line in enumerate(read(path).splitlines(), 1):
+ if WRONG in line:
+ hits.append(f"{os.path.relpath(path, REPO)}:{n}")
+check(f"2d. `{WRONG}` appears in no shipped doc (hits: {hits})", not hits)
+
+# MUTATION: the old heading must fail 2c's comparison.
+bad = DOCS.replace(f"#### `{real_id}`", f"#### `sensor.{WRONG}`")
+mb = re.search(r"^#### `(sensor\.[a-z0-9_]+)`\n\n(?:.*\n){0,6}?.*stale-input watchdog",
+ bad, re.M)
+check("2e. MUTATION: the old heading is found and does not match",
+ mb is not None and mb.group(1) != real_id)
+
+print()
+if fails:
+ print(f"FAILED: {len(fails)}")
+ sys.exit(1)
+print("all ok")