diff --git a/CHANGELOG.md b/CHANGELOG.md index 24629a8..3bc5b4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable changes to `ebus-sdk` are recorded here. Format follows [Keep a Chan ### Added +- `DeviceSpec` and `DeviceTreeBuilder`: a device-level declaration and a tree-aware, incremental builder, for publishers whose shape is a tree rather than one device. `build_from_declarations` materializes exactly one device and creates the observable model itself keyed by capability, which fits the single-device proxy the SDK was first written for and cannot express what the eBus framework actually describes: a root whose circuits, lugs, MID and DERs are child devices, each with its own id, `$state`, `$description` and capability set. Three independent consumers had hand-rolled the same layer on top of `homie.Device(parent=...)`, which is evidence about the SDK rather than about them. Device class, id and parent are device-level facts, so they live on `DeviceSpec` rather than being repeated on every property of the device; `device_type` defaults to `energy.ebus.device.{device_class}`, which matters more than a convenience default because the SDK stores `Device.type` verbatim and validates nothing against a registry, so the derived form is the main guard against a type that ships misspelled. The builder accepts a `GroupedPropertyDict` it does not own and keys each device's group by device rather than capability, since two children both exposing `info` otherwise collide in the model while remaining perfectly distinct on the wire; a `PropertySpec` naming its own `model_group` still wins, so a consumer with an existing model keeps its keying. Late-bound ids are first class: `device_id` may be a callable returning `None` while an asynchronous identifier has not arrived, `add()` returns `None` and remembers the spec, and `resolve_deferred()` resolves a whole generation including children waiting behind a deferred parent. That is worth the machinery because a child published under a wrong-but-stable id leaves retained topics that outlive restarts and firmware updates. `add()` is idempotent because incremental lifecycles re-fire; `remove()` is depth-first, grandchild before parent, derived from the live tree rather than a caller-maintained ordering, so nothing ever observes an orphaned child, and it also deletes the model entries the builder added plus any group it created that is now empty. `on_created` carries per-child side effects so consumers do not post-process the returned tree. ([#57](https://github.com/electrification-bus/python-sdk/issues/57)) + - `PropertySpec` reaches property-level parity with the private declaration types that multi-device publishers were keeping instead of using it. Seven new fields, each defaulting to what the spec did before it existed, so no existing declaration set changes: `round_to` (decimal places applied on publish, which the property already supported and the declaration could not reach); `initial_value` (a seed applied through the model at build, overridden by the builder's `values=` argument); `retained=False` (an event property rather than a state); `internal_only` (the model tracks the value and the wire never sees it, so no Homie property is created and a capability whose specs are all internal gets no node); `conditionally_settable` (settability decided per instance at runtime, materialized not-settable so `$description` stays honest and no `/set` topic is opened on a property that would reject the command); and `source_id` / `model_group`, which split the observable-model identity from the wire identity. That last split is the load-bearing one: `capability` was simultaneously the Homie node id and the model group key, which is the same string only while one device is in play, and two child devices in a tree that both expose `info` collide in a shared model while remaining perfectly distinct on the wire. Two contradictions are now refused when the spec is constructed rather than when it publishes: `settable` with `conditionally_settable`, and `internal_only` with either. ([#58](https://github.com/electrification-bus/python-sdk/issues/58)) ### Fixed diff --git a/README.md b/README.md index 1c3ce67..8ec6993 100644 --- a/README.md +++ b/README.md @@ -310,7 +310,9 @@ Helpers that mirror the observable model onto the Homie tree, so you never hand- The declarative "schema" layer for proxies (see [`doc/building-a-proxy.md`](doc/building-a-proxy.md)): - **PropertySpec** - declares one eBus property (capability/node, id, datatype, unit, scale, settable, plus `round_to`, `initial_value`, `retained`, `internal_only`, `conditionally_settable`, and the `source_id` / `model_group` model-identity splits) -- **build_from_declarations** - materializes a set of specs into Homie nodes/properties, the observable model, and their bindings in one call +- **build_from_declarations** - materializes a set of specs into Homie nodes/properties, the observable model, and their bindings in one call (one device) +- **DeviceSpec** - declares one device in a tree (class, id or late-bound id resolver, parent, model group, `on_created` hook) +- **DeviceTreeBuilder** - materializes a set of `DeviceSpec`s into a parent/child tree over an externally-owned model: late-bound ids, idempotent `add()`, depth-first `remove()` - **resolve** / **specs_and_values** / **ResolvedProperty** - the two-tier mapping (hand-authored `mapping` first, generic `fallback` for the rest) that turns source fields into specs and scaled values ### topology.py diff --git a/doc/building-a-proxy.md b/doc/building-a-proxy.md index 14b05c2..dc74e84 100644 --- a/doc/building-a-proxy.md +++ b/doc/building-a-proxy.md @@ -68,6 +68,7 @@ homie.Property.set_value(...) ──► MQTT (ebus/5/// str: + """The observable-model group a spec's value lives in. + + An explicit `model_group` on the spec always wins: the caller is naming a + group in a model they own. Otherwise `default_group` applies, which is how a + device tree gives each device its own group; with neither, the group is the + capability, which is what a single-device build has always done. + """ + if spec.model_group is not None: + return spec.model_group + return default_group if default_group is not None else spec.capability + + +def _materialize( + device: Device, + model: GroupedPropertyDict, + specs: Iterable[PropertySpec], + *, + node_type: Callable[[str], str], + node_name: Callable[[str], str], + default_group: Optional[str] = None, +) -> _Materialized: + """Build one device's nodes, properties, model entries and bindings. + + The single materialization path, shared by `build_from_declarations` (one + device, model groups keyed by capability) and `DeviceTreeBuilder` (many + devices, model groups keyed per device). Runs inside one + `device.state_transition()`, so a device announces its structure once. + """ grouped: dict[str, list[PropertySpec]] = {} for spec in specs: grouped.setdefault(spec.capability, []).append(spec) homie_props: dict = {} declared: dict[tuple, PropertySpec] = {} + model_keys: list = [] + created_groups: list = [] with device.state_transition(): for capability, cap_specs in grouped.items(): # A node exists to carry published properties. If every spec on this @@ -204,12 +250,14 @@ def build_from_declarations( else None ) for spec in cap_specs: - group = spec.group_key + group = _group_for(spec, default_group) if not model.has_group(group): model.create_group(group) + created_groups.append(group) py_type = spec.python_type if spec.python_type is not None else python_type_for(spec.datatype) model.add_property(group, ObservableProperty(id=spec.model_key, type=py_type)) declared[(capability, spec.prop_id)] = spec + model_keys.append((group, spec.model_key)) # An entity_setter is the translator toward the entity, so it is # registered whenever one is given and the model can reach it. if spec.entity_setter is not None and (spec.settable or spec.internal_only): @@ -238,8 +286,22 @@ def build_from_declarations( homie_prop.set_set_callback(partial(model.set_entity, group, spec.model_key)) homie_props[(capability, spec.prop_id)] = homie_prop - # Seed declared initial values first, then let an explicit `values` entry - # override: the runtime map is the more specific statement of the two. + return _Materialized(homie_props, declared, model_keys, created_groups) + + +def _seed( + model: GroupedPropertyDict, + declared: dict, + values: Optional[dict] = None, + *, + default_group: Optional[str] = None, +) -> None: + """Seed values through the model, so they publish via the bindings. + + Declared `initial_value`s first, then any explicit `values` entry overriding + them: a caller passing a runtime map is being more specific than the static + declaration. Entries naming an undeclared property are ignored. + """ seed: dict[tuple, Any] = { key: spec.initial_value for key, spec in declared.items() if spec.initial_value is not None } @@ -247,8 +309,243 @@ def build_from_declarations( seed.update({key: value for key, value in values.items() if key in declared}) for key, value in seed.items(): spec = declared[key] - model.set_value(spec.group_key, spec.model_key, value) - return homie_props + model.set_value(_group_for(spec, default_group), spec.model_key, value) + + +@dataclass(frozen=True, eq=False) +class DeviceSpec: + """Declaration of one device in a tree: what it is, where it sits, what it carries. + + The device-level counterpart to `PropertySpec`. Device class, device id and + parent are device-level facts, so they live here rather than being repeated + on every property of the device. + + * `device_class` is the eBus class (`circuit`, `bess`, `distribution-enclosure`). + `device_type` defaults to `energy.ebus.device.{device_class}`, and that + default is the main guard a consumer gets: the SDK stores `Device.type` + verbatim and validates nothing against a registry, so a hand-written + misspelling ships silently. Prefer the default; override only for a type + outside the eBus namespace. + * `device_id` is either the id or a callable returning it, returning `None` + while it is still unknown. Child ids are often only known once an + asynchronous identifier arrives (a DER's serial number), and a child + published under a wrong-but-stable id leaves retained topics that outlive + restarts and firmware updates, so waiting is worth the deferral machinery. + Ids are used verbatim: run them through `sanitize_homie_id` yourself if + they come from a vendor. + * `parent` names the parent DEVICE SPEC, or `None` for a child of the + builder's root. The builder resolves it to a live `Device`. + * `model_group` is this device's group in the externally-owned model, + defaulting to its resolved device id. A `PropertySpec` that names its own + `model_group` still wins, so a consumer with an existing model keyed its + own way keeps that keying. + * `on_created` runs once, with the live `Device`, right after the device and + its properties exist. For per-child side effects (ACL emission, registry + entries) that would otherwise force the caller to post-process the tree. + + Compared by IDENTITY, not by value: a `DeviceSpec` stands for one device in + one tree, and two devices declared with identical fields are still two + devices. It is also what the builder keys its bookkeeping on. + """ + + device_class: str + specs: Sequence[PropertySpec] = () + device_id: Union[str, Callable[[], Optional[str]]] = "" + parent: Optional["DeviceSpec"] = None + model_group: Union[str, Callable[[], str], None] = None + device_type: Optional[str] = None + name: Optional[str] = None + on_created: Optional[Callable[[Device], None]] = None + + def resolve_device_id(self) -> Optional[str]: + """This device's id, or None while a late-bound id is still unresolved.""" + if callable(self.device_id): + return self.device_id() + return self.device_id or None + + def resolve_device_type(self) -> str: + """`device_type` if given, else the eBus type derived from `device_class`.""" + return self.device_type or f"energy.ebus.device.{self.device_class}" + + def resolve_model_group(self, device_id: str) -> str: + """This device's model group: `model_group` if given, else its device id.""" + if callable(self.model_group): + return self.model_group() + return self.model_group or device_id + + +class DeviceTreeBuilder: + """Materialize a set of `DeviceSpec`s into a live parent/child device tree. + + `build_from_declarations` builds exactly ONE device and creates the + observable model itself, keyed by capability. That fits a single-device + proxy and cannot express the shape the eBus framework actually describes: a + root device whose circuits, lugs, MID and DERs are child devices, each with + its own id, `$state`, `$description` and capability set. + + This builder covers that shape, and differs from the single-device one in + four ways that all follow from there being more than one device: + + 1. **The model is external.** It is passed in, never created, and each + device gets its own group (its id by default). Keying by capability + would collide the moment two children both expose `info`. + 2. **Ids can be late-bound.** `add()` returns `None` for a spec whose id is + not yet knowable and remembers it; `resolve_deferred()` retries, and a + deferred parent unblocking its deferred children resolves in one call. + 3. **It is incremental.** Devices come and go over a tree's life, so `add()` + is idempotent (lifecycles re-fire) and `remove()` tears one down. + 4. **Removal is depth-first**, grandchild before parent, derived from the + live tree rather than a caller-maintained ordering, so nothing ever + observes an orphaned child. + + Batching: each `add()` announces its own device, and the parent republishes + its `$description` to name the new child. To collapse a burst of adds into + one parent announcement, wrap them in the parent's `state_transition()`. + """ + + def __init__( + self, + root: Device, + model: GroupedPropertyDict, + *, + node_type: Callable[[str], str] = _default_node_type, + node_name: Callable[[str], str] = lambda capability: capability, + ) -> None: + self._root = root + self._model = model + self._node_type = node_type + self._node_name = node_name + self._devices: dict = {} + self._homie_props: dict = {} + self._model_keys: dict = {} + self._created_groups: dict = {} + self._deferred: list = [] + + def add(self, spec: DeviceSpec) -> Optional[Device]: + """Materialize `spec` as a device, or defer it while its id is unknown. + + Returns the live `Device`, or `None` when the spec (or an ancestor of + it) has no id yet, in which case it is remembered for + `resolve_deferred()`. Idempotent: re-adding a spec already built returns + the same `Device` without touching the tree, because incremental + lifecycles re-fire and a second add must not republish or duplicate. + """ + existing = self._devices.get(spec) + if existing is not None: + return existing + + if spec.parent is None: + parent_device: Optional[Device] = self._root + else: + parent_device = self._devices.get(spec.parent) or self.add(spec.parent) + if parent_device is None: + self._defer(spec) # the parent is itself waiting on an id + return None + + device_id = spec.resolve_device_id() + if device_id is None: + self._defer(spec) + return None + + device = Device( + device_id, + name=spec.name or device_id, + type=spec.resolve_device_type(), + parent=parent_device, + ) + group = spec.resolve_model_group(device_id) + built = _materialize( + device, + self._model, + spec.specs, + node_type=self._node_type, + node_name=self._node_name, + default_group=group, + ) + _seed(self._model, built.declared, default_group=group) + + self._devices[spec] = device + self._homie_props[spec] = built.homie_props + self._model_keys[spec] = built.model_keys + self._created_groups[spec] = built.created_groups + if spec in self._deferred: + self._deferred.remove(spec) + if spec.on_created is not None: + spec.on_created(device) + return device + + def resolve_deferred(self) -> list: + """Retry every deferred spec, returning the devices that could now be built. + + Repeats while progress is being made, so a parent whose id has just + arrived and the children waiting behind it resolve in one call rather + than one call per generation. + """ + built: list = [] + progress = True + while progress: + progress = False + for spec in list(self._deferred): + device = self.add(spec) + if device is not None: + built.append(device) + progress = True + return built + + def remove(self, spec: DeviceSpec) -> None: + """Tear down `spec`'s device and everything under it, grandchild first. + + `Device.delete()` walks the live tree depth-first, so the ordering comes + from the tree rather than from a list the caller has to keep correct. + The model entries this builder added for the removed devices are deleted + too, along with any group it created that is now empty; a group the + caller created, or one still in use, is left alone. + """ + device = self._devices.get(spec) + if device is None: + if spec in self._deferred: + self._deferred.remove(spec) # never built, just stop waiting for it + return + + doomed = {id(d) for d in _descendants(device)} + removed = [s for s, d in self._devices.items() if id(d) in doomed] + device.delete() + for gone in removed: + for group, model_key in self._model_keys.pop(gone, []): + self._model.delete_property(group, model_key) + for group in self._created_groups.pop(gone, []): + if not self._model.items(group): + self._model.delete_group(group) + self._devices.pop(gone, None) + self._homie_props.pop(gone, None) + + 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) + + def homie_properties(self, spec: DeviceSpec) -> dict: + """`{(capability, prop_id): homie.Property}` for `spec`, as the single-device builder returns. + + Empty for a spec that is not built. An `internal_only` property has no + Homie twin and so is absent, exactly as in `build_from_declarations`. + """ + return self._homie_props.get(spec, {}) + + def deferred(self) -> list: + """The specs waiting on an id, in the order they were first attempted.""" + return list(self._deferred) + + def _defer(self, spec: DeviceSpec) -> None: + if spec not in self._deferred: + self._deferred.append(spec) + + +def _descendants(device: Device) -> list: + """`device` and every device beneath it, parents before children.""" + found = [device] + for child in device.children(): + found.extend(_descendants(child)) + return found @dataclass(frozen=True) diff --git a/tests/test_device_tree.py b/tests/test_device_tree.py new file mode 100644 index 0000000..28d8366 --- /dev/null +++ b/tests/test_device_tree.py @@ -0,0 +1,303 @@ +"""Tests for DeviceSpec + DeviceTreeBuilder (GH #57). + +One test per acceptance criterion, plus the tree shape the criteria assume. +""" + +import pytest + +from ebus_sdk import ( + DeviceSpec, + DeviceTreeBuilder, + Device, + GroupedPropertyDict, + PropertyDatatype, + PropertySpec, + Unit, +) + +INFO = [PropertySpec("info", "serial-number", PropertyDatatype.STRING)] +METER = [PropertySpec("meter", "active-power", PropertyDatatype.FLOAT, Unit.WATT)] + + +@pytest.fixture +def root(mock_paho): + device = Device( + "enclosure-1", + type="energy.ebus.device.distribution-enclosure", + mqtt_cfg={"host": "localhost", "port": 1883}, + ) + device.start_mqtt_client() + return device + + +def _topics(mock_paho): + """Published topics, in call order.""" + return [str(c.args[0]) for c in mock_paho.publish.call_args_list if c.args] + + +# --- Criterion 1: the model is externally owned and keyed per device --------- + + +def test_two_children_sharing_a_capability_do_not_collide_in_the_model(root, mock_paho): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + + bess = builder.add(DeviceSpec("bess", INFO, device_id="bess-1")) + pv = builder.add(DeviceSpec("pv", INFO, device_id="pv-1")) + + # Same capability, same property id, two devices: distinct on the wire and + # now distinct in the model, because the group is the device. + model.set_value("bess-1", "serial-number", "B-1") + model.set_value("pv-1", "serial-number", "P-1") + assert model.value("bess-1", "serial-number") == "B-1" + assert model.value("pv-1", "serial-number") == "P-1" + assert bess.get_node("info").get_property("serial-number").value() == "B-1" + assert pv.get_node("info").get_property("serial-number").value() == "P-1" + + +def test_the_builder_never_creates_the_model(root): + model = GroupedPropertyDict() + model.create_group("preexisting") + builder = DeviceTreeBuilder(root, model) + builder.add(DeviceSpec("bess", INFO, device_id="bess-1")) + # It adds to the caller's model and leaves what was already there. + assert "preexisting" in model.groups() + assert "bess-1" in model.groups() + + +def test_a_property_spec_model_group_still_wins(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + spec = PropertySpec("info", "serial-number", PropertyDatatype.STRING, model_group="span-info") + builder.add(DeviceSpec("bess", [spec], device_id="bess-1")) + # The caller keyed their own model; the device default does not override it. + assert "span-info" in model.groups() + assert "bess-1" not in model.groups() + + +def test_device_model_group_can_be_named_or_computed(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + builder.add(DeviceSpec("bess", INFO, device_id="bess-1", model_group="battery")) + builder.add(DeviceSpec("pv", INFO, device_id="pv-1", model_group=lambda: "solar")) + assert "battery" in model.groups() + assert "solar" in model.groups() + + +# --- The tree shape the criteria assume ------------------------------------- + + +def test_builder_materializes_a_parent_child_grandchild_tree(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + + bess_spec = DeviceSpec("bess", INFO, device_id="bess-1") + mid_spec = DeviceSpec("mid", METER, device_id="bess-1-mid", parent=bess_spec) + bess = builder.add(bess_spec) + mid = builder.add(mid_spec) + + assert bess.parent() is root and mid.parent() is bess + assert mid.root() is root + # Homie derives the description's parent/root/children from the live tree. + assert mid.description()["root"] == "enclosure-1" + assert mid.description()["parent"] == "bess-1" + assert bess.description()["children"] == ["bess-1-mid"] + assert "bess-1" in root.description()["children"] + + +def test_device_type_defaults_from_device_class_and_can_be_overridden(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + default = builder.add(DeviceSpec("circuit", INFO, device_id="c-1")) + override = builder.add(DeviceSpec("circuit", INFO, device_id="c-2", device_type="vendor.thing")) + assert default.type() == "energy.ebus.device.circuit" + assert override.type() == "vendor.thing" + + +def test_adding_a_child_whose_parent_spec_is_not_built_builds_the_parent_first(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + bess_spec = DeviceSpec("bess", INFO, device_id="bess-1") + mid_spec = DeviceSpec("mid", METER, device_id="bess-1-mid", parent=bess_spec) + + mid = builder.add(mid_spec) # parent never added explicitly + assert mid is not None + assert builder.device_for(bess_spec) is not None + assert mid.parent() is builder.device_for(bess_spec) + + +# --- Criterion 2: late-bound device ids are first class ---------------------- + + +def test_a_spec_with_an_unresolved_id_defers_instead_of_publishing(root, mock_paho): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + serial = {} + + spec = DeviceSpec("bess", INFO, device_id=lambda: serial.get("value")) + assert builder.add(spec) is None + assert builder.deferred() == [spec] + assert builder.device_for(spec) is None + # Nothing was published under a placeholder id: a wrong-but-stable id leaves + # retained topics that outlive restarts. + assert not [t for t in _topics(mock_paho) if "/info/" in t] + + serial["value"] = "bess-serial-9" + built = builder.resolve_deferred() + assert [d.id() for d in built] == ["bess-serial-9"] + assert builder.deferred() == [] + assert builder.device_for(spec).id() == "bess-serial-9" + + +def test_a_deferred_parent_unblocks_its_deferred_children_in_one_call(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + serial = {} + + bess_spec = DeviceSpec("bess", INFO, device_id=lambda: serial.get("value")) + mid_spec = DeviceSpec( + "mid", METER, device_id=lambda: f"{serial['value']}-mid" if serial else None, parent=bess_spec + ) + assert builder.add(bess_spec) is None + assert builder.add(mid_spec) is None + assert set(builder.deferred()) == {bess_spec, mid_spec} + + serial["value"] = "tg-1" + built = builder.resolve_deferred() + # One call resolves the generation and everything waiting behind it. + assert sorted(d.id() for d in built) == ["tg-1", "tg-1-mid"] + assert builder.device_for(mid_spec).parent() is builder.device_for(bess_spec) + + +def test_resolving_deferred_when_nothing_can_be_built_is_a_noop(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + spec = DeviceSpec("bess", INFO, device_id=lambda: None) + builder.add(spec) + assert builder.resolve_deferred() == [] + assert builder.deferred() == [spec] + + +# --- Criterion 3: deletion is depth-first, grandchild before parent ---------- + + +def test_remove_tears_down_grandchild_before_parent(root, mock_paho): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + bess_spec = DeviceSpec("bess", INFO, device_id="bess-1") + mid_spec = DeviceSpec("mid", METER, device_id="bess-1-mid", parent=bess_spec) + builder.add(bess_spec) + builder.add(mid_spec) + + before = len(mock_paho.publish.call_args_list) + builder.remove(bess_spec) + after = _topics(mock_paho)[before:] + + # The transient matters: a settled-state comparison cannot catch an + # observer briefly seeing a child whose parent is already gone. + mid_state = after.index("ebus/5/bess-1-mid/$state") + bess_state = after.index("ebus/5/bess-1/$state") + assert mid_state < bess_state + + +def test_remove_drops_the_devices_and_their_model_entries(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + bess_spec = DeviceSpec("bess", INFO, device_id="bess-1") + mid_spec = DeviceSpec("mid", METER, device_id="bess-1-mid", parent=bess_spec) + builder.add(bess_spec) + builder.add(mid_spec) + assert "bess-1" in model.groups() and "bess-1-mid" in model.groups() + + builder.remove(bess_spec) + assert builder.device_for(bess_spec) is None + assert builder.device_for(mid_spec) is None # the grandchild went with it + assert "bess-1" not in model.groups() + assert "bess-1-mid" not in model.groups() + assert root.children_ids() == [] + + +def test_remove_leaves_a_group_the_builder_did_not_create(root): + model = GroupedPropertyDict() + model.create_group("shared") + builder = DeviceTreeBuilder(root, model) + spec = PropertySpec("info", "serial-number", PropertyDatatype.STRING, model_group="shared") + device_spec = DeviceSpec("bess", [spec], device_id="bess-1") + builder.add(device_spec) + + builder.remove(device_spec) + # The property this builder added is gone; the caller's group is not. + assert "shared" in model.groups() + assert model.get("shared", "serial-number") is None + + +def test_removing_a_deferred_spec_stops_waiting_for_it(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + spec = DeviceSpec("bess", INFO, device_id=lambda: None) + builder.add(spec) + builder.remove(spec) + assert builder.deferred() == [] + + +# --- Criterion 4: add() is idempotent --------------------------------------- + + +def test_add_is_idempotent(root, mock_paho): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + spec = DeviceSpec("bess", INFO, device_id="bess-1") + + first = builder.add(spec) + quiet = len(mock_paho.publish.call_args_list) + second = builder.add(spec) + + assert second is first + assert len(root.children()) == 1 + # A re-fired lifecycle must not republish or duplicate the device. + assert len(mock_paho.publish.call_args_list) == quiet + + +# --- Criterion 5: per-child side effects ------------------------------------ + + +def test_on_created_runs_once_with_the_live_device(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + seen = [] + spec = DeviceSpec("bess", INFO, device_id="bess-1", on_created=seen.append) + + device = builder.add(spec) + builder.add(spec) # idempotent: no second side effect + + assert seen == [device] + # The device is fully built when the hook runs, not a bare shell. + assert seen[0].get_node("info") is not None + + +# --- Accessors --------------------------------------------------------------- + + +def test_homie_properties_returns_the_twins_like_the_single_device_builder(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + specs = [ + PropertySpec("info", "serial-number", PropertyDatatype.STRING), + PropertySpec("info", "raw", PropertyDatatype.INTEGER, internal_only=True), + ] + spec = DeviceSpec("bess", specs, device_id="bess-1") + builder.add(spec) + + props = builder.homie_properties(spec) + assert set(props) == {("info", "serial-number")} # internal_only has no twin + assert builder.homie_properties(DeviceSpec("pv", INFO, device_id="pv-1")) == {} + + +def test_specs_are_compared_by_identity(root): + model = GroupedPropertyDict() + builder = DeviceTreeBuilder(root, model) + a = DeviceSpec("circuit", INFO, device_id="c-1") + b = DeviceSpec("circuit", INFO, device_id="c-1") + assert a != b # identical fields, still two declarations + builder.add(a) + assert builder.device_for(b) is None