fix: honour the documented mqtt_cfg=None default instead of raising by cayossarian · Pull Request #10 · electrification-bus/python-sdk

cayossarian · 2026-08-02T04:59:41Z

Fixes #9 — constructing a root Device with its own documented default raised
AttributeError, because __init__ called connect_broker() unconditionally for roots and
mqtt_cfg=None reached MqttClient.from_config(None).

The change

if parent is None:
    if mqtt_cfg is not None:
        self.connect_broker()

mqtt_cfg=None now means no transport: the tree composes $description, resolves ids and
topics, and holds property values, but never publishes. mqtt_cfg={} is untouched and still
connects on ebus-mqtt-client's defaults, so the only input whose behaviour changes is the
one that previously raised.

The child guard at :1227 is qualified to match. It treated any clientless root as an error,
conflating "never started / already stopped" with "transport-free by design" — only the first
is a mistake:

if parent is not None and parent.root().mqttc is None and parent.root()._mqtt_cfg is not None:

Both expressions carry a short comment, since the shapes are load-bearing: a truthiness test
would fold {} into the no-transport path, and dropping the _mqtt_cfg term would make every
transport-free tree single-node.

No new state

stop() already ends with root.mqttc = None (:1456), and every consumer path already
tolerates a missing client — publish, publish_empty, subscribe, get_mqtt_client,
start_mqtt_client, is_connected, stop. :1227 was the only place where a clientless tree
was fatal rather than quiet. This makes an existing state reachable by construction rather than
only via stop().

Tests

Six regression tests in TestDeviceWithoutTransport:

the declared default constructs (the reported crash)
mqtt_cfg={} still connects — guards the backward-compatibility claim above
children attach to a transport-free root, two levels deep
such a tree still composes $description, including parent/root and unit round-trip
set_value and stop() on it are no-ops rather than errors
the original guard still fires for a configured root that has lost its client

pytest: 462 passed. ruff check . and ruff format --check . clean under the pinned
0.15.21. (tests/test_json_format.py has 5 failures on my machine that reproduce identically
on an unmodified checkout — a missing jsonschema locally, unrelated to this change.)

Not included

Issue #9 notes that a transport-free tree emits ~1,593 WARNING lines for a 31-device model,
because a missing client is logged at WARNING in twelve places. Correct when you meant to
publish, noise when you did not — but it is a logging-policy call across Property, Node,
and Device, so it is deliberately not bundled here. Happy to send it separately if you want
it.

Device.__init__ called connect_broker() unconditionally for roots, so the annotated `mqtt_cfg: Optional[dict] = None` reached MqttClient.from_config(None) and died on None.get("host", ...). Constructing a root with its own default argument raised AttributeError. Skip the connect when mqtt_cfg is None, giving a device tree with no transport: it still composes $description, resolves ids and topics, and holds property values, but never publishes. mqtt_cfg={} keeps its existing meaning, so the only input whose behaviour changes is the one that previously raised. The child guard is qualified to match. It treated any clientless root as an error, conflating "never started / already stopped" with "transport-free by design"; only the former is a mistake. This adds no new state to the class — stop() already leaves root.mqttc as None, and every consumer path already tolerates it. Closes #9

dcj

Thanks, this is a clean fix for a real regression (the documented mqtt_cfg=None default raising), and the transport-free Device tree it unlocks is the producer-side twin of the 0.13.0 bring-your-own-transport seam. Verified locally: your 6 tests pass, full suite green (486), ruff clean. Merging into the 0.14.0 line; #8 and #11 tracked as follow-ups.

@cayossarian

…p flat controller) Bumps __version__ to 0.14.0 and cuts CHANGELOG [Unreleased] to [0.14.0]. Added: Property.set_format() (SDK-6do.2), examples/simple-tree-device (SDK-5le). Fixed: Device(mqtt_cfg=None) no longer raises and builds a transport-free tree (#9/#10, thanks @cayossarian). Removed: examples/simple-controller (flat tree-unaware discovery is the dead model; SDK-4zv tracks the tree-aware replacement). README documents transport-free construction. All additive; no signature or semantics change to the existing API. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

@cayossarian

* fix: publish a device's $state after its children in refresh_tree refresh_tree() walked the tree pre-order, publishing a device completely -- description, nodes, state -- before recursing. The root therefore announced $state=ready while the children its own $description names had published nothing at all. On a 37-device enclosure tree the root went ready at publish 1 and the last child at publish 73. This is the normal path, not an edge case: on_connect calls refresh_tree() for SDK-owned clients on both the initial connect and every reconnect. Homie 5 invites a controller to gate on the root's $state, and one that does proceeded against a tree whose children had not described themselves. After a broker restart a previously-healthy consumer re-read a root claiming ready with nothing under it. Description and nodes now publish first, then descendants, then this device's own state. The set of messages is unchanged and only the order differs: on the same tree, 74 publishes and 74 unique topics before and after, identical topic sets, with the root's $state moving from index 1 to index 73. The regression test fails on main and passes here. The existing suite passes either way -- no test asserted the old order, which is why it survived. * review fixups: ruff format, drop orphaned _publish_self, depth-2 order assertion - ruff format collapse on child_states (the only CI blocker; our pin is 0.15.21) - delete Device._publish_self(), which this change left with zero callers and a docstring still claiming it serves the reconnect cascade - assert the state-after-children rule at depth 2 in the existing three-level test: a fix that reordered only the root passed all 546 tests as written - reframe the comment and CHANGELOG entry as a producer-side narrowing rather than a guarantee consumers may build on, and add the LWT/atomic-commit reason (a lost root makes every child lost, so the final publish is one atomic flip) - credit @cayossarian in the CHANGELOG, matching #10/#12/#20 Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --------- Co-authored-by: Donald Clark Jackson <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

cayossarian mentioned this pull request Aug 2, 2026

Transport-free device tree logs a missing client at WARNING, thousands of times #11

Closed

dcj approved these changes Aug 2, 2026

View reviewed changes

dcj merged commit 3610130 into electrification-bus:main Aug 2, 2026
1 check passed

cayossarian mentioned this pull request Aug 2, 2026

feat: type Controller's mqttc by what the SDK calls on an injected client #12

Merged

Navigation Menu

Repository navigation

fix: honour the documented mqtt_cfg=None default instead of raising - #10

fix: honour the documented mqtt_cfg=None default instead of raising#10
dcj merged 1 commit into
electrification-bus:mainfrom
cayossarian:fix/device-mqtt-cfg-none-default

cayossarian commented Aug 2, 2026

Uh oh!

dcj left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cayossarian commented Aug 2, 2026

The change

No new state

Tests

Not included

Uh oh!

dcj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants