From 9f75ae820e553f9d257a50e9afe3bcef4391b7cc Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:53:40 -0700 Subject: [PATCH 1/3] fix(control): a target only where the device declares the control Three refusal edges the #161 work left behind, all folding into the unreleased 3.1.0. `priority_is_settable` read an absent `$settable` as permission, which is right for a circuit that declares `load-shed/priority` and omits the attribute, and wrong for a device that declares no such property at all. The BESS, the MID, the lugs and the PV are all in that position, so `set_circuit_priority_target("bess")` resolved a topic for a control that device never published. `_declared_settable` now separates the two: an undeclared property is never settable, whatever the per-property default for a declared one says. The relay avoided this incidentally, because its default is already refusal. The flat adapter still built a target for a circuit id the panel never published: `get_prop` answers `""` for an id nothing published, `""` parses as `false`, and `not false` read as permission -- so any id was writable on any panel, the snapshot's own synthetic `unmapped_tab_*` keys included. It now answers from the node type in `$description`, which is the same question `schema_1` was already answering, and `SchemaAdapter` states the guarantee rather than leaving it to each implementation. An id no circuit answers to was refused with "declares its relay non-commandable" and audited as `relay not commandable`, which asserts something about a circuit that does not exist and sends whoever reads it to a panel's commissioning to explain a typo. The two cases now carry distinct messages and distinct audit details. `SchemaAdapter` gains `has_circuit` for it, consulted only once a target has already been refused -- so it can relabel a refusal but never cause one -- and both adapters implement it on circuit membership rather than on topology, since a BESS is not a circuit whose relay happens to be locked. --- CHANGELOG.md | 16 ++- packages/schema-0/CHANGELOG.md | 7 ++ .../src/span_panel_api_schema_0/adapter.py | 23 +++- .../src/span_panel_api_schema_0/consumer.py | 31 +++++- packages/schema-1/CHANGELOG.md | 9 ++ .../src/span_panel_api_schema_1/adapter.py | 23 ++++ .../src/span_panel_api_schema_1/circuits.py | 51 ++++++--- src/span_panel_api/mqtt/client.py | 67 +++++++++-- src/span_panel_api/protocol.py | 32 ++++++ tests/conftest.py | 11 +- tests/test_control_interceptor.py | 105 ++++++++++++++++++ tests/test_mqtt_connect_flow.py | 26 +++-- tests/test_mqtt_homie.py | 24 +++- tests/test_protocol_conformance.py | 1 + tests/test_schema_one_control_refusal.py | 85 ++++++++++++++ tests/test_schema_zero_adapter.py | 86 +++++++++++--- 16 files changed, 533 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8e822b..7e2467a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,8 @@ beta corrected in an earlier beta does not appear at all: from the point of view ## [3.1.0] A security release. Three things a caller could not previously find out — whether a control command was delivered, whether the panel's bootstrap traffic was encrypted, and whether the CA behind the MQTT broker is still the one that was there yesterday — -now have answers. **Install the matching adapter**: this release replaces four `SchemaAdapter` members, so `span-panel-api-schema-0` / `-1` must move to 1.1.0 at the same time. The extras (`span-panel-api[schema-0]`) carry the floor; a direct install of -the adapter distribution does not, and a 1.0.0 adapter against this bootstrap is rejected at discovery with a named error rather than misbehaving. +now have answers. **Install the matching adapter**: this release replaces four `SchemaAdapter` members and adds one, so `span-panel-api-schema-0` / `-1` must move to 1.1.0 at the same time. The extras (`span-panel-api[schema-0]`) carry the floor; a direct +install of the adapter distribution does not, and a 1.0.0 adapter against this bootstrap is rejected at discovery with a named error rather than misbehaving. ### Fixed @@ -32,6 +32,18 @@ the adapter distribution does not, and a 1.0.0 adapter against this bootstrap is A locked relay keeps a settable priority, which is the combination real panels publish and which `switch` 0.3 and `load-shed` 0.3 scope separately. + **Both adapters also refuse a circuit id the panel never published**, where the flat one used to build a topic for it. Its lookup read an unpublished value as the empty string, which parses as "not always-on" and read as permission, so any id at all was + writable on any panel — including the synthetic `unmapped_tab_*` keys the snapshot itself invents. The two adapters answer the same question and now answer it the same way, and `SchemaAdapter` states the guarantee rather than leaving it to each + implementation. + + **And a device that declares no such property is refused as well**, which is not the same absence as a declared property carrying no `$settable`. Under the parent/child schema an absent `$settable` on `load-shed/priority` means settable — that is the + documented case where firmware declares the property and omits the attribute — but a BESS, a MID or the lugs declare no `load-shed` node at all, and reading their silence as permission resolved a write topic for a control those devices never offered. + +- **A refused circuit command names the refusal it actually made.** An id the panel carries no circuit under was refused with "declares its relay non-commandable" and audited as `relay not commandable`, which asserts something about a circuit that does not + exist: it sends whoever reads it to a panel's commissioning to explain a mistyped id. The two cases now carry distinct messages and distinct `detail` values (`no such circuit`), and the distinction matters most in the audit trail, because `detail` + reaches `after_publish` and the Home Assistant integration writes it into a security log where it is read as a fact about the panel. `SchemaAdapter` gains `has_circuit` for it — consulted only once a target has already been refused, so it can relabel a + refusal but never cause one. + - **A control the library refused before resolving an address is no longer invisible to `ControlInterceptor`.** `after_publish` is contracted to see every command, refusals included, but five refusals happened while resolving the target and therefore never reached the publish path at all: a relay declared non-commandable, a priority declared locked, a charger with no settable limit, a panel with no islanding control, and an adopted property that is not settable. A consumer building a security audit on `after_publish` — which is what the Home Assistant integration does — would have had a hole in it exactly where the interesting cases are, the highest-consequence control in the system among them. diff --git a/packages/schema-0/CHANGELOG.md b/packages/schema-0/CHANGELOG.md index b7ad454..52585e4 100644 --- a/packages/schema-0/CHANGELOG.md +++ b/packages/schema-0/CHANGELOG.md @@ -31,6 +31,13 @@ Requires `span-panel-api` **3.1.0 or newer**, and the two must be upgraded toget A locked relay keeps a settable priority: always-on is not never-backup on either schema. + **And both refuse a circuit id this panel never published.** Absence-reads-as-permission is right for the two flags and wrong for the node itself: `get_prop` answers `""` for an id nothing published, `""` parses as `false`, and `not false` is permission + — so an unknown id produced a well-formed topic aimed at nothing, including for the synthetic `unmapped_tab_*` keys the snapshot invents. `schema_1` already refused an id its tree did not carry; two adapters answering the same question differently was + the defect, and the question is answered here from the node type in `$description`. + +- **`HomieDeviceConsumer.is_circuit_node` is public**, and `SchemaZeroAdapter` gains `has_circuit`, which delegates to it. `SchemaAdapter` declares `has_circuit` in 3.1.0 so a transport can tell "no such circuit" apart from "this circuit's control is + locked" when it reports a refusal. One reading behind both answers: what counts as a circuit here decides the refusal and its stated reason together, so the two cannot drift. + ## [1.0.0] First release as a standalone distribution. Requires `span-panel-api` 3.0.0 or newer. diff --git a/packages/schema-0/src/span_panel_api_schema_0/adapter.py b/packages/schema-0/src/span_panel_api_schema_0/adapter.py index f63e671..80e7368 100644 --- a/packages/schema-0/src/span_panel_api_schema_0/adapter.py +++ b/packages/schema-0/src/span_panel_api_schema_0/adapter.py @@ -66,10 +66,11 @@ def find_node_by_type(self, type_str: str) -> str | None: def set_circuit_relay_target(self, circuit_id: str) -> ControlTarget | None: """Where this circuit's relay is commanded, or None if it may not be. - None on an always-on circuit. `_target` is pure string formatting from a - node id, so without the lookup this aimed a write at a relay the panel - commissioned as permanently closed — and the refusal was already in the - values this adapter parses, as `is_user_controllable`. + None on an always-on circuit, and None on an id the panel published no + circuit under. `_target` is pure string formatting from a node id, so + without the lookup this aimed a write at a relay the panel commissioned + as permanently closed — and the refusal was already in the values this + adapter parses, as `is_user_controllable`. """ if not self._consumer.relay_is_settable(circuit_id): return None @@ -79,12 +80,24 @@ def set_circuit_priority_target(self, circuit_id: str) -> ControlTarget | None: """Where this circuit's shed priority is written, or None if it may not be. None on a never-backup circuit, which is the flat spelling of the - `$settable` lock v1.0 publishes on `load-shed/priority`. + `$settable` lock v1.0 publishes on `load-shed/priority`, and None on an + id the panel published no circuit under. """ if not self._consumer.priority_is_settable(circuit_id): return None return self._target(circuit_id, "shed-priority") + def has_circuit(self, circuit_id: str) -> bool: + """Whether the panel carries a circuit under this id. + + The same lookup the two target builders make before they build anything, + asked separately so the transport can say which of the two refusals it + is reporting. One reading: a change to what counts as a circuit here + moves both answers together, and the pair cannot drift into a refusal + whose stated reason is the other one's. + """ + return self._consumer.is_circuit_node(circuit_id) + def set_dominant_power_source_target(self) -> ControlTarget | None: core_node = self._consumer.find_node_by_type(TYPE_CORE) if core_node is None: diff --git a/packages/schema-0/src/span_panel_api_schema_0/consumer.py b/packages/schema-0/src/span_panel_api_schema_0/consumer.py index 256540b..a94cbb3 100644 --- a/packages/schema-0/src/span_panel_api_schema_0/consumer.py +++ b/packages/schema-0/src/span_panel_api_schema_0/consumer.py @@ -121,7 +121,16 @@ def relay_is_settable(self, node_id: str) -> bool: Absent reads as commandable, for the reason the flag exists — it marks the exception, and defaulting to locked would refuse every relay on a panel that omits it. + + **That default is why the node has to exist first.** ``get_prop`` + answers ``""`` for a node nothing ever published, which parses as + ``always-on = false`` and reads as permission, so an id no circuit + answers to produced a write target — the same defect the parent/child + adapter refuses by finding no such device in its tree. Both adapters + answer this question, and they have to answer it the same way. """ + if not self.is_circuit_node(node_id): + return False return not _parse_bool(self._acc.get_prop(node_id, "always-on")) def priority_is_settable(self, node_id: str) -> bool: @@ -130,7 +139,13 @@ def priority_is_settable(self, node_id: str) -> bool: ``never-backup`` is the flat spelling of what v1.0 expresses as mutability of ``load-shed/priority``, and it is already read into ``SpanCircuitSnapshot.is_never_backup``. Same reading, second surface. + + Gated on the node existing for the reason ``relay_is_settable`` gives: + an unpublished property reads ``""``, which is indistinguishable from a + published ``false`` and would make every unknown id writable. """ + if not self.is_circuit_node(node_id): + return False return not _parse_bool(self._acc.get_prop(node_id, "never-backup")) def circuit_nodes_missing_names(self) -> list[str]: @@ -191,7 +206,7 @@ def _rebuild_dirty_circuits(self, dirty: frozenset[str]) -> SpanPanelSnapshot: continue # drop old unmapped entries; will recompute below updated_circuits[cid] = circ for node_id in dirty: - if self._is_circuit_node(node_id): + if self.is_circuit_node(node_id): meta = feed_metadata.get(node_id, {}) device_type = meta.get("device_type", "circuit") relative_position = meta.get("relative_position", "") @@ -245,8 +260,16 @@ def _find_lugs_node(self, direction: str) -> str | None: TYPE_EVSE: "evse", } - def _is_circuit_node(self, node_id: str) -> bool: - """Check if node is a circuit device.""" + def is_circuit_node(self, node_id: str) -> bool: + """Whether the panel published a circuit under this node id. + + False for an id nothing published and for a node of any other type, and + those are the same answer: the panel carries no circuit by that name. + Public because it is the flat schema's whole answer to "is there + anything here to command" — the command path asks it before building a + target, and the adapter asks it again to tell an unknown id apart from a + circuit that declares its control locked. + """ return self._acc.get_node_type(node_id) in self._CIRCUIT_LIKE_TYPES def _build_feed_metadata(self) -> dict[str, dict[str, str]]: @@ -607,7 +630,7 @@ def _build_snapshot(self) -> SpanPanelSnapshot: # Circuits circuits: dict[str, SpanCircuitSnapshot] = {} for node_id in self._acc.all_node_types(): - if self._is_circuit_node(node_id): + if self.is_circuit_node(node_id): meta = feed_metadata.get(node_id, {}) device_type = meta.get("device_type", "circuit") relative_position = meta.get("relative_position", "") diff --git a/packages/schema-1/CHANGELOG.md b/packages/schema-1/CHANGELOG.md index 03fe3aa..6422ca0 100644 --- a/packages/schema-1/CHANGELOG.md +++ b/packages/schema-1/CHANGELOG.md @@ -40,6 +40,15 @@ Requires `span-panel-api` **3.1.0 or newer**, and the two must be upgraded toget Also `None` for a circuit id the tree does not carry, rather than a topic addressed to a device nobody published. + **An undeclared property is refused too, and it is not the same absence as an unannounced attribute.** `priority_is_settable` reads an absent `$settable` on `load-shed/priority` as permission, which is correct for the case it exists for — firmware that + declares the property and omits the attribute — but that default was also answering for devices with no `load-shed` node at all. The BESS, the MID, the lugs and the PV are all in that position in the reference tree, so + `set_circuit_priority_target("bess")` resolved a topic for a control that device never published. `_declared_settable` now distinguishes the two: a property the device does not declare is never settable, whatever the per-property default for a declared + one says. The relay avoided this incidentally, because its default is already refusal. + +- **`SchemaOneAdapter.has_circuit`**, which `SchemaAdapter` declares in 3.1.0 so a transport can tell "no such circuit" apart from "this circuit's control is locked" when it reports a refusal. It answers on the device _type_, not on membership of the tree: + every device is addressable by id, and a BESS reported as a circuit would have its refusal read as a claim about a relay it does not have. It is deliberately not a gate on the two target builders — those refuse on the declaration, which is the + specification's rule and what authorises a write, and putting a type check in front of it would be a second, weaker rule where the authorisation lives. + - **`spec_lock.json`: `peer` becomes `peers`, and the eBus emitter is tracked as one.** `ebus-panel-sim` produced the reference tree and was recorded nowhere, which is the whole reason that capture went three releases stale without anything objecting. It now carries a pin of the same shape panelbench has — repo, ref, role, commit, tag, released version, and the specification commit it implements — so a scheduled job can ask whether the producer has moved and a test can ask whether our checkout is the release the bytes are attributed to. diff --git a/packages/schema-1/src/span_panel_api_schema_1/adapter.py b/packages/schema-1/src/span_panel_api_schema_1/adapter.py index c7efe7d..cece256 100644 --- a/packages/schema-1/src/span_panel_api_schema_1/adapter.py +++ b/packages/schema-1/src/span_panel_api_schema_1/adapter.py @@ -38,6 +38,7 @@ PROP_PRIORITY, PROP_RELAY, STATE_READY, + TYPE_CIRCUIT, ) from span_panel_api_schema_1.description import device_type from span_panel_api_schema_1.field_metadata import build_field_metadata @@ -237,6 +238,28 @@ def set_circuit_priority_target(self, circuit_id: str) -> ControlTarget | None: return None return self._target(circuit_id, NODE_LOAD_SHED, PROP_PRIORITY) + def has_circuit(self, circuit_id: str) -> bool: + """Whether the tree carries a circuit under this id. + + The type is the question, not mere membership. Every device in the tree + is addressable by id — the BESS, the MID, the lugs, a charger — and none + of them is a circuit, so answering on membership alone would report a + BESS as a circuit whose relay the panel declares non-commandable. It + declares no relay at all. + + A child that has not described itself yet reads as absent, because it + has declared nothing: type, nodes and properties all arrive together in + one `$description`, so there is no state in which it is known to be a + circuit and unknown whether its controls are settable. + + This is a diagnostic, deliberately not a gate on the two target builders + above. Those refuse on the *declaration*, which is the specification's + rule and is what authorises a write; adding a type check in front of it + would put a second, weaker rule where the authorisation lives. + """ + device = self._child(circuit_id) + return device is not None and device_type(device) == TYPE_CIRCUIT + def set_dominant_power_source_target(self) -> ControlTarget | None: """The settable successor: `shed/asserted-islanding-state` on the panel. diff --git a/packages/schema-1/src/span_panel_api_schema_1/circuits.py b/packages/schema-1/src/span_panel_api_schema_1/circuits.py index 3cd8990..3250bed 100644 --- a/packages/schema-1/src/span_panel_api_schema_1/circuits.py +++ b/packages/schema-1/src/span_panel_api_schema_1/circuits.py @@ -139,20 +139,33 @@ def _tabs(device: DiscoveredDevice) -> list[int]: return tabs -def _declared_settable(device: DiscoveredDevice, node: str, prop: str, *, when_absent: bool) -> bool: +def _declared_settable(device: DiscoveredDevice, node: str, prop: str, *, when_unannotated: bool) -> bool: """Read the Homie ``$settable`` attribute off one property's definition. - ``when_absent`` is the answer for a property that carries no such attribute, - and it is a per-property judgement rather than one rule — which is why it is - a parameter instead of a default. See the two callers below for why they - answer it differently. + **An undeclared property is never settable, whatever ``when_unannotated`` + says.** The two absences are different claims and were once collapsed onto + one answer: a device that declares ``load-shed/priority`` and omits + ``$settable`` has left a mutable property unannotated, while a device with no + ``load-shed`` node at all — a BESS, a MID, the lugs — has not declared the + property, and there is nothing on it to be settable. Answering the second + with the first's default produced a write target for a control the device + never offered, which is the defect the refusal exists to prevent. + + ``when_unannotated`` is therefore the narrower question it now names: what a + *declared* property means when it carries no ``$settable``. That is a + per-property judgement rather than one rule — which is why it is a parameter + instead of a default. See the two callers below for why they answer it + differently. + + A definition that is not a mapping is treated as undeclared for the same + reason: a malformed declaration has not said the property is settable. """ definition = device.get_node_properties(node).get(prop) if not isinstance(definition, dict): - return when_absent + return False settable = definition.get(ATTR_SETTABLE) if settable is None: - return when_absent + return when_unannotated if isinstance(settable, bool): return settable return str(settable).strip().lower() != "false" @@ -168,11 +181,21 @@ def priority_is_settable(device: DiscoveredDevice) -> bool: expresses never-backup as *mutability*, so the signal is the Homie ``$settable`` attribute on the property definition. - Absent therefore means settable: treating an unannounced circuit as locked - would mark every circuit never-backup on a panel that does not publish the - attribute. + An unannounced ``$settable`` therefore means settable: treating an + unannounced circuit as locked would mark every circuit never-backup on a + panel that does not publish the attribute. + + **An unannounced attribute is not an undeclared property**, and only the + first of those means settable. A device carrying no ``load-shed`` node, or a + ``load-shed`` node with no ``priority`` on it, has not published a shed + priority for anything to be settable *on* — every non-circuit device in the + tree is in that position, the BESS and the MID and the lugs among them. The + permissive default is for the documented case where firmware declares the + property and omits the attribute, and reading it as permission for a device + that declared neither produced a write target for a control that device + never offered. `_declared_settable` separates the two. """ - return _declared_settable(device, NODE_LOAD_SHED, PROP_PRIORITY, when_absent=True) + return _declared_settable(device, NODE_LOAD_SHED, PROP_PRIORITY, when_unannotated=True) def relay_is_settable(device: DiscoveredDevice) -> bool: @@ -209,14 +232,16 @@ def relay_is_settable(device: DiscoveredDevice) -> bool: describing one correctly omits the attribute rather than publishing ``false``. Absence is therefore the announcement. Priority answers the other way because its catalog entry carries no condition at all. The two - properties are not making the same kind of claim. + properties are not making the same kind of claim. Both refuse a device that + declares no such property at all, which is a third case and not either + default. Across the two production enclosures we hold captures from — 27 circuits — ``$settable`` is present on ``switch/relay`` exactly when ``relay-controllable`` is ``true``, without exception, which is the specification's rule showing up in hardware. """ - return _declared_settable(device, NODE_SWITCH, PROP_RELAY, when_absent=False) and _flag( + return _declared_settable(device, NODE_SWITCH, PROP_RELAY, when_unannotated=False) and _flag( device, NODE_SWITCH, PROP_RELAY_CONTROLLABLE, default=True ) diff --git a/src/span_panel_api/mqtt/client.py b/src/span_panel_api/mqtt/client.py index 82d5fd8..1e870ba 100644 --- a/src/span_panel_api/mqtt/client.py +++ b/src/span_panel_api/mqtt/client.py @@ -694,15 +694,18 @@ async def set_circuit_relay(self, circuit_id: str, state: str) -> PublishOutcome error -- see `PublishState`. Raises: - SpanPanelServerError: the panel declares this circuit's relay - non-commandable, so there is nothing to publish to. Raised the - way `set_evse_charge_limit` raises for a charger with no - settable limit, and recorded through the interceptor first. + SpanPanelServerError: the panel carries no such circuit, or declares + this circuit's relay non-commandable, so there is nothing to + publish to. Raised the way `set_evse_charge_limit` raises for a + charger with no settable limit, and recorded through the + interceptor first. """ - target = self._require_adapter().set_circuit_relay_target(circuit_id) + adapter = self._require_adapter() + target = adapter.set_circuit_relay_target(circuit_id) if target is None: - await self._refuse_control( - device_id=circuit_id, + await self._refuse_circuit_control( + adapter, + circuit_id=circuit_id, value=state, detail="relay not commandable", message=f"Circuit {circuit_id!r} declares its relay non-commandable", @@ -720,19 +723,59 @@ async def set_circuit_priority(self, circuit_id: str, priority: str) -> PublishO What happened to the command. See `PublishState`. Raises: - SpanPanelServerError: the circuit is commissioned never-backup, so - its priority is not writable. + SpanPanelServerError: the panel carries no such circuit, or the + circuit is commissioned never-backup, so its priority is not + writable. """ - target = self._require_adapter().set_circuit_priority_target(circuit_id) + adapter = self._require_adapter() + target = adapter.set_circuit_priority_target(circuit_id) if target is None: - await self._refuse_control( - device_id=circuit_id, + await self._refuse_circuit_control( + adapter, + circuit_id=circuit_id, value=priority, detail="priority not settable", message=f"Circuit {circuit_id!r} declares its shed priority not settable", ) return await self._publish_control(target, priority, self._control_deadlines.priority) + async def _refuse_circuit_control( + self, + adapter: SchemaAdapter, + *, + circuit_id: str, + value: str, + detail: str, + message: str, + ) -> NoReturn: + """Refuse a circuit command under the reason that is actually true of it. + + A circuit target builder returns None for two unrelated reasons, and + this picks between them. `detail` and `message` describe the *control* + being locked, which is the caller's case; an id the panel carries no + circuit under is this method's case, and it says so instead. + + The distinction is not cosmetic in either direction. "Declares its relay + non-commandable" reads as a statement about a circuit that exists, so it + sends whoever is debugging a mistyped id off to look at a panel's + commissioning. And `detail` reaches a consumer's `after_publish` -- the + Home Assistant integration writes it into a security log -- where it is + read as a fact about the panel rather than as this library's best guess. + + `has_circuit` is asked only once a target has already been refused, so a + panel that answers it strangely cannot turn a permitted command into a + refused one; the worst it can do is mislabel a refusal that was going to + happen either way. + """ + if not adapter.has_circuit(circuit_id): + await self._refuse_control( + device_id=circuit_id, + value=value, + detail="no such circuit", + message=f"Panel carries no circuit {circuit_id!r}", + ) + await self._refuse_control(device_id=circuit_id, value=value, detail=detail, message=message) + # -- PanelControlProtocol ---------------------------------------------- async def set_dominant_power_source(self, value: str) -> PublishOutcome: diff --git a/src/span_panel_api/protocol.py b/src/span_panel_api/protocol.py index f35a394..2e7406c 100644 --- a/src/span_panel_api/protocol.py +++ b/src/span_panel_api/protocol.py @@ -249,6 +249,15 @@ def set_circuit_relay_target(self, circuit_id: str) -> ControlTarget | None: authorisation, and a topic built by string formatting alone authorises nothing. + **None also where the panel carries no circuit under that id.** Both + adapters resolve the id before they build anything, so an id nothing + published yields no target rather than a well-formed topic aimed at + nothing. The flat adapter's lookup used to default an unpublished value + to the empty string, which parses as "not always-on" and reads as + permission; the two adapters answer the same question and now answer it + the same way. `has_circuit` reports this case separately, so a transport + can say which of the two refusals it is raising. + Widening the return type does not move `ADAPTER_CONTRACT_VERSION` either, and the direction is why. An older adapter returns a `ControlTarget` where this now permits `ControlTarget | None`, which is @@ -265,6 +274,29 @@ def set_circuit_priority_target(self, circuit_id: str) -> ControlTarget | None: None where the panel declares the priority locked -- `never-backup` under the flat schema, `$settable` on `load-shed/priority` under v1.0 -- which is the same reading `SpanCircuitSnapshot.is_never_backup` reports. + + None also where the panel carries no circuit under that id, and where + the device carries no shed priority to write: under v1.0 an absent + `$settable` on a *declared* `load-shed/priority` means settable, but a + device that declares no such property has offered no such control, and + the two are not the same absence. + """ + + def has_circuit(self, circuit_id: str) -> bool: + """Whether this panel carries a circuit under that id at all. + + Exists so a refused command can name the right refusal. Both circuit + target builders return None for two unrelated reasons -- the panel has + no such circuit, or it has one and declares the control locked -- and a + transport that cannot tell them apart has to pick one message for both. + Picking "declares its relay non-commandable" for an id no circuit + answers to states a fact about a device that does not exist, and the + transport's audit trail carries that reason to whoever reads it. + + Membership of the *circuit* set, not of the topology: a device this + schema models some other way -- a battery, a MID, the lugs -- is not a + circuit whose controls happen to be locked, so it answers False and its + refusal reads as the absence it is. """ def set_dominant_power_source_target(self) -> ControlTarget | None: diff --git a/tests/conftest.py b/tests/conftest.py index ae05185..afb7400 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,7 +18,7 @@ import span_panel_api._http as _http_mod from span_panel_api.models import V2HomieSchema from span_panel_api.mqtt.control import ControlDeadlines -from span_panel_api_schema_0.const import TOPIC_PREFIX, TYPE_CORE +from span_panel_api_schema_0.const import TOPIC_PREFIX, TYPE_CIRCUIT, TYPE_CORE _DOTENV = Path(__file__).parent.parent / ".env" @@ -66,6 +66,15 @@ def _reset_ssl_cache() -> None: # Minimal Homie description that makes the device "ready" MINIMAL_DESCRIPTION = json.dumps({"nodes": {"core": {"type": TYPE_CORE}}}) +DESCRIBED_CIRCUIT = "aabbccdd11223344556677889900aabb" +"""A circuit id that `DESCRIPTION_WITH_CIRCUIT` actually declares.""" + +# The same panel plus one circuit, for tests that command one. A circuit the +# panel never declared yields no target and is refused, so `MINIMAL_DESCRIPTION` +# cannot stand in: a control test on it would be asserting that a write to an +# unknown circuit reaches the broker. +DESCRIPTION_WITH_CIRCUIT = json.dumps({"nodes": {"core": {"type": TYPE_CORE}, DESCRIBED_CIRCUIT: {"type": TYPE_CIRCUIT}}}) + def flat_schema(panel_size: int = 32) -> V2HomieSchema: """A flat-schema REST response declaring ``panel_size`` breaker spaces. diff --git a/tests/test_control_interceptor.py b/tests/test_control_interceptor.py index 69a3ee8..698ca3c 100644 --- a/tests/test_control_interceptor.py +++ b/tests/test_control_interceptor.py @@ -71,6 +71,10 @@ def _client(*, connected: bool = True, deadline: float = 0.05) -> SpanMqttClient adapter.set_circuit_relay_target.return_value = ControlTarget( topic=RELAY_TOPIC, device_id=SERIAL, node_id=CIRCUIT, property_id="relay" ) + # The panel has this circuit. Stated rather than left to a MagicMock's + # truthiness, because it is what makes every refusal below the "this control + # is locked" case rather than the "no such circuit" one. + adapter.has_circuit.return_value = True adapter.register_property_callback.side_effect = lambda cb: lambda: None client._adapter = adapter client._observe(adapter) @@ -362,3 +366,104 @@ async def test_no_interceptor_installed_still_raises(self) -> None: with pytest.raises(SpanPanelServerError): await client.set_circuit_relay(CIRCUIT, "OPEN") + + +class TestARefusedCommandNamesTheRefusalItActuallyMade: + """Two reasons produce no target, and they are not interchangeable. + + "Declares its relay non-commandable" asserts something about a circuit that + exists -- that it was commissioned always-on -- and sends whoever reads it to + a panel's commissioning to find out why. An id no circuit answers to has + nothing to do with commissioning, and a mistyped id investigated as a + commissioning question costs the whole investigation. + + The `detail` matters more than the message, because it travels further: it + lands in `after_publish`, which the Home Assistant integration writes into a + security log, where "relay not commandable" is read as a fact about the + panel rather than as this library's best guess at one. + """ + + @pytest.mark.asyncio + async def test_an_unknown_circuit_is_refused_as_an_unknown_circuit(self) -> None: + client = _client() + recorder = _Recorder() + client.set_control_interceptor(recorder) + assert isinstance(client._adapter, MagicMock) + client._adapter.set_circuit_relay_target.return_value = None + client._adapter.has_circuit.return_value = False + + with pytest.raises(SpanPanelServerError, match="carries no circuit"): + await client.set_circuit_relay(CIRCUIT, "OPEN") + await _settle() + + assert recorder.after[0][1].detail == "no such circuit" + + @pytest.mark.asyncio + async def test_a_known_circuit_is_refused_as_a_locked_relay(self) -> None: + """The other half, so neither test can pass on the other's account.""" + client = _client() + recorder = _Recorder() + client.set_control_interceptor(recorder) + assert isinstance(client._adapter, MagicMock) + client._adapter.set_circuit_relay_target.return_value = None + client._adapter.has_circuit.return_value = True + + with pytest.raises(SpanPanelServerError, match="non-commandable"): + await client.set_circuit_relay(CIRCUIT, "OPEN") + await _settle() + + assert recorder.after[0][1].detail == "relay not commandable" + + @pytest.mark.asyncio + async def test_the_priority_makes_the_same_distinction(self) -> None: + """A shed priority refused for an id nothing published is not a circuit + "commissioned never-backup" either.""" + client = _client() + recorder = _Recorder() + client.set_control_interceptor(recorder) + assert isinstance(client._adapter, MagicMock) + client._adapter.set_circuit_priority_target.return_value = None + client._adapter.has_circuit.return_value = False + + with pytest.raises(SpanPanelServerError, match="carries no circuit"): + await client.set_circuit_priority(CIRCUIT, "NEVER") + await _settle() + + assert recorder.after[0][1].detail == "no such circuit" + + @pytest.mark.asyncio + async def test_the_absent_circuit_is_still_a_refusal_with_no_topic(self) -> None: + """Everything else about the record is what it was: no topic, FAILED, + and `before_publish` not consulted.""" + client = _client() + recorder = _Recorder(veto=_Refusal("only admins may do that")) + client.set_control_interceptor(recorder) + assert isinstance(client._adapter, MagicMock) + client._adapter.set_circuit_relay_target.return_value = None + client._adapter.has_circuit.return_value = False + assert client._bridge is not None + + with pytest.raises(SpanPanelServerError): + await client.set_circuit_relay(CIRCUIT, "OPEN") + await _settle() + + client._bridge._client.publish.assert_not_called() + assert recorder.before == [] + command, outcome = recorder.after[0] + assert command.device_id == CIRCUIT + assert command.topic is None + assert outcome.state is PublishState.FAILED + assert outcome.topic is None + + @pytest.mark.asyncio + async def test_a_permitted_command_never_asks(self) -> None: + """The lookup is consulted only once a target has already been refused, + so a panel that answers it strangely cannot turn a permitted command + into a refused one.""" + client = _client() + assert isinstance(client._adapter, MagicMock) + client._adapter.has_circuit.return_value = False + + await client.set_circuit_relay(CIRCUIT, "OPEN") + + client._adapter.has_circuit.assert_not_called() diff --git a/tests/test_mqtt_connect_flow.py b/tests/test_mqtt_connect_flow.py index a2feb53..c1626e7 100644 --- a/tests/test_mqtt_connect_flow.py +++ b/tests/test_mqtt_connect_flow.py @@ -20,7 +20,14 @@ from span_panel_api.mqtt.const import MQTT_FULL_REBUILD_AFTER_FAILURES, MQTT_RECONNECT_MIN_DELAY_S from span_panel_api.mqtt.models import MqttClientConfig -from conftest import FAST_CONTROL_DEADLINES, MINIMAL_DESCRIPTION, SERIAL, TOPIC_PREFIX_SERIAL +from conftest import ( + DESCRIBED_CIRCUIT, + DESCRIPTION_WITH_CIRCUIT, + FAST_CONTROL_DEADLINES, + MINIMAL_DESCRIPTION, + SERIAL, + TOPIC_PREFIX_SERIAL, +) def _make_bridge() -> AsyncMqttBridge: @@ -410,13 +417,17 @@ async def test_set_circuit_relay(self, mqtt_client_mock: MagicMock) -> None: connect_task = asyncio.create_task(client.connect()) await asyncio.sleep(0.05) - client._on_message(f"{TOPIC_PREFIX_SERIAL}/$description", MINIMAL_DESCRIPTION) + # With the circuit declared: a command aimed at an id the panel never + # published is refused, so a publishing test needs a panel that carries + # the circuit it is commanding. + client._on_message(f"{TOPIC_PREFIX_SERIAL}/$description", DESCRIPTION_WITH_CIRCUIT) + # Named, or connect() spends its whole circuit-name wait on this one. + client._on_message(f"{TOPIC_PREFIX_SERIAL}/{DESCRIBED_CIRCUIT}/name", "Kitchen Lights") client._on_message(f"{TOPIC_PREFIX_SERIAL}/$state", "ready") await asyncio.wait_for(connect_task, timeout=5.0) # Publish relay command - circuit_id = "aabbccdd11223344556677889900aabb" - await client.set_circuit_relay(circuit_id, "OPEN") + await client.set_circuit_relay(DESCRIBED_CIRCUIT, "OPEN") mqtt_client_mock.publish.assert_called() @pytest.mark.asyncio @@ -426,12 +437,13 @@ async def test_set_circuit_priority(self, mqtt_client_mock: MagicMock) -> None: connect_task = asyncio.create_task(client.connect()) await asyncio.sleep(0.05) - client._on_message(f"{TOPIC_PREFIX_SERIAL}/$description", MINIMAL_DESCRIPTION) + client._on_message(f"{TOPIC_PREFIX_SERIAL}/$description", DESCRIPTION_WITH_CIRCUIT) + # Named, or connect() spends its whole circuit-name wait on this one. + client._on_message(f"{TOPIC_PREFIX_SERIAL}/{DESCRIBED_CIRCUIT}/name", "Kitchen Lights") client._on_message(f"{TOPIC_PREFIX_SERIAL}/$state", "ready") await asyncio.wait_for(connect_task, timeout=5.0) - circuit_id = "aabbccdd11223344556677889900aabb" - await client.set_circuit_priority(circuit_id, "NEVER") + await client.set_circuit_priority(DESCRIBED_CIRCUIT, "NEVER") mqtt_client_mock.publish.assert_called() @pytest.mark.asyncio diff --git a/tests/test_mqtt_homie.py b/tests/test_mqtt_homie.py index 3b1405b..9fd3ef9 100644 --- a/tests/test_mqtt_homie.py +++ b/tests/test_mqtt_homie.py @@ -1017,6 +1017,22 @@ def test_capabilities(self): # --------------------------------------------------------------------------- +CONTROLLED_CIRCUIT = "aabbccdd112233445566778899001122" + + +def _adapter_carrying_the_controlled_circuit() -> SchemaZeroAdapter: + """A flat adapter whose panel has actually declared the circuit under test. + + The description matters: a circuit the panel never published is refused, so + a publishing test that skipped it would be asserting that a write to an + unknown id reaches the wire. + """ + adapter = SchemaZeroAdapter(serial_number=SERIAL, schema=flat_schema(32)) + adapter.handle_message(f"{PREFIX}/$description", _make_description({CONTROLLED_CIRCUIT: {"type": TYPE_CIRCUIT}})) + adapter.handle_message(f"{PREFIX}/$state", HOMIE_STATE_READY) + return adapter + + class TestSpanMqttClientControl: @pytest.mark.asyncio async def test_set_circuit_relay_publishes(self): @@ -1029,12 +1045,12 @@ async def test_set_circuit_relay_publishes(self): broker_config=config, control_deadlines=FAST_CONTROL_DEADLINES, ) - client._adapter = SchemaZeroAdapter(serial_number=SERIAL, schema=flat_schema(32)) + client._adapter = _adapter_carrying_the_controlled_circuit() mock_bridge = acking_bridge() client._bridge = mock_bridge - await client.set_circuit_relay("aabbccdd112233445566778899001122", "OPEN") + await client.set_circuit_relay(CONTROLLED_CIRCUIT, "OPEN") mock_bridge.publish.assert_called_once_with( f"{TOPIC_PREFIX}/{SERIAL}/aabbccdd112233445566778899001122/relay/set", @@ -1052,12 +1068,12 @@ async def test_set_circuit_priority_publishes(self): broker_config=config, control_deadlines=FAST_CONTROL_DEADLINES, ) - client._adapter = SchemaZeroAdapter(serial_number=SERIAL, schema=flat_schema(32)) + client._adapter = _adapter_carrying_the_controlled_circuit() mock_bridge = acking_bridge() client._bridge = mock_bridge - await client.set_circuit_priority("aabbccdd112233445566778899001122", "NEVER") + await client.set_circuit_priority(CONTROLLED_CIRCUIT, "NEVER") mock_bridge.publish.assert_called_once_with( f"{TOPIC_PREFIX}/{SERIAL}/aabbccdd112233445566778899001122/shed-priority/set", diff --git a/tests/test_protocol_conformance.py b/tests/test_protocol_conformance.py index a2a2e9e..db062cc 100644 --- a/tests/test_protocol_conformance.py +++ b/tests/test_protocol_conformance.py @@ -67,6 +67,7 @@ def test_schema_adapter_declares_its_methods() -> None: "find_node_by_type", "set_circuit_relay_target", "set_circuit_priority_target", + "has_circuit", "set_dominant_power_source_target", "dominant_power_source_payload", "set_evse_charge_limit_target", diff --git a/tests/test_schema_one_control_refusal.py b/tests/test_schema_one_control_refusal.py index f16ac2b..cf8467a 100644 --- a/tests/test_schema_one_control_refusal.py +++ b/tests/test_schema_one_control_refusal.py @@ -293,3 +293,88 @@ def test_an_unannounced_priority_settable_is_still_writable() -> None: adapter = _adapter(_redeclared(CONTROLLABLE_CIRCUIT, "load-shed", "priority", settable=None)) assert adapter.set_circuit_priority_target(CONTROLLABLE_CIRCUIT) is not None + + +# --------------------------------------------------------------------------- +# An undeclared property is not an unannounced attribute +# --------------------------------------------------------------------------- + +_NON_CIRCUITS = ("bess", "bess-mid", "pv", "lugs-upstream", "lugs-downstream") +"""Devices in the capture that carry no `load-shed` node at all.""" + + +def test_the_capture_carries_devices_with_no_shed_priority(adapter: SchemaOneAdapter) -> None: + """The premise, asserted rather than assumed: these devices declare nothing + about a shed priority, so there is a property-undeclared case to refuse.""" + tree = parent_child_tree() + + for device_id in _NON_CIRCUITS: + nodes = json.loads(tree[device_id]["$description"])["nodes"] + assert "load-shed" not in nodes, device_id + + +@pytest.mark.parametrize("device_id", _NON_CIRCUITS) +def test_a_device_that_declares_no_priority_yields_no_priority_target(adapter: SchemaOneAdapter, device_id: str) -> None: + """The permissive default is for a *declared* property, and these declare none. + + `priority_is_settable` reads an absent `$settable` as permission, which is + right for the documented case where firmware declares `load-shed/priority` + and omits the attribute. A BESS, a MID or the lugs declare no `load-shed` + node at all — they have offered no shed priority for anything to be settable + on — and answering them with the same default resolved a write topic for a + control the device never published. The relay avoided this incidentally, + because its default is refusal. + """ + assert adapter.set_circuit_priority_target(device_id) is None + assert adapter.set_circuit_relay_target(device_id) is None + + +def test_a_priority_target_exists_exactly_on_the_circuits(adapter: SchemaOneAdapter) -> None: + """Over the whole capture at once, the way the relay invariant is asserted. + + Every device in the tree is addressable by id, and only the circuits declare + a shed priority — so a target on anything else is a target for a control + that device never offered, whatever its `$settable` would have defaulted to. + """ + tree = parent_child_tree() + + for device_id, topics in tree.items(): + is_circuit = json.loads(topics["$description"])["type"].endswith(".circuit") + assert (adapter.set_circuit_priority_target(device_id) is not None) is is_circuit, device_id + + +def test_a_declared_priority_missing_from_its_node_is_undeclared_too(adapter: SchemaOneAdapter) -> None: + """The narrower half of the same case: the node is there, the property is not. + + Removing `priority` leaves `load-shed` declared, so nothing but the property + lookup distinguishes this from the capture's ordinary circuit. It has to + refuse for the same reason the BESS does -- there is no property to be + settable. + """ + tree = _copy() + description = json.loads(tree[CONTROLLABLE_CIRCUIT]["$description"]) + del description["nodes"]["load-shed"]["properties"]["priority"] + tree[CONTROLLABLE_CIRCUIT]["$description"] = json.dumps(description) + + assert _adapter(tree).set_circuit_priority_target(CONTROLLABLE_CIRCUIT) is None + + +# --------------------------------------------------------------------------- +# Telling "no such circuit" apart from "the circuit says no" +# --------------------------------------------------------------------------- + + +def test_has_circuit_answers_for_the_circuits_and_nothing_else(adapter: SchemaOneAdapter) -> None: + """Membership of the circuit set, not of the topology. + + The transport asks this to name the refusal it is raising, and a BESS + reported as a circuit would have its refusal read as "declares its relay + non-commandable" -- a claim about a relay that device does not have. + """ + assert adapter.has_circuit(CONTROLLABLE_CIRCUIT) is True + assert adapter.has_circuit(LOCKED_CIRCUIT) is True, "locked is not absent" + + for device_id in _NON_CIRCUITS: + assert adapter.has_circuit(device_id) is False, device_id + assert adapter.has_circuit(PANEL) is False + assert adapter.has_circuit("0" * 32) is False diff --git a/tests/test_schema_zero_adapter.py b/tests/test_schema_zero_adapter.py index ed8bf9f..b0c146e 100644 --- a/tests/test_schema_zero_adapter.py +++ b/tests/test_schema_zero_adapter.py @@ -8,9 +8,12 @@ from __future__ import annotations +import json + import pytest from span_panel_api_schema_0 import SchemaZeroAdapter +from span_panel_api_schema_0.const import TYPE_CIRCUIT, TYPE_CORE from conftest import flat_schema from span_panel_api.protocol import SchemaAdapter @@ -40,51 +43,102 @@ def test_subscribes_to_the_single_panel_wildcard(adapter: SchemaZeroAdapter) -> CIRCUIT = "ac3dccda46a94b98878a227df6fed588" -def test_circuit_setter_topics_address_the_panel_device(adapter: SchemaZeroAdapter) -> None: - relay = adapter.set_circuit_relay_target(CIRCUIT) - priority = adapter.set_circuit_priority_target(CIRCUIT) +def _describe(adapter: SchemaZeroAdapter, nodes: dict[str, str]) -> None: + """Publish a `$description` declaring `nodes` as node id → type.""" + adapter.handle_message( + f"ebus/5/{SERIAL}/$description", + json.dumps({"nodes": {node_id: {"type": node_type} for node_id, node_type in nodes.items()}}), + ) + + +@pytest.fixture +def described(adapter: SchemaZeroAdapter) -> SchemaZeroAdapter: + """An adapter whose panel has declared `CIRCUIT` as a circuit. + + Every settability test below needs this. The flat schema answers "may this + be commanded" out of published values, and an unpublished value is the empty + string — so on a panel that has declared nothing, both flags read as + permission and every id looks writable. Declaring the node is what makes the + subsequent value assertions about the values. + """ + _describe(adapter, {CIRCUIT: TYPE_CIRCUIT}) + return adapter + + +def test_circuit_setter_topics_address_the_panel_device(described: SchemaZeroAdapter) -> None: + relay = described.set_circuit_relay_target(CIRCUIT) + priority = described.set_circuit_priority_target(CIRCUIT) assert relay is not None and relay.topic == f"ebus/5/{SERIAL}/{CIRCUIT}/relay/set" assert priority is not None and priority.topic == f"ebus/5/{SERIAL}/{CIRCUIT}/shed-priority/set" +def test_a_circuit_the_panel_never_declared_yields_no_target(described: SchemaZeroAdapter) -> None: + """The refusal `schema_1` already made, from the lookup flat has for it. + + `get_prop` answers `""` for an id nothing published, `""` parses as `false`, + and `not false` is permission — so an unknown circuit produced a well-formed + topic aimed at nothing, on a panel that had declared one real circuit beside + it. Two adapters answering the same question differently is the bug; the + question is "does this panel carry a circuit by this name", and the flat + schema answers it from the node type in `$description`. + """ + unknown = "0" * 32 + + assert described.set_circuit_relay_target(unknown) is None + assert described.set_circuit_priority_target(unknown) is None + assert described.has_circuit(unknown) is False + # And the declared sibling is untouched: the refusal is about the id. + assert described.has_circuit(CIRCUIT) is True + + +def test_a_node_of_another_type_is_not_a_circuit(adapter: SchemaZeroAdapter) -> None: + """A node the panel published under a non-circuit type carries no circuit + controls, and is refused as an absent circuit rather than as a locked one.""" + _describe(adapter, {"core": TYPE_CORE}) + + assert adapter.has_circuit("core") is False + assert adapter.set_circuit_relay_target("core") is None + assert adapter.set_circuit_priority_target("core") is None + + def _publish(adapter: SchemaZeroAdapter, node: str, prop: str, value: str) -> None: adapter.handle_message(f"ebus/5/{SERIAL}/{node}/{prop}", value) -def test_an_always_on_circuit_yields_no_relay_target(adapter: SchemaZeroAdapter) -> None: +def test_an_always_on_circuit_yields_no_relay_target(described: SchemaZeroAdapter) -> None: """The same refusal the v1.0 side makes, from the value flat publishes for it. `always-on` is already read into `is_user_controllable`, so the panel had told this adapter the relay was locked and only the snapshot listened; the topic builder was pure string formatting from a node id. """ - _publish(adapter, CIRCUIT, "always-on", "true") + _publish(described, CIRCUIT, "always-on", "true") - assert adapter.set_circuit_relay_target(CIRCUIT) is None + assert described.set_circuit_relay_target(CIRCUIT) is None # Only the relay. Always-on is not never-backup, on either schema. - assert adapter.set_circuit_priority_target(CIRCUIT) is not None + assert described.set_circuit_priority_target(CIRCUIT) is not None -def test_a_never_backup_circuit_yields_no_priority_target(adapter: SchemaZeroAdapter) -> None: - _publish(adapter, CIRCUIT, "never-backup", "true") +def test_a_never_backup_circuit_yields_no_priority_target(described: SchemaZeroAdapter) -> None: + _publish(described, CIRCUIT, "never-backup", "true") - assert adapter.set_circuit_priority_target(CIRCUIT) is None - assert adapter.set_circuit_relay_target(CIRCUIT) is not None + assert described.set_circuit_priority_target(CIRCUIT) is None + assert described.set_circuit_relay_target(CIRCUIT) is not None -def test_a_published_false_reads_as_permission(adapter: SchemaZeroAdapter) -> None: +def test_a_published_false_reads_as_permission(described: SchemaZeroAdapter) -> None: """A panel that publishes the flags as `false` must not be refused. Absence already reads as permission; this is the other half, and it is the one a producer actually exercises -- a clone of a real panel writes `always-on: "false"` out rather than omitting it. """ - _publish(adapter, CIRCUIT, "always-on", "false") - _publish(adapter, CIRCUIT, "never-backup", "false") + _publish(described, CIRCUIT, "always-on", "false") + _publish(described, CIRCUIT, "never-backup", "false") - assert adapter.set_circuit_relay_target(CIRCUIT) is not None - assert adapter.set_circuit_priority_target(CIRCUIT) is not None + assert described.set_circuit_relay_target(CIRCUIT) is not None + assert described.set_circuit_priority_target(CIRCUIT) is not None def test_dominant_power_source_topic_is_none_before_the_core_node_is_known( From a7819709e0d8b32e8da62b18f5b76c8322e39f6f Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:54:10 -0700 Subject: [PATCH 2/3] chore(capture): a missing pin exits with prose, not a traceback `pinned_release()` indexed straight into the lock document, so a `spec_lock.json` with no `peers` block, or no `ebus-panel-sim` inside it, came out as a bare `KeyError` where every other malformed input in this script exits with a sentence naming what was wrong. --- scripts/capture_parent_child_reference.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/scripts/capture_parent_child_reference.py b/scripts/capture_parent_child_reference.py index e3bd5be..82b1773 100644 --- a/scripts/capture_parent_child_reference.py +++ b/scripts/capture_parent_child_reference.py @@ -127,6 +127,19 @@ def _mappings(value: object, where: str) -> list[dict[str, object]]: return [_mapping(item, f"{where}[{index}]") for index, item in enumerate(_sequence(value, where))] +def _required(source: Mapping[str, object], key: str, where: str) -> object: + """One key that has to be there, reported the way everything else here is. + + Indexing straight into the mapping says the same thing as a bare `KeyError` + traceback, which is the one failure mode in this file that makes a reader + work out what the script wanted. Every other malformed input exits with a + sentence naming it. + """ + if key not in source: + raise SystemExit(f"{where} has no {key!r} entry") + return source[key] + + def _text(source: Mapping[str, object], key: str, default: str) -> str: value = source.get(key) return default if value is None else str(value) @@ -177,8 +190,9 @@ def pinned_release() -> str: """ with LOCK.open(encoding="utf-8") as handle: lock: object = json.load(handle) - peers = _mapping(_mapping(lock, "spec_lock.json")["peers"], "peers") - return _text(_mapping(peers[PEER], f"peers.{PEER}"), "version", "") + document = _mapping(lock, "spec_lock.json") + peers = _mapping(_required(document, "peers", "spec_lock.json"), "peers") + return _text(_mapping(_required(peers, PEER, "peers"), f"peers.{PEER}"), "version", "") # --------------------------------------------------------------------------- From b641419ab3f6254a35e1f4bea62221eca23948e2 Mon Sep 17 00:00:00 2001 From: cayossarian <23534755+cayossarian@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:56:46 -0700 Subject: [PATCH 3/3] docs(changelog): name has_circuit as the third adapter-contract change `_derive_required_members` makes every public `SchemaAdapter` member mandatory of every adapter wheel, so adding one is a contract change and belongs beside the four `set_*_topic` renames rather than buried in a prose bullet. No new mismatch a consumer can hit -- a 1.0.0 adapter was already rejected by the renames -- but the rejection names this member too, and the changelog should say so before somebody meets it. --- CHANGELOG.md | 4 ++++ packages/schema-0/CHANGELOG.md | 7 +++++-- packages/schema-1/CHANGELOG.md | 9 ++++++--- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e2467a..289b998 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,10 @@ install of the adapter distribution does not, and a 1.0.0 adapter against this b reaches `after_publish` and the Home Assistant integration writes it into a security log where it is read as a fact about the panel. `SchemaAdapter` gains `has_circuit` for it — consulted only once a target has already been refused, so it can relabel a refusal but never cause one. + **`has_circuit` is a required protocol member**, and therefore the third adapter-contract change in this release alongside the four `set_*_topic` renames and the widened return types: `_derive_required_members` makes every public `SchemaAdapter` member + mandatory of every adapter wheel, so an adapter without it is rejected at discovery. That is not a new mismatch anyone can hit — a 1.0.0 adapter was already rejected by the renames — and the rejection names this member alongside them, with the same + remedy. `ADAPTER_CONTRACT_VERSION` still does not move: an added member is caught by name at discovery, which is what the constant's own docstring reserves it for. + - **A control the library refused before resolving an address is no longer invisible to `ControlInterceptor`.** `after_publish` is contracted to see every command, refusals included, but five refusals happened while resolving the target and therefore never reached the publish path at all: a relay declared non-commandable, a priority declared locked, a charger with no settable limit, a panel with no islanding control, and an adopted property that is not settable. A consumer building a security audit on `after_publish` — which is what the Home Assistant integration does — would have had a hole in it exactly where the interesting cases are, the highest-consequence control in the system among them. diff --git a/packages/schema-0/CHANGELOG.md b/packages/schema-0/CHANGELOG.md index 52585e4..1fb8f19 100644 --- a/packages/schema-0/CHANGELOG.md +++ b/packages/schema-0/CHANGELOG.md @@ -35,8 +35,11 @@ Requires `span-panel-api` **3.1.0 or newer**, and the two must be upgraded toget — so an unknown id produced a well-formed topic aimed at nothing, including for the synthetic `unmapped_tab_*` keys the snapshot invents. `schema_1` already refused an id its tree did not carry; two adapters answering the same question differently was the defect, and the question is answered here from the node type in `$description`. -- **`HomieDeviceConsumer.is_circuit_node` is public**, and `SchemaZeroAdapter` gains `has_circuit`, which delegates to it. `SchemaAdapter` declares `has_circuit` in 3.1.0 so a transport can tell "no such circuit" apart from "this circuit's control is - locked" when it reports a refusal. One reading behind both answers: what counts as a circuit here decides the refusal and its stated reason together, so the two cannot drift. +- **BREAKING: `SchemaZeroAdapter` gains `has_circuit`, a new required `SchemaAdapter` member.** 3.1.0 declares it so a transport can tell "no such circuit" apart from "this circuit's control is locked" when it reports a refusal. Every public member of that + protocol is mandatory of every adapter wheel — `_derive_required_members` enforces it by name — so this is the third contract change in the pair, alongside the four `set_*_topic` renames and the widened return types. It adds no new mismatch a consumer + can hit: a 1.0.0 wheel was already rejected by the renames, and the rejection names this member too, with the same remedy (upgrade both distributions together). + + `HomieDeviceConsumer.is_circuit_node` is public for it, and `has_circuit` delegates there rather than re-deriving. One reading behind both answers: what counts as a circuit here decides the refusal and its stated reason together, so the two cannot drift. ## [1.0.0] diff --git a/packages/schema-1/CHANGELOG.md b/packages/schema-1/CHANGELOG.md index 6422ca0..30b2731 100644 --- a/packages/schema-1/CHANGELOG.md +++ b/packages/schema-1/CHANGELOG.md @@ -45,9 +45,12 @@ Requires `span-panel-api` **3.1.0 or newer**, and the two must be upgraded toget `set_circuit_priority_target("bess")` resolved a topic for a control that device never published. `_declared_settable` now distinguishes the two: a property the device does not declare is never settable, whatever the per-property default for a declared one says. The relay avoided this incidentally, because its default is already refusal. -- **`SchemaOneAdapter.has_circuit`**, which `SchemaAdapter` declares in 3.1.0 so a transport can tell "no such circuit" apart from "this circuit's control is locked" when it reports a refusal. It answers on the device _type_, not on membership of the tree: - every device is addressable by id, and a BESS reported as a circuit would have its refusal read as a claim about a relay it does not have. It is deliberately not a gate on the two target builders — those refuse on the declaration, which is the - specification's rule and what authorises a write, and putting a type check in front of it would be a second, weaker rule where the authorisation lives. +- **BREAKING: `SchemaOneAdapter` gains `has_circuit`, a new required `SchemaAdapter` member.** 3.1.0 declares it so a transport can tell "no such circuit" apart from "this circuit's control is locked" when it reports a refusal. Every public member of that + protocol is mandatory of every adapter wheel — `_derive_required_members` enforces it by name — so this is the third contract change in the pair, alongside the four `set_*_topic` renames and the widened return types. It adds no new mismatch a consumer + can hit: a 1.0.0 wheel was already rejected by the renames, and the rejection names this member too, with the same remedy (upgrade both distributions together). + + It answers on the device _type_, not on membership of the tree: every device is addressable by id, and a BESS reported as a circuit would have its refusal read as a claim about a relay it does not have. It is deliberately not a gate on the two target + builders — those refuse on the declaration, which is the specification's rule and what authorises a write, and putting a type check in front of it would be a second, weaker rule where the authorisation lives. - **`spec_lock.json`: `peer` becomes `peers`, and the eBus emitter is tracked as one.** `ebus-panel-sim` produced the reference tree and was recorded nowhere, which is the whole reason that capture went three releases stale without anything objecting. It now carries a pin of the same shape panelbench has — repo, ref, role, commit, tag, released version, and the specification commit it implements — so a scheduled job can ask whether the producer has moved and a test can ask whether our checkout is the