diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index 4e04c4d..65f2780 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -1,5 +1,41 @@ # Changelog +## 0.1.6 + +Findings from installing this on a live system, replacing a working YAML +implementation. Every one of these was silent - the add-on looked healthy while +being completely unable to do its job. + +- **`run.sh` must use `#!/usr/bin/with-contenv sh`.** The HA base images run + s6-overlay, which starts services with a SANITISED environment. With a plain + shebang, SUPERVISOR_TOKEN is simply absent and every Home Assistant call + returns 401 - while `homeassistant_api: true` makes the permissions look + correctly granted. Startup now logs the token length and probes the Core API, + so the next person sees it in one line. +- **Bump `version:` for every change.** Supervisor keys the built image by + version, so editing source and rebuilding silently reuses the old image. Two + fixes appeared not to work because of this. +- **Dependencies come from apk, not pip.** Alpine is musl and there are no musl + wheels for aiohttp; pip would compile it on the client's Pi. +- **paho-mqtt 1.x and 2.x are both supported.** Alpine ships 1.x, which has no + `CallbackAPIVersion`; that raised and took the whole add-on down with it. +- **MQTT can no longer take down control.** Publisher construction is wrapped - + observability must never stop the controller. +- **MQTT discovery is published from `on_connect`.** paho drops QoS-0 publishes + issued before the CONNACK, so announcing straight after `connect()` published + nothing at all while logging "MQTT connected". +- **Repeated failures log at most once a minute.** The control loop retries every + second; unthrottled warnings rolled the log buffer and destroyed the startup + diagnostics needed to debug the 401 above. +- **`auto_start` works.** The store's defaults supplied `auto: False`, so the + fallback to the option could never fire. + +Known issue: after deleting the MQTT entities from the registry during +development, Home Assistant would not re-adopt them from retained discovery - +not even after clearing the retained topics and reconnecting. The add-on +publishes correct discovery and live state (verified on the broker); this is an +HA-side adoption problem and affects status entities only, never control. + ## 0.1.0 First packaged release. Ports the control loop and the monthly maintenance diff --git a/goodwe_controller/Dockerfile b/goodwe_controller/Dockerfile index de7dfad..ffbf906 100644 --- a/goodwe_controller/Dockerfile +++ b/goodwe_controller/Dockerfile @@ -3,14 +3,13 @@ FROM ${BUILD_FROM} ENV LANG=C.UTF-8 PYTHONUNBUFFERED=1 PYTHONDONTWRITEBYTECODE=1 -RUN apk add --no-cache python3 py3-pip +# ⚠️ Install the dependencies from APK, not pip. Alpine ships musl, and there are +# no musl wheels for aiohttp on PyPI - pip would compile it from source on the +# client's Raspberry Pi, which needs a toolchain we would then have to ship and +# takes many minutes. apk has prebuilt packages for both. +RUN apk add --no-cache python3 py3-aiohttp py3-paho-mqtt WORKDIR /opt/goodwe -COPY requirements.txt ./ -# --break-system-packages: Alpine marks its python "externally managed" (PEP 668). -# This is a single-purpose container, so there is no environment to protect and a -# venv would only add a layer and a PATH to get wrong. -RUN pip3 install --no-cache-dir --break-system-packages -r requirements.txt COPY app/ ./app/ COPY run.sh /run.sh diff --git a/goodwe_controller/app/hass.py b/goodwe_controller/app/hass.py index 979602d..7ea8bb1 100644 --- a/goodwe_controller/app/hass.py +++ b/goodwe_controller/app/hass.py @@ -7,6 +7,7 @@ leak at a client site. That is one of the main reasons this is an add-on. import logging import os +import time import aiohttp @@ -21,9 +22,23 @@ class HomeAssistant: def __init__(self, session: aiohttp.ClientSession, token: str | None = None): self.session = session self.token = token or os.environ.get("SUPERVISOR_TOKEN", "") + self._last_moan: dict[str, float] = {} if not self.token: _LOG.error("SUPERVISOR_TOKEN missing - is this running as an add-on?") + def _moan(self, key: str, msg: str, *args) -> None: + """Log a recurring failure at most once a minute. + + ⚠️ The control loop retries every second, so an unthrottled warning here + writes six lines a second forever - which rolls the add-on's log buffer + and destroys exactly the startup diagnostics an installer needs. A fault + that repeats is not more informative for being repeated. + """ + now = time.monotonic() + if now - self._last_moan.get(key, -999) >= 60: + self._last_moan[key] = now + _LOG.warning(msg, *args) + @property def _headers(self) -> dict: return {"Authorization": f"Bearer {self.token}", "Content-Type": "application/json"} @@ -48,7 +63,7 @@ class HomeAssistant: resp.raise_for_status() return await resp.json() except (aiohttp.ClientError, TimeoutError) as err: - _LOG.warning("read %s failed: %s", entity_id, err) + self._moan(f"read:{entity_id}", "read %s failed: %s", entity_id, err) return None async def number(self, entity_id: str, invert: bool = False) -> float | None: @@ -62,7 +77,7 @@ class HomeAssistant: try: value = float(raw) except ValueError: - _LOG.warning("%s is not numeric: %r", entity_id, raw) + self._moan(f"nan:{entity_id}", "%s is not numeric: %r", entity_id, raw) return None return -value if invert else value @@ -97,12 +112,14 @@ class HomeAssistant: # value is rejected outright, not clamped. Always log it: # silently dropped commands are how a controller ends up # believing something the hardware never did. - _LOG.error("service %s.%s rejected (%s): %s", + self._moan(f"svc:{domain}.{service}", + "service %s.%s rejected (%s): %s", domain, service, resp.status, body[:200]) return False return True except (aiohttp.ClientError, TimeoutError) as err: - _LOG.warning("service %s.%s failed: %s", domain, service, err) + self._moan(f"svcerr:{domain}.{service}", "service %s.%s failed: %s", + domain, service, err) return False async def set_number(self, entity_id: str, value: float) -> bool: diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 1557339..54872dd 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -384,13 +384,45 @@ async def amain() -> None: async with aiohttp.ClientSession() as session: hass = HomeAssistant(session) - broker = await hass.mqtt_service() - pub = MqttPublisher( - broker.get("host") if broker else None, - broker.get("port", 1883) if broker else 1883, - broker.get("username") if broker else None, - broker.get("password") if broker else None, - ) + # Startup self-check: prove we can actually reach the Core API before + # anything tries to control an inverter with it. A 401 here is a + # permissions/token problem, not a configuration mistake, and saying so + # explicitly saves an installer from re-checking entity ids for an hour. + import os as _os + _tok = _os.environ.get("SUPERVISOR_TOKEN", "") + _LOG.info("supervisor token: %s (%d chars); env has: %s", + "present" if _tok else "MISSING", len(_tok), + ",".join(sorted(k for k in _os.environ if "TOKEN" in k.upper())) or "none") + try: + async with session.get("http://supervisor/core/api/", + headers={"Authorization": f"Bearer {_tok}"}, + timeout=10) as _r: + _LOG.info("core api probe: HTTP %s %s", _r.status, (await _r.text())[:80]) + except Exception as _e: # noqa: BLE001 + _LOG.error("core api probe failed: %s", _e) + + # ⚠️ Status publishing must NEVER be able to stop the controller. A + # broken broker, a missing library, an API change in paho - all of it is + # observability, and the battery does not care. Caught broadly and on + # purpose: this crashed the add-on once already (paho 1.x vs 2.x) and + # took the control loop down with it. + try: + broker = await hass.mqtt_service() + pub = MqttPublisher( + broker.get("host") if broker else None, + broker.get("port", 1883) if broker else 1883, + broker.get("username") if broker else None, + broker.get("password") if broker else None, + ) + except Exception as err: # noqa: BLE001 + _LOG.warning("MQTT unavailable (%s) - continuing without status entities", err) + + class _NoMqtt: + enabled = False + def publish(self, *a, **k): pass + def close(self): pass + + pub = _NoMqtt() controller = Controller(opts, hass, store, pub) runner = await web.start(controller, port=8099) diff --git a/goodwe_controller/app/mqtt.py b/goodwe_controller/app/mqtt.py index 3f30103..7c8fadf 100644 --- a/goodwe_controller/app/mqtt.py +++ b/goodwe_controller/app/mqtt.py @@ -17,21 +17,34 @@ except ImportError: # pragma: no cover - container always has it _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 RS485 Controller", + "name": "GoodWe", "manufacturer": "GoodWe (via RS485 meter emulation)", "model": "ES/BP series", } -# (key, name, unit, device_class, state_class, icon) +# (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", "W", "power", "measurement", None), - ("grid", "GoodWe grid power", "W", "power", "measurement", None), - ("battery", "GoodWe battery power", "W", "power", "measurement", None), - ("soc", "GoodWe battery SoC", "%", "battery", "measurement", None), - ("phase", "GoodWe maintenance phase", None, None, None, "mdi:battery-sync"), - ("status", "GoodWe controller status", None, None, None, "mdi:heart-pulse"), + ("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), + ("phase", "goodwe_maintenance_phase", "Maintenance phase", None, None, None, "mdi:battery-sync"), + ("status", "goodwe_controller_status", "Controller status", None, None, None, "mdi:heart-pulse"), ] BASE = "goodwe_ctl" @@ -45,24 +58,37 @@ class MqttPublisher: if not self.enabled: _LOG.info("MQTT not configured - status entities will not be published") return - self.client = mqtt.Client(mqtt.CallbackAPIVersion.VERSION2, - client_id="goodwe_rs485_controller") + # 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() - self._announce() _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: - for key, name, unit, dev_class, state_class, icon in SENSORS: + for key, object_id, name, unit, dev_class, state_class, icon in SENSORS: cfg = { "name": name, + "object_id": object_id, "unique_id": f"{BASE}_{key}", "state_topic": f"{BASE}/{key}", "availability_topic": AVAILABILITY, diff --git a/goodwe_controller/app/store.py b/goodwe_controller/app/store.py index fdf4444..91ac8a2 100644 --- a/goodwe_controller/app/store.py +++ b/goodwe_controller/app/store.py @@ -18,12 +18,15 @@ from datetime import datetime, timezone _LOG = logging.getLogger("goodwe.store") +# ⚠️ "auto" is deliberately NOT in here. Controller falls back to the add-on's +# `auto_start` option only when the key is absent - if a default supplied False, +# the fallback could never fire and auto_start would silently do nothing on +# every fresh install. DEFAULTS = { "phase": "idle", "phase_started": None, "last_completed": None, "last_start_attempt": None, - "auto": False, } diff --git a/goodwe_controller/config.yaml b/goodwe_controller/config.yaml index 4f82e0b..bdd928a 100644 --- a/goodwe_controller/config.yaml +++ b/goodwe_controller/config.yaml @@ -1,5 +1,5 @@ name: GoodWe RS485 Controller -version: "0.1.0" +version: "0.1.6" slug: goodwe_controller description: >- Drives a GoodWe ES/BP battery inverter over RS485 by emulating its smart diff --git a/goodwe_controller/requirements.txt b/goodwe_controller/requirements.txt index e7522ba..790e518 100644 --- a/goodwe_controller/requirements.txt +++ b/goodwe_controller/requirements.txt @@ -1,2 +1,5 @@ -aiohttp==3.10.11 -paho-mqtt==2.1.0 +# Installed from Alpine packages in the Dockerfile (py3-aiohttp, py3-paho-mqtt), +# not by pip - there are no musl wheels for aiohttp and compiling it on a client's +# Pi is not acceptable. Listed here for reference and for local development: +aiohttp>=3.9 +paho-mqtt>=2.0 diff --git a/goodwe_controller/run.sh b/goodwe_controller/run.sh index 1f48da2..4ef4ac7 100644 --- a/goodwe_controller/run.sh +++ b/goodwe_controller/run.sh @@ -1,6 +1,16 @@ -#!/usr/bin/env sh -# The add-on entrypoint. Deliberately thin: everything that can fail in an -# interesting way belongs in Python where it can be logged and tested. +#!/usr/bin/with-contenv sh +# ⚠️ THE SHEBANG IS LOAD-BEARING. It must be `with-contenv`, not `env sh`. +# +# The Home Assistant base images run s6-overlay, which starts services with a +# SANITISED environment: the container's variables live in +# /run/s6/container_environment and are only re-imported by `with-contenv`. +# With a plain `#!/usr/bin/env sh` the add-on starts perfectly, serves its UI, +# and then fails EVERY Home Assistant call with 401 Unauthorized - because +# SUPERVISOR_TOKEN is simply absent, while `homeassistant_api: true` in +# config.yaml makes the permissions look correctly granted. +# +# Diagnosed 2026-08-23 the slow way. The startup probe in main.py now prints the +# token length so the next person sees it in one line instead of an hour. set -e cd /opt/goodwe exec python3 -m app.main