diff --git a/CHANGELOG.md b/CHANGELOG.md index 255c2d2..d7b7439 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ All notable changes to `ebus-sdk` are recorded here. Format follows [Keep a Chan ### Added +- `Device.declare_lost()`: a way to announce deliberate death. `Device` modeled three teardowns and implemented one, so a producer that knew it was failing (a fatal error handler, a supervisor about to kill it, hardware that has gone away, a simulator acting the part) could only announce `disconnected`, which is a lie, or reach around the SDK to the concrete client; `DeviceState.LOST` was published nowhere in `homie.py` except inside the `will()` descriptor, and the will fires only on an *unclean* disconnect, which the clean disconnect `stop()` performs deliberately suppresses. It is TREE-level like `will()` and `stop()`, publishing the ROOT's `$state` (per the Homie 5 effective-state rule that covers every descendant in one publish) and emitting exactly the topic and payload `will()` describes, so the declared and will-driven paths cannot drift; to mark a single device lost, `set_state(DeviceState.LOST)` on that device remains the right call, and `declare_lost()` would blank the whole tree's liveness. The state move and the publish happen together, and the move is unconditional, because publishing a state the `Device` does not hold is exactly how a later `refresh_tree()` silently republishes `ready` over it. It returns whether `$state` actually moved, reusing `set_state`'s True-changed/False-already-there convention: on an injected transport that distinguishes "queued, now drain" from "already lost, nothing to wait for", and it is deliberately not a delivery signal, which it could not honestly be there. Owned clients flush; injected clients queue on the caller's loop, since `publish_and_flush` is owned-only and off the `MqttDeviceTransport` surface. Reported by [@cayossarian](https://github.com/cayossarian), whose async-transport drain sequence shaped the contract, and adopted by `ebus-panel-sim` in place of the reach-around that produced four separate downstream bugs. ([#46](https://github.com/electrification-bus/python-sdk/issues/46)) + +- `Device.stop(announce=False)`: tear down without publishing anything, leaving the retained `$state` exactly as it stands. The counterpart to `declare_lost()`: the default `announce=True` would overwrite a just-declared `lost` with `disconnected`, and the state move now lives inside the announcing branch so it cannot. Named `announce` rather than the `graceful` a downstream reached for, because "graceful" conflates the announcement with the bounded clean disconnect, and the teardown stays bounded and clean in both modes; only the announcement differs. Unpaired it leaves whatever was published last, typically a stale `ready`, and nothing will correct that, so the docstring and the README say so plainly. Adds nothing to the injected-transport surface: it only skips a publish, never adds a call. ([#46](https://github.com/electrification-bus/python-sdk/issues/46)) + - `Property.invalidate_publish_cache()`: forget what a property last published, for anything that deletes its retained value topic behind its back. The publish-on-change skip below assumes the broker still holds the payload the property last sent, so an operator wiping the broker, or a call to `Device.clear_retained_topic()` aimed at a value topic, leaves that assumption false and the next `set_value()` of the same value would be skipped against an empty topic. `Device.delete_all_from_mqtt()` now calls it on every property it clears; `clear_value()` and `Node.delete_property()` reset the memo themselves, so only the raw-topic paths need it. Distinct from `_ever_published`: this says "I no longer know what the broker holds", not "I have never published". ([#50](https://github.com/electrification-bus/python-sdk/issues/50)) ### Changed diff --git a/README.md b/README.md index 6680881..3694b5b 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ client.on_connect(device.refresh_tree) # re-announce the retained tree o client.connect() # host connects on its own loop ``` -`device.will()` returns the tree's Last Will descriptor and `device.refresh_tree()` republishes the whole tree; the `set_will` / `on_connect` / `connect` calls above are illustrative of your host's own MQTT API. Property values publish once the client is connected (the SDK gates on `is_connected()`, not on its own `start()`, which a caller-driven client never calls). `device.stop()` publishes a final retained `$state=disconnected` through the client and returns immediately, without flushing or closing it. `on_disconnect=` is inert for an injected client; register disconnect handling on your own client. +`device.will()` returns the tree's Last Will descriptor and `device.refresh_tree()` republishes the whole tree; the `set_will` / `on_connect` / `connect` calls above are illustrative of your host's own MQTT API. Property values publish once the client is connected (the SDK gates on `is_connected()`, not on its own `start()`, which a caller-driven client never calls). `device.stop()` publishes a final retained `$state=disconnected` through the client and returns immediately, without flushing or closing it; `device.stop(announce=False)` publishes nothing and leaves the retained `$state` as it stands, for a caller that published its own final state first (see `declare_lost()` below). `on_disconnect=` is inert for an injected client; register disconnect handling on your own client. For the inbound direction, if the tree has settable properties whose callbacks are async coroutines, pass `Device(async_loop=)`: inbound `/set` arrives on the transport's network thread, and this schedules the callback onto your loop (set once for the whole tree, not per property). A synchronous callback runs inline and needs no loop. @@ -95,10 +95,33 @@ That matters because ordering is a guarantee the SDK maintains on your behalf. A So if your transport hands work onward rather than publishing inline (an `MqttDeviceTransport` over a natively-async client, say, where `publish()` enqueues and returns), **drain a single queue in order** rather than dispatching each publish independently. The failure is invisible in testing: it holds by luck under a fast broker and breaks under a slow first publish. -`publish()` returning without having reached the wire is otherwise entirely legitimate: the protocol types every return as `object` precisely because the SDK discards them. It does mean teardown needs a drain point of your own, because `Device.stop()` publishes the final `$state` and returns without flushing. Close the queue before closing the client, or that last message is lost behind it. +`publish()` returning without having reached the wire is otherwise entirely legitimate: the protocol types every return as `object` precisely because the SDK discards them. It does mean teardown needs a drain point of your own, because `Device.stop()` publishes the final `$state` and returns without flushing. Close the queue before closing the client, or that last message is lost behind it. The same obligation applies to `declare_lost()`, which queues the `lost` and returns; its `True` return means "queued, now drain", `False` means "already lost, nothing to wait for". **A producer should own its MQTT connection.** The example above owns a *dedicated* client and connects it itself, so the SDK's Last Will (`$state=lost` on an ungraceful death) works normally: prefer this for any producer whose liveness matters. A *shared* connection owned by a host (Home Assistant is the archetype: one connection, up before your code loads, its single will already spent on the host's own) **cannot carry an eBus will**, because MQTT allows one will per connection. A producer publishing through such a connection therefore never signals ungraceful death: a crash leaves a stale retained `$state=ready`, and consumers render a dead device as alive. Reconnect is still handled (wire `refresh_tree()` to the host's reconnect callback and gate on `is_connected()`), but permanent death is not, and there is no portable substitute (a host that owns the connection also will not forward the MQTT 5 publish properties that would let `$state` expire). So do not publish a liveness-bearing device through a connection you do not own: if a host environment forbids a dedicated connection, run the producer as a **separate adapter** with its own connection rather than borrowing the host's. (The injected-client seam is still the right tool for the *consumer* role, `Controller(mqttc=...)`, which has no `$state` and no will to lose, and for tests.) +#### Announcing death: the three teardowns + +A device has three ways to stop, and they mean different things to a consumer rendering `$state` into availability: + +| Teardown | `$state` left retained | How | +| --- | --- | --- | +| Graceful shutdown | `disconnected` | `device.stop()` | +| Ungraceful death (crash, power loss) | `lost` | the Last Will, which fires only on an *unclean* disconnect | +| Deliberate death | `lost` | `device.declare_lost()` | + +The third is for a producer that knows it is failing: a fatal error handler, a supervisor about to kill it, hardware that has gone away, or a simulator acting the part. `disconnected` would tell consumers the shutdown was orderly and expected, which is a lie. + +```python +device.declare_lost() # root's $state=lost, published and (owned path) flushed +device.stop(announce=False) # tear down without overwriting it with `disconnected` +``` + +`declare_lost()` is **tree-level**, like `will()` and `stop()`: it publishes the *root's* `$state`, which per the Homie 5 effective-state rule makes every descendant lost too, and it publishes exactly the topic and payload `will()` describes so the two paths cannot drift. To mark one device lost (a proxy whose single upstream vanished), use `set_state(DeviceState.LOST)` on that device instead. + +It moves the state and publishes it together, and the move is unconditional: publishing a state the `Device` does not hold is how a later `refresh_tree()` silently republishes `ready` over it. It returns whether `$state` actually moved, the same convention as `set_state`: on an injected transport, a `True` from a connected tree is your cue to drain, and `False` means the root was already lost. It is not a delivery signal and cannot be one there. Publishing is skipped entirely when the broker is unreachable (the state still moves, and the next connect republishes it), so `True` does not by itself prove anything was queued. It does not stop the client. + +`stop(announce=False)` unpaired leaves whatever was published last, typically a stale `ready`, and nothing will correct it: the clean disconnect `stop()` performs suppresses the LWT. Neither call substitutes for the will, because a crashed process calls nothing. + #### Clearing a value vs. an empty-string value Homie 5 distinguishes two things that both look "empty" on the wire, and the SDK handles each automatically: @@ -147,7 +170,7 @@ Device(id='mid-1', type='...metering', parent=bess) panel.children()[0].delete() ``` -Children may have children of their own. A single Last Will registered on the root marks the entire tree `lost` if the publisher process dies — controllers compute effective state per the Homie 5 precedence table (see [`HOMIE_EFFECTIVE_STATE_TABLE`](src/ebus_sdk/homie.py)). +Children may have children of their own. A single Last Will registered on the root marks the entire tree `lost` if the publisher process dies, and `root.declare_lost()` publishes the same thing deliberately when the publisher knows it is dying — controllers compute effective state per the Homie 5 precedence table (see [`HOMIE_EFFECTIVE_STATE_TABLE`](src/ebus_sdk/homie.py)). `$description` republishes are minimized: structural changes made inside one `state_transition()` collapse to a single consolidated publish at exit (not one per `add_node`), and `publish_description()` is a no-op when the description content (ignoring its `version` timestamp) is unchanged — so a `state_transition()` that changes nothing structural does not re-emit the (potentially multi-KB) `$description`. A reconnect always republishes regardless, to restore retained state. Note this suppresses the redundant `$description` payload, not the `$state` `init`→`ready` edge of an empty transition. Property *values* are minimized the same way and with the same reconnect carve-out (see [Unchanged values are not republished](#unchanged-values-are-not-republished)). @@ -256,7 +279,7 @@ MQTT transport lives in the separate [`ebus-mqtt-client`](https://github.com/ele Core Homie convention implementation: -- **Device** - Represents a Homie device; pass `parent=` to build a child in a tree, or `on_disconnect=` for a push disconnect hook (`clean: bool`) +- **Device** - Represents a Homie device; pass `parent=` to build a child in a tree, or `on_disconnect=` for a push disconnect hook (`clean: bool`); `declare_lost()` announces deliberate death and `stop(announce=False)` tears down without announcing - **Node** - Groups related properties within a device - **Property** - Individual data points (sensors, controls) - **Controller** - Discovers and monitors Homie devices on a broker; navigates trees and computes effective state; `set_on_disconnect_callback` for push disconnect notification diff --git a/doc/building-a-proxy.md b/doc/building-a-proxy.md index c0b7a38..06a1107 100644 --- a/doc/building-a-proxy.md +++ b/doc/building-a-proxy.md @@ -147,13 +147,13 @@ A proxy is not one flat device. Per the eBus [`proxy.md`](https://github.com/ele - Publish **one child device per proxied device**, each `Device(id=..., type=..., parent=root)`. The proxied measurements live here. - Name each child `{proxier-id}-{proxied-id}` (the proxied id is the device's stable serial when it has one). Consumers correlate a proxy and a native publisher of the same physical device by `info/serial-number`, not by device id. -Children share the root's single MQTT connection automatically (that is what `parent=` does), and one Last Will on the root marks the whole tree `lost` if the process dies. See [Device Trees](../README.md#device-trees-parent--child) in the README. +Children share the root's single MQTT connection automatically (that is what `parent=` does), and one Last Will on the root marks the whole tree `lost` if the process dies; `root.declare_lost()` publishes exactly the same thing deliberately, when the bridge knows it is dying rather than crashing. See [Device Trees](../README.md#device-trees-parent--child) in the README. ## Lifecycle and state - **Batch structural changes.** Adding N nodes/properties inside one `with device.state_transition():` collapses to a single `$description` publish and one `init` to `ready` edge, instead of N. Always build a device's structure inside a transition. - **Connect before you publish.** `Device(..., mqtt_cfg=...)` connects asynchronously. If you build and publish before the broker connection is established, the first retained `$description` / `$state` the broker keeps can be a pre-connect snapshot until the SDK's on-connect refresh corrects it. Wait for `device.mqttc.is_connected()` before the initial build so the first retained state is correct. -- **Drive `$state` from availability.** When your upstream reports a device offline, set the child `DeviceState.LOST` (and `READY` when it returns). The root's Last Will covers process death. +- **Drive `$state` from availability.** When your upstream reports ONE device offline, `set_state(DeviceState.LOST)` on that child (and `READY` when it returns). When the whole bridge is dying, `root.declare_lost()` publishes the root's `$state=lost`, which per the Homie 5 effective-state rule covers every descendant in a single publish; follow it with `stop(announce=False)` so the teardown does not overwrite it with `disconnected`. Do not reach for `declare_lost()` for one dead upstream: it blanks the entire tree's liveness. The root's Last Will still covers process death, which neither call can, since a crashed process calls nothing. ## Settable / bidirectional properties (control back to the device) diff --git a/doc/consuming-a-homie-tree.md b/doc/consuming-a-homie-tree.md index 571d206..21a58a6 100644 --- a/doc/consuming-a-homie-tree.md +++ b/doc/consuming-a-homie-tree.md @@ -117,7 +117,7 @@ Read that re-arming as a warning. If you take the first call as a barrier and st Two things `is_tree_complete()` deliberately does not mean: -- **Not liveness.** A device counts as described once its `$description` has been parsed, whatever its `$state`. A declared child that is `lost` has still told you what it is. Use `get_effective_state()` for liveness. +- **Not liveness.** A device counts as described once its `$description` has been parsed, whatever its `$state`. A declared child that is `lost` has still told you what it is. Use `get_effective_state()` for liveness. Note `lost` is not always a crash: a producer that knows it is dying can publish it deliberately (`Device.declare_lost()`), so you may see it arrive from a publisher that is otherwise healthy and still connected. Your obligation is unchanged, which is the point: react to the state you are told, never to how you imagine it was produced. - **Not permanence.** It is true of the tree you can see right now. It says nothing about the tree a second from now. ## Checklist diff --git a/doc/ha-discovery-bridge.md b/doc/ha-discovery-bridge.md index d7ea21e..86398b2 100644 --- a/doc/ha-discovery-bridge.md +++ b/doc/ha-discovery-bridge.md @@ -140,9 +140,9 @@ Home Assistant's registry and recorder internals evolve between releases; the `( - Device discovered, or `$description` changed: publish (retained) `homeassistant/device//config`. - Device removed (empty retained `$state`): publish an empty retained payload to that config topic, so HA drops the device. -- Transient offline (`lost` / `disconnected`): handled by the availability template. The entity shows unavailable but is NOT removed. Set `clear_on_lost=True` if you would rather clear discovery on `lost` as well. +- Transient offline (`lost` / `disconnected`): handled by the availability template. The entity shows unavailable but is NOT removed. Set `clear_on_lost=True` if you would rather clear discovery on `lost` as well. Note `lost` may be a deliberate declaration (`Device.declare_lost()`) rather than a crash, so a `clear_on_lost=True` deployment drops the entity's discovery config on an intentional retirement too. That is usually what you want for a retirement and rarely what you want for a transient upstream fault; drive transient faults from the affected child's own `$state` instead. - `bridge.stop()` is a graceful shutdown: it restores the Controller's prior callbacks but LEAVES the discovery configs it published, so Home Assistant keeps the exported entities across a bridge restart (they read values directly from the `ebus/` topics and keep working while the bridge is down). The bridge is a context manager, so a `with HaDiscoveryBridge(controller) as bridge:` block guarantees `stop()` runs on exit. -- `bridge.clear_all()` is permanent retirement: it removes every discovery config the bridge published (empty retained payload), so Home Assistant drops those devices. Call it when the bridge and its exported devices are going away for good; `clear_on_stop=True` routes `stop()` through it. This mirrors the device lifecycle exactly: `stop()` is to the bridge what a graceful device shutdown (leave `$state` retained) is to a device, and `clear_all()` is what `Device.delete()` is. +- `bridge.clear_all()` is permanent retirement: it removes every discovery config the bridge published (empty retained payload), so Home Assistant drops those devices. Call it when the bridge and its exported devices are going away for good; `clear_on_stop=True` routes `stop()` through it. This mirrors the device lifecycle exactly: `stop()` is to the bridge what `Device.stop()` (announce `disconnected`, leave the retained data) is to a device, and `clear_all()` is what `Device.delete()` is. `HaDiscoveryBridge` chains onto (does not clobber) any callbacks already registered on the Controller: its handler runs first, then the pre-existing one. diff --git a/src/ebus_sdk/homie.py b/src/ebus_sdk/homie.py index 5d966b3..6df8720 100644 --- a/src/ebus_sdk/homie.py +++ b/src/ebus_sdk/homie.py @@ -1742,7 +1742,7 @@ def is_connected(self) -> bool: mqttc = self.root().mqttc return bool(mqttc and mqttc.is_connected()) - def stop(self, *, flush_timeout: float = 1.0, stop_timeout: float = 2.0) -> None: + def stop(self, *, announce: bool = True, flush_timeout: float = 1.0, stop_timeout: float = 2.0) -> None: """Gracefully and promptly tear down this device tree's MQTT connection. Publishes a final ``$state=disconnected`` for the root (best-effort) then @@ -1766,6 +1766,15 @@ def stop(self, *, flush_timeout: float = 1.0, stop_timeout: float = 2.0) -> None closing the client, so it never blocks the caller's loop. The caller stops and disconnects its own client. ``flush_timeout``/``stop_timeout`` apply to the owned path only. + + ``announce=False`` tears down without publishing anything, leaving the + retained ``$state`` exactly as it stands. Pair it with ``declare_lost()`` + for a producer that is dying rather than shutting down: that publishes + ``lost`` first, and the default ``announce=True`` would then overwrite it + with ``disconnected``. Unpaired it leaves whatever was published last, + typically a stale ``ready``, and nothing will correct that: the clean + disconnect on the owned path suppresses the LWT (see above). The teardown + itself stays bounded and clean in both modes; only the announcement differs. """ root = self.root() mqttc = root.mqttc @@ -1774,8 +1783,11 @@ def stop(self, *, flush_timeout: float = 1.0, stop_timeout: float = 2.0) -> None return # Best-effort graceful $state=disconnected. publish_and_flush is bounded # and returns False (never blocks/raises) when the broker is unreachable, - # so this can't stall shutdown. - if mqttc.is_connected(): + # so this can't stall shutdown. Note the state move lives inside this branch, + # so announce=False cannot overwrite a $state a caller just declared. + if not announce: + logger.info(f"reason=deviceStopSilent,id={root._id}") + elif mqttc.is_connected(): root._state = DeviceState.DISCONNECTED state_topic = f"{EBUS_HOMIE_DOMAIN}/{EBUS_HOMIE_VERSION_MAJOR}/{root._id}/$state" if root._owns_client and root._owned_client is not None: @@ -1798,6 +1810,73 @@ def stop(self, *, flush_timeout: float = 1.0, stop_timeout: float = 2.0) -> None root.mqttc = None root._owned_client = None + def declare_lost(self, *, flush_timeout: float = 1.0) -> bool: + """Declare this device tree dead: move the ROOT to ``$state=lost`` and publish it. + + The third teardown, alongside graceful shutdown (``stop()``, which announces + ``disconnected``) and ungraceful death (the Last Will, which fires only on an + UNCLEAN disconnect). This is for a producer that knows it is dying: a fatal + error handler, a supervisor about to kill it, hardware that has gone away, or + a simulator acting the part. Such a producer previously had to announce + ``disconnected``, which is a lie, or reach around the SDK to its client. + + TREE-level, like ``will()`` and ``stop()``. It publishes the ROOT's ``$state``, + which per the Homie 5 effective-state rule makes every descendant lost too, and + it publishes exactly the topic and payload ``will()`` describes so the declared + and will-driven paths cannot drift. To mark ONE device lost (a proxy whose + single upstream vanished), call ``set_state(DeviceState.LOST)`` on that device + instead: this method would blank the whole tree's liveness. + + The state move and the publish happen together, and the move is unconditional. + Publishing a state the Device does not hold is how a later ``refresh_tree()`` + silently republishes ``ready`` over it. For the same reason a reconnect after + this re-asserts ``lost`` until something sets the device back. + + Owned client: the publish is flushed, bounded by ``flush_timeout``. Injected + client (bring-your-own-transport): the message is handed to the caller's loop + with no flush, since ``publish_and_flush`` is owned-only and off the + ``MqttDeviceTransport`` surface, so draining it before closing the client is + the caller's obligation. Publishing is skipped when the broker is unreachable; + the state still moves, and the next connect republishes it. + + Returns True if ``$state`` actually moved to ``lost``, False if the root was + already lost, the same convention as ``set_state``. On an injected transport + True means "queued, now drain" and False means "nothing to wait for". It is + NOT a delivery signal and cannot be one there. + + Does NOT stop the client. Follow with ``stop(announce=False)`` to tear down + without overwriting the ``lost`` just published. + """ + root = self.root() + if root._transition_depth > 0: + # _end_state_transition() publishes READY on exit, which would land on top + # of this. Warn rather than refuse: the caller may be dying mid-transition. + logger.warning(f"reason=deviceDeclareLostInsideStateTransition,id={root._id}") + changed = root._state != DeviceState.LOST + root._state = DeviceState.LOST + mqttc = root.mqttc + if mqttc is None: + _log_missing_client(f"reason=deviceDeclareLostNoMqttClient,id={root._id}", by_design=root._transport_free()) + return changed + if not mqttc.is_connected(): + # An injected transport that queues while offline could deliver a stale + # `lost` long after recovery, which is worse than not sending it. + logger.info(f"reason=deviceDeclareLostBrokerUnreachable,id={root._id}") + return changed + state_topic = f"{EBUS_HOMIE_DOMAIN}/{EBUS_HOMIE_VERSION_MAJOR}/{root._id}/$state" + # Ownership decides, never isinstance: a caller may legitimately inject a real + # MqttClient (driven by asyncio_driver), and publish_and_flush/stop must not be + # called on a client the SDK does not own. + if root._owns_client and root._owned_client is not None: + flushed = root._owned_client.publish_and_flush( + state_topic, DeviceState.LOST.value, qos=root._qos, retain=True, timeout=flush_timeout + ) + logger.info(f"reason=deviceDeclareLostPublished,id={root._id},flushed={flushed}") + else: + mqttc.publish(state_topic, DeviceState.LOST.value, retain=True, qos=root._qos) + logger.info(f"reason=deviceDeclareLostPublishedInjected,id={root._id}") + return changed + def description(self) -> dict: """ Returns a dict of the $description attribute of the Device @@ -1941,9 +2020,11 @@ def delete_all_from_mqtt(self) -> None: * to permanently REMOVE a device, use delete(), which additionally clears the retained $state (the Homie removal signal) and detaches from the parent tree; - * for graceful SHUTDOWN, a client manages $state itself (set_state to - DISCONNECTED, or rely on the LWT publishing $state=lost), typically leaving - retained values in place so consumers recover state across a restart. + * for SHUTDOWN, $state is managed by the teardown itself: stop() publishes + DISCONNECTED, declare_lost() publishes LOST for a producer that knows it is + dying, and the LWT publishes LOST on an unclean disconnect. All three + typically leave retained values in place so consumers recover state across + a restart. Does NOT republish anything and does NOT publish node descriptions. """ @@ -2009,7 +2090,13 @@ def delete(self) -> None: On a root: clears all retained data for this device. (Does not stop the MQTT client — that's the caller's responsibility, after which the - LWT publish covers the whole tree.) + LWT publish covers the whole tree.) Note this REMOVES the device: an absent + retained ``$state`` is the Homie removal signal, so do not follow it with + ``declare_lost()``, which would resurrect the device on the broker as a bare + ``$state=lost`` with no ``$description``. (The root's will is a separate + matter: it is armed on the connection, not on the device, so it still fires + if the process then dies uncleanly.) To retire a tree as dead but still + present, use ``declare_lost()`` plus ``stop(announce=False)`` instead. While delete() is running, this device acts as if it were mid state_transition so descendants' delete()-triggered parent-flap @@ -2371,6 +2458,10 @@ def will(self) -> dict: client it is merely given after that client has connected. Children share the root's connection, so this always describes the root regardless of which device in the tree it is called on. + + ``declare_lost()`` publishes this same topic and payload explicitly, for a + producer that knows it is dying: the will fires only on an unclean + disconnect, and the clean disconnect ``stop()`` performs suppresses it. """ root = self.root() return { diff --git a/tests/test_homie_device.py b/tests/test_homie_device.py index 693f75b..21d77f8 100644 --- a/tests/test_homie_device.py +++ b/tests/test_homie_device.py @@ -2312,6 +2312,153 @@ def test_no_mqtt_client_is_noop(self, mock_paho): device.stop() # must not raise mock_client.stop.assert_not_called() + def test_announce_false_publishes_nothing_and_still_closes_the_owned_client(self, mock_paho): + device, mock_client = _make_device(mock_paho, device_id="dev-silent") + mock_client.is_connected.return_value = True + mock_client.publish.reset_mock() + + device.stop(announce=False) + + mock_client.publish_and_flush.assert_not_called() + mock_client.publish.assert_not_called() + # Teardown itself is unchanged: still bounded, still clean. + mock_client.stop.assert_called_once() + assert device.mqttc is None + + def test_announce_false_leaves_a_declared_lost_state_intact(self, mock_paho): + """The pairing #46 exists for: declare death, then tear down without lying about it.""" + device, mock_client = _make_device(mock_paho, device_id="dev-dying") + mock_client.is_connected.return_value = True + device.declare_lost() + mock_client.publish.reset_mock() + mock_client.publish_and_flush.reset_mock() + + device.stop(announce=False) + + assert device._state == DeviceState.LOST + assert DeviceState.DISCONNECTED.value not in _state_payloads_for(mock_client, "dev-dying") + for call in mock_client.publish_and_flush.call_args_list: + assert call.args[1] != DeviceState.DISCONNECTED.value + + +class TestDeviceDeclareLost: + """Device.declare_lost(): announce deliberate death, tree-level (#46). + + The third teardown. stop() announces `disconnected` and the will announces + `lost` only on an unclean disconnect, so a producer that knows it is dying + previously had to lie or reach around the SDK. + """ + + def test_publishes_lost_retained_and_flushes_on_the_owned_path(self, mock_paho): + device, mock_client = _make_device(mock_paho, device_id="dev-1") + mock_client.is_connected.return_value = True + + assert device.declare_lost() is True + + mock_client.publish_and_flush.assert_called_once() + args, kwargs = mock_client.publish_and_flush.call_args + assert args[0] == f"{EBUS_HOMIE_DOMAIN}/{EBUS_HOMIE_VERSION_MAJOR}/dev-1/$state" + assert args[1] == DeviceState.LOST.value == "lost" + # A plain str, not the StrEnum member: on Python < 3.11 (CI runs 3.10) + # str(DeviceState.LOST) is "DeviceState.LOST", so a transport that stringifies + # its payload would put that on the wire. `==` alone cannot see the difference. + assert type(args[1]) is str + assert kwargs["retain"] is True + assert kwargs["timeout"] == 1.0 + assert device._state == DeviceState.LOST + + def test_payload_and_topic_match_the_will(self, mock_paho): + """Anti-drift: the declared path and the LWT path must say the identical thing.""" + device, mock_client = _make_device(mock_paho, device_id="dev-1") + mock_client.is_connected.return_value = True + + device.declare_lost() + + topic, payload = mock_client.publish_and_flush.call_args.args[:2] + assert (topic, payload) == (device.will()["topic"], device.will()["payload"]) + + def test_returns_true_once_then_false_when_already_lost(self, mock_paho): + device, mock_client = _make_device(mock_paho, device_id="dev-1") + mock_client.is_connected.return_value = True + + assert device.declare_lost() is True # queued, now drain + assert device.declare_lost() is False # already lost, nothing to wait for + + def test_moves_state_so_a_later_refresh_republishes_lost(self, mock_paho): + """#46's downstream bug 4: publishing a state the Device does not hold lets + the next refresh_tree() silently republish `ready` over it.""" + device, mock_client = _make_device(mock_paho, device_id="dev-1") + mock_client.is_connected.return_value = True + device.declare_lost() + mock_client.publish.reset_mock() + + device.refresh_tree() + + assert _state_payloads_for(mock_client, "dev-1") == [DeviceState.LOST.value] + + def test_from_a_child_declares_the_root(self, mock_paho): + """Tree-level, like will() and stop(): a lost root makes every descendant lost.""" + root, mock_client = _make_device(mock_paho, device_id="panel-1") + child = Device(id="circuit-a", parent=root) + mock_client.is_connected.return_value = True + mock_client.publish_and_flush.reset_mock() + + assert child.declare_lost() is True + + assert mock_client.publish_and_flush.call_args.args[0].endswith("/panel-1/$state") + assert root._state == DeviceState.LOST + + def test_moves_state_even_when_the_broker_is_unreachable(self, mock_paho): + device, mock_client = _make_device(mock_paho, device_id="dev-1") + mock_client.is_connected.return_value = False + + assert device.declare_lost() is True + + mock_client.publish_and_flush.assert_not_called() + # The state still moved, so the next connect republishes it. + assert device._state == DeviceState.LOST + + def test_no_mqtt_client_still_moves_state(self, mock_paho): + device, _ = _make_device(mock_paho, device_id="dev-1") + device.mqttc = None # e.g. after stop() + + assert device.declare_lost() is True # must not raise + assert device._state == DeviceState.LOST + + def test_warns_inside_an_open_state_transition(self, mock_paho, caplog): + """_end_state_transition() publishes READY on exit, landing on top of this.""" + device, mock_client = _make_device(mock_paho, device_id="dev-1") + mock_client.is_connected.return_value = True + + with caplog.at_level(logging.WARNING, logger="homie"), device.state_transition(): + device.declare_lost() + + assert any("DeclareLostInsideStateTransition" in r.message for r in caplog.records) + + def test_warns_when_the_roots_transition_is_open_not_the_callers(self, mock_paho, caplog): + """The guard reads the ROOT's depth, not the caller's, because it is the root's + transition exit that publishes READY over the just-declared `lost`. A bridge + dying while it builds a child inside `with root.state_transition():` is exactly + that case, and _transition_depth is strictly per-device.""" + root, mock_client = _make_device(mock_paho, device_id="panel-1") + mock_client.is_connected.return_value = True + child = Device(id="circuit-a", parent=root) + + with caplog.at_level(logging.WARNING, logger="homie"), root.state_transition(): + assert child._transition_depth == 0 # only the ROOT's transition is open + child.declare_lost() + + assert any("DeclareLostInsideStateTransition" in r.message for r in caplog.records) + + def test_does_not_stop_the_client(self, mock_paho): + device, mock_client = _make_device(mock_paho, device_id="dev-1") + mock_client.is_connected.return_value = True + + device.declare_lost() + + mock_client.stop.assert_not_called() + assert device.mqttc is not None + class TestDeviceBYOTransport: """Bring-your-own-transport: inject a client into a root Device (#14). @@ -2366,6 +2513,39 @@ def test_stop_injected_broker_down_is_silent(self, mock_paho): client.stop.assert_not_called() assert device.mqttc is None + def test_declare_lost_injected_publishes_without_flush_or_close(self, mock_paho): + client = _mock_mqtt_client() + client.is_connected.return_value = True + device = Device(id="panel-1", mqttc=client) + + client.reset_mock() # ignore construction-time publishes + assert device.declare_lost() is True + + client.publish.assert_called_once() + topic, payload = client.publish.call_args.args + assert topic == f"{EBUS_HOMIE_DOMAIN}/{EBUS_HOMIE_VERSION_MAJOR}/panel-1/$state" + assert payload == DeviceState.LOST.value + assert type(payload) is str # not the StrEnum member; see TestDeviceDeclareLost + assert client.publish.call_args.kwargs["retain"] is True + # Ownership decides, not isinstance: an injected client can itself be a real + # MqttClient, so neither owned-only method may be reached (#46, bugs 2 and 3). + client.publish_and_flush.assert_not_called() + client.stop.assert_not_called() + assert device.mqttc is client # declare_lost() does not tear down + + def test_stop_announce_false_injected_publishes_nothing_and_closes_nothing(self, mock_paho): + client = _mock_mqtt_client() + client.is_connected.return_value = True + device = Device(id="panel-1", mqttc=client) + + client.reset_mock() + device.stop(announce=False) + + client.publish.assert_not_called() + client.publish_and_flush.assert_not_called() + client.stop.assert_not_called() + assert device.mqttc is None + def test_mqttc_and_mqtt_cfg_are_mutually_exclusive(self, mock_paho): with pytest.raises(ValueError, match="mqtt_cfg= and mqttc="): Device(id="panel-1", mqtt_cfg={"host": "x"}, mqttc=_mock_mqtt_client()) @@ -2445,6 +2625,43 @@ def is_connected(self): device = Device(id="panel-1", type="dev.test", mqttc=client) device.stop() # must not reach start()/stop() on a client that has neither + def test_declare_lost_and_silent_stop_stay_on_the_minimal_transport_surface(self): + """The teardown additions must not widen the injected surface (#46). + + A bare MagicMock answers any attribute, so an accidental publish_and_flush() + or stop() on an injected client passes silently there. This stub does not. + """ + from ebus_sdk import MqttDeviceTransport + + class Minimal: + is_running = True + + def __init__(self): + self.published = [] + + def publish(self, topic, data, qos=1, retain=False): + self.published.append((topic, data, retain)) + return None + + def subscribe(self, sub, param, qos=1): + return None + + def is_connected(self): + return True + + client = Minimal() + assert isinstance(client, MqttDeviceTransport) + device = Device(id="panel-1", type="dev.test", mqttc=client) + + assert device.declare_lost() is True # must not reach publish_and_flush() + device.stop(announce=False) # must not reach stop() + + assert ( + f"{EBUS_HOMIE_DOMAIN}/{EBUS_HOMIE_VERSION_MAJOR}/panel-1/$state", + DeviceState.LOST.value, + True, + ) in client.published + def test_owned_client_handle_is_none_when_injected(self): client = _mock_mqtt_client() device = Device(id="panel-1", mqttc=client) @@ -2674,6 +2891,7 @@ def test_transport_free_tree_reports_missing_client_at_debug(self, caplog): prop.set_subscribe() prop.start_mqtt_client() root.start_mqtt_client() + root.declare_lost() root.stop() assert self._no_client_records(caplog, logging.WARNING) == []