Files
goodwe-addon/goodwe_controller/app/mqtt.py
T
adminandClaude Opus 5 26c5ee8adc
checks / unit (pull_request) Successful in 4m58s
TEL-07: a second MQTT device for inverter B, and A keeps the one it has
MQTT discovery announced ONE device and hung every entity off one unique_id
prefix, so at two inverters B's per-unit telemetry had nowhere to go. It has
somewhere now, and not one entity that already exists moved.

⚠️ THE ASYMMETRY IS DELIBERATE. The first roster entry's per-unit rows are
announced on THE DEVICE IT ALREADY HAS; every later entry gets its own device,
via_device the controller. "One device per inverter" is tidier and is the change
that renames every entity on the live install - and because HA keeps the id it
composed at FIRST discovery, a rename produces a NEW entity beside a DEAD one:
the history stays behind and every dashboard, template and automation naming it
goes on pointing at a sensor that quietly stopped updating, including the three
live watchdog automations. The composed ids come out symmetric anyway
(sensor.goodwe_inverter_a_battery_power / ..._inverter_b_...), so the tidy
design's only real benefit is bought here without its cost.

  * four per-unit rows - setpoint, battery power, SoC, SAFETY-02 sign latch -
    announced only from a roster of two. At N=1 nothing is announced and
    nothing is published: no second device, no renamed entity, no added topic.
  * grid and p1_sample_age_s stay SITE-scoped and are announced ONCE, at every
    roster length 1-6. One P1, one grid connection, and p1_sample_age_s is a
    watchdog input - a second copy is a second answer to "is the meter fresh?".
  * inverter.roster_names() is the one answer to what the units are called,
    read by build_roster() and by the publisher, which must know the roster
    before a Controller exists because announcing happens on the CONNACK.
  * DECISIONS/TEL-07-second-mqtt-device.md records the two designs side by side.

VERIFIED BY DIFFING THE ANNOUNCED PAYLOADS, not by reading the code. The
release/1.0 baseline in test_mqtt.py was generated by running that commit's
module, not transcribed. Roster of one: 9 topics, 0 missing, 0 changed, 0 added.
Roster of two: 0 missing, 0 changed, 8 added.

MEASURED, NOT ASSERTED. Sixteen mutations through tools/mutation_run.sh; twelve
are in the file as a running table, each named with the property it breaks.
Three of those properties exist because the table found their ABSENCE - dropping
the unit from the state topic, moving `grid` into the per-unit set, and
`units=()` in main()'s publisher call were all GREEN on the first run, as was a
roster fallback naming two nameless entries `inverter-a`. Part 9 enumerates what
the file still cannot see, with sizes.

⚠️ INVERTER B DOES NOT PHYSICALLY EXIST (~2026-09-12). Every roster of two here
is SIMULATED, from the datasheet and serial in NOTES.md §1.1b. Nothing about B
has been measured and no entity described here has been seen on a broker.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
2026-09-05 14:14:53 +02:00

386 lines
19 KiB
Python

