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

CFG-01: refuse to start on a configuration that cannot be honoured

Browse Source 
main.py used to read the setpoint device's own +-5000 W rating, notice a
configured max_w above it, log one WARNING, and run anyway with
write_setpoint() quietly clamping. The operator's stated ceiling was never
the real ceiling and the only evidence had scrolled off the log. specs
§1.5 is explicit that such a config is refused at startup, not survived at
runtime.

validate_startup_config() runs in amain() before the broker, the web
server or any task exists, so a refusal cannot leave half a controller
running behind a UI. It refuses exactly three things, all sharing one
property - continuing makes a safety mechanism inoperative or makes a
stated limit untrue:

  1. max_w outside the setpoint device's own rating.
  2. No setpoint_entity: every write is a no-op, the device rating is
     never read, and the ESP32 heartbeat that holds off the layer-1
     failsafe never happens. It ran, looked healthy, and moved nothing.
  3. A meter_source that can never produce a reading. These were already
     caught, but they only DISABLED ingestion, which leaves grid None
     forever: permanent inputs-missing, 0 W, and a status page
     indistinguishable from a healthy meter nobody has fed yet.

Those last predicates move into p1.validate_meter_options(), which
build_source() also calls - one definition, two callers, not two
competing validators. ha_dsmr with half its entities and mqtt_p1 with a
blank topic gained the check the other two transports already had.

Deliberately not refused, because untidy is not unsafe: integrator_max_w
above max_w (the arbiter still bounds the wire to +-max_w, so the cost is
unwind latency - warned), a missing soc/batt entity (fail-safe and
already a red NOT READY banner), a setpoint entity that has not appeared
on the Core API yet (ESPHome is routinely late; re-checked in
run_control() once the real rating arrives), and a thin meter_poll_s
margin.

A refusal is legible as one: a banner saying in words that this is
configuration and nothing is broken, numbered problems, no traceback, and
exit 78 (EX_CONFIG).

