Skip to content

Commit 2e939dc

Browse files
dcjclaude
andauthored
fix: Node.delete_property() republishes $description (#41)
add_property() has always called publish_description(); its mirror never did. Deleting a property cleared the retained value topic but left the device in `ready` with a $description that still named the property, and nothing corrected it afterwards, so the broker held a self-contradicting device indefinitely. The two halves of the same API disagreed about whether mutating a node's property set is a structural change. It is. Deletions batch inside state_transition() exactly as additions do, so N deletions still collapse to one $description publish. Closes #35. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent abb53be commit 2e939dc

3 files changed

Lines changed: 64 additions & 2 deletions

File tree

CHANGELOG.md

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

99
- Tooling: the ruff lint selection is now declared explicitly (`select = ["E4", "E7", "E9", "F"]`) rather than inherited from ruff's defaults, and CI moves from ruff 0.15.21 to 0.16.1. Ruff 0.16 widened its default selection (UP, LOG, BLE, I, RUF and more) and began formatting Python code blocks embedded in Markdown, so an unchanged codebase reported 0 or 381 violations depending only on which ruff you happened to run, and four docs files showed phantom format diffs locally that CI never saw. Pinning the set decouples "what this project lints for" from "what version of ruff is installed"; `extend-exclude = ["*.md"]` keeps prose out of both check and format. Verified clean on both 0.16.1 and 0.15.21, with no source changes. Widening the rule set (the 381) is now a deliberate act rather than an upgrade side-effect. ([#39](https://github.com/electrification-bus/python-sdk/issues/39))
1010

11+
### Fixed
12+
13+
- `Node.delete_property()` now republishes `$description`, as its mirror `Node.add_property()` always has. Deleting a property cleared the retained value topic but left the device in `ready` with a `$description` that still named the property, and nothing corrected it afterwards, so the broker held a self-contradicting device indefinitely. The two halves of the same API disagreed about whether mutating a node's property set is a structural change; it is. Deletions batch inside `device.state_transition()` exactly as additions do, so N deletions still collapse to one `$description` publish. ([#35](https://github.com/electrification-bus/python-sdk/issues/35))
14+
1115
## [0.18.1] — 2026-08-07
1216

1317
### Added

src/ebus_sdk/homie.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,15 +1172,29 @@ def get_property(self, property_id: str) -> Optional[Property]:
11721172

11731173
def delete_property(self, property_id: str) -> bool:
11741174
"""
1175-
Remove property and clear its MQTT topic
1176-
Returns True if removed, False if not found
1175+
Remove property, clear its MQTT topic, and republish $description.
1176+
1177+
The mirror of add_property(): both mutate the node's property set, so
1178+
both must re-announce it. Without the republish the broker kept a device
1179+
in `ready` whose $description still named a property that no longer
1180+
existed, and nothing ever corrected it.
1181+
1182+
Batching several deletions inside `device.state_transition()` collapses
1183+
the republishes to one, exactly as it does for additions.
1184+
1185+
Returns True if removed, False if not found.
11771186
"""
11781187
if property_id not in self._properties:
11791188
logger.warning(f"reason=nodeDeletePropertyNotFound,nodeId={self._id},propertyId={property_id}")
11801189
return False
11811190
property = self._properties[property_id]
11821191
property.clear_value()
11831192
del self._properties[property_id]
1193+
# Delete from the dict BEFORE republishing, so the new $description
1194+
# reflects the removal (add_property() has the same ordering rule).
1195+
device = self.device()
1196+
if device:
1197+
device.publish_description()
11841198
logger.info(f"reason=nodeDeletedProperty,nodeId={self._id},propertyId={property_id}")
11851199
return True
11861200

tests/test_homie_device.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,50 @@ def test_delete_missing_property(self):
772772
n = Node(id="core")
773773
assert n.delete_property("missing") is False
774774

775+
def test_delete_property_republishes_description(self, mock_paho):
776+
"""The mirror of add_property() must also re-announce the property set.
777+
778+
Without the republish the broker keeps a device in `ready` whose
779+
$description still names a property that no longer exists, with nothing
780+
to correct it later.
781+
"""
782+
device, mock_client = _make_device(mock_paho, device_id="dev-1")
783+
with device.state_transition():
784+
node = device.add_node_from_dict({"id": "core", "type": "sensor"})
785+
node.add_property_from_dict({"id": "temp", "datatype": PropertyDatatype.FLOAT})
786+
node.add_property_from_dict({"id": "humidity", "datatype": PropertyDatatype.FLOAT})
787+
mock_client.publish.reset_mock()
788+
789+
assert node.delete_property("temp") is True
790+
791+
descriptions = [c[0][1] for c in mock_client.publish.call_args_list if c[0][0].endswith("/dev-1/$description")]
792+
assert descriptions, f"delete_property published no $description: {mock_client.publish.call_args_list}"
793+
published = json.loads(descriptions[-1])
794+
props = published["nodes"]["core"]["properties"]
795+
assert "temp" not in props, f"deleted property still in published $description: {props}"
796+
assert "humidity" in props, f"surviving property missing from $description: {props}"
797+
798+
def test_delete_property_batches_inside_state_transition(self, mock_paho):
799+
"""N deletions in one transition collapse to one $description publish.
800+
801+
Same guarantee add_node/add_property already give, so the two halves of
802+
the API stay symmetric under batching.
803+
"""
804+
device, mock_client = _make_device(mock_paho, device_id="dev-1")
805+
with device.state_transition():
806+
node = device.add_node_from_dict({"id": "core", "type": "sensor"})
807+
for pid in ("a", "b", "c"):
808+
node.add_property_from_dict({"id": pid, "datatype": PropertyDatatype.FLOAT})
809+
mock_client.publish.reset_mock()
810+
811+
with device.state_transition():
812+
for pid in ("a", "b", "c"):
813+
node.delete_property(pid)
814+
815+
descriptions = [c for c in mock_client.publish.call_args_list if c[0][0].endswith("/dev-1/$description")]
816+
assert len(descriptions) == 1, f"expected 1 consolidated $description, got {len(descriptions)}"
817+
assert json.loads(descriptions[0][0][1])["nodes"]["core"]["properties"] == {}
818+
775819

776820
class TestNodeDescription:
777821
def test_description(self):

0 commit comments

Comments
 (0)