"""Optional MQTT discovery, so the controller's state appears as real HA entities.
Optional on purpose: the add-on runs headless without a broker and simply
publishes nothing. Nothing in the control path depends on this - if MQTT breaks,
the battery keeps being controlled correctly and only the dashboard goes stale.
That separation is deliberate; observability must never be able to take down
control.
"""
import json
import logging
try:
import paho.mqtt.client as mqtt
except ImportError: # pragma: no cover - container always has it
mqtt = None
_LOG = logging.getLogger("goodwe.mqtt")
# ⚠️ The DEVICE name is half of every entity_id. Home Assistant composes
# entity_id from device name + entity name, so "GoodWe RS485 Controller" plus
# "GoodWe battery power" yields
# sensor.goodwe_rs485_controller_goodwe_battery_power. object_id in the
# discovery payload did NOT override it (tested on HA 2026.8). So the device is
# named "GoodWe" and the entities are named without repeating it - that is what
# makes the ids short, predictable, and identical on every install.
DEVICE = {
"identifiers": ["goodwe_rs485_controller"],
"name": "GoodWe",
"manufacturer": "GoodWe (via RS485 meter emulation)",
"model": "ES/BP series",
}
# (key, object_id, name, unit, device_class, state_class, icon)
#
# ⚠️ object_id is what pins the entity_id. Without it Home Assistant derives the
# id from the DEVICE name plus the entity name and produces
# `sensor.goodwe_rs485_controller_goodwe_battery_power` - unpredictable, ugly,
# and different if anyone renames the device. Dashboards and documentation need
# these ids to be stable across every install, so they are declared, not derived.
SENSORS = [
("setpoint", "goodwe_setpoint", "Setpoint", "W", "power", "measurement", None),
("grid", "goodwe_grid_power", "Grid power", "W", "power", "measurement", None),
("battery", "goodwe_battery_power", "Battery power", "W", "power", "measurement", None),
("soc", "goodwe_battery_soc", "Battery SoC", "%", "battery", "measurement", None),
# SAFETY-31. `sum(|Bi|) - |sum(Bi)|` over MEASURED per-inverter power: the
# power going round between the inverters, which the grid sensor above
# CANNOT SHOW because it nets to zero there. The two are published side by
# side on purpose - "grid 0 W, circulating 6000 W" is the whole phenomenon
# in two rows of a dashboard, and no arithmetic on the grid figure alone can
# ever produce the second one.
#
# ⚠️ Published as 0 at the shipped one-inverter roster, not omitted, and
# that is a deliberate difference from `p1_age` below. An absent entity is
# right for a watchdog input nothing is feeding; this is a quantity whose
# value is KNOWN at N=1 - identically zero, for every reading - and a
# dashboard row reading 0 W is the honest report of it.
("circulation", "goodwe_circulating_power", "Circulating power", "W", "power", "measurement", "mdi:sync-alert"),
("phase", "goodwe_maintenance_phase", "Maintenance phase", None, None, None, "mdi:battery-sync"),
("status", "goodwe_controller_status", "Controller status", None, None, None, "mdi:heart-pulse"),
# ⚠️ THE LIVE ENTITY IS `sensor.goodwe_p1_sample_age`, NOT what this
# object_id spells. An earlier version of this comment said the id "must be
# exactly sensor.p1_sample_age_s" - that name has never existed on any HA,
# and believing it is how a safety layer gets disarmed by someone tidying up.
#
# On HA 2026.8 `object_id` in a discovery payload does NOT determine the
# entity_id. HA composes it from the device name plus the entity name:
# "GoodWe RS485 Controller" + "P1 sample age" -> sensor.goodwe_p1_sample_age.
# Verified against the live system 2026-08-26; the firmware subscribes to
# that composed name (goodwe-master-stage4-failsafe.yaml, homeassistant
# sensor p1_age_s), and its own comment records that sensor.p1_sample_age_s
# never existed.
#
# So: do NOT "fix" the firmware to match the object_id below, and do not
# rename the device or this entity's friendly name - either changes the
# composed id and silently disarms the ingest guard, because has_state()
# simply stays false forever and nothing reports it. The firmware calls this
# out as the failure that "FAILS SILENTLY AND LOOKS CORRECT".
#
# The value: seconds since the newest accepted P1 telegram, republished
# every second so a meter frozen at a constant value still shows a climbing
# age - the false-trip this entity exists to remove.
#
# ⚠️ Separately, and not fixed here: the firmware's ingest guard tests this
# 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),
]
BASE = "goodwe_ctl"
AVAILABILITY = f"{BASE}/availability"
# --- TEL-07: a SECOND device, for inverter B, and NOT one per inverter -------
#
# ⚠️ READ THE DEVICE-NAME NOTE ABOVE BEFORE CHANGING ANYTHING HERE. The device
# name is half of every entity_id, so re-scoping `DEVICE` renames every entity
# the live install already has - and HA keeps the id it composed at FIRST
# discovery, so what a rename actually produces is a NEW entity beside a dead
# one: the history stays behind on the old id, and every dashboard, template
# and automation that names it (including `goodwe_master_watchdog_held_*` and
# `goodwe_master_parked_at_0_w_*`) keeps pointing at the entity that stopped
# updating. That is silent. So:
#
# `DEVICE` IS NOT TOUCHED AND ITS ENTITIES ARE NOT TOUCHED. It is the SITE
# device - the meter's view and the controller's own state - and at the
# shipped roster of one it is also, in effect, inverter A's.
#
# THE FIRST ROSTER ENTRY GETS NO NEW DEVICE. Its per-unit rows are announced
# ON THE DEVICE IT ALREADY HAS, under new `unique_id`s and new names. Nothing
# existing is renamed, so nothing existing loses its history.
#
# EVERY LATER ENTRY GETS ITS OWN DEVICE (`via_device` the site one).
#
# ⚠️ THE ASYMMETRY IS THE POINT, AND IT WILL LOOK LIKE AN OVERSIGHT. "One
# device per inverter", with A as `inverter-a` and B as `inverter-b`, is tidy,
# symmetrical, and is the change that destroys the live system's recorded
# history. DO NOT "CLEAN THIS UP". The composed entity_ids come out symmetric
# anyway - `sensor.goodwe_inverter_a_battery_power` and
# `sensor.goodwe_inverter_b_battery_power` - which is the whole benefit of the
# tidy version, without the cost.
#
# WHAT IS PER-UNIT AND WHAT IS SITE, and why the split is where it is:
#
# PER-UNIT setpoint (this unit's share of the site target, the number its
# own heartbeat re-emits), battery power (`inv.batt`, this unit's
# own meter reading), SoC (`inv.soc`, THIS pack - the site row is
# the EMPTIEST pack, which at N>=2 may be the other one), and the
# SAFETY-02 sign latch, which is per actuator by construction.
#
# SITE `grid` and `p1_age`. There is ONE P1 meter and ONE grid
# connection; announcing them per inverter would invent two of a
# thing there is one of. `p1_sample_age_s` is additionally a
# WATCHDOG INPUT (see `omit` below and the firmware note above), so
# a second copy of it is a second copy of a safety signal - two
# answers to "is the meter fresh?", one of which is always the one
# nobody is looking at. Also site: circulating power (it is
# `sum(|Bi|) - |sum(Bi)|`, a property of the SET), the controller
# status, the maintenance phase, and the capacity-tariff peak.
#
# NOT ANNOUNCED, AND SAID OUT LOUD RATHER THAN LEFT LOOKING FINISHED: the
# card also lists "inverter AC power" and per-unit HEARTBEATS as per-unit.
# Neither has a source in this add-on today - nothing reads an AC-power
# register per unit, and `inv.last_beat` is a monotonic loop clock with no
# wall-clock age published anywhere. Announcing them would create entities
# nothing ever feeds, which is exactly what the `omit` note below refuses to
# do for `p1_sample_age_s`. They are gaps, not omissions.
UNIT_SENSORS = [
("setpoint", "setpoint", "Setpoint", "W", "power", "measurement", None),
("battery", "battery_power", "Battery power", "W", "power", "measurement", None),
("soc", "battery_soc", "Battery SoC", "%", "battery", "measurement", None),
("sign_latch", "sign_latch", "Sign latch", None, None, None, "mdi:flash-alert"),
]
# ⚠️ A ROSTER OF ONE ANNOUNCES NOTHING NEW - not a device, not an entity, not a
# topic. Every install running today is a roster of one, and "the upgrade added
# four entities nobody asked for" is a change to a live system's entity list
# for no information: at N=1 the site rows ARE this unit's rows, exactly.
PER_UNIT_MIN_ROSTER = 2
def _slug(text) -> str:
"""Home Assistant's own object_id spelling: lowercase, `_`, no runs.
⚠️ ASCII-ONLY ON PURPOSE. `str.isalnum()` is true for 'é' and for '٤', and
HA transliterates those rather than keeping them, so treating them as
alphanumeric here would produce an id this module claims and HA does not.
They become separators, which is conservative in the right direction: two
names that differ ONLY outside ASCII collide, and `unit_slugs()` below is
what stops a collision becoming two entities with one `unique_id`.
"""
out = []
for ch in str(text).strip().lower():
out.append(ch if (ch.isascii() and ch.isalnum()) else "_")
slug = "".join(out)
while "__" in slug:
slug = slug.replace("__", "_")
return slug.strip("_")
def _title(name) -> str:
"""`inverter-b` -> `Inverter B`. The half of the entity_id HA composes."""
words = str(name).replace("-", " ").replace("_", " ").split()
return " ".join(w[:1].upper() + w[1:] for w in words) or str(name)
def unit_slugs(units) -> list:
"""One id-safe slug per roster entry, in roster order, ALL DISTINCT.
⚠️ THE DE-COLLISION IS NOT DECORATION. `validate.py` rule 9(c) refuses two
entries with the SAME name; it does not refuse `inverter-b` beside
`Inverter B`, and those two slug to one string. Two entities published with
one `unique_id` is not an error in HA - the second one is DROPPED, so one
inverter's telemetry silently stops existing. The suffix is the roster
INDEX, so it is stable for a given roster and does not depend on what the
other unit is called.
"""
slugs, seen = [], set()
for i, name in enumerate(units):
slug = _slug(name) or f"inverter_{i + 1}"
while slug in seen:
slug = f"{slug}_{i + 1}"
seen.add(slug)
slugs.append(slug)
return slugs
def announced_units(units) -> list:
"""The roster, in order - or EMPTY below `PER_UNIT_MIN_ROSTER`.
⚠️ ONE ANSWER TO "WHICH UNITS GET PER-UNIT ENTITIES", read by the
announcement here AND by `Controller.publish()` in main.py. Two spellings of
the same predicate is how an entity gets announced that nothing publishes
to, or published to on a topic nothing announced - and at N=1 the second
would be an added topic, which AC 4 forbids outright.
"""
names = [str(u) for u in units]
if len(names) < PER_UNIT_MIN_ROSTER:
return []
return names
def unit_state_key(slug: str, key: str) -> str:
"""The `publish()` key - i.e. the state topic under `BASE` - for one unit.
Written once, here, because main.py builds it and this module announces it;
a second transcription is a topic announced and never fed.
"""
return f"unit/{slug}/{key}"
def unit_device(name, slug: str) -> dict:
"""B's OWN device. ⚠️ Never returned for the first roster entry."""
return {
"identifiers": [f"{DEVICE['identifiers'][0]}_{slug}"],
"name": f"{DEVICE['name']} {_title(name)}",
"manufacturer": DEVICE["manufacturer"],
"model": DEVICE["model"],
# Hangs B under the controller in HA's device tree rather than beside
# it. Cosmetic, and the one line that says in the UI what the asymmetry
# above means: there is one controller, and it drives both.
"via_device": DEVICE["identifiers"][0],
}
def _payload(name, object_id, unique_id, state_topic, device,
unit, dev_class, state_class, icon) -> dict:
"""One discovery payload.
⚠️ KEY ORDER IS PART OF THE OUTPUT. `json.dumps` writes keys in insertion
order, so this dict's construction order IS the bytes on the wire, and AC 1
is a BYTE comparison against what `release/1.0` publishes. Do not reorder.
"""
cfg = {
"name": name,
"object_id": object_id,
"unique_id": unique_id,
"state_topic": state_topic,
"availability_topic": AVAILABILITY,
"device": device,
}
if unit:
cfg["unit_of_measurement"] = unit
if dev_class:
cfg["device_class"] = dev_class
if state_class:
cfg["state_class"] = state_class
if icon:
cfg["icon"] = icon
return cfg
def discovery_payloads(units=(), omit=()) -> list:
"""Every `(topic, payload)` this add-on announces, in announcement order.
Split out of `_announce()` so that the acceptance criterion can be graded as
a DIFF OF THE ANNOUNCED PAYLOADS against `release/1.0`, rather than as a
reading of the code. `_announce()` publishes exactly this list and nothing
else, which is itself checked.
"""
omit = set(omit)
out = []
for key, object_id, name, unit, dev_class, state_class, icon in SENSORS:
if key in omit:
continue
out.append((
f"homeassistant/sensor/{BASE}_{key}/config",
_payload(name, object_id, f"{BASE}_{key}", f"{BASE}/{key}",
DEVICE, unit, dev_class, state_class, icon)))
names = announced_units(units)
slugs = unit_slugs(names)
site_slug = _slug(DEVICE["name"])
for i, (name, slug) in enumerate(zip(names, slugs)):
# ⚠️ `i == 0` IS THE ASYMMETRY, IN ONE LINE. The first roster entry
# keeps the device it already has and carries its name in the ENTITY
# name instead; every later one gets its own device and does not.
# Either way HA composes device-name + entity-name into the same
# string, which is why `object_id` below can be written once for both.
device = DEVICE if i == 0 else unit_device(name, slug)
prefix = f"{_title(name)} " if i == 0 else ""
for key, oid, label, unit, dev_class, state_class, icon in UNIT_SENSORS:
unique_id = f"{BASE}_unit_{slug}_{key}"
out.append((
f"homeassistant/sensor/{unique_id}/config",
_payload(prefix + label,
f"{site_slug}_{slug}_{oid}",
unique_id,
f"{BASE}/{unit_state_key(slug, key)}",
device, unit, dev_class, state_class, icon)))
return out
class MqttPublisher:
def __init__(self, host, port, username=None, password=None, omit=(),
units=()):
# `omit` drops sensor keys from discovery entirely. ⚠️ Announcing a
# sensor that nothing will ever publish to is not harmless here:
# p1_sample_age_s is a watchdog input, and an entity that exists but is
# never fed is a worse signal than one that does not exist at all.
self.omit = set(omit)
# TEL-07. The roster's NAMES, in roster order - `inverter.roster_names()`,
# not a second reading of the option. Defaults to empty, which
# `announced_units()` treats exactly as a roster of one: nothing
# per-unit is announced, so every caller that has not been taught about
# the roster keeps publishing what it published before.
self.units = [str(u) for u in units]
self.enabled = mqtt is not None and bool(host)
self.client = None
if not self.enabled:
_LOG.info("MQTT not configured - status entities will not be published")
return
# paho-mqtt 2.x requires a callback API version; 1.x has no such
# argument and Alpine ships 1.x. Support both rather than pinning, so
# the container can use the distro package instead of compiling.
try:
self.client = mqtt.Client(mqtt.CallbackAPIVersion.VERSION2,
client_id="goodwe_rs485_controller")
except AttributeError:
self.client = mqtt.Client(client_id="goodwe_rs485_controller")
if username:
self.client.username_pw_set(username, password or "")
self.client.will_set(AVAILABILITY, "offline", retain=True)
# ⚠️ Announce from on_connect, never straight after connect(). paho
# processes the CONNACK on its network thread, so a publish issued
# immediately after connect() is made while still disconnected - and
# paho DROPS QoS-0 publishes when disconnected, silently. The result is
# an add-on that logs "MQTT connected" and creates no entities at all.
# As a bonus, this also re-announces after every reconnect.
self.client.on_connect = lambda *_args, **_kw: self._announce()
try:
self.client.connect(host, int(port), keepalive=60)
self.client.loop_start()
_LOG.info("MQTT connected to %s:%s", host, port)
except OSError as err:
_LOG.warning("MQTT connect failed (%s) - continuing without it", err)
self.enabled = False
def _announce(self) -> None:
# ⚠️ NO PAYLOAD IS BUILT HERE. `discovery_payloads()` is the whole
# announcement, so the check that diffs it against `release/1.0` is
# diffing what actually goes on the wire - and a payload built here
# instead would be one this add-on publishes and no check has ever seen.
for topic, cfg in discovery_payloads(self.units, self.omit):
self.client.publish(topic, json.dumps(cfg), retain=True)
self.client.publish(AVAILABILITY, "online", retain=True)
def publish(self, values: dict) -> None:
if not self.enabled or self.client is None:
return
try:
for key, value in values.items():
if value is None:
continue
self.client.publish(f"{BASE}/{key}", str(value))
except OSError as err: # pragma: no cover
_LOG.debug("MQTT publish failed: %s", err)
def close(self) -> None:
if not self.enabled or self.client is None:
return
try:
self.client.publish(AVAILABILITY, "offline", retain=True)
self.client.loop_stop()
self.client.disconnect()
except OSError: # pragma: no cover
pass