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

goodwe_controller/app/inverter.py

@@ -357,6 +357,37 @@ def roster_entries(opts: dict) -> list:
     return list(entries)
 
 
+def roster_names(opts: dict) -> list:
+    """The unit names, in roster order. THE one answer, for two callers.
+
+    ⚠️ IT EXISTS BECAUSE MQTT DISCOVERY CANNOT WAIT FOR A ROSTER. TEL-07 gives
+    every unit past the first its own MQTT device, and `MqttPublisher` is
+    constructed in `main()` BEFORE `Controller.__init__` builds the roster -
+    announcing happens on the broker's CONNACK, which can land first. Building a
+    second roster there would construct a second set of `Inverter`s off the same
+    Store; transcribing the naming rule there would be a third spelling of it.
+    So the naming rule lives here, `build_roster()` reads it, and the two cannot
+    drift.
+
+    ⚠️ THE FALLBACK NAME IS POSITIONAL, NOT `UNIT_A`. Two nameless entries both
+    called `inverter-a` would share one Store namespace, so one unit's SAFETY-02
+    latch would be the other's - a fault on B latching A is the exact failure
+    DEV-02's namespaced keys removed. Rule 9 refuses a nameless entry outright;
+    this only decides what the refusal is allowed to call it in the meantime.
+
+    An EMPTY `inverters:` is the flat-key shape and its one unit is `UNIT_A`,
+    which is what every install running today reads.
+    """
+    entries = roster_entries(opts)
+    if not entries:
+        return [UNIT_A]
+    names = []
+    for i, entry in enumerate(entries):
+        entry = entry if isinstance(entry, dict) else {}
+        names.append(str(entry.get("name") or "").strip() or f"inverter-{i + 1}")
+    return names
+
+
 def build_roster(opts: dict, store, sign_cfg=None) -> list:
     """The site's inverters, in roster order.
 
@@ -414,15 +445,15 @@ def build_roster(opts: dict, store, sign_cfg=None) -> list:
             store=store, sign_cfg=sign_cfg)]
 
     roster = []
-    for i, entry in enumerate(entries):
+    names = roster_names(opts)
+    # ⚠️ NAMES COME FROM `roster_names()`, NOT FROM A SECOND READING OF THE
+    # ENTRIES. TEL-07 announces one MQTT device and one `unique_id` per unit off
+    # these names, and it has to build them BEFORE a `Controller` exists - so a
+    # second spelling of the fallback here would be an entity announced under a
+    # name no unit answers to. `zip` pairs them positionally, which is what
+    # `roster_names()` is defined to be.
+    for entry, name in zip(entries, names):
         entry = entry if isinstance(entry, dict) else {}
-        # ⚠️ THE FALLBACK NAME IS POSITIONAL, NOT `UNIT_A`. Two nameless entries
-        # both called `inverter-a` would share one Store namespace, so one
-        # unit's SAFETY-02 latch would be the other's - a fault on B latching A
-        # is the exact failure DEV-02's namespaced keys removed. Rule 9 refuses
-        # a nameless entry outright; this only decides what the refusal is
-        # allowed to call it in the meantime.
-        name = str(entry.get("name") or "").strip() or f"inverter-{i + 1}"
         roster.append(Inverter(
             name,
             setpoint_entity=entry.get("setpoint_entity", ""),

goodwe_controller/app/main.py

@@ -53,11 +53,12 @@ from .hass import HomeAssistant
 # and two checks read `app.main.LINK_STALE_S` to build a stale link, so the
 # window an operator would look up here is the one the roster actually uses.
 from .inverter import (LINK_STALE_S, UNIT_A, build_roster,
-                       roster_entries)
+                       roster_entries, roster_names)
 from .maintenance import (
     IDLE, POWER_MOVING, MaintConfig, Maintenance, arbitrate as maint_arbitrate,
 )
-from .mqtt import MqttPublisher
+from .mqtt import (MqttPublisher, announced_units, unit_slugs,
+                   unit_state_key)
 from . import peakguard
 from .p1 import P1Ingest, build_source, is_enabled
 from .price import Uncertainty, price_option
@@ -3073,6 +3074,28 @@ class Controller:
         if self.p1_enabled:
             # Recomputed here, once a second, on purpose - see P1Ingest.
             values["p1_age"] = round(self.p1.published_age_s, 1)
+        # TEL-07. THE PER-UNIT ROWS, AND THE EMPTY LIST IS THE GATE.
+        #
+        # ⚠️ THERE IS NO `if len(self.inverters) > 1` HERE, DELIBERATELY.
+        # `announced_units()` answers `[]` at a roster of one, `zip` then yields
+        # nothing, and the values dict comes out byte-identical to what shipped -
+        # so the condition that decides what is ANNOUNCED is the same expression
+        # that decides what is PUBLISHED. Written as a second predicate, the two
+        # could disagree, and both directions are bad: an announced entity
+        # nothing feeds (the hazard `omit` exists for) or, at N=1, a published
+        # topic nothing announced, which AC 4 forbids outright.
+        #
+        # ⚠️ AND THE SITE ROWS ABOVE DO NOT MOVE. `battery` stays the SUM and
+        # `soc` stays the EMPTIEST pack; these are additions beside them, not a
+        # re-pointing of them, because those two ids are on the live install's
+        # dashboards and in its automations.
+        names = announced_units([inv.name for inv in self.inverters])
+        for inv, slug in zip(self.inverters, unit_slugs(names)):
+            values[unit_state_key(slug, "setpoint")] = inv.target
+            values[unit_state_key(slug, "battery")] = inv.batt
+            values[unit_state_key(slug, "soc")] = inv.soc
+            values[unit_state_key(slug, "sign_latch")] = (
+                "tripped" if inv.sign_wd.tripped else "clear")
         self.mqtt.publish(values)
 
     async def shutdown(self) -> None:
@@ -3779,6 +3802,13 @@ async def amain() -> None:
                 broker.get("username") if broker else None,
                 broker.get("password") if broker else None,
                 omit=() if is_enabled(opts) else ("p1_age",),
+                # TEL-07. ⚠️ THE NAMES, NOT A ROSTER. `build_roster()` here
+                # would construct a SECOND set of `Inverter`s off the same
+                # Store, minutes before `Controller.__init__` constructs the
+                # real ones; `roster_names()` is the naming rule on its own and
+                # is what `build_roster()` itself reads. At a roster of one this
+                # announces nothing new - see `announced_units()`.
+                units=roster_names(opts),
             )
         except Exception as err:  # noqa: BLE001
             _LOG.warning("MQTT unavailable (%s) - continuing without status entities", err)

goodwe_controller/app/mqtt.py

@@ -90,14 +90,239 @@ SENSORS = [
 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=()):
+    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:
@@ -130,29 +355,12 @@ class MqttPublisher:
             self.enabled = False
 
     def _announce(self) -> None:
-        for key, object_id, name, unit, dev_class, state_class, icon in SENSORS:
-            if key in self.omit:
-                continue
-            cfg = {
-                "name": name,
-                "object_id": object_id,
-                "unique_id": f"{BASE}_{key}",
-                "state_topic": f"{BASE}/{key}",
-                "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
-            self.client.publish(
-                f"homeassistant/sensor/{BASE}_{key}/config",
-                json.dumps(cfg), retain=True,
-            )
+        # ⚠️ 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: