admin/goodwe-addon
1
0
Fork 0
Code Issues Pull Requests 4 Actions Packages Projects Releases Wiki Activity

Six silent failures found by installing this on a live system

Browse Source 
Migrated the reference site off the YAML packages and onto the add-on. Every
bug below presented identically: the add-on starts, logs "started", serves its
UI, and cannot do its job.

- run.sh needs #!/usr/bin/with-contenv sh. s6-overlay sanitises the environment
  for services, so a plain shebang means SUPERVISOR_TOKEN is absent and every
  Core API call is 401 - while homeassistant_api: true makes permissions look
  granted. Startup now prints the token length and probes the API.
- Supervisor keys the image by config.yaml `version`, so rebuilding without a
  bump reuses the old image. Two fixes appeared not to work because of it.
- Alpine is musl and has no aiohttp wheel on PyPI; deps now come from apk so
  nothing compiles on a client's Pi.
- Alpine ships paho-mqtt 1.x, which has no CallbackAPIVersion. That raised at
  construction and took the control loop down with it - so MQTT setup is now
  wrapped too. Observability must never be able to stop the controller.
- MQTT discovery is published from on_connect: paho silently drops QoS-0
  publishes issued before the CONNACK, so the previous code announced nothing
  while logging "MQTT connected".
- Repeated failures now log once a minute. Six warnings a second rolled the log
  buffer and destroyed the startup diagnostics needed to find the 401.
- auto_start could never fire, because the store's defaults always supplied
  auto: False for the fallback to find.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016NckgXecasQb2eSsPYNSW6
This commit is contained in:
admin
2026-08-23 02:25:58 +02:00
co-authored by Claude Opus 5
parent 1abca15520
commit 594f8f9fc9
9 changed files with 162 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/hass.py
+21 -4
View File
@@ -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:
goodwe_controller/app/main.py
+39 -7
View File
@@ -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)
goodwe_controller/app/mqtt.py
+38 -12
View File
@@ -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,
goodwe_controller/app/store.py
+4 -1
View File
@@ -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,
}