Skip to content

Commit 01b17f8

Browse files
dcjclaude
andcommitted
feat: root capabilities and extend(), and make materialization idempotent
Two gaps reported against 0.21.0's DeviceTreeBuilder, plus the idempotence both of them need. root's own surfaces (an enclosure's aggregate metering, its state, its controls) had no declarative expression. A producer had to hand-roll the root beside the builder: one model, two construction styles, and the root outside every guarantee the builder gives. The root already exists, so this materializes onto it; the model group defaults to the root's device id, matching how add() keys a child. first published. add() short-circuits an already-built spec, so the builder modeled devices appearing and disappearing but not growing. The workaround was unsafe rather than absent: Device.add_node and Node.add_property both replace wholesale, so re-declaring a live device dropped the previous node's properties from $description while leaving their retained topics on the broker, and only delete_node clears those. Description and broker then disagree, across restarts. Idempotence, at three levels, which both features need and which build_from_declarations now gets too: node reused rather than replaced (the mechanism behind the description-versus-broker divergence above) property reused rather than re-added (add_property replaces and republishes with force=True) the transition itself not opened at all when nothing would be created. An empty state_transition still emits init -> ready, and that edge forces every controller on the bus to resync, so a re-declaration that changes nothing must not cost one. The third was found by a test asserting extend() publishes nothing on a re-declare: content was already idempotent, the state flap was not. Closes #67 Closes #68 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3e97e16 commit 01b17f8

4 files changed

Lines changed: 262 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,16 @@ All notable changes to `ebus-sdk` are recorded here. Format follows [Keep a Chan
66

77
### Added
88

9+
- `DeviceTreeBuilder.add_root_capabilities()`: the tree's root can carry its own capabilities. `add()` only ever creates children, so a root's own surfaces (an enclosure's aggregate metering, its state, its controls) had no declarative expression and had to be hand-rolled beside the builder, which left one model with two construction styles and put the root outside every guarantee the builder gives (idempotence, ordered teardown, model cleanup). The root already exists, so this materializes onto it rather than constructing anything; the model group defaults to the root's device id, matching how `add()` keys a child. `root_capabilities()` reads back what has accumulated. ([#67](https://github.com/electrification-bus/python-sdk/issues/67))
10+
11+
- `DeviceTreeBuilder.extend()`: a device that already exists can grow a capability. A capability set is not always known when a device is first published, since a storage system commissioned at runtime gives an enclosure shed and forecast surfaces it did not have at boot, and `add()` short-circuits an already-built spec, so the builder modeled devices appearing and disappearing but not a device growing. The workaround was unsafe rather than merely absent: `Device.add_node` and `Node.add_property` both replace wholesale, so re-declaring a live device through `build_from_declarations` dropped the previous node's properties from `$description` while leaving their retained topics on the broker, and only `delete_node` clears those. The result was a tree whose description and whose broker state disagreed, persisting across restarts. `extend()` materializes inside one `state_transition()` and folds the new model keys into the same bookkeeping `remove()` uses. ([#68](https://github.com/electrification-bus/python-sdk/issues/68))
12+
913
- `node_id` on `build_from_declarations` and `DeviceTreeBuilder`: a callable mapping a capability to the Homie node id it materializes onto, defaulting to the capability itself. The node id was hardcoded to the capability name, which is right until one device carries two instances of the same capability (two lugs, two meters), at which point the second silently lands on the first one's node. `node_type` and `node_name` were already callables, so the id was the one part of a node a caller could not choose. Renaming is all it does: the declaration's vocabulary stays `capability`, the model group still comes from the spec, and the returned map is still keyed by the declared capability, so a caller who ignores it sees no change. Pairs with `PropertySpec.model_group` from 0.21.0, which separates the same two instances in the model the way this separates them on the wire; using one without the other moves the collision rather than removing it. ([#47](https://github.com/electrification-bus/python-sdk/issues/47))
1014

15+
### Changed
16+
17+
- Materializing declarations is now idempotent, at three levels. An existing Homie **node** is reused rather than replaced (`Device.add_node` is a wholesale `self._nodes.update(...)`, which is the mechanism behind the description-versus-broker divergence above); an existing Homie **property** is reused rather than re-added (`Node.add_property` replaces and republishes with `force=True`); and a materialization that would create nothing **does not open a state transition at all**. That last one matters most: an empty transition still emits `init` then `ready`, and that edge forces every controller on the bus to resync, so a re-declaration that changes nothing must not cost one. Together these make `build_from_declarations` safe to call twice, which is what lets a re-fired incremental lifecycle be a genuine no-op rather than a quieter republish.
18+
1119
### Fixed
1220

1321
- `build_from_declarations` and `DeviceTreeBuilder` now REUSE an observable property the model already holds instead of replacing it. `_materialize` guarded the model group with `has_group` and then, two lines later, added the property unconditionally, and `GroupedPropertyDict.add_property` is a wholesale `self._properties[property_id] = property`. So a producer handing over a model it had already populated got that property swapped for a fresh one, losing its value and, worse, every callback and `entity_setter` attached to it: `$description` kept advertising `settable: true` while the actuator behind it was gone, and an arriving `/set` did nothing. Nor was it self-healing, since the builder path seeds only a static `initial_value` and `Property.set_value` fires callbacks only on an actual change, so a value written once at group creation never republished. This is the exact case `DeviceTreeBuilder` documents as the reason it accepts a model rather than creating one, which made the gap a documented guarantee the code did not provide. The builder now records only properties it actually created, so removing a device deletes what it added and leaves what the producer owned. A spec whose python type disagrees with the property already in the model raises rather than binding a Homie twin to a mismatched observable. ([#66](https://github.com/electrification-bus/python-sdk/issues/66))

doc/building-a-proxy.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,8 @@ Four things the tree builder does that the single-device one has no need to:
196196
- **`add()` is idempotent.** Incremental lifecycles re-fire, and a second `add()` of a built spec returns the same `Device` without republishing anything.
197197
- **`remove()` is depth-first**, grandchild before parent, derived from the live tree rather than an ordering you maintain, so nothing ever observes an orphaned child. It also deletes the model entries it added, and any group it created that is now empty.
198198

199+
The root is not only a parent: `builder.add_root_capabilities(specs)` materializes capabilities onto the root device itself, keyed in the model by the root's device id, and `builder.extend(spec, specs)` gives an already-built device a capability it did not have at boot. Both are idempotent, and a call that would create nothing does not open a state transition at all, so a re-fired lifecycle costs no `init` to `ready` edge (an empty one still forces every controller on the bus to resync).
200+
199201
Each `add()` announces its own device and makes the parent republish its `$description`. To collapse a burst of adds into one parent announcement, wrap them in the parent's `state_transition()`. Use `on_created` for per-child side effects rather than post-processing the returned tree.
200202

201203
## Lifecycle and state

src/ebus_sdk/declaration.py

Lines changed: 127 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
from __future__ import annotations
2020

21+
from contextlib import nullcontext
2122
from dataclasses import dataclass
2223
from functools import partial
2324
from typing import Any, Callable, Iterable, Optional, Sequence, Union
@@ -259,22 +260,29 @@ def _materialize(
259260
declared: dict[tuple, PropertySpec] = {}
260261
model_keys: list = []
261262
created_groups: list = []
262-
with device.state_transition():
263+
# Open a state transition only if there is something to announce. An empty
264+
# one still emits init -> ready, and that edge forces every controller on the
265+
# bus to resync, so a re-declaration that changes nothing must not cost one.
266+
needs_transition = _needs_materializing(device, model, grouped, node_id, default_group)
267+
with device.state_transition() if needs_transition else nullcontext():
263268
for capability, cap_specs in grouped.items():
264269
# A node exists to carry published properties. If every spec on this
265270
# capability is internal, creating one would announce an empty node.
266271
published = [spec for spec in cap_specs if not spec.internal_only]
267-
node = (
268-
device.add_node_from_dict(
272+
# Reuse an existing node. Device.add_node is a wholesale
273+
# `self._nodes.update(...)`, so re-declaring would drop the previous
274+
# node's properties from $description while LEAVING their retained
275+
# topics on the broker (only delete_node clears those), producing a
276+
# tree whose description and whose broker state disagree.
277+
node = None
278+
if published:
279+
node = device.get_node(node_id(capability)) or device.add_node_from_dict(
269280
{
270281
"id": node_id(capability),
271282
"name": node_name(capability),
272283
"type": node_type(capability),
273284
}
274285
)
275-
if published
276-
else None
277-
)
278286
for spec in cap_specs:
279287
group = _group_for(spec, default_group)
280288
if not model.has_group(group):
@@ -315,6 +323,14 @@ def _materialize(
315323
prop_dict["round_to"] = spec.round_to
316324
if not spec.retained:
317325
prop_dict["retained"] = False
326+
# Reuse likewise: Node.add_property replaces wholesale and
327+
# republishes with force=True, so re-declaring an unchanged
328+
# property would re-announce it. A datatype that actually changed
329+
# is caught on the model side above, which raises.
330+
existing_prop = node.get_property(spec.prop_id)
331+
if existing_prop is not None:
332+
homie_props[(capability, spec.prop_id)] = existing_prop
333+
continue
318334
homie_prop = node.add_property_from_dict(prop_dict)
319335
bind_property_to_homie(model, group, spec.model_key, homie_prop)
320336
# Inbound/control path for a settable property with a translator:
@@ -327,6 +343,31 @@ def _materialize(
327343
return _Materialized(homie_props, declared, model_keys, created_groups)
328344

329345

346+
def _needs_materializing(
347+
device: Device,
348+
model: GroupedPropertyDict,
349+
grouped: dict,
350+
node_id: Callable[[str], str],
351+
default_group: Optional[str],
352+
) -> bool:
353+
"""True when any spec still has something to create on the device or the model.
354+
355+
Answered BEFORE the transition opens, because the question is whether to open
356+
one at all: an init -> ready edge that announces nothing is a cost paid by
357+
every controller on the bus.
358+
"""
359+
for capability, cap_specs in grouped.items():
360+
for spec in cap_specs:
361+
if model.get(_group_for(spec, default_group), spec.model_key) is None:
362+
return True
363+
if spec.internal_only:
364+
continue
365+
node = device.get_node(node_id(capability))
366+
if node is None or node.get_property(spec.prop_id) is None:
367+
return True
368+
return False
369+
370+
330371
def _seed(
331372
model: GroupedPropertyDict,
332373
declared: dict,
@@ -482,6 +523,10 @@ def __init__(
482523
self._model_keys: dict = {}
483524
self._created_groups: dict = {}
484525
self._deferred: list = []
526+
# The root is not a DeviceSpec, so its bookkeeping lives beside the
527+
# per-spec maps rather than inside them. Nothing removes a root.
528+
self._root_props: dict = {}
529+
self._root_model_keys: list = []
485530

486531
def add(self, spec: DeviceSpec) -> Optional[Device]:
487532
"""Materialize `spec` as a device, or defer it while its id is unknown.
@@ -582,6 +627,82 @@ def remove(self, spec: DeviceSpec) -> None:
582627
self._devices.pop(gone, None)
583628
self._homie_props.pop(gone, None)
584629

630+
def add_root_capabilities(self, specs: Iterable[PropertySpec], *, model_group: Optional[str] = None) -> dict:
631+
"""Materialize capabilities onto the tree's ROOT device.
632+
633+
`add()` only ever creates children, so a root's own capabilities (an
634+
enclosure's aggregate metering, its state, its control surfaces) had no
635+
declarative expression and had to be hand-rolled beside the builder: one
636+
model, two construction styles, and the root outside every guarantee the
637+
builder gives.
638+
639+
The root already exists, so this materializes onto it rather than
640+
constructing anything. `model_group` defaults to the root's device id,
641+
matching how `add()` keys a child's group. Idempotent, so a re-fired
642+
lifecycle re-declares nothing.
643+
644+
Returns `{(capability, prop_id): homie.Property}` for the root, and the
645+
map accumulates across calls, so `add_root_capabilities` twice returns
646+
everything the root has.
647+
"""
648+
group = model_group or self._root.id()
649+
built = _materialize(
650+
self._root,
651+
self._model,
652+
specs,
653+
node_type=self._node_type,
654+
node_name=self._node_name,
655+
node_id=self._node_id,
656+
default_group=group,
657+
)
658+
_seed(self._model, built.declared, default_group=group)
659+
self._root_props.update(built.homie_props)
660+
self._root_model_keys.extend(built.model_keys)
661+
return dict(self._root_props)
662+
663+
def root_capabilities(self) -> dict:
664+
"""`{(capability, prop_id): homie.Property}` materialized onto the root so far."""
665+
return dict(self._root_props)
666+
667+
def extend(self, spec: DeviceSpec, specs: Iterable[PropertySpec]) -> dict:
668+
"""Give a device this builder already built additional capabilities.
669+
670+
A device's capability set is not always known when it is first published:
671+
a storage system is commissioned and the enclosure gains shed and
672+
forecast surfaces it did not have at boot. `add()` short-circuits an
673+
already-built spec, so the builder modeled devices appearing and
674+
disappearing but not a device GROWING.
675+
676+
Materializes inside one `state_transition()`, so the device announces
677+
once, and folds the new model keys into the same bookkeeping `remove()`
678+
uses. Idempotent: extending with a capability already present is a no-op
679+
rather than a republish, because incremental lifecycles re-fire.
680+
681+
Raises `KeyError` for a spec that is not built. Use `add()` first; a
682+
deferred device has no tree to extend.
683+
"""
684+
device = self._devices.get(spec)
685+
if device is None:
686+
raise KeyError(
687+
f"{spec.device_class}: not built, so there is nothing to extend. "
688+
"add() it first (a deferred device has no tree yet)."
689+
)
690+
group = spec.resolve_model_group(device.id())
691+
built = _materialize(
692+
device,
693+
self._model,
694+
specs,
695+
node_type=self._node_type,
696+
node_name=self._node_name,
697+
node_id=self._node_id,
698+
default_group=group,
699+
)
700+
_seed(self._model, built.declared, default_group=group)
701+
self._homie_props[spec].update(built.homie_props)
702+
self._model_keys[spec].extend(built.model_keys)
703+
self._created_groups[spec].extend(built.created_groups)
704+
return dict(self._homie_props[spec])
705+
585706
def device_for(self, spec: DeviceSpec) -> Optional[Device]:
586707
"""The live `Device` for `spec`, or None if it is deferred or removed."""
587708
return self._devices.get(spec)

0 commit comments

Comments
 (0)