Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,16 @@ All notable changes to `ebus-sdk` are recorded here. Format follows [Keep a Chan

### Added

- `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))

- `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))

- `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))

### Changed

- 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.

### Fixed

- `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))
Expand Down
2 changes: 2 additions & 0 deletions doc/building-a-proxy.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ Four things the tree builder does that the single-device one has no need to:
- **`add()` is idempotent.** Incremental lifecycles re-fire, and a second `add()` of a built spec returns the same `Device` without republishing anything.
- **`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.

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).

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.

## Lifecycle and state
Expand Down
133 changes: 127 additions & 6 deletions src/ebus_sdk/declaration.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from __future__ import annotations

from contextlib import nullcontext
from dataclasses import dataclass
from functools import partial
from typing import Any, Callable, Iterable, Optional, Sequence, Union
Expand Down Expand Up @@ -259,22 +260,29 @@ def _materialize(
declared: dict[tuple, PropertySpec] = {}
model_keys: list = []
created_groups: list = []
with device.state_transition():
# Open a state transition only if there is something to announce. An empty
# one 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.
needs_transition = _needs_materializing(device, model, grouped, node_id, default_group)
with device.state_transition() if needs_transition else nullcontext():
for capability, cap_specs in grouped.items():
# A node exists to carry published properties. If every spec on this
# capability is internal, creating one would announce an empty node.
published = [spec for spec in cap_specs if not spec.internal_only]
node = (
device.add_node_from_dict(
# Reuse an existing node. Device.add_node is a wholesale
# `self._nodes.update(...)`, so re-declaring would drop the previous
# node's properties from $description while LEAVING their retained
# topics on the broker (only delete_node clears those), producing a
# tree whose description and whose broker state disagree.
node = None
if published:
node = device.get_node(node_id(capability)) or device.add_node_from_dict(
{
"id": node_id(capability),
"name": node_name(capability),
"type": node_type(capability),
}
)
if published
else None
)
for spec in cap_specs:
group = _group_for(spec, default_group)
if not model.has_group(group):
Expand Down Expand Up @@ -315,6 +323,14 @@ def _materialize(
prop_dict["round_to"] = spec.round_to
if not spec.retained:
prop_dict["retained"] = False
# Reuse likewise: Node.add_property replaces wholesale and
# republishes with force=True, so re-declaring an unchanged
# property would re-announce it. A datatype that actually changed
# is caught on the model side above, which raises.
existing_prop = node.get_property(spec.prop_id)
if existing_prop is not None:
homie_props[(capability, spec.prop_id)] = existing_prop
continue
homie_prop = node.add_property_from_dict(prop_dict)
bind_property_to_homie(model, group, spec.model_key, homie_prop)
# Inbound/control path for a settable property with a translator:
Expand All @@ -327,6 +343,31 @@ def _materialize(
return _Materialized(homie_props, declared, model_keys, created_groups)


def _needs_materializing(
device: Device,
model: GroupedPropertyDict,
grouped: dict,
node_id: Callable[[str], str],
default_group: Optional[str],
) -> bool:
"""True when any spec still has something to create on the device or the model.

Answered BEFORE the transition opens, because the question is whether to open
one at all: an init -> ready edge that announces nothing is a cost paid by
every controller on the bus.
"""
for capability, cap_specs in grouped.items():
for spec in cap_specs:
if model.get(_group_for(spec, default_group), spec.model_key) is None:
return True
if spec.internal_only:
continue
node = device.get_node(node_id(capability))
if node is None or node.get_property(spec.prop_id) is None:
return True
return False


def _seed(
model: GroupedPropertyDict,
declared: dict,
Expand Down Expand Up @@ -482,6 +523,10 @@ def __init__(
self._model_keys: dict = {}
self._created_groups: dict = {}
self._deferred: list = []
# The root is not a DeviceSpec, so its bookkeeping lives beside the
# per-spec maps rather than inside them. Nothing removes a root.
self._root_props: dict = {}
self._root_model_keys: list = []

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

def add_root_capabilities(self, specs: Iterable[PropertySpec], *, model_group: Optional[str] = None) -> dict:
"""Materialize capabilities onto the tree's ROOT device.

`add()` only ever creates children, so a root's own capabilities (an
enclosure's aggregate metering, its state, its control surfaces) had no
declarative expression and had to be hand-rolled 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 rather than
constructing anything. `model_group` defaults to the root's device id,
matching how `add()` keys a child's group. Idempotent, so a re-fired
lifecycle re-declares nothing.

Returns `{(capability, prop_id): homie.Property}` for the root, and the
map accumulates across calls, so `add_root_capabilities` twice returns
everything the root has.
"""
group = model_group or self._root.id()
built = _materialize(
self._root,
self._model,
specs,
node_type=self._node_type,
node_name=self._node_name,
node_id=self._node_id,
default_group=group,
)
_seed(self._model, built.declared, default_group=group)
self._root_props.update(built.homie_props)
self._root_model_keys.extend(built.model_keys)
return dict(self._root_props)

def root_capabilities(self) -> dict:
"""`{(capability, prop_id): homie.Property}` materialized onto the root so far."""
return dict(self._root_props)

def extend(self, spec: DeviceSpec, specs: Iterable[PropertySpec]) -> dict:
"""Give a device this builder already built additional capabilities.

A device's capability set is not always known when it is first published:
a storage system is commissioned and the enclosure gains shed and
forecast surfaces it did not have at boot. `add()` short-circuits an
already-built spec, so the builder modeled devices appearing and
disappearing but not a device GROWING.

Materializes inside one `state_transition()`, so the device announces
once, and folds the new model keys into the same bookkeeping `remove()`
uses. Idempotent: extending with a capability already present is a no-op
rather than a republish, because incremental lifecycles re-fire.

Raises `KeyError` for a spec that is not built. Use `add()` first; a
deferred device has no tree to extend.
"""
device = self._devices.get(spec)
if device is None:
raise KeyError(
f"{spec.device_class}: not built, so there is nothing to extend. "
"add() it first (a deferred device has no tree yet)."
)
group = spec.resolve_model_group(device.id())
built = _materialize(
device,
self._model,
specs,
node_type=self._node_type,
node_name=self._node_name,
node_id=self._node_id,
default_group=group,
)
_seed(self._model, built.declared, default_group=group)
self._homie_props[spec].update(built.homie_props)
self._model_keys[spec].extend(built.model_keys)
self._created_groups[spec].extend(built.created_groups)
return dict(self._homie_props[spec])

def device_for(self, spec: DeviceSpec) -> Optional[Device]:
"""The live `Device` for `spec`, or None if it is deferred or removed."""
return self._devices.get(spec)
Expand Down
Loading