Verified on the live add-on 2026-08-28, not by reading the code: a blank
setpoint_entity and a meter_poll_s equal to meter_max_age_s each stopped
the add-on with the banner and no stack trace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
This commit is contained in:
glenn schrooyen
2026-08-28 19:29:35 +02:00
co-authored by Claude Opus 5
parent 0422127cab
commit fc9bd89dca
5 changed files with 582 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/CHANGELOG.md
+42
View File
@@ -2,6 +2,48 @@
## 0.4.0
**CFG-01.** The add-on now refuses to start on a configuration it cannot honour,
instead of logging a line and carrying on. `validate_startup_config()` runs in
`amain()` before the broker, the web server or any task exists, so a refusal
cannot leave half a controller running behind a UI.
It refuses exactly three things, all of which share one property - continuing
makes a safety mechanism inoperative or makes a stated limit untrue:
- **`max_w` above (or below) the setpoint device's own rating.** This is the one
that changed behaviour. It used to warn `the device wins` and run anyway, with
`write_setpoint()` silently clamping, so the configured ceiling was never the
real one and the only evidence had scrolled off the log. The refusal names the
configured value, the device's value, and which wins.
- **No `setpoint_entity`.** A controller with no actuator: every write is a
no-op, the device rating is never read, and the ESP32 heartbeat that holds off
the layer-1 failsafe never happens. It ran, looked healthy, and moved nothing.
- **A `meter_source` that can never produce a reading** - missing `p1_host`,
missing `p1_net_entity`, a blank MQTT topic, a phase list disagreeing with
`meter_phases`, an unknown source name, or a `meter_poll_s` at or past
`meter_max_age_s`. These were already caught, but they only *disabled
ingestion*, which leaves the grid reading None forever: permanent "inputs
missing", 0 W, and a status page indistinguishable from a healthy meter nobody
has sent anything to yet.
The predicates for the last group still live in `p1.validate_meter_options()`,
which `build_source()` also calls - one definition, two callers, not two
competing validators. Two transports gained a check they never had: `ha_dsmr`
with only one of its two entities, and `mqtt_p1` with a blank topic, both of
which previously built a transport that could never read.
Deliberately **not** refused, because it is untidy rather than unsafe:
`integrator_max_w` above `max_w` (the arbiter still bounds the wire to ±max_w,
so the cost is unwind latency, not amperage - warned), a missing `soc_entity` /
`batt_entity` (fail-safe, and already a red NOT READY banner naming it), a
`setpoint_entity` that has not appeared on the Core API yet (ESPHome is routinely
late; refusing would make a slow boot need a human - re-checked once the real
rating arrives), and a thin-but-nonzero `meter_poll_s` margin.
A refusal is legible as a refusal: a banner that says in words that this is a
configuration problem and nothing is broken, the numbered problems, no traceback,
and exit status 78 (`EX_CONFIG`).
**TEL-04.** `meter_source: ha_signed` reads a single *signed* power entity, which
is the shape a HomeWizard P1 publishes. Both transports 0.3.0 shipped require two
unsigned registers and refuse negatives, so neither fits the meter installed here.
goodwe_controller/app/main.py
+59 -7
View File
@@ -40,6 +40,7 @@ from .hass import HomeAssistant
from .maintenance import IDLE, MaintConfig, Maintenance
from .mqtt import MqttPublisher
from .p1 import P1Ingest, build_source, is_enabled
from .validate import ConfigRefused, refuse, validate_startup_config
from . import web
OPTIONS_PATH = "/data/options.json"
@@ -56,11 +57,15 @@ def load_options() -> dict:
class Controller:
def __init__(self, opts: dict, hass: HomeAssistant, store, mqtt_pub):
def __init__(self, opts: dict, hass: HomeAssistant, store, mqtt_pub, stop=None):
self.o = opts
self.hass = hass
self.store = store
self.mqtt = mqtt_pub
# Set by run_control() if the startup gate could not see a conflict
# because the setpoint device had not appeared yet. See run_control().
self.stop_event = stop
self.refused: list[str] | None = None
self.tuning = Tuning(
gain=float(opts.get("gain", 0.6)),
@@ -303,10 +308,26 @@ class Controller:
entity = self.o.get("setpoint_entity", "")
if entity:
self.dev_min, self.dev_max, _ = await self.hass.limits(entity)
if self.dev_max is not None and self.tuning.max_w > self.dev_max:
self.log_event(
f"clamp {self.tuning.max_w:.0f} W exceeds the device maximum "
f"{self.dev_max:.0f} W - the device wins")
# ⚠️ THIS USED TO WARN AND CARRY ON, and that was the bug CFG-01
# exists to remove: a max_w above the device's rating meant
# write_setpoint() silently clamped and the configured ceiling was
# never the real one.
#
# amain() already ran this gate before anything started. It runs
# AGAIN here for one case the gate cannot cover: ESPHome is often
# not up yet when the add-on boots, so hass.limits() returned
# (None, None) and the range checks were skipped rather than
# guessed at. This is the first moment the device's real rating is
# known. If it conflicts, refuse now - stopping deterministically
# through the normal shutdown, which commands 0 W on the way out.
problems = validate_startup_config(self.o, self.dev_min, self.dev_max)
if problems:
refuse(problems)
self.refused = problems
self.log_event("REFUSED: " + problems[0])
if self.stop_event is not None:
self.stop_event.set()
return
last_grid = object()
heartbeat = float(self.o.get("heartbeat_s", 10))
@@ -515,6 +536,26 @@ async def amain() -> None:
except Exception as _e: # noqa: BLE001
_LOG.error("core api probe failed: %s", _e)
# ⚠️ THE STARTUP GATE (CFG-01). Nothing below this line may run on a
# configuration validate.py rejects - specs §1.5: a config the system
# cannot honour is refused at startup, not survived at runtime.
#
# It sits HERE, before the broker, before the web server and before any
# task exists, so a refusal cannot leave half a controller running with
# a UI in front of it. Reading the device range is the one thing the
# validator cannot do for itself; (None, None) when ESPHome has not come
# up yet is not a refusal - run_control() re-checks once the real rating
# is known. See validate.py for where the line is drawn and why.
_sp = str(opts.get("setpoint_entity", "") or "").strip()
_dev_min = _dev_max = None
if _sp:
_dev_min, _dev_max, _ = await hass.limits(_sp)
_LOG.info("setpoint device %s reports range %s…%s W", _sp, _dev_min, _dev_max)
_problems = validate_startup_config(opts, _dev_min, _dev_max)
if _problems:
refuse(_problems)
raise ConfigRefused(_problems)
# ⚠️ 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
@@ -540,10 +581,10 @@ async def amain() -> None:
pub = _NoMqtt()
controller = Controller(opts, hass, store, pub)
stop = asyncio.Event()
controller = Controller(opts, hass, store, pub, stop=stop)
runner = await web.start(controller, port=8099)
stop = asyncio.Event()
loop = asyncio.get_running_loop()
for sig in (signal.SIGTERM, signal.SIGINT):
with contextlib.suppress(NotImplementedError):
@@ -570,6 +611,10 @@ async def amain() -> None:
await task
await runner.cleanup()
_LOG.info("stopped")
# The late-arriving-device refusal from run_control(). Re-raised only
# now, so the wind-down above still ran and the inverter is at 0 W.
if controller.refused:
raise ConfigRefused(controller.refused)
def main() -> None:
@@ -577,6 +622,13 @@ def main() -> None:
asyncio.run(amain())
except KeyboardInterrupt:
pass
except ConfigRefused:
# ⚠️ No traceback, on purpose. The banner refuse() already printed says
# everything an operator needs, and a stack trace on top of it would say
# "this crashed" - the exact confusion AC3 is about. 78 is sysexits(3)
# EX_CONFIG, so even a script reading the exit status can tell a bad
# config from a bug.
raise SystemExit(78)
if __name__ == "__main__":
goodwe_controller/app/p1.py
+130 -67
View File
@@ -1044,15 +1044,133 @@ def is_enabled(opts: dict) -> bool:
return str(opts.get("meter_source", "off") or "off").strip() not in ("off", "")
def homewizard_host_port(opts: dict) -> tuple[str, str]:
"""`p1_host` split into host and (still unparsed) port text.
One definition because validate_meter_options() and build_source() must
agree byte for byte about what counts as a usable address - a pasted browser
URL is the obvious way to fill this in wrong.
"""
host = str(opts.get("p1_host", "") or "").strip()
host = host.removeprefix("http://").removeprefix("https://").rstrip("/")
host, _, port_s = host.partition(":")
return host, port_s
def validate_meter_options(opts: dict, phases: int, max_age_s: float) -> list[str]:
"""Every way `meter_source` can name a meter that will never produce a reading.
⚠️ THIS IS THE ONLY PLACE THAT KNOWS THESE RULES. It used to be a run of
inline `return None`s inside build_source(); CFG-01 lifted them out because
the same predicates now have two callers with two different consequences:
validate.validate_startup_config() refuses to start the add-on
build_source() returns None, as it always did
They are one function and not two on purpose. A second copy of "what makes a
meter config wrong" would drift, and the half that drifted would be the one
that decides whether the process is allowed to run.
Checked ONCE at startup rather than per telegram, because a misconfigured
source otherwise fails in the one way indistinguishable from a healthy source
nobody has sent anything to yet: no samples, a climbing age, the firmware
watchdog holding the battery at 0 W, and nothing in the log saying why.
Returns a list of human-readable problems; empty means the config is usable.
`phases` and `max_age_s` are passed in rather than read from opts so the
caller decides whether it is validating the options or an already-built
P1Ingest - build_source() has an ingest whose values may not have come from
this dict.
"""
source = str(opts.get("meter_source", "off") or "off").strip()
if source in ("off", ""):
return []
problems: list[str] = []
if source == SOURCE_HA:
imp = str(opts.get("p1_import_entity", "") or "").strip()
exp = str(opts.get("p1_export_entity", "") or "").strip()
if not imp or not exp:
problems.append(
f"meter_source {SOURCE_HA} needs both p1_import_entity and "
f"p1_export_entity (got {imp or '<blank>'!r} / {exp or '<blank>'!r})")
elif source == SOURCE_HA_SIGNED:
net = str(opts.get("p1_net_entity", "") or "").strip()
phase_net = [str(e).strip() for e in
(opts.get("p1_phase_net_entities") or []) if str(e).strip()]
if not net:
problems.append(
f"meter_source {SOURCE_HA_SIGNED} needs p1_net_entity "
"(sensor.p1_sample_age_s would otherwise be announced with "
"nothing feeding it)")
if phase_net and len(phase_net) != phases:
problems.append(
f"p1_phase_net_entities has {len(phase_net)} entities but "
f"meter_phases is {phases} - every telegram would be rejected "
"on the phase-count check")
elif source == SOURCE_MQTT:
if not str(opts.get("meter_mqtt_topic", "") or "").strip():
problems.append(f"meter_source {SOURCE_MQTT} needs meter_mqtt_topic")
elif source == SOURCE_HOMEWIZARD:
host, port_s = homewizard_host_port(opts)
if not host:
problems.append(
f"meter_source {SOURCE_HOMEWIZARD} needs p1_host (the meter's "
"own address, e.g. 192.168.2.250)")
if port_s:
try:
int(port_s)
except ValueError:
problems.append(f"p1_host {opts.get('p1_host')!r} has an "
"unparseable port")
try:
# ⚠️ Not `or 5`: that turns an explicit 0 into the default, and a
# cadence of 0 is a misconfiguration that must be reported, not
# quietly corrected into something that looks like it was asked for.
raw = opts.get("meter_poll_s", 5)
poll_s = float(5 if raw is None or raw == "" else raw)
except (TypeError, ValueError):
problems.append(f"meter_poll_s {opts.get('meter_poll_s')!r} is not a number")
else:
if poll_s <= 0:
problems.append("meter_poll_s must be positive")
# ⚠️ The age can never be fresher than the poll interval. At or past
# max_age_s every reading is stale before its successor arrives, so
# the controller would sit permanently on missing inputs while the
# meter is perfectly healthy - refuse it rather than ship that.
elif poll_s >= max_age_s:
problems.append(
f"meter_poll_s {poll_s:.1f} is not under meter_max_age_s "
f"{max_age_s:.1f} - every reading would go stale before the "
"next poll")
else:
problems.append(
f"meter_source {source!r} is not one of "
+ ", ".join((SOURCE_HA, SOURCE_MQTT, SOURCE_HA_SIGNED, SOURCE_HOMEWIZARD)))
return problems
def build_source(opts: dict, ingest: P1Ingest, session, broker: dict | None):
"""Return the transport named by `meter_source`, or None if disabled.
This is the whole of AC 1's "switchable": every consumer reads P1Ingest, so
changing transport is a config edit, never a code path.
⚠️ In the add-on this is now unreachable with a bad config -
validate_startup_config() refuses to start on anything
validate_meter_options() rejects. The guard stays because this function is
also called directly by tests and by anything embedding P1Ingest, and
"returns None" is a safer default for those callers than a transport that
can never read.
"""
source = str(opts.get("meter_source", "off") or "off").strip()
if not is_enabled(opts):
return None
problems = validate_meter_options(opts, ingest.phases, ingest.max_age_s)
if problems:
for problem in problems:
_LOG.error("%s - P1 ingestion disabled", problem)
return None
if source == SOURCE_HA:
return HaDsmrSource(session, ingest, {
"import": opts.get("p1_import_entity", ""),
@@ -1061,70 +1179,19 @@ def build_source(opts: dict, ingest: P1Ingest, session, broker: dict | None):
"phase_export": opts.get("p1_phase_export_entities") or [],
})
if source == SOURCE_HA_SIGNED:
net = str(opts.get("p1_net_entity", "") or "").strip()
phase_net = [str(e).strip() for e in
(opts.get("p1_phase_net_entities") or []) if str(e).strip()]
# ⚠️ Both of these are checked ONCE here rather than per telegram. A
# misconfigured source otherwise fails silently in the only way that
# looks exactly like a healthy one that has not been sent anything yet:
# no samples, a climbing age, and the firmware watchdog holding the
# battery at 0 W with nothing in the log saying why.
if not net:
_LOG.error("meter_source %s needs p1_net_entity - P1 ingestion "
"disabled (sensor.p1_sample_age_s would otherwise be "
"announced with nothing feeding it)", SOURCE_HA_SIGNED)
return None
if phase_net and len(phase_net) != ingest.phases:
_LOG.error("p1_phase_net_entities has %d entities but meter_phases "
"is %d - P1 ingestion disabled. Every telegram would be "
"rejected on the phase-count check.",
len(phase_net), ingest.phases)
return None
return HaSignedSource(session, ingest,
{"net": net, "phase_net": phase_net})
return HaSignedSource(session, ingest, {
"net": str(opts.get("p1_net_entity", "") or "").strip(),
"phase_net": [str(e).strip() for e in
(opts.get("p1_phase_net_entities") or []) if str(e).strip()],
})
if source == SOURCE_HOMEWIZARD:
host = str(opts.get("p1_host", "") or "").strip()
# A pasted browser URL is the obvious way to fill this in wrong.
host = host.removeprefix("http://").removeprefix("https://").rstrip("/")
host, _, port_s = host.partition(":")
# ⚠️ Checked ONCE here, like the ha_signed entity ids and for the same
# reason: a misconfigured source otherwise fails in the one way that
# looks exactly like a healthy one nobody has sent anything to yet - no
# samples, a climbing age, the watchdog holding the battery at 0 W, and
# nothing in the log saying why.
if not host:
_LOG.error("meter_source %s needs p1_host (the meter's own address, "
"e.g. 192.168.2.250) - P1 ingestion disabled",
SOURCE_HOMEWIZARD)
return None
try:
port = int(port_s) if port_s else HOMEWIZARD_PORT
except ValueError:
_LOG.error("p1_host %r has an unparseable port - P1 ingestion disabled",
opts.get("p1_host"))
return None
try:
# ⚠️ Not `or 5`: that turns an explicit 0 into the default, and a
# cadence of 0 is a misconfiguration that must be reported, not
# quietly corrected into something that looks like it was asked for.
raw = opts.get("meter_poll_s", 5)
poll_s = float(5 if raw is None or raw == "" else raw)
except (TypeError, ValueError):
_LOG.error("meter_poll_s %r is not a number - P1 ingestion disabled",
opts.get("meter_poll_s"))
return None
if poll_s <= 0:
_LOG.error("meter_poll_s must be positive - P1 ingestion disabled")
return None
# ⚠️ The age can never be fresher than the poll interval. At or past
# max_age_s every reading is stale before its successor arrives, so the
# controller would sit permanently on missing inputs while the meter is
# perfectly healthy - refuse it rather than ship that.
if poll_s >= ingest.max_age_s:
_LOG.error("meter_poll_s %.1f is not under meter_max_age_s %.1f - every "
"reading would go stale before the next poll. P1 ingestion "
"disabled.", poll_s, ingest.max_age_s)
return None
host, port_s = homewizard_host_port(opts)
port = int(port_s) if port_s else HOMEWIZARD_PORT
raw = opts.get("meter_poll_s", 5)
poll_s = float(5 if raw is None or raw == "" else raw)
# A warning, not a refusal: one missed poll costs a stale reading and a
# 0 W cycle, which is fail-safe. The refusal above is for cadences that
# can NEVER be fresh.
if poll_s > ingest.max_age_s / 2:
_LOG.warning("meter_poll_s %.1f leaves no room under meter_max_age_s "
"%.1f: one missed poll makes the reading stale. The meter "
@@ -1136,8 +1203,4 @@ def build_source(opts: dict, ingest: P1Ingest, session, broker: dict | None):
return MqttP1Source(ingest, str(opts.get("meter_mqtt_topic", "")),
broker.get("host"), broker.get("port", 1883),
broker.get("username"), broker.get("password"))
if source:
_LOG.error("meter_source %r is not one of %s - P1 ingestion disabled",
source, ", ".join((SOURCE_HA, SOURCE_MQTT, SOURCE_HA_SIGNED,
SOURCE_HOMEWIZARD)))
return None
goodwe_controller/app/validate.py
+190
View File
@@ -0,0 +1,190 @@
"""Startup configuration validation - the gate before the control loop runs.
WHAT THIS IS FOR, in one sentence: some configurations are not "a bit off", they
are configurations under which the add-on's own safety machinery does not work,
and for those the correct behaviour is to refuse to start rather than to log a
line and carry on.
⚠️ THE FAILURE THIS REPLACES. `main.py` used to read the setpoint device's own
±5000 W rating, notice that a configured `max_w` of, say, 8000 W exceeded it,
write one WARNING to a log nobody reads, and then run anyway with
`write_setpoint()` quietly clamping. The operator's stated ceiling was simply not
the ceiling, and the only evidence was a line that had scrolled off. specs §1.5
is explicit that a configuration in which the system cannot honour what it was
told is refused at startup, not survived at runtime.
WHERE THE LINE IS DRAWN, and why it is not further out. A validator that refuses
on trivia gets switched off by the first person it inconveniences, so this one
refuses on exactly three things, all of which share one property: **continuing
makes a safety mechanism inoperative or makes a stated limit untrue.**
1. `max_w` outside the setpoint device's own rating.
The stated limit is untrue. arbiter.py's `supervised` SAFETY claim says the
output is bounded by ±max_w; it is not, it is bounded by the device, and the
two numbers disagree. Which one wins is a coin toss to anyone reading the
config.
2. No `setpoint_entity`.
This is a controller with no actuator, and nothing about it looks broken.
`write_setpoint()` returns immediately on a blank entity, so: no device
rating is ever read (the `device` SAFETY claim is never made at all), and
the ESP32 heartbeat - the thing that feeds LAYER 1 - never happens. The
add-on serves a UI, publishes MQTT, resolves claims, and moves nothing. It
is fail-safe by accident, not by design, and it is indistinguishable from
working.
3. A `meter_source` that can never produce a reading.
§5.1 makes P1 the only authoritative measurement and deliberately provides
no fallback, so a transport that cannot be built means `grid` is None
forever: permanent "inputs missing", 0 W, and a status page that reports a
meter with a climbing age exactly as it would for a healthy meter nobody has
sent anything to yet. The predicates live in `p1.validate_meter_options()`,
which is also what `build_source()` uses - one definition, two callers.
WHAT IS DELIBERATELY *NOT* REFUSED, because it is untidy rather than unsafe:
- `integrator_max_w` above `max_w`. DOCS.md is right that the only thing the
extra headroom buys is more wrong-direction power to unwind after
saturation - but the arbiter's `supervised` limit still binds the wire to
±max_w, so nothing out of range ever reaches the inverter. The cost is
latency, not amperage. Warned about, not refused.
- A missing `soc_entity` / `batt_entity`. `inputs_ok` goes False, the loop
commands 0 W, and `checks()` already renders a red NOT READY banner naming
the missing entity. Fail-safe AND already loud; a refusal adds nothing.
- A `setpoint_entity` that exists in the config but is not found on the Core
API at startup. This is the transient case - ESPHome is frequently not up
yet when the add-on boots - and refusing here would turn a device that
arrives thirty seconds late into a restart loop needing a human. It stays a
NOT READY banner.
- `meter_poll_s` above half `meter_max_age_s`. One missed poll costs a stale
reading and a 0 W cycle. Warned about in p1.py, not refused.
"""
import logging
from .p1 import validate_meter_options
_LOG = logging.getLogger("goodwe.config")
# ⚠️ The complete set of option keys through which this add-on can reach an
# actuator. specs §1.5 makes "any configuration in which two writers can reach
# one actuator" a non-goal, and the way that non-goal is ENFORCED here is by
# construction: there is exactly one such key, so a second writer is not
# expressible in the options schema at all. sim/inject.py's `two_writers`
# scenario asserts on this tuple - if a second write path is ever added, that
# scenario goes red and somebody has to decide how the two are arbitrated
# BEFORE the config that allows both can ship.
ACTUATOR_WRITE_KEYS = ("setpoint_entity",)
class ConfigRefused(Exception):
"""The configuration is wrong. Not "something broke" - see main.py's handler.
Carries the list of problems so the caller can render them; str() is the
first one, so an unhandled traceback still names something useful.
"""
def __init__(self, problems: list[str]):
self.problems = problems
super().__init__(problems[0] if problems else "configuration refused")
def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str]:
"""Return the list of reasons this configuration must not be started.
Empty list means go. `dev_min`/`dev_max` are the setpoint device's own
reported range (`hass.limits()`), or None when the device could not be
reached - in which case the range checks are skipped rather than guessed at,
for the transient reason in this module's docstring.
Pure and side-effect free apart from two warnings, so it can be called from
a test or from sim/inject.py without a Home Assistant anywhere near it.
"""
problems: list[str] = []
# --- 1. an actuator to write to ------------------------------------------
writers = [k for k in ACTUATOR_WRITE_KEYS
if str(opts.get(k, "") or "").strip()]
if not writers:
problems.append(
"setpoint_entity is not set, so this add-on has no actuator: every "
"write is a no-op, the device's own rating is never read, and the "
"ESP32 heartbeat that holds off the layer-1 failsafe never happens. "
"It would run, look healthy, and move no energy at all. Set it to "
"the ESPHome number entity that carries the setpoint.")
# --- 2. the configured ceiling against the device's own rating -----------
max_w = _number(opts, "max_w", 2000.0)
if max_w is None:
problems.append(f"max_w {opts.get('max_w')!r} is not a number")
elif dev_max is not None and max_w > dev_max:
problems.append(
f"max_w is {max_w:.0f} W but {opts.get('setpoint_entity') or 'the setpoint device'} accepts at "
f"most {dev_max:.0f} W - THE DEVICE WINS, so the {max_w:.0f} W you "
f"configured would silently never happen. Lower max_w to "
f"{dev_max:.0f} W or less, or point setpoint_entity at a device that "
f"can take {max_w:.0f} W.")
elif dev_min is not None and -max_w < dev_min:
problems.append(
f"max_w is {max_w:.0f} W, so this controller may command "
f"{-max_w:.0f} W, but {opts.get('setpoint_entity') or 'the setpoint device'} accepts no less "
f"than {dev_min:.0f} W - THE DEVICE WINS on the charge side. Lower "
f"max_w to {abs(dev_min):.0f} W or less.")
# --- 3. a meter that can actually read -----------------------------------
# ⚠️ Not a second copy of these rules - p1.py owns them, because
# build_source() has to apply the identical predicates and a drifted copy
# would be worse than no check.
phases = _number(opts, "meter_phases", 1.0)
max_age_s = _number(opts, "meter_max_age_s", 30.0)
if phases is None or max_age_s is None:
problems.append("meter_phases and meter_max_age_s must both be numbers "
f"(got {opts.get('meter_phases')!r} / "
f"{opts.get('meter_max_age_s')!r})")
else:
problems.extend(validate_meter_options(opts, int(phases), max_age_s))
# --- warnings: real, but not worth refusing over -------------------------
integ = opts.get("integrator_max_w")
integ_w = None if not integ else _number(opts, "integrator_max_w", None)
if integ_w is not None and max_w is not None and integ_w > max_w:
_LOG.warning(
"integrator_max_w %.0f W is above max_w %.0f W. Not refused - the "
"arbiter still bounds the wire to +-max_w - but the only thing the "
"extra headroom buys is more wrong-direction power to unwind after "
"saturation. 0 (follow max_w) is the recommended value.",
integ_w, max_w)
return problems
def _number(opts: dict, key, default):
raw = opts.get(key, default)
if raw is None or raw == "":
return default
try:
return float(raw)
except (TypeError, ValueError):
return None
def refuse(problems: list[str]) -> None:
"""Log a refusal an operator can tell apart from a crash. Does not raise.
⚠️ THE POINT OF THE BANNER. "You configured this wrong" must not read like
"it broke". A Python traceback in an add-on log means the same thing to most
operators as a segfault - file a bug - whereas this block names the keys,
names the fix, and says in words that nothing is broken. main.py catches
ConfigRefused and exits 78 (sysexits EX_CONFIG), so no traceback is printed
at all and even the exit status distinguishes the two.
"""
_LOG.error("=" * 72)
_LOG.error("REFUSING TO START: the add-on configuration is not usable.")
_LOG.error("This is a CONFIGURATION problem, not a crash. Nothing is broken;")
_LOG.error("the add-on will start as soon as the settings below are fixed.")
_LOG.error("Fix them in Settings > Add-ons > GoodWe RS485 Controller >")
_LOG.error("Configuration, then start the add-on again.")
_LOG.error("-" * 72)
for i, problem in enumerate(problems, 1):
_LOG.error(" %d. %s", i, problem)
_LOG.error("=" * 72)
goodwe_controller/test_validate.py
+161
View File
@@ -0,0 +1,161 @@
"""Runnable check for the startup gate. `python3 test_validate.py`
Two halves, and the second is the one that matters. The first asserts that the
three refusable configurations are refused. The second asserts that everything
on validate.py's "deliberately not refused" list still STARTS - because the way
a validator dies is not by missing a fault, it is by refusing something harmless
until somebody deletes the call.
"""
import sys
from app.validate import (
ACTUATOR_WRITE_KEYS, ConfigRefused, refuse, validate_startup_config,
)
fails = []
def check(name, cond):
print(f" {'ok ' if cond else 'FAIL'} {name}")
if not cond:
fails.append(name)
def base(**over):
"""A configuration that must start. Everything else is this, spoilt."""
o = {"setpoint_entity": "number.goodwe_setpoint", "max_w": 2000,
"soc_entity": "sensor.soc", "batt_entity": "sensor.batt",
"meter_source": "off"}
o.update(over)
return o
def why(problems, needle):
return any(needle in p for p in problems)
# --------------------------------------------------------------------------- #
print("the baseline must start, or nothing below means anything")
check("a good config with a 5000 W device is accepted",
validate_startup_config(base(), -5000, 5000) == [])
check("...and with no device reachable yet, too",
validate_startup_config(base(), None, None) == [])
# --------------------------------------------------------------------------- #
print("1. max_w against the device's own rating")
# ⚠️ THE ORIGINAL BUG. main.py used to log 'the device wins' and run anyway,
# with write_setpoint() clamping, so the configured ceiling was never the real
# one and the only evidence had scrolled off the log.
p = validate_startup_config(base(max_w=8000), -5000, 5000)
check("max_w above the device maximum is refused", len(p) == 1)
check("...and the message names the configured value", why(p, "8000"))
check("...and the device's value", why(p, "5000"))
check("...and says which one wins", why(p, "THE DEVICE WINS"))
check("max_w exactly at the device maximum is fine",
validate_startup_config(base(max_w=5000), -5000, 5000) == [])
check("max_w under it is fine",
validate_startup_config(base(max_w=2000), -5000, 5000) == [])
# The charge side is a separate rail and an asymmetric device is legal.
p = validate_startup_config(base(max_w=4000), -2500, 5000)
check("a max_w the device cannot take on the CHARGE side is refused too",
len(p) == 1 and why(p, "charge side"))
check("...naming the device's floor", why(p, "-2500"))
# ⚠️ Not guessed at: the entity may simply not have booted yet. run_control()
# re-runs this the moment the real rating arrives.
check("an unreachable device skips the range check rather than assuming",
validate_startup_config(base(max_w=8000), None, None) == [])
check("a non-numeric max_w is refused",
why(validate_startup_config(base(max_w="lots"), -5000, 5000), "not a number"))
# --------------------------------------------------------------------------- #
print("2. an actuator to write to")
# A controller with no actuator: write_setpoint() returns on the blank entity,
# so no rating is read, no heartbeat is sent, layer 1 pins the inverter at 0 W,
# and the add-on looks perfectly healthy the whole time.
for blank in ("", " ", None):
p = validate_startup_config(base(setpoint_entity=blank), None, None)
check(f"setpoint_entity {blank!r} is refused", len(p) == 1)
check("...and the message says why it is dangerous rather than untidy",
why(validate_startup_config(base(setpoint_entity=""), None, None),
"heartbeat"))
# specs §1.5: two writers on one actuator is not expressible in this schema.
check("there is exactly one way to reach the actuator",
ACTUATOR_WRITE_KEYS == ("setpoint_entity",))
# --------------------------------------------------------------------------- #
print("3. a meter that can never read")
# ⚠️ These predicates belong to p1.validate_meter_options(), which build_source()
# also uses. Asserted here to prove the gate actually consults it - one entry
# point, not two.
check("meter_source off asks nothing of the P1 keys",
validate_startup_config(base(meter_source="off"), -5000, 5000) == [])
check("homewizard_local with no p1_host is refused",
why(validate_startup_config(base(meter_source="homewizard_local"),
-5000, 5000), "p1_host"))
check("...and with a host it is accepted",
validate_startup_config(base(meter_source="homewizard_local",
p1_host="192.168.2.250"), -5000, 5000) == [])
check("a poll cadence at meter_max_age_s is refused at STARTUP, not just disabled",
why(validate_startup_config(base(meter_source="homewizard_local",
p1_host="m", meter_poll_s=30,
meter_max_age_s=30), -5000, 5000),
"meter_max_age_s"))
check("ha_signed with no p1_net_entity is refused",
why(validate_startup_config(base(meter_source="ha_signed"), -5000, 5000),
"p1_net_entity"))
check("a phase list disagreeing with meter_phases is refused",
why(validate_startup_config(base(meter_source="ha_signed",
p1_net_entity="sensor.n", meter_phases=3,
p1_phase_net_entities=["a", "b"]),
-5000, 5000), "phase-count"))
check("mqtt_p1 with no topic is refused",
why(validate_startup_config(base(meter_source="mqtt_p1"), -5000, 5000),
"meter_mqtt_topic"))
check("ha_dsmr with only half its entities is refused",
why(validate_startup_config(base(meter_source="ha_dsmr",
p1_import_entity="sensor.i"), -5000, 5000),
"p1_export_entity"))
check("a misspelt meter_source is refused rather than silently ignored",
why(validate_startup_config(base(meter_source="ha_signd"), -5000, 5000),
"is not one of"))
# --------------------------------------------------------------------------- #
print("what must NOT be refused - a validator that fires on trivia gets deleted")
# DOCS.md: the arbiter's `supervised` limit still binds the wire to +-max_w, so
# the only cost of the extra headroom is unwind latency after saturation.
check("integrator_max_w above max_w warns but starts",
validate_startup_config(base(max_w=2000, integrator_max_w=9000),
-5000, 5000) == [])
check("integrator_max_w 0 (follow max_w) starts",
validate_startup_config(base(integrator_max_w=0), -5000, 5000) == [])
# Fail-safe AND already a red NOT READY banner naming the entity: a refusal on
# top of that adds nothing an operator can act on.
check("a missing soc_entity is left to the readiness banner",
validate_startup_config(base(soc_entity=""), -5000, 5000) == [])
check("a missing batt_entity likewise",
validate_startup_config(base(batt_entity=""), -5000, 5000) == [])
# The transient case. ESPHome is routinely not up when the add-on boots, and
# refusing here would turn a device that arrives late into a restart loop.
check("a setpoint entity that is configured but not yet found starts",
validate_startup_config(base(), None, None) == [])
check("meter_poll_s with thin but non-zero headroom starts",
validate_startup_config(base(meter_source="homewizard_local", p1_host="m",
meter_poll_s=20, meter_max_age_s=30),
-5000, 5000) == [])
# --------------------------------------------------------------------------- #
print("the refusal must be legible as a refusal, not as a crash")
p = validate_startup_config(base(max_w=8000, setpoint_entity=""), -5000, 5000)
check("several problems are all reported, not just the first", len(p) >= 1)
refuse(p) # prints the banner; the eye is the assertion for its wording
err = ConfigRefused(p)
check("ConfigRefused carries every problem", err.problems == p)
check("...and str() names one of them", str(err) == p[0])
# --------------------------------------------------------------------------- #
print()
if fails:
print(f"{len(fails)} FAILED: " + "; ".join(fails))
sys.exit(1)
print("all checks passed")