Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
547d796c5d | ||
|
|
fc9bd89dca | ||
|
|
0422127cab | ||
|
|
80ebb96e7d | ||
|
|
c70239cac2 | ||
|
|
08f17f65dd |
+1
-1
@@ -12,7 +12,7 @@ a GoodWe GW5000S-BP.
|
||||
|
||||
| | |
|
||||
|---|---|
|
||||
| running | `local_goodwe_controller` **0.2.0**, `boot: auto`, holding grid at zero |
|
||||
| running | `local_goodwe_controller` **0.4.0**, `boot: auto`, holding grid at zero |
|
||||
| installed as | a **Local** add-on: `/addons/goodwe_controller` on HA OS at 192.168.2.158 |
|
||||
| repo | Gitea `admin/goodwe-addon` (ssh://git@192.168.2.147:2222) — **private** |
|
||||
| tuning | gain 0.6, `max_w` 5000, slew 1000, deadband 15, step 10, saturation 500/3, heartbeat 10 s |
|
||||
|
||||
@@ -1,5 +1,100 @@
|
||||
# Changelog
|
||||
|
||||
## 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 only where continuing produces one of two outcomes: **(a)** a stated
|
||||
limit is untrue - the operator configured a bound and a lower layer silently
|
||||
enforces a different one - or **(b)** the add-on is inert and indistinguishable
|
||||
from healthy. (b) is not a special case of (a): with no `setpoint_entity`, layer
|
||||
1 pinning the inverter at 0 W is the failsafe *working*. The system does not
|
||||
become unsafe, it becomes a convincing impression of a working controller.
|
||||
|
||||
Four things qualify:
|
||||
|
||||
- **`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.
|
||||
- **`meter_max_age_s` above the firmware's ingest veto.** The schema accepts
|
||||
`int(5,300)`; the ESP32's threshold is a compile-time `max_age_s: "30"` and it
|
||||
trips layer 1 at `p1_age_s >= 30`. Set 120 and the add-on treats a 119 s
|
||||
reading as fresh and keeps commanding, while the firmware has already vetoed
|
||||
and is holding the inverter at 0 W - healthy meter, running controller,
|
||||
nothing moving, and no line anywhere saying the number you set was overruled.
|
||||
Structurally the same fault as the `max_w` case. Only *above* the veto is
|
||||
refused: below it the add-on gives up first, which is fail-safe and visible on
|
||||
its own status page. Mirrored as `validate.FIRMWARE_INGEST_VETO_S` - it cannot
|
||||
be imported from a compile-time YAML substitution, so it carries a note about
|
||||
the one direction in which it can go stale.
|
||||
|
||||
`max_w` at or below zero is refused too. Schema-blocked on a real install, but
|
||||
`validate_startup_config()` is now also the simulation suite's entry point, and
|
||||
a non-positive `max_w` inverts the arbiter's `supervised` bounds into a
|
||||
contradiction that resolves to 0 W - i.e. it would read as a safety trip rather
|
||||
than as the typo it is.
|
||||
|
||||
The predicates for the meter 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.
|
||||
|
||||
**TEL-05.** `meter_source: homewizard_local` polls the meter's own local API
|
||||
directly. This exists because of something the HA paths cannot do: Home Assistant
|
||||
fires no state change for a repeated reading and exposes no arrival signal at
|
||||
all, so `sensor.p1_sample_age_s` on those transports measures time since the
|
||||
*value changed*, not since the meter *reported*. Real captured data has gaps of
|
||||
42 s and 97 s between changes on a perfectly healthy meter - long enough for a
|
||||
30 s staleness threshold to command 0 W for no reason. Every HTTP response to the
|
||||
local API is a genuine arrival, so the age means what it says.
|
||||
|
||||
Verified on the rig against a literally constant 350.0 W for 70 s - the exact
|
||||
state where the HA path recorded zero samples in 45 s - with 14 arrivals and the
|
||||
age never exceeding the poll cadence.
|
||||
|
||||
Set `p1_host` to the meter's address and `meter_poll_s` (default 5 s, matching
|
||||
the meter's own ~5 s update). Still defaults to `off`; nothing changes for an
|
||||
install that does not opt in.
|
||||
|
||||
Freeze detection is separate and deliberately not thresholded: a meter answering
|
||||
200 OK with a stale number *is* arriving, so no arrival stamp can flag it. The
|
||||
energy registers carry that as `unchanged_s`, surfaced on the status line for a
|
||||
human to read rather than alarmed on - at a converged -10 W a 1 Wh register needs
|
||||
about six minutes to move, so any threshold would false-trip at the target
|
||||
operating point.
|
||||
|
||||
## Unreleased
|
||||
|
||||
**TEL-05.** A fourth `meter_source`, `homewizard_local`, which polls a
|
||||
|
||||
@@ -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,22 @@ 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):
|
||||
self.o = opts
|
||||
self.hass = hass
|
||||
self.store = store
|
||||
self.mqtt = mqtt_pub
|
||||
# ⚠️ REQUIRED, AND KEYWORD-ONLY, AFTER A REVIEW CATCH. This started life
|
||||
# as `stop=None`, which made a Controller that CANNOT STOP ITSELF the
|
||||
# default one. On the late-refusal path in run_control() that default
|
||||
# meant: log the banner, return, and leave the process alive forever
|
||||
# with no control loop, no exit and no further output - the exact
|
||||
# "inert but looks healthy" failure this whole module exists to refuse.
|
||||
# amain() always passed a real Event, so it was latent, but test_p1.py
|
||||
# was already constructing Controllers without one. No default now, so
|
||||
# the broken shape does not typecheck.
|
||||
self.stop_event = stop
|
||||
self.refused: list[str] | None = None
|
||||
|
||||
self.tuning = Tuning(
|
||||
gain=float(opts.get("gain", 0.6)),
|
||||
@@ -303,10 +315,29 @@ 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])
|
||||
# Signal, then return WITHOUT entering the loop. amain() wakes
|
||||
# on this, runs the normal wind-down (which commands 0 W) and
|
||||
# re-raises ConfigRefused afterwards, so the exit code is 78 and
|
||||
# the inverter is left at zero rather than latched.
|
||||
self.stop_event.set()
|
||||
return
|
||||
|
||||
last_grid = object()
|
||||
heartbeat = float(self.o.get("heartbeat_s", 10))
|
||||
@@ -337,7 +368,8 @@ class Controller:
|
||||
"status": "running" if self.auto else "stopped",
|
||||
}
|
||||
# ⚠️ ONLY when P1 ingestion is actually running. The ESP32's stale-input
|
||||
# watchdog subscribes to sensor.p1_sample_age_s and forces the layer-1
|
||||
# watchdog subscribes to sensor.goodwe_p1_sample_age - see the entity-id
|
||||
# note in mqtt.py; the composed name is not the object_id - and forces layer-1
|
||||
# failsafe once it reaches max_age_s. With meter_source off there is no
|
||||
# ingester feeding it, so published_age_s would be time-since-startup
|
||||
# climbing without bound - i.e. every existing install would cross the
|
||||
@@ -514,6 +546,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
|
||||
@@ -539,10 +591,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):
|
||||
@@ -569,6 +621,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:
|
||||
@@ -576,6 +632,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__":
|
||||
|
||||
@@ -45,13 +45,32 @@ SENSORS = [
|
||||
("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"),
|
||||
# ⚠️ This one deliberately breaks the goodwe_ prefix above: the entity id
|
||||
# must be exactly `sensor.p1_sample_age_s`, because SAFETY-01's firmware
|
||||
# watchdog subscribes to that literal id and the ENV-01 simulation rig
|
||||
# asserts on it. Renaming it silently disarms a safety layer. It is seconds
|
||||
# since the newest accepted P1 telegram, republished every second so that a
|
||||
# meter frozen at a constant value still shows a climbing age - which is the
|
||||
# false-trip that this entity exists to remove.
|
||||
# ⚠️ 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),
|
||||
]
|
||||
|
||||
|
||||
+130
-67
@@ -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
|
||||
|
||||
@@ -0,0 +1,284 @@
|
||||
"""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 only where **continuing produces one of two outcomes:**
|
||||
|
||||
(a) **a stated limit is untrue** - the operator configured a bound, a lower
|
||||
layer silently enforces a different one, and nothing says which won; or
|
||||
(b) **the add-on is inert and indistinguishable from healthy** - it starts,
|
||||
serves its UI, reports green, and moves no energy, with no symptom that
|
||||
separates it from a correctly-idle system.
|
||||
|
||||
⚠️ (b) IS NOT A SPECIAL CASE OF (a), and an earlier draft of this file wrongly
|
||||
implied it was by claiming every refusal here "makes a safety mechanism
|
||||
inoperative". It does not: when `setpoint_entity` is blank, layer 1 pinning the
|
||||
inverter at 0 W is the failsafe WORKING. The problem is not that the system
|
||||
becomes unsafe, it is that it becomes a very convincing impression of a working
|
||||
controller - and an operator has no way to tell. That is its own reason to
|
||||
refuse, and stating it as one is what stops the rule being stretched later to
|
||||
cover mere untidiness.
|
||||
|
||||
Both halves have to stay narrow. (a) is not "any two numbers that could
|
||||
disagree"; it is a bound the operator SET being overridden by a layer they
|
||||
cannot see. (b) is not "anything degraded"; it is inert AND silent - if a red
|
||||
NOT READY banner already names the problem, (b) does not apply.
|
||||
|
||||
1. `max_w` outside the setpoint device's own rating. [(a)]
|
||||
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`. [(b)]
|
||||
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.
|
||||
Layer 1 then holds the inverter at 0 W, which is the failsafe working
|
||||
exactly as designed - this is (b), not (a). Nothing is unsafe; it is simply
|
||||
impossible to tell from the outside that the thing is doing nothing.
|
||||
|
||||
3. A `meter_source` that can never produce a reading. [(b)]
|
||||
§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.
|
||||
|
||||
4. `meter_max_age_s` above the firmware's own ingest veto. [(a)]
|
||||
The purest example of (a) in the system, and structurally the same fault as
|
||||
1. The add-on's schema accepts `int(5,300)`. The firmware's threshold is a
|
||||
COMPILE-TIME SUBSTITUTION - `max_age_s: "30"` in
|
||||
goodwe-master-stage4-failsafe.yaml - used by both its monotonic receipt
|
||||
clock and its `ingest_stale` veto, which trips layer 1 on
|
||||
`p1_age_s >= 30`. Set 120 and the add-on believes a 119 s reading is fresh
|
||||
and keeps commanding into it, while the firmware vetoed at 30 s and is
|
||||
holding the inverter at 0 W. Status page: healthy meter, running
|
||||
controller, nothing moving, and no line anywhere saying the number you set
|
||||
was overruled. That firmware file's own comment says a lower bound on this
|
||||
"belongs in CFG-01, not here" - so here it is.
|
||||
|
||||
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 is_enabled, validate_meter_options
|
||||
|
||||
_LOG = logging.getLogger("goodwe.config")
|
||||
|
||||
# ⚠️ A MIRROR OF A FIRMWARE CONSTANT, AND IT CANNOT BE IMPORTED. The firmware
|
||||
# threshold is a compile-time YAML substitution baked into the ESP32 image
|
||||
# (`max_age_s: "30"`, master/goodwe-master-stage4-failsafe.yaml), so nothing at
|
||||
# runtime can ask the board what value it was built with. This number is a copy,
|
||||
# and a copy of a constant in another repo is a thing that goes stale.
|
||||
#
|
||||
# What makes that acceptable here: the copy can only ever be WRONG IN THE SAFE
|
||||
# DIRECTION for the check below. If the firmware's veto is raised above 30, this
|
||||
# refuses some configs that would in fact have worked - annoying, visible
|
||||
# immediately, and fixed by editing one line. It cannot start accepting a config
|
||||
# the firmware vetoes unless the firmware's number goes DOWN, which would be a
|
||||
# tightening nobody does silently.
|
||||
#
|
||||
# ⚠️ TEL-06 IS EDITING `ingest_stale` DIRECTLY. If that ticket changes what the
|
||||
# veto MEANS - e.g. thresholding arrival rather than the sensor's value - this
|
||||
# constant's justification has to be re-read, not just its number. Do not assume
|
||||
# a matching integer means a matching rule.
|
||||
FIRMWARE_INGEST_VETO_S = 30.0
|
||||
|
||||
# ⚠️ 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")
|
||||
# ⚠️ config.yaml's int(100,5000) already blocks this on a real install, so
|
||||
# the check is not for the Supervisor path - it is because
|
||||
# validate_startup_config() is now also sim/inject.py's entry point and can
|
||||
# be handed any dict at all. A max_w of 0 or below inverts the arbiter's
|
||||
# `supervised` bounds (lo=-max_w becomes the HIGHER number), and a limit
|
||||
# claim whose lo exceeds its hi is exactly the contradiction the arbiter
|
||||
# resolves to 0 W - i.e. it would look like a safety trip rather than the
|
||||
# nonsense config it is.
|
||||
elif max_w <= 0:
|
||||
problems.append(
|
||||
f"max_w is {max_w:.0f} W and must be positive - it is a magnitude, "
|
||||
"not a signed setpoint. At or below zero the arbiter's supervised "
|
||||
f"limit becomes lo={-max_w:.0f} / hi={max_w:.0f}, which is a "
|
||||
"contradiction it resolves to 0 W, so the controller would look "
|
||||
"like it was tripping rather than misconfigured.")
|
||||
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))
|
||||
|
||||
# --- 4. the staleness window the FIRMWARE will actually enforce -----------
|
||||
# ⚠️ Only when P1 ingestion is on, and that is a real scope limit rather than
|
||||
# caution: meter_max_age_s is read by nothing but P1Ingest, and the veto it
|
||||
# collides with keys off `p1_age_s.has_state()`, which is only ever published
|
||||
# when ingestion is running (see Controller.publish()). With meter_source off
|
||||
# there is no sensor, the veto is inert, and the number governs nothing.
|
||||
#
|
||||
# Strictly greater, not >=: the firmware trips at `>= 30` and P1Ingest goes
|
||||
# stale at `> max_age_s`, so 30 against 30 has the firmware fractionally
|
||||
# stricter by one sample. That is the shipped default and the tested pair.
|
||||
# BELOW 30 is fine and deliberately not refused - the add-on then gives up
|
||||
# first, which is fail-safe and visible on its own status page. Only ABOVE is
|
||||
# the silent case, where the add-on keeps commanding into a reading the board
|
||||
# has already rejected.
|
||||
if (is_enabled(opts) and max_age_s is not None
|
||||
and max_age_s > FIRMWARE_INGEST_VETO_S):
|
||||
problems.append(
|
||||
f"meter_max_age_s is {max_age_s:.0f} s but the ESP32 firmware vetoes "
|
||||
f"any P1 sample at {FIRMWARE_INGEST_VETO_S:.0f} s and forces the "
|
||||
"layer-1 failsafe - THE FIRMWARE WINS, and it wins silently. This "
|
||||
f"add-on would treat a {max_age_s - 1:.0f} s reading as fresh and "
|
||||
"keep commanding, while the inverter sat at 0 W with a healthy-"
|
||||
"looking meter on the status page. Lower meter_max_age_s to "
|
||||
f"{FIRMWARE_INGEST_VETO_S:.0f} s or less (30 s is the tested value), "
|
||||
"or rebuild the firmware with a matching max_age_s and update "
|
||||
"validate.FIRMWARE_INGEST_VETO_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)
|
||||
@@ -1,5 +1,5 @@
|
||||
name: GoodWe RS485 Controller
|
||||
version: "0.3.0"
|
||||
version: "0.4.0"
|
||||
slug: goodwe_controller
|
||||
description: >-
|
||||
Drives a GoodWe ES/BP battery inverter over RS485 by emulating its smart
|
||||
|
||||
@@ -1346,6 +1346,17 @@ check("with P1 on the age sensor IS announced",
|
||||
from app.main import Controller # noqa: E402
|
||||
|
||||
|
||||
def _never():
|
||||
"""A stop Event these publish-only tests never set.
|
||||
|
||||
`stop` is a REQUIRED keyword argument (CFG-01): a Controller that cannot
|
||||
stop itself used to be the default one, and on the late-refusal path that
|
||||
meant a live process with no control loop and no exit. These cases only call
|
||||
publish(), so any Event will do - but they may not skip it.
|
||||
"""
|
||||
return asyncio.Event()
|
||||
|
||||
|
||||
class _Store:
|
||||
data = {}
|
||||
|
||||
@@ -1368,14 +1379,14 @@ class _Pub:
|
||||
|
||||
|
||||
pub_off = _Pub()
|
||||
Controller({"meter_source": "off"}, None, _Store(), pub_off).publish()
|
||||
Controller({"meter_source": "off"}, None, _Store(), pub_off, stop=_never()).publish()
|
||||
check("with P1 off, p1_age is absent from the published payload",
|
||||
"p1_age" not in pub_off.last)
|
||||
check("...while the normal status keys are still published",
|
||||
"setpoint" in pub_off.last and "grid" in pub_off.last)
|
||||
|
||||
pub_on = _Pub()
|
||||
Controller({"meter_source": SOURCE_HA}, None, _Store(), pub_on).publish()
|
||||
Controller({"meter_source": SOURCE_HA}, None, _Store(), pub_on, stop=_never()).publish()
|
||||
check("with P1 on, p1_age is published", "p1_age" in pub_on.last)
|
||||
check("...as a number, so has_state() becomes true only once we feed it",
|
||||
isinstance(pub_on.last["p1_age"], float))
|
||||
@@ -1384,14 +1395,14 @@ check("...as a number, so has_state() becomes true only once we feed it",
|
||||
# age sensor is a hard prerequisite for the FW-01 flash, and it has to appear on
|
||||
# the transport that can actually read the meter in this house.
|
||||
pub_sig = _Pub()
|
||||
Controller({"meter_source": SOURCE_HA_SIGNED}, None, _Store(), pub_sig).publish()
|
||||
Controller({"meter_source": SOURCE_HA_SIGNED}, None, _Store(), pub_sig, stop=_never()).publish()
|
||||
check("with ha_signed selected, p1_age is published too",
|
||||
"p1_age" in pub_sig.last and isinstance(pub_sig.last["p1_age"], float))
|
||||
|
||||
# ⚠️ And on homewizard_local, where it is the one age FW-01 may actually
|
||||
# threshold - every other transport's age measures when the VALUE changed.
|
||||
pub_hw = _Pub()
|
||||
Controller({"meter_source": SOURCE_HOMEWIZARD}, None, _Store(), pub_hw).publish()
|
||||
Controller({"meter_source": SOURCE_HOMEWIZARD}, None, _Store(), pub_hw, stop=_never()).publish()
|
||||
check("with homewizard_local selected, p1_age is published",
|
||||
"p1_age" in pub_hw.last and isinstance(pub_hw.last["p1_age"], float))
|
||||
|
||||
@@ -1421,7 +1432,7 @@ def p1_rows(ctl):
|
||||
|
||||
|
||||
def _fed(source):
|
||||
ctl = Controller({"meter_source": source}, None, _Store(), _Pub())
|
||||
ctl = Controller({"meter_source": source}, None, _Store(), _Pub(), stop=_never())
|
||||
ctl.p1.submit(make_sample(source, 350.0, 0.0, phases=1,
|
||||
ingest_mono=time.monotonic()))
|
||||
return ctl
|
||||
|
||||
@@ -0,0 +1,309 @@
|
||||
"""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 asyncio
|
||||
import sys
|
||||
|
||||
from app.validate import (
|
||||
ACTUATOR_WRITE_KEYS, FIRMWARE_INGEST_VETO_S, 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("4. meter_max_age_s against the firmware's compile-time veto")
|
||||
# ⚠️ The schema allows int(5,300); the firmware's threshold is baked in at 30 s
|
||||
# and wins silently. Same shape as the max_w case: a configured bound overridden
|
||||
# by a lower layer, with nothing saying so.
|
||||
hw = dict(meter_source="homewizard_local", p1_host="m")
|
||||
p = validate_startup_config(base(**hw, meter_max_age_s=120), -5000, 5000)
|
||||
check("a meter_max_age_s above the firmware veto is refused", len(p) == 1)
|
||||
check("...naming the configured value", why(p, "120"))
|
||||
check("...naming the firmware's value", why(p, "30"))
|
||||
check("...and saying which one wins", why(p, "THE FIRMWARE WINS"))
|
||||
check("the tested pair 30/30 is accepted",
|
||||
validate_startup_config(base(**hw, meter_max_age_s=30), -5000, 5000) == [])
|
||||
check("one second above it is refused",
|
||||
validate_startup_config(base(**hw, meter_max_age_s=31), -5000, 5000) != [])
|
||||
# Below the veto the add-on gives up FIRST, which is fail-safe and visible on
|
||||
# its own status page. Only above is the silent case.
|
||||
check("below the veto is fine - the add-on being stricter is not a conflict",
|
||||
validate_startup_config(base(**hw, meter_max_age_s=10,
|
||||
meter_poll_s=4), -5000, 5000) == [])
|
||||
# ⚠️ Scope: meter_max_age_s is read by nothing but P1Ingest, and the veto keys
|
||||
# off a sensor only published when ingestion runs. With the meter off it governs
|
||||
# nothing, so refusing would be pure trivia.
|
||||
check("with meter_source off the number governs nothing and is not refused",
|
||||
validate_startup_config(base(meter_source="off", meter_max_age_s=300),
|
||||
-5000, 5000) == [])
|
||||
check("the constant is the firmware's, not a rounded guess",
|
||||
FIRMWARE_INGEST_VETO_S == 30.0)
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
print("max_w must be a positive magnitude")
|
||||
# Schema-blocked on a real install, but validate_startup_config() is also
|
||||
# sim/inject.py's entry point and can be handed any dict at all.
|
||||
for bad in (0, -1, -2000):
|
||||
p = validate_startup_config(base(max_w=bad), -5000, 5000)
|
||||
check(f"max_w {bad} is refused", len(p) == 1 and why(p, "must be positive"))
|
||||
check("...and the message explains it would read as a trip, not a mistake",
|
||||
why(validate_startup_config(base(max_w=0), -5000, 5000), "contradiction"))
|
||||
check("the smallest sane max_w still starts",
|
||||
validate_startup_config(base(max_w=1), -5000, 5000) == [])
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
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("the LATE refusal, when the device only shows up after the gate has run")
|
||||
# ⚠️ This path had no test at all and no live demonstration - the review's
|
||||
# catch. ESPHome is routinely not up when the add-on boots, so hass.limits()
|
||||
# returns (None, None), the range checks are skipped, and run_control() is the
|
||||
# first place the device's real rating is ever known. Everything below is about
|
||||
# what happens when it turns out to conflict.
|
||||
from app.main import Controller # noqa: E402
|
||||
|
||||
|
||||
class _LateHass:
|
||||
"""Reports the range only run_control() gets to see."""
|
||||
|
||||
def __init__(self, lo, hi):
|
||||
self.lo, self.hi = lo, hi
|
||||
self.writes = []
|
||||
|
||||
async def limits(self, entity):
|
||||
return (self.lo, self.hi, 1.0)
|
||||
|
||||
async def number(self, *a, **k):
|
||||
return 0.0
|
||||
|
||||
async def set_number(self, entity, value):
|
||||
self.writes.append(value)
|
||||
return True
|
||||
|
||||
|
||||
class _Store2:
|
||||
data = {}
|
||||
|
||||
def set(self, *a):
|
||||
pass
|
||||
|
||||
|
||||
class _Pub2:
|
||||
enabled = False
|
||||
|
||||
def publish(self, *a, **k):
|
||||
pass
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
|
||||
def late(opts_over, lo, hi):
|
||||
"""Drive run_control() against a device that only reports (lo, hi) now.
|
||||
|
||||
⚠️ Returns whether run_control() EXITED ON ITS OWN. That is the assertion
|
||||
that matters: on a refusal it must return without entering the while loop,
|
||||
and on a clean config it must not return at all. So the clean case is stopped
|
||||
from outside after it has had time to write, rather than waited on - a
|
||||
wait_for() here would just time out, which is the correct behaviour dressed
|
||||
up as a failure.
|
||||
"""
|
||||
async def go():
|
||||
stop = asyncio.Event()
|
||||
hass = _LateHass(lo, hi)
|
||||
# A device narrower than the shipped 5000 W schema ceiling is exactly
|
||||
# the case that cannot be reached through the Supervisor on the live
|
||||
# install, so it is reached here instead.
|
||||
c = Controller(base(**opts_over), hass, _Store2(), _Pub2(), stop=stop)
|
||||
c.auto = True
|
||||
task = asyncio.create_task(c.run_control())
|
||||
for _ in range(30): # up to ~3 s
|
||||
if task.done():
|
||||
break
|
||||
await asyncio.sleep(0.1)
|
||||
exited = task.done()
|
||||
if not exited:
|
||||
c.stopping = True
|
||||
await asyncio.wait_for(task, timeout=5)
|
||||
return c, stop, hass, exited
|
||||
return asyncio.run(go())
|
||||
|
||||
c, stop, hass, exited = late({"max_w": 5000}, -3600.0, 3600.0)
|
||||
check("a device narrower than max_w, discovered late, is refused",
|
||||
c.refused is not None and len(c.refused) == 1)
|
||||
check("...naming both values", why(c.refused, "5000") and why(c.refused, "3600"))
|
||||
check("...signalling amain to wind down", stop.is_set())
|
||||
check("...and returning instead of looping forever", exited)
|
||||
# The whole point: run_control() must not fall through into the while loop. A
|
||||
# single write here would mean it commanded the inverter under a config it had
|
||||
# just declared unusable.
|
||||
check("...having never entered the control loop, so it wrote nothing",
|
||||
hass.writes == [])
|
||||
check("...and it is recorded in the event log an operator reads",
|
||||
any("REFUSED" in e for e in c.events))
|
||||
|
||||
# The same path when there is no conflict: it must NOT refuse, and it must
|
||||
# actually regulate - asserted by a write reaching the device, not by absence.
|
||||
c2, stop2, hass2, exited2 = late({"max_w": 2000}, -5000.0, 5000.0)
|
||||
check("a late device that agrees does not trip the re-check", c2.refused is None)
|
||||
check("...and the control loop runs, writing to the device", hass2.writes != [])
|
||||
check("...and does not signal a shutdown", not stop2.is_set() and not exited2)
|
||||
|
||||
# ⚠️ THE FOOTGUN THE REVIEW FOUND. `stop` used to default to None, so this
|
||||
# construction was legal and the refusal path then returned into a live process
|
||||
# with no control loop, no exit and no further output.
|
||||
try:
|
||||
Controller(base(), _LateHass(-5000.0, 5000.0), _Store2(), _Pub2())
|
||||
check("a Controller with no stop Event is rejected at construction", False)
|
||||
except TypeError:
|
||||
check("a Controller with no stop Event is rejected at construction", True)
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
print()
|
||||
if fails:
|
||||
print(f"{len(fails)} FAILED: " + "; ".join(fails))
|
||||
sys.exit(1)
|
||||
print("all checks passed")
|
||||
Reference in New Issue
Block a user