diff --git a/CLAUDE.md b/CLAUDE.md index 66589d3..b725f95 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,6 +47,8 @@ loop→Indigo writes go straight through `device_sync.apply_states` (thread-safe | `bridge_client.py` | Bridge-node client (outbound export): hello+attach handshake that fails closed on version skew, endpoint CRUD, fire-and-forget `set_state`, §5 event callbacks. Attach refusals are triaged (§1.1): `version_mismatch`/`mass_removal_refused` halt with a `halted_reason`, `endpoint_map_invalid` holds the socket open un-attached in a `recovery` state so the §3.11 rebuild stays reachable, anything else reconnects on the normal backoff | | `export_store.py` | The export allow-list (PRD-indigo-matter-export §5.1): `ExportEntry` (device id + role + name override + options) and an `RLock`'d store persisted as ONE JSON string in `pluginPrefs["matterExports"]`, schema-versioned. A blob it cannot parse is moved aside to `matterExports.corrupt` and the store starts empty — user config is never silently discarded | | `export_catalog.py` | Indigo device → eligible Matter roles, or an `Excluded(reason)` shown in the picker (PRD §5.2, XAC9). The loop guard (XNG3/XAC6) is `pluginId` and nothing else, checked before any type reasoning. Type dispatch walks the IOM **class-name chain**, not `isinstance` — the indigo module is a MagicMock under test | +| `export_handlers.py` | The **outbound** handler table, keyed by §4.2 **role** (the inbound registry is keyed by cluster; outbound there is no cluster, only a user-declared role) — `states_for` / `diff` / `dispatch` per role. E3 roles only: plug, on/off light, dimmable, colour-temp, extended colour. `handler_for` returns `None` for the E4 roles the §5.1 dialog can already put in the allow-list. Hue diffs carry a ±1° tolerance (Matter's 0–254 hue round-trips ±1°); saturation deliberately has none | +| `export_bridge.py` | The outbound engine: owns the `BridgeClient` and everything the Indigo callbacks *mean* for it. The client exists **only** while the allow-list is non-empty (XG5) and starts/stops on the dialog's empty↔non-empty transitions. The attach endpoint provider **re-runs `export_catalog.classify` on every attach** — the store is a past user declaration, not a guard — and skips-with-warning anything deleted/excluded/re-typed or carrying an E4 role (an unknown role fails the *whole* attach). State pushes are fire-and-forget onto the loop; `on_command` dispatches `indigo.*` from the loop thread, the same discipline `device_sync.apply_states` already uses | | `launch_agent.py` | Generic launchd LaunchAgent machinery (npm/npx/node resolution, plist authoring, applied-plist digest, orphan/EADDRINUSE reaping), driven by a frozen `AgentSpec` that carries one agent's identity. Extracted so the Matter **bridge node** can be a second agent without duplicating it (PRD-indigo-matter-export §4.2 / XOQ3) | | `server_process.py` | `ServerProcess` = the matter-server (controller) specialisation of `LaunchAgent`: its prefs, its argv, its pinned version. Gated by the `serverLocation` pref — the config asks "is matter-server on this Mac?"; `local` (turnkey default) manages it here on loopback, `remote` connects to a server elsewhere. `manageLaunchAgent`/host/port are derived from that in `startup` (see `plugin.py:server_location`) | | `commission_jobs.py` | Commissioning job state machine (API.md §3.2/§3.3) | diff --git a/docs/HANDOVER.md b/docs/HANDOVER.md index e971073..8e3dce8 100644 --- a/docs/HANDOVER.md +++ b/docs/HANDOVER.md @@ -499,11 +499,23 @@ Domio no longer commissions; it relays a **share code** (Apple Home is admin 1; **Export side (E1, `docs/BRIDGE_PROTOCOL.md`):** `bridge_protocol.py` the wire contract (envelope, §3 commands, §1.1 error codes, §4.2 roles, normalised `BridgeCommand`/`StatusReport`/`PairingReport`/`FabricInfo`; **no** rename firewall — we own both ends) · `bridge_client.py` the client (hello+attach handshake, **fails closed** on `protocolVersion` skew via `on_version_skew` + halt, endpoint CRUD, fire-and-forget `set_state`, §5 event callbacks) · `bridge-node/` the TypeScript node · `tests/fixtures/bridge_protocol/frames.json` the ONE golden-frame file both suites read (§7; `npm test` copies it into the TS build). -**Export side (E2, allow-list + UI):** `export_store.py` the allow-list model (`ExportEntry` = device id + §4.2 role + name override + options; `RLock`'d, one schema-versioned JSON string in `pluginPrefs["matterExports"]`, unparseable blobs moved aside to `matterExports.corrupt` rather than discarded) · `export_catalog.py` the PRD §5.2 mapping (eligible roles + safe default, or `Excluded(reason)`; loop guard is `pluginId` only — XNG3/XAC6 — and type dispatch walks the IOM class-name chain because `isinstance` is unusable against the MagicMock'd indigo module) · `MenuItems.xml` → **Manage Matter Exports…**, the UI-D dialog: no `` (so it gets a single Close button and the in-dialog buttons do the work), filter textfield + Apply-filter button + single-select device `menu` with `dynamicReload` (a multi-select `list` has NO CallbackMethod, so master-detail is impossible with one) + role menu + name/polarity fields + Add/Remove buttons + a readonly `exportStatus` textfield (Indigo labels cannot change at runtime) + a readonly summary list. `plugin.py` builds the store in `startup` and owns the callbacks. `bridge_client` is still **not** wired to the store — that is E3. +**Export side (E2, allow-list + UI):** `export_store.py` the allow-list model (`ExportEntry` = device id + §4.2 role + name override + options; `RLock`'d, one schema-versioned JSON string in `pluginPrefs["matterExports"]`, unparseable blobs moved aside to `matterExports.corrupt` rather than discarded) · `export_catalog.py` the PRD §5.2 mapping (eligible roles + safe default, or `Excluded(reason)`; loop guard is `pluginId` only — XNG3/XAC6 — and type dispatch walks the IOM class-name chain because `isinstance` is unusable against the MagicMock'd indigo module) · `MenuItems.xml` → **Manage Matter Exports…**, the UI-D dialog: no `` (so it gets a single Close button and the in-dialog buttons do the work), filter textfield + Apply-filter button + single-select device `menu` with `dynamicReload` (a multi-select `list` has NO CallbackMethod, so master-detail is impossible with one) + role menu + name/polarity fields + Add/Remove buttons + a readonly `exportStatus` textfield (Indigo labels cannot change at runtime) + a readonly summary list. `plugin.py` builds the store in `startup` and owns the callbacks. + +**Export side (E3b, the outbound pipeline):** `export_handlers.py` the per-**role** handler table — the mirror of `matter_handlers/`, keyed by §4.2 role because outbound there is no cluster, only a user declaration. Three methods per role: `states_for` (the §4.2 snapshot), `diff` (changed keys only; hue carries a ±1° tolerance because Matter's 0–254 hue round-trips ±1°, saturation deliberately does not because 0–100↔0–254 is exact) and `dispatch` (§4.2 command → `indigo.device.turnOn/turnOff`, `indigo.dimmer.setBrightness`, `indigo.dimmer.setColorLevels`). **E3 roles only**; `handler_for` answers `None` for the E4 roles the dialog can already write into the allow-list · `export_bridge.py` the engine: owns the `BridgeClient`, starts it only while the allow-list is non-empty (XG5) and stops it on the deliberate §3.1 `replace_all` attach when it empties; its endpoint provider re-runs `export_catalog.classify` on **every** attach and skips-with-warning anything deleted, excluded, re-typed, or carrying an E4 role (an unknown role fails the *whole* attach with `internal` — E3a); `on_command` resolves through the store and dispatches on the loop thread · `plugin.py` `deviceUpdated`/`deviceDeleted` + the `_exports_changed` seam. + +**E3b decisions worth not re-deriving.** +- **`subscribeToChanges` is conditional and one-way.** It subscribes to *every* device on the server ("a significant amount of traffic" per the IOM reference), so it is issued only once the allow-list is non-empty — at `startup`, or from the dialog the first time a user exports something (it is a plain request to the server, not a startup-only registration). There is **no unsubscribe in the canonical reference**, so it is never turned off again; `deviceUpdated`'s guard is a plain `frozenset` attribute on the Plugin (`self._exported_ids`, refreshed only by `_exports_changed`) so a non-exported device costs one hash lookup — no lock, no rebuild, no allocation. +- **`indigo.*` device commands from the loop thread are *unverified from the docs*.** Not "existing house discipline" — `device_sync.apply_states`'s precedent covers state *writes* on our own devices, which is a different claim from `indigo.device.turnOn` on somebody else's. It is left on the loop because `on_command` is a **single seam**: one method, one call site, so moving it to `run_in_executor` is a local change the day the loop is seen stalling. Bulk Indigo IPC does *not* get that latitude — `export_bridge.endpoint_specs` is one blocking `indigo.devices[id]` copy per exported device, and `bridge_client._gather_endpoints` runs it in an executor because this loop is shared with the inbound matter-server client (a slow Indigo server would otherwise stall live Matter updates behind an export reconcile). +- **Attach deadline scales:** `bridge_client.attach_timeout_for(n) = max(8.0, 2.0 + 0.15n)`. The node answers an attach only after its ~100ms-paced removals (§3.3), so ~80 endpoints spend 8s in pacing alone — the flat deadline would have timed out on exactly the databases that need export most. Zero protocol change. +- **…and `n` is the REMOVALS, not the endpoints sent.** For an ordinary reconnect the two track each other (the node's set came from our last attach), so `len(specs)` is a fine default. For `export_bridge._replace_all_then_stop` — the §3.1 un-export — they are opposites: zero sent, *everything* removed. It passes `attach_timeout_for()` explicitly, captured in `exports_changed` because the store is already empty by the time the un-export runs. Defaulting there gave a 60-device un-export the 8s floor: timeout mid-reconcile → a false "accessories may linger" warning → `close()` yanking the socket out from under a node that was working fine. +- **Colour is one-of, in both directions.** Matter's `colorMode` makes hue/sat and colour temperature mutually exclusive, and the node's push side picks hue/sat when a device reports both (`bridge-node/src/endpoints.ts` `colorPatch`). So `_set_color_temp` zeroes RGB alongside the white write and `_set_color` zeroes `whiteLevel` alongside the RGB write — each only when the device actually has that channel (`_number(dev, …) is not None`), because an all-zero RGB write to a CT-only driver is its own way to black out a room. A device with **no** white channel skips `setColorTemp` entirely (debug line) rather than inventing `whiteLevel=100` for a channel it does not have. **Needs the live E2E on a real RGBW driver** — this is the one change in the batch whose behaviour is a guess about drivers rather than about our own code. Two things to watch for on that run: (a) whether a driver treats an all-zero RGB write as "off" rather than "no colour"; (b) `colorPatch`'s mode arbitration — an RGBW device pushes `colorTempMireds` *and* `saturation` in the same snapshot, and the node sets `colorMode` from whichever it sees last, so a CT change still lands as `CurrentHueAndCurrentSaturation` with saturation 0. That is pre-existing (it happened with *stale* RGB before, which was worse), it is node-side, and it is deliberately **not** touched here. +- **Hue is omitted below saturation 20** (`export_handlers.SATURATION_HUE_FLOOR`). Indigo stores colour as three integer 0–100 channels, so hue is *recovered*, and near the grey axis those integers carry almost no angular information — measured worst-case round-trip error: 180° at sat 0, 30° at sat 1, 6° at sat 5, 1° at sat 20+. The ±1° `HUE_TOLERANCE_DEGREES` only covers the last of those, so every pastel change was pushing a `hue` nobody asked for; it converges (`setColorLevels` is absolute) but the Home colour wheel visibly jumps on the way. No protocol change — §3.4 state maps are partial by design, and the node's `colorPatch` already handles a saturation-only patch. +- **A role change in the dialog is remove+re-add** (§4.1 refuses a role change in place), and the accessory is new to every ecosystem afterwards — it loses its Home-app name and room. `exportStatus` says so before the user finds out. +- **`bridgeWsPort` is deliberately NOT in `PluginConfig.xml` yet.** PRD §5.5's Export section is a whole panel (enable/disable wholesale, both ports, the pairing readout) and belongs with E6/E7; a lone port field would ship an Export section that cannot start, stop or pair anything. `bridge_client` already reads the pref, so a hand-set `.indiPref` value is the escape hatch for E3's manually-run node. **E2 hardening (PR #122) — read before building E3.** The store persists *then* commits: `_commit` writes the pref, flushes through the injected `save_prefs` (`indigo.server.savePluginPrefs`), and only then adopts the new map in memory, rolling the pref back if the flush raises — so memory and prefs can never disagree. It holds a `prefs_getter` callable, not the `pluginPrefs` object, because Indigo may rebind that on a PluginConfig save. A load failure is carried in `store.load_error` and shown in the dialog instead of "Nothing is exported yet.", and `matterExports.corrupt` is **first-rescue-wins** (a second corruption never overwrites it). -> **The store is NOT the guard. E3 MUST re-classify at endpoint-build time.** The injected `entry_validator` re-runs the loop guard over entries restored from prefs, and `ExportEntry.from_dict` enforces the options shape per role (`invert` only on `windowCovering`) — but both run at *load*, against the database as it was then. A device can change type, gain our `pluginId`, or be replaced between load and endpoint build. Every endpoint E3 builds must call `export_catalog.classify` again and refuse anything that comes back `Excluded`; treating a store hit as proof of eligibility reintroduces exactly the loop (XNG3/XAC6) the guard exists to prevent. +> **The store is NOT the guard — re-classify at endpoint-build time.** (Honoured by `export_bridge._spec_for` since E3b; keep it that way.) The injected `entry_validator` re-runs the loop guard over entries restored from prefs, and `ExportEntry.from_dict` enforces the options shape per role (`invert` only on `windowCovering`) — but both run at *load*, against the database as it was then. A device can change type, gain our `pluginId`, or be replaced between load and endpoint build. Every endpoint E3 builds must call `export_catalog.classify` again and refuse anything that comes back `Excluded`; treating a store hit as proof of eligibility reintroduces exactly the loop (XNG3/XAC6) the guard exists to prevent. **Key invariants:** node-details has NO `endpoints` key (derive from flat `attributes`); `attribute_updated` data is `[node_id,"ep/cl/at",value]`; `node_removed` is a bare id; `server_info` is a bare connect frame (`sdk_version`/`fabric_id`). Setpoints/modes are attribute **writes**, not commands. diff --git a/indigo-matter.indigoPlugin/Contents/Info.plist b/indigo-matter.indigoPlugin/Contents/Info.plist index b61ee3e..a631867 100644 --- a/indigo-matter.indigoPlugin/Contents/Info.plist +++ b/indigo-matter.indigoPlugin/Contents/Info.plist @@ -20,7 +20,7 @@ IwsApiVersion 1.0.0 PluginVersion - 2026.7.27 + 2026.7.28 ServerApiVersion 3.6 diff --git a/indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_client.py b/indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_client.py index a86bf7c..532812b 100644 --- a/indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_client.py +++ b/indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_client.py @@ -47,8 +47,42 @@ #: derives a fresh passcode (crypto), and a factory reset rebuilds the node. LONG_TIMEOUT = 30.0 -#: The attach must complete inside the node's 10s unattached timeout (§2). +#: Floor for the attach deadline. The node's own 10s timer (§2) bounds how long +#: we may take to *send* an attach, not how long it may take to answer one, so +#: this deadline is free to exceed it — and has to (see below). ATTACH_TIMEOUT = 8.0 +#: Fixed cost of an attach: one round trip plus the node's own bookkeeping. +ATTACH_TIMEOUT_BASE = 2.0 +#: Marginal cost per endpoint in the desired set. An ``attach`` reconciles by +#: removing what is no longer wanted before it answers, and the node paces bulk +#: removals ~100ms apart (§3.3) — so the answer to a large attach arrives on the +#: far side of ``0.1s × removals`` (E3a, measured). 0.15 leaves headroom for the +#: creates in the same reconcile without making a hung node cost minutes. +ATTACH_TIMEOUT_PER_ENDPOINT = 0.15 + + +def attach_timeout_for(endpoint_count: int) -> float: + """The deadline for one ``attach`` carrying ``endpoint_count`` endpoints. + + THE formula (E3a decision, zero protocol change): ~80 endpoints lands just + over 8s of pacing alone, so a fixed deadline would time out exactly on the + databases that most need export to work. Everything smaller keeps the flat + :data:`ATTACH_TIMEOUT` floor, so the common case is unchanged. + + **The count is asymmetric, and callers have to know which one they mean.** + What the node spends its time on is the ~100ms-paced REMOVALS (§3.3), not + the creates — and an attach's removals are everything it holds that the + desired set omits. For an ordinary reconnect the two counts track each + other: the node's set came from our last attach, so drift is bounded by + whatever the user changed while the socket was down, and ``len(specs)`` is + a fine proxy. For the ONE caller that deliberately sends nothing — + ``export_bridge._replace_all_then_stop``, the §3.1 un-export — they are + opposites: zero sent, *everything* removed. That caller passes its own + ``timeout`` over the removal count; defaulting would hand a 60-device + un-export the 8s floor and time it out mid-reconcile. + """ + return max(ATTACH_TIMEOUT, + ATTACH_TIMEOUT_BASE + ATTACH_TIMEOUT_PER_ENDPOINT * max(0, int(endpoint_count))) #: Attach refusals that reconnecting cannot fix, with the remedy the user needs. #: Everything NOT listed here (``internal``, ``malformed_args``, …) is treated as @@ -177,10 +211,16 @@ async def _handshake(self, first: Any) -> None: "connected to bridge node (bridge %s, matter.js %s), attaching", hello.bridge_version, hello.matter_js_version, ) + # The desired set is read BEFORE the connection is declared usable. It + # is blocking Indigo IPC (see :meth:`_attach`) so it happens off the + # loop either way, but doing the hop here keeps ``connected`` meaning + # "the attach is on its way" rather than "we are still deciding what to + # send" — which is what every waiter on ``wait_connected`` assumes. + specs = await self._gather_endpoints() # Requests are legal from here on: attach is one. self._mark_connected() try: - status = await self._attach(None, replace_all=False, timeout=ATTACH_TIMEOUT, inline=True) + status = await self._attach(specs, replace_all=False, timeout=None, inline=True) except BridgeProtocolError as exc: self._handle_attach_refused(exc) return @@ -303,7 +343,7 @@ def _notify(self, callback: Optional[Callable], *args: Any) -> None: # Commands (§3) # ------------------------------------------------------------------ async def attach(self, endpoints: Optional[list] = None, *, replace_all: bool = False, - timeout: float = ATTACH_TIMEOUT) -> StatusReport: + timeout: Optional[float] = None) -> StatusReport: """Declare this client and deliver the full desired endpoint set (§3.1). Called by the handshake on every (re)connect, which is what makes a fresh @@ -314,19 +354,43 @@ async def attach(self, endpoints: Optional[list] = None, *, replace_all: bool = ``replace_all`` is the §3.1 opt-in required to empty a non-empty live set; the plugin passes it only on the deliberate allow-list-emptied path (PRD §7), so a stale client can never un-export everything by default. + + ``timeout`` defaults to :func:`attach_timeout_for` over the set actually + being sent — which is why it cannot be a default argument value. """ return await self._attach(endpoints, replace_all=replace_all, timeout=timeout, inline=False) + async def _gather_endpoints(self) -> list: + """Read the injected endpoint provider, OFF the loop. + + The provider is ``export_bridge.endpoint_specs``, which does one + synchronous ``indigo.devices[id]`` IPC copy PER exported device — 60 + exports is 60 blocking round trips to IndigoServer. This loop is shared + with the inbound matter-server client, so running that inline would + stall live Matter device updates behind an export reconcile every time + the bridge reconnects. + + The provider itself deliberately stays an ordinary blocking callable: + it is the injected seam's public shape, and making it a coroutine would + push the same problem into every implementation instead of solving it + once here. + """ + loop = asyncio.get_running_loop() + return await loop.run_in_executor(None, self._endpoint_provider) + async def _attach(self, endpoints: Optional[list], *, replace_all: bool, - timeout: float, inline: bool) -> StatusReport: + timeout: Optional[float], inline: bool) -> StatusReport: """The attach itself. ``inline`` selects how the response is waited for. The handshake issues its attach before the run loop's listen loop owns the socket, so there is no dispatcher to resolve a pending future — it pumps the socket itself. A caller re-attaching on a live connection goes through the normal correlated path. + """ - specs = self._endpoint_provider() if endpoints is None else endpoints + specs = await self._gather_endpoints() if endpoints is None else endpoints + if timeout is None: + timeout = attach_timeout_for(len(specs)) frame = self.proto.build_attach(self.plugin_version, specs, replace_all=replace_all) if inline: result = await self._handshake_request(frame, timeout, bridge_protocol.CMD_ATTACH) diff --git a/indigo-matter.indigoPlugin/Contents/Server Plugin/export_bridge.py b/indigo-matter.indigoPlugin/Contents/Server Plugin/export_bridge.py new file mode 100644 index 0000000..3b3175c --- /dev/null +++ b/indigo-matter.indigoPlugin/Contents/Server Plugin/export_bridge.py @@ -0,0 +1,680 @@ +"""The outbound export engine — Indigo device changes ⇄ the bridge node. + +``plugin.py`` stays lifecycle glue: it owns the Indigo callbacks and nothing +else. Everything those callbacks *mean* for export lives here — when the +:class:`bridge_client.BridgeClient` exists at all, what the desired endpoint set +is, how a device change becomes a ``set_state``, and how an ecosystem command +becomes an ``indigo.*`` call. A bare ``§N`` below is ``docs/BRIDGE_PROTOCOL.md``. + +Four disciplines worth knowing before editing: + +* **The client exists only while something is exported (XG5).** A fresh install + is inert: no allow-list, no socket, no log noise. The dialog transitions + empty↔non-empty mid-session, so :meth:`ExportBridge.exports_changed` starts + and stops the client rather than the plugin's ``startup`` deciding once. In E3 + the bridge *node* is started by hand — launchd is E7 — so "not running" is the + normal case and is reported once per streak, not once per retry. + +* **The store is not the guard; :func:`export_catalog.classify` is.** The + endpoint provider re-classifies every entry on every attach (the E2 handover's + standing requirement). An allow-list entry is a user *declaration*, made at + some point in the past against a device that has since been deleted, disabled, + reconfigured, or taken over by another plugin. Sending the node a spec built + from a stale declaration is how an accessory ends up controlling the wrong + thing. + +* **A role the plugin cannot bridge is skipped, loudly, not sent.** The §5.1 + dialog already offers ``doorLock``/``windowCovering``/the sensors as roles, so + the allow-list can hold E4 entries today. An unknown role fails the *whole* + ``attach`` on the node side (E3a), so one E4 export would silently un-export + every working one. Skip-with-warning keeps the blast radius at one device, and + the count is surfaced in the dialog's status line. + +* **Nothing here may block Indigo's thread.** ``deviceUpdated`` runs on Indigo's + callback thread for *every* device on the server. State pushes are submitted + to the loop and never awaited (§3.4); the result is logged by a done-callback + so a failed push is never silent. + +* **...and nothing here may block the loop either.** The reverse direction has + the same rule and one fewer guarantee: whether ``indigo.*`` device *commands* + are safe from a non-Indigo thread is unverified from the docs. Bulk Indigo IPC + is kept off the loop (:meth:`ExportBridge.endpoint_specs` runs in an executor), + and :meth:`ExportBridge.on_command` is deliberately the single remaining seam + — one method, so it can follow the day the loop is seen stalling. +""" +from __future__ import annotations + +from typing import Any, Callable, Optional + +import bridge_protocol +import export_catalog +import export_handlers +from bridge_client import TERMINAL_ATTACH_ERRORS, BridgeClient, attach_timeout_for +from bridge_protocol import EndpointSpec + +#: How many consecutive watchdog ticks a disconnected bridge client tolerates +#: before the log escalates from debug to a single warning. Ticks are ~15s, so +#: this is ~1 minute — the same shape (and the same reasoning) as the +#: matter-server counter in ``plugin._health_tick``. +DISCONNECT_WARN_TICKS = 4 + + +class ExportBridge: + """Owns the bridge client and everything the Indigo callbacks mean for it. + + :param store: the :class:`export_store.ExportStore` allow-list. + :param runtime: the :class:`async_runtime.AsyncRuntime` the client runs on. + :param logger: the plugin logger. + :param prefs_getter: callable returning the *current* prefs mapping — a + callable, not the mapping, for the same reason ``ExportStore`` takes one + (Indigo rebinds ``pluginPrefs`` when a config dialog is saved). + :param plugin_version: reported to the node in ``attach`` (§3.1). + :param plugin_id: this plugin's bundle id — the catalog's loop guard. + :param device_getter: ``id → indigo device or None``. Injected so this + module unit-tests without the Indigo runtime. + :param client_factory: builds the :class:`BridgeClient`; injected for tests. + """ + + # The seams ARE the API, exactly as BridgeClient's callbacks are. + # pylint: disable=too-many-arguments + def __init__(self, store, runtime, logger, prefs_getter: Callable[[], dict], *, + plugin_version: str = "unknown", + plugin_id: str = export_catalog.DEFAULT_PLUGIN_ID, + device_getter: Optional[Callable[[int], Any]] = None, + client_factory: Optional[Callable[..., BridgeClient]] = None) -> None: + self._store = store + self._runtime = runtime + self._logger = logger + self._prefs_getter = prefs_getter + self._plugin_version = plugin_version + self._plugin_id = plugin_id + self._device_getter = device_getter or _indigo_device + self._client_factory = client_factory or BridgeClient + + #: The live client, or ``None`` while nothing is exported (XG5). + self.client: Optional[BridgeClient] = None + #: Last reason each device was skipped by the provider, so a permanent + #: skip (an E4 role) logs once rather than on every reconnect. + self._skipped: dict[int, str] = {} + #: Consecutive watchdog ticks seen disconnected. + self._disconnect_ticks = 0 + #: Set once the "the node is not running" line has been said for this + #: outage, so a manually-started-later node does not fill the log first. + self._unreachable_reported = False + #: Same latch shape, one per condition that persists until a human or a + #: reconnect changes it. Each is cleared by :meth:`_on_attached`, which + #: is the only event that means "whatever that was, it is over". + #: Without them the watchdog says the same sentence every 15s forever, + #: and a halted bridge says it again for every device change in the + #: house — burying the one line that would explain the outage. + self._halted_reported = False + self._recovery_reported = False + #: The last attach-refusal code reported, so a *transient* refusal — + #: which reconnects on the normal backoff and refuses again — is said + #: once per streak rather than once per cycle. + self._refusal_reported: Optional[str] = None + #: Device ids whose ``device_updated`` is currently failing, so a stuck + #: device does not write a traceback per state change. + self._update_failed: set[int] = set() + #: The allow-list size as of the last :meth:`exports_changed`. The + #: un-export path needs it: by the time it runs the store is already + #: empty, and its attach deadline has to cover REMOVING that many + #: endpoints (see :func:`bridge_client.attach_timeout_for`). + self._last_export_count = len(store) + + # ------------------------------------------------------------------ + # Lifecycle + # ------------------------------------------------------------------ + @property + def active(self) -> bool: + """True while a client exists (whether or not it is connected).""" + return self.client is not None + + def start(self) -> None: + """Create and run the client. Idempotent.""" + if self.client is not None: + return + self.client = self._client_factory( + self._logger, self._prefs_getter(), + plugin_version=self._plugin_version, + endpoint_provider=self.endpoint_specs, + on_command=self.on_command, + on_attached=self._on_attached, + on_attach_refused=self._on_attach_refused, + on_version_skew=self._on_version_skew, + on_drift_detected=self._on_drift_detected, + on_repeated_failure=self._on_unreachable, + ) + self._unreachable_reported = False + self._disconnect_ticks = 0 + self._fire(self.client.run(), "bridge client run loop", + lost="nothing will be exported until the plugin is reloaded") + self._logger.info( + "Matter export: connecting to the bridge node (%d device(s) exported)", + len(self._store)) + + def stop(self, timeout: float = 4.0) -> None: + """Close the client. Idempotent; never raises at shutdown.""" + client, self.client = self.client, None + if client is None: + return + try: + self._runtime.submit(client.close()).result(timeout=timeout) + except Exception as exc: # pylint: disable=broad-except + self._logger.debug("bridge client close error: %s", exc) + + def exports_changed(self) -> None: + """The allow-list changed — start or stop the client to match (XG5). + + Called by every path that mutates the store. Incremental endpoint + updates are the caller's job (:meth:`upsert`/:meth:`remove`); this is + only the empty↔non-empty transition. + """ + self._skipped.clear() + count = len(self._store) + # Captured BEFORE it is overwritten: the un-export below has to size its + # deadline over what the node is about to remove, and the store that + # would have told it is already empty by the time we get here. + removing, self._last_export_count = self._last_export_count, count + if count: + self.start() + elif self.client is not None: + # PRD §7 "allow-list emptied": endpoints go, pairings stay. The node + # needs the §3.1 opt-in for that, so it is a deliberate attach + # rather than a disconnect — and only THEN do we close. + self._replace_all_then_stop(removing) + + def _replace_all_then_stop(self, removing: int) -> None: + """Un-export everything with the §3.1 intent, then drop the client. + + The attach and the close are one coroutine rather than two awaited + steps, for two reasons: closing the socket before the attach is written + would lose the un-export entirely, and *waiting* for the attach would + block whichever Indigo thread emptied the list — which can be the + device-delete callback, not just a menu click. + + ``removing`` is the size of the allow-list *before* it was emptied, and + it — not the empty list being sent — is what the deadline is built from. + The node answers an attach only after its ~100ms-paced removals (§3.3), + so this one request costs `0.1s × every endpoint it holds`. Letting the + client default from ``len([])`` gave it the 8s floor, which a set of + much over 60 blows straight through: the attach "fails" on a timeout + while the node is still working, the user is told accessories "may + linger", and then ``close()`` pulls the socket out mid-reconcile — the + one outcome the warning was describing. The count is an upper bound (a + skipped export was never sent, so is not there to remove), which is the + safe direction for a deadline. + """ + client = self.client + if client is None: + return + self.client = None # inert immediately; the close is in flight + self._logger.info("Matter export: allow-list is now empty — removing every " + "exported accessory (pairings are kept)") + + async def _un_export() -> None: + try: + await client.attach([], replace_all=True, + timeout=attach_timeout_for(removing)) + except Exception as exc: # pylint: disable=broad-except + self._logger.warning( + "Matter export: could not tell the bridge node the export list is empty " + "(%s). Accessories may linger in paired ecosystems until it restarts.", exc) + finally: + # The socket must be released whatever happened above — + # including a CancelledError at shutdown, which is a + # BaseException and so walks straight past the handler. + await client.close() + + self._fire(_un_export(), "un-exporting everything", + lost="exported accessories will linger in paired ecosystems") + + # ------------------------------------------------------------------ + # The endpoint provider (§3.1 attach reconcile source) + # ------------------------------------------------------------------ + def endpoint_specs(self) -> list: + """Build the desired endpoint set from the allow-list, re-classified. + + Read fresh on every (re)connect, never cached: ``attach`` is a full + reconcile and the allow-list may have changed while the socket was down. + + **Blocking, and called off the loop on purpose.** Each entry costs one + synchronous ``indigo.devices[id]`` IPC copy, so a 60-device allow-list + is 60 round trips to IndigoServer. ``bridge_client._attach`` runs this + in an executor for that reason — the loop it would otherwise occupy is + shared with the inbound matter-server client, and a slow Indigo server + would stall live Matter device updates behind an export reconcile. + """ + specs = [] + for entry in self._store.all(): + spec = self._spec_for(entry) + if spec is not None: + specs.append(spec) + return specs + + def _spec_for(self, entry) -> Optional[EndpointSpec]: + """One §4.1 ``EndpointSpec``, or ``None`` with a warning.""" + device_id = entry.indigo_device_id + dev = self._device_getter(device_id) + if dev is None: + return self._skip(device_id, "the Indigo device no longer exists") + verdict = export_catalog.classify(dev, self._plugin_id) + if isinstance(verdict, export_catalog.Excluded): + return self._skip(device_id, f"it is no longer exportable: {verdict.reason}") + if entry.role not in verdict.eligible_roles: + return self._skip(device_id, f"it no longer offers the role {entry.role!r} " + f"(now: {', '.join(verdict.eligible_roles)})") + handler = export_handlers.handler_for(entry.role) + if handler is None: + return self._skip(device_id, f"the role {entry.role!r} cannot be bridged yet — " + "sensors, locks, coverings and thermostats land in E4") + try: + states = handler.states_for(dev) + except Exception as exc: # pylint: disable=broad-except + self._logger.exception(exc) + # The exception TEXT is deliberately not part of the dedupe key: a + # message carrying a timestamp, an address or an attempt counter + # differs on every attach and would defeat the latch entirely, + # turning a permanently-broken device into a warning per reconnect. + return self._skip(device_id, "its state could not be read", detail=str(exc)) + self._skipped.pop(device_id, None) + return EndpointSpec( + indigo_device_id=device_id, + role=entry.role, + label=entry.label_for(str(getattr(dev, "name", "") or "")), + reachable=reachable_of(dev), + states=states, + options=dict(entry.options), + ) + + def _skip(self, device_id: int, why: str, detail: str = "") -> None: + """Warn once per reason, then keep quiet — the provider runs per connect. + + ``why`` is the dedupe key and must be stable for a stable cause; + ``detail`` is free-form context that goes in the line but never in the + key (see the ``states_for`` call site for why that distinction exists). + """ + if self._skipped.get(device_id) != why: + self._skipped[device_id] = why + self._logger.warning( + "Matter export: device %s is in the export list but will NOT be bridged — %s%s.", + device_id, why, f" ({detail})" if detail else "") + + # ------------------------------------------------------------------ + # Indigo → node + # ------------------------------------------------------------------ + def device_updated(self, orig_dev: Any, new_dev: Any) -> None: + """Push what changed about an **already-known-exported** device. + + The caller has already established that this device is in the allow-list + — that check is a set lookup on Indigo's thread and must stay there. + """ + entry = self._store.get(new_dev.id) + if entry is None: # removed between the check and here + return + handler = export_handlers.handler_for(entry.role) + if handler is None: + return # already warned by the provider + client = self._live_client("the state update", new_dev.id) + if client is None: + return + # Order matters only in that frames are applied in receipt order (§1): + # identity first, then availability, then state. + if entry.name_override is None and orig_dev.name != new_dev.name: + self.upsert(new_dev.id) + elif reachable_of(orig_dev) != reachable_of(new_dev): + # An upsert already carries `reachable`, so only send the split + # §3.5 command when we are not sending a whole spec anyway. + self._fire(client.set_reachable(new_dev.id, reachable_of(new_dev)), + f"set_reachable dev {new_dev.id}") + try: + states = handler.diff(orig_dev, new_dev) + except Exception as exc: # pylint: disable=broad-except + # Once per device per streak: this fires on every change of an + # exported device, so a lamp on a dimmer ramp would otherwise write + # one traceback per step — and a traceback with no device in it is + # not a lead anyway. + if new_dev.id not in self._update_failed: + self._update_failed.add(new_dev.id) + self._logger.error( + "Matter export: could not work out what changed about %s (id %s, exported " + "as %s) — %s. Its accessory will show stale state until this clears.", + getattr(new_dev, "name", ""), new_dev.id, entry.role, exc) + self._logger.exception(exc) + return + self._update_failed.discard(new_dev.id) + if states: + self._fire(client.set_state(new_dev.id, states), f"set_state dev {new_dev.id}") + + def _live_client(self, what: str, device_id: int) -> Optional[BridgeClient]: + """The client, but only while it can actually take an endpoint command. + + An incremental CRUD frame sent before ``attach`` completes is refused + with ``not_attached`` (§1.1) — and would be pointless anyway, because + the attach that is about to happen carries the full desired set and + reconciles it (§3.1). So "not attached yet" is a no-op, not an error. + + Two of the three ways to be un-attached are NOT that, though, and both + used to leave through this same silent ``return``: + + * **halted** — no reconnect is coming and no attach will reconcile + anything, so the ecosystem shows stale state until a human acts; + * **recovery** — the node is serving nothing at all until its + endpoint-number map is rebuilt (§1.1). + + ``BridgeClient._log_dropped_state_push`` says exactly this, loudly, and + is unreachable from here: the gate happens before ``set_state`` is ever + called. So the message is replicated rather than routed through — the + client's version cannot name the operation, and this one can. + """ + client = self.client + if client is None: + self._logger.debug( + "Matter export: no bridge client; dropping %s for device %s", what, device_id) + return None + if client.attached: + return client + if client.halted: + self._logger.debug("Matter export: bridge client halted; dropping %s for device %s", + what, device_id) + if not self._halted_reported: + self._halted_reported = True + self._logger.warning( + "Matter export: the bridge client is HALTED (%s) — device %s and everything " + "after it is NOT reaching any ecosystem, and nothing will retry on its own.", + client.halted_reason or "no reason recorded", device_id) + elif client.recovery: + self._logger.debug("Matter export: bridge in recovery; dropping %s for device %s", + what, device_id) + if not self._recovery_reported: + self._recovery_reported = True + self._logger.warning( + "Matter export: the bridge node is awaiting an endpoint-map rebuild — " + "device %s and everything after it is NOT reaching any ecosystem.", device_id) + else: + self._logger.debug( + "Matter export: bridge node not attached; dropping %s for device %s " + "(the next attach reconciles it)", what, device_id) + return None + + def upsert(self, device_id: int) -> None: + """(Re)send one endpoint's full spec (§3.2). Fire-and-forget.""" + client = self._live_client("upsert_endpoint", device_id) + if client is None: + return + entry = self._store.get(device_id) + if entry is None: + return + spec = self._spec_for(entry) + if spec is None: + return + self._fire(client.upsert_endpoint(spec), f"upsert_endpoint dev {device_id}") + + def remove(self, device_id: int) -> None: + """Drop one endpoint (§3.3). Fire-and-forget; idempotent on the node.""" + self._skipped.pop(device_id, None) + self._update_failed.discard(device_id) + client = self._live_client("remove_endpoint", device_id) + if client is None: + return + self._fire(client.remove_endpoint(device_id), f"remove_endpoint dev {device_id}") + + def replace(self, device_id: int) -> None: + """Re-create one endpoint, because its **role** changed. + + §4.1 rejects a role change on an existing endpoint (``role_change``) — + ecosystems cache the Matter device type per endpoint — so the only way + through is remove-then-add. The accessory is genuinely new to every + paired ecosystem afterwards: it loses its name and room assignment + there, which is why the dialog says so out loud rather than letting the + user discover it in the Home app. + """ + client = self._live_client("the role change", device_id) + if client is None: + return + + async def _recreate() -> None: + await client.remove_endpoint(device_id) + entry = self._store.get(device_id) + if entry is None: + return + spec = self._spec_for(entry) + if spec is not None: + await client.upsert_endpoint(spec) + + self._fire(_recreate(), f"role change for dev {device_id}") + + # ------------------------------------------------------------------ + # Node → Indigo (§5 command events; runs on the loop thread) + # ------------------------------------------------------------------ + def on_command(self, command: bridge_protocol.BridgeCommand) -> None: + """Apply one ecosystem-originated action to its Indigo device. + + Called from the client's frame loop, i.e. on the asyncio thread. + ``indigo.*`` device commands being safe to issue from a non-Indigo + thread is **unverified from the docs** — ``device_sync.apply_states``'s + precedent covers state *writes* on our own devices, which is not the + same claim. It is kept here rather than hedged because this is the + single seam: one method, one call, so moving it to ``run_in_executor`` + is a local change the day the loop is seen stalling on Indigo IPC. + """ + device_id = command.indigo_device_id + entry = self._store.get(device_id) + if entry is None: + # PRD §7 race row: the endpoint outlived the allow-list entry. + self._logger.warning( + "Matter export: the bridge node sent %r for Indigo device %s, which is not " + "exported — ignoring. The accessory should disappear at the next reconnect.", + command.command, device_id) + return + handler = export_handlers.handler_for(entry.role) + if handler is None: + self._logger.warning( + "Matter export: %r arrived for device %s exported as %s, a role this version " + "cannot bridge — ignoring.", command.command, device_id, entry.role) + return + dev = self._device_getter(device_id) + if dev is None: + self._logger.warning( + "Matter export: %r arrived for device %s, which no longer exists in Indigo — " + "ignoring.", command.command, device_id) + return + try: + if not handler.dispatch(command.command, command.args, dev): + self._logger.warning( + "Matter export: the bridge node sent %r for device %s (%s), which that role " + "does not define — ignoring.", command.command, device_id, entry.role) + except Exception as exc: # pylint: disable=broad-except + self._logger.error( + "Matter export: %r failed for device %s (%s) with args %r — %s. The ecosystem " + "still shows the state it asked for; pushing the real one back.", + command.command, device_id, entry.role, command.args, exc) + self._logger.exception(exc) + self._correct(handler, dev, device_id) + + def _correct(self, handler, dev: Any, device_id: int) -> None: + """Push the device's real state after a command we could not apply (F5). + + An ecosystem applies a command optimistically the moment it sends it — + the Home tile flips before anything reaches Indigo. If the dispatch then + fails, logging and returning leaves Home showing "on" and the lamp off, + permanently, until something else happens to that device. Re-reading and + pushing the truth is the only thing that closes that gap, and it is safe + to do unconditionally: ``set_state`` is fire-and-forget and the node + echo-guards its own writes (§6.4). + """ + client = self._live_client("the corrective state push", device_id) + if client is None: + return + try: + states = handler.states_for(dev) + except Exception as exc: # pylint: disable=broad-except + self._logger.warning( + "Matter export: could not read device %s back to correct the ecosystem (%s) — " + "it will show the failed command's state until the next attach.", device_id, exc) + return + if states: + self._fire(client.set_state(device_id, states), + f"corrective set_state dev {device_id}") + + # ------------------------------------------------------------------ + # Client callbacks + # ------------------------------------------------------------------ + def _on_attached(self, status) -> None: + """A successful attach ends every outage, so it clears every latch.""" + self._disconnect_ticks = 0 + self._unreachable_reported = False + self._halted_reported = False + self._recovery_reported = False + self._refusal_reported = None + self._logger.info("Matter export: bridge node attached — %d endpoint(s) live, %s", + status.endpoint_count, + "commissioned" if status.commissioned else "not yet paired") + + def _on_attach_refused(self, code: str, details: str) -> None: + """Surface a refusal with its remedy. The client has already triaged it. + + Terminal refusals (:data:`bridge_client.TERMINAL_ATTACH_ERRORS`) are + said every time: each one is a distinct decision the node made and the + client either halts or parks in recovery, so there is no loop to + throttle. Everything else is retried on the ordinary backoff and will + refuse again in ~30s, forever — so those get the same once-per-streak + latch as ``_on_unreachable``, cleared by the attach that eventually + succeeds. + """ + if code == bridge_protocol.ERR_ENDPOINT_MAP_INVALID: + self._logger.error( + "Matter export: the bridge node is serving NOTHING because its endpoint-number " + "map is unreadable (%s). Nothing will be exported until it is rebuilt — and a " + "rebuild WILL duplicate accessories in ecosystems that are already paired.", + details) + return + if code not in TERMINAL_ATTACH_ERRORS: + if self._refusal_reported == code: + return + self._refusal_reported = code + self._logger.error("Matter export: the bridge node refused the connection (%s: %s). " + "Nothing is being exported.", code, details) + + def _on_version_skew(self, hello) -> None: + self._logger.error( + "Matter export: the bridge node speaks protocol version %s, this plugin speaks %s " + "(node %s). Export is STOPPED and pairings are untouched — restart the bridge agent " + "so it picks up the node that ships with this plugin.", + hello.protocol_version, bridge_protocol.PROTOCOL_VERSION, hello.bridge_version) + + def _on_drift_detected(self, drift: list) -> None: + self._logger.error( + "Matter export: endpoint-number DRIFT detected — %s. Exported accessories may have " + "swapped identities in paired ecosystems. This is never repaired automatically.", + ", ".join(f"{d.unique_id}: expected {d.expected}, got {d.actual}" for d in drift)) + + def _on_unreachable(self, attempts: int) -> None: + """The node is not answering. In E3 that usually means it is not running.""" + if self._unreachable_reported: + return + self._unreachable_reported = True + self._logger.warning( + "Matter export: the bridge node is not responding after %d attempts on port %s. " + "In this build the node is started by hand — check it is running. Indigo devices " + "are unaffected; exported accessories will show as unavailable.", + attempts, + str(self._prefs_getter().get(bridge_protocol.PREF_WS_PORT) + or bridge_protocol.DEFAULT_WS_PORT)) + + # ------------------------------------------------------------------ + # Watchdog + # ------------------------------------------------------------------ + def health_tick(self) -> None: + """One watchdog pass. No I/O — it only reads client state and logs.""" + client = self.client + if client is None: + return + # Both of these persist until a human acts, so the tick that notices + # them is a tick that will notice them again in 15s, and in 15s after + # that — the same latch the drop path uses, for the same reason. + if client.halted: + if not self._halted_reported: + self._halted_reported = True + self._logger.warning( + "Matter export: the bridge client is HALTED (%s) — nothing is being exported " + "and it will not retry on its own.", + client.halted_reason or "no reason recorded") + return + if client.recovery: + if not self._recovery_reported: + self._recovery_reported = True + self._logger.warning("Matter export: the bridge node is awaiting an endpoint-map " + "rebuild; nothing is being exported.") + return + if client.attached: + self._disconnect_ticks = 0 + return + self._disconnect_ticks += 1 + if self._disconnect_ticks == DISCONNECT_WARN_TICKS: + self._logger.warning("Matter export: still not attached to the bridge node after " + "~1 min") + else: + self._logger.debug("Matter export: bridge node not currently attached") + + # ------------------------------------------------------------------ + # Internals + # ------------------------------------------------------------------ + def _fire(self, coro, what: str, lost: str = "") -> None: + """Schedule ``coro`` on the loop and never wait for it. + + The result is still collected by a done-callback: a ``set_state`` that + failed looks exactly like "the ecosystem is showing stale state", so it + must never be silent (§3.4). An un-retrieved future would swallow it. + + ``lost`` names the standing consequence of the coroutine never running + at all, and its presence is what promotes a scheduling failure from + debug to warning. Most callers have none: a dropped ``set_state`` or + ``upsert_endpoint`` is re-delivered by the next attach, so the loop + being down is a transient the system already recovers from. The run + loop and the un-export have no such backstop — if those are never + scheduled, nothing later puts them right. + """ + try: + future = self._runtime.submit(coro) + except Exception as exc: # pylint: disable=broad-except + coro.close() + if lost: + self._logger.warning("Matter export: could not schedule %s (%s) — %s.", + what, exc, lost) + else: + self._logger.debug("Matter export: could not schedule %s (%s)", what, exc) + return + future.add_done_callback(lambda fut: self._log_future(fut, what)) + + def _log_future(self, future, what: str) -> None: + if future.cancelled(): + return + exc = future.exception() + if exc is not None: + self._logger.warning("Matter export: %s failed — %s", what, exc) + + +def reachable_of(dev: Any) -> bool: + """§4.1 ``reachable`` for an Indigo device (XAC8). + + ``enabled`` is the user's comm-enabled flag and ``configured`` is Indigo's + "this device's config dialog has been run" flag; a device failing either is + one an ecosystem should grey out rather than time out against. Both are + real base-class properties, and both default to *unreachable* when absent — + a device we cannot read is not a device we should claim is fine. + """ + return bool(getattr(dev, "enabled", False)) and bool(getattr(dev, "configured", False)) + + +def _indigo_device(device_id: int) -> Any: + """``indigo.devices[device_id]`` or ``None``. Imported lazily, see below.""" + # The import is deferred so this module stays importable (and unit-testable) + # without the Indigo runtime, the same posture export_catalog/export_store + # take. Every real call site is inside the running plugin. + import indigo # pylint: disable=import-outside-toplevel + + try: + return indigo.devices[int(device_id)] + except Exception: # pylint: disable=broad-except + return None diff --git a/indigo-matter.indigoPlugin/Contents/Server Plugin/export_handlers.py b/indigo-matter.indigoPlugin/Contents/Server Plugin/export_handlers.py new file mode 100644 index 0000000..66736f2 --- /dev/null +++ b/indigo-matter.indigoPlugin/Contents/Server Plugin/export_handlers.py @@ -0,0 +1,404 @@ +"""Per-role **outbound** handlers — the mirror image of ``matter_handlers/``. + +Inbound, a handler is keyed by Matter *cluster* because that is what the wire +gives us. Outbound there is no cluster: the plugin has an Indigo device and a +user-declared **role** (``export_catalog``/BRIDGE_PROTOCOL §4.2), so the +registry here is keyed by role. That is the whole of PRD §XG7 — "adding a +device-type mapping in future is an isolated, mechanical change": a new role is +a new class plus one line in :data:`HANDLERS`, and the zoo test in +``tests/test_export_handlers.py`` fails until its §4.2 vocabularies are covered +in both directions. + +Each handler owns three things, and no more: + +* :meth:`ExportHandler.states_for` — the §4.2 ``set_state`` snapshot for a + device (what the ecosystem should be showing); +* :meth:`ExportHandler.diff` — the *changed* subset of that snapshot, which is + what ``deviceUpdated`` actually pushes. An empty dict means "nothing to say"; +* :meth:`ExportHandler.dispatch` — one §4.2 ``command`` event turned into a real + ``indigo.*`` call. + +**Only the E3 roles live here.** ``doorLock``, ``windowCovering``, the sensors +and ``thermostat`` are E4; the allow-list can already hold them (the §5.1 dialog +offers them as roles) but :func:`handler_for` returns ``None``, and +``export_bridge`` skips those exports with a warning rather than sending the +bridge node a role it cannot serve. That is deliberate: a role the node rejects +fails the **whole** ``attach`` with ``internal`` (E3a), so one E4 export would +otherwise take every E3 export down with it. + +**Destructive commands are not implemented here and that is not an oversight.** +``doorLock``'s ``lock``/``unlock`` are E4's, and PRD §7 requires that we never +auto-confirm a destructive state change; the seam for that gate is +:meth:`ExportHandler.dispatch`, where an E4 lock handler must consult the +inbound lock conventions before it calls ``indigo.device.lock``. + +Units are Indigo-natural at this boundary (§4.2): brightness and saturation are +0–100 in **both** directions, hue is 0–360, colour temperature is mireds on the +wire and Kelvin in Indigo. The node owns every Matter wire conversion. +""" +from __future__ import annotations + +import colorsys +import logging +from typing import Any, Callable, Optional + +import indigo # provided by the Indigo runtime + +import export_catalog +from matter_handlers.color_control import kelvin_to_mireds, mireds_to_kelvin + +#: Handlers are stateless singletons in :data:`HANDLERS` with no plugin logger +#: injected, so the one line they need to say goes to the module logger — the +#: same posture ``export_catalog`` takes. It is debug-only by design: everything +#: a user must act on is logged by ``export_bridge``, which knows the device. +_LOG = logging.getLogger(__name__) + +# -------------------------------------------------------------------------- +# §4.2 state keys and command names. Spelled once here; the zoo test pins each +# against ``bridge_protocol.ROLE_STATE_KEYS`` / ``ROLE_COMMANDS`` so a typo is a +# test failure rather than a silently ignored key on the node. +# -------------------------------------------------------------------------- +STATE_ON_OFF = "onOff" +STATE_LEVEL = "level" +STATE_COLOR_TEMP_MIREDS = "colorTempMireds" +STATE_HUE = "hue" +STATE_SATURATION = "saturation" + +COMMAND_ON_OFF = "onOff" +COMMAND_SET_LEVEL = "setLevel" +COMMAND_SET_COLOR_TEMP = "setColorTemp" +COMMAND_SET_COLOR = "setColor" + +#: §4.2 bounds for ``colorTempMireds``. The node clamps too, but sending a value +#: outside the declared domain would be *us* breaking the contract — and a +#: device reporting ``whiteTemperature`` 0 (the "unknown" spelling several +#: plugins use) converts to an infinite mired value if left unguarded. +MIREDS_MIN = 153 +MIREDS_MAX = 500 + +#: Hue tolerance, in degrees, below which :meth:`ExportHandler.diff` treats a +#: change as noise. The node converts 0–360 to Matter's 0–254 hue and back, and +#: 360/254 ≈ 1.4°, so a colour we pushed can come back one degree different +#: through no one's fault. Without this, every ecosystem-originated colour +#: change would echo a spurious ``set_state`` straight back out. +#: +#: Saturation deliberately has NO tolerance: 0–100 → 0–254 → 0–100 is a +#: widening then a narrowing, so it round-trips exactly. +HUE_TOLERANCE_DEGREES = 1 + +#: Saturation below which ``hue`` is OMITTED from the state snapshot entirely. +#: +#: Indigo stores colour as three integer 0–100 channels, so hue is recovered +#: from them rather than stored — and near the grey axis those integers carry +#: almost no angular information. Measured worst-case round-trip error over all +#: 360 degrees (``rgb → hue,sat → rgb → hue``): +#: +#: =========== ===== ==== ==== ==== ==== ==== ==== ===== +#: saturation 0 1 2 3 5 10 15 20+ +#: max hue error 180° 30° 15° 10° 6° 3° 2° 1° +#: =========== ===== ==== ==== ==== ==== ==== ==== ===== +#: +#: :data:`HUE_TOLERANCE_DEGREES` only absorbs the last column, so below 20 every +#: pastel change pushed a ``hue`` the ecosystem never asked for. It converges — +#: ``setColorLevels`` is absolute — but the Home colour wheel visibly jumps +#: while it does. At saturation 0 hue is not merely noisy but undefined (grey +#: reads back as hue 0 whatever was sent), so omitting is the honest answer: +#: §3.4 state maps are partial by design, and an absent key means "not telling +#: you", which is exactly the truth here. +SATURATION_HUE_FLOOR = 20 + +#: Sentinel for "this key was absent before", which is a change, not a match. +_MISSING = object() + + +# -------------------------------------------------------------------------- +# Colour conversion (pure) +# -------------------------------------------------------------------------- +def rgb_to_hue_saturation(red: float, green: float, blue: float) -> tuple[int, int]: + """Indigo RGB levels (0–100 each) → §4.2 ``(hue 0-360, saturation 0-100)``.""" + hue, saturation, _value = colorsys.rgb_to_hsv(red / 100.0, green / 100.0, blue / 100.0) + return round(hue * 360) % 360, round(saturation * 100) + + +def hue_saturation_to_rgb(hue: float, saturation: float) -> tuple[int, int, int]: + """§4.2 ``(hue, saturation)`` → Indigo RGB levels (0–100 each). + + Value is pinned to 1.0 — full vibrance. Brightness is a separate §4.2 key + (``level`` → ``brightnessLevel``), so folding it into RGB here would make a + dim red and a bright dark-red indistinguishable. This matches the inbound + convention in ``matter_handlers.color_control.matter_xy_to_rgb``. + """ + red, green, blue = colorsys.hsv_to_rgb((hue % 360) / 360.0, + _clamp(saturation, 0, 100) / 100.0, 1.0) + return round(red * 100), round(green * 100), round(blue * 100) + + +def _clamp(value: float, low: float, high: float) -> float: + return max(low, min(high, value)) + + +def _number(dev: Any, name: str) -> Optional[float]: + """A numeric device attribute, or ``None`` if it is absent/unset/not a number. + + Indigo's colour attributes are ``None`` on a device that has no colour + channel, and a MagicMock device (tests, and any exotic proxy) answers every + attribute truthily — so the type check is load-bearing, not defensive + padding. + """ + value = getattr(dev, name, None) + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + return float(value) + + +# -------------------------------------------------------------------------- +# Handlers +# -------------------------------------------------------------------------- +class ExportHandler: + """One §4.2 role's outbound vocabulary.""" + + #: The §4.2 role this handler serves. + role: str = "" + #: Per-key diff tolerances; anything absent compares exactly. + tolerances: dict[str, float] = {} + + # -- state (plugin → node) ------------------------------------------ + def states_for(self, dev: Any) -> dict: + """The full §4.2 state snapshot for ``dev``. + + Keys whose value the device cannot supply are **omitted**, never + defaulted: ``set_state`` args are a partial map by design (§3.4), and a + fabricated 0 would be pushed to every ecosystem as fact. + """ + raise NotImplementedError + + def diff(self, orig_dev: Any, new_dev: Any) -> dict: + """The changed §4.2 keys only. Empty dict = nothing to push.""" + before = self.states_for(orig_dev) + after = self.states_for(new_dev) + return { + key: value for key, value in after.items() + if self._changed(key, before.get(key, _MISSING), value) + } + + def _changed(self, key: str, before: Any, after: Any) -> bool: + if before is _MISSING: + return True + tolerance = self.tolerances.get(key) + if tolerance is not None and isinstance(before, (int, float)) \ + and isinstance(after, (int, float)): + return abs(after - before) > tolerance + return before != after + + # -- commands (node → plugin) --------------------------------------- + def commands(self) -> dict[str, Callable[[dict, Any], None]]: + """``command name → handler``. Subclasses extend, never replace.""" + return {} + + def dispatch(self, command: str, args: dict, dev: Any) -> bool: + """Run one §4.2 command against ``dev``. ``False`` = not our command. + + The caller logs the ``False`` case: only it knows which device and which + role were involved, which is the version of that line worth having. + """ + handler = self.commands().get(command) + if handler is None: + return False + handler(dict(args or {}), dev) + return True + + +class OnOffExport(ExportHandler): + """``onOffPlugInUnit`` / ``onOffLight`` — the whole of the relay export.""" + + role = export_catalog.ROLE_ON_OFF_PLUG + + def states_for(self, dev: Any) -> dict: + return {STATE_ON_OFF: bool(getattr(dev, "onState", False))} + + def commands(self) -> dict[str, Callable[[dict, Any], None]]: + return {**super().commands(), COMMAND_ON_OFF: self._on_off} + + @staticmethod + def _on_off(args: dict, dev: Any) -> None: + if bool(args.get("value")): + indigo.device.turnOn(dev) + else: + indigo.device.turnOff(dev) + + +class OnOffLightExport(OnOffExport): + """Same vocabulary as the plug; a different Matter device type on the node.""" + + role = export_catalog.ROLE_ON_OFF_LIGHT + + +class DimmableLightExport(OnOffExport): + """``dimmableLight`` — adds ``level`` (0–100 both sides, no conversion).""" + + role = export_catalog.ROLE_DIMMABLE_LIGHT + + def states_for(self, dev: Any) -> dict: + states = super().states_for(dev) + brightness = _number(dev, "brightness") + if brightness is not None: + states[STATE_LEVEL] = int(_clamp(round(brightness), 0, 100)) + return states + + def commands(self) -> dict[str, Callable[[dict, Any], None]]: + return {**super().commands(), COMMAND_SET_LEVEL: self._set_level} + + @staticmethod + def _set_level(args: dict, dev: Any) -> None: + level = args.get(STATE_LEVEL) + if not isinstance(level, (int, float)) or isinstance(level, bool): + raise ValueError(f"setLevel without a numeric level: {args!r}") + indigo.dimmer.setBrightness(dev, value=int(_clamp(round(level), 0, 100))) + + +class ColorTemperatureLightExport(DimmableLightExport): + """``colorTemperatureLight`` — adds ``colorTempMireds`` over Indigo's Kelvin.""" + + role = export_catalog.ROLE_COLOR_TEMPERATURE_LIGHT + + def states_for(self, dev: Any) -> dict: + states = super().states_for(dev) + kelvin = _number(dev, "whiteTemperature") + if kelvin: # 0 and None both mean "this device is not telling us" + states[STATE_COLOR_TEMP_MIREDS] = int( + _clamp(kelvin_to_mireds(kelvin), MIREDS_MIN, MIREDS_MAX)) + return states + + def commands(self) -> dict[str, Callable[[dict, Any], None]]: + return {**super().commands(), COMMAND_SET_COLOR_TEMP: self._set_color_temp} + + @staticmethod + def _set_color_temp(args: dict, dev: Any) -> None: + """Set white temperature, preserving the white channel's own level. + + ``setColorLevels`` documents whiteTemperature as used *in combination + with* whiteLevel, so sending the temperature alone risks a driver + reading whiteLevel as 0 and turning the lamp off — a colour tweak that + blacks out the room is a worse bug than a colour tweak that misses. + + Three guards, all about not depending on someone else to be careful: + + * **the mireds are clamped here.** The node clamps too, but that is the + *other process*: an unclamped 1 mired is a 1,000,000 K write and 10000 + mireds is 100 K, both outside Indigo's own 1200–15000 domain, and both + would reach the driver if the node ever stopped clamping or a command + arrived from anywhere else. ``round`` rather than ``int`` because + truncating 369.9 to 369 is a different colour, not a rounding detail; + * **no white channel means no command.** ``whiteLevel is None`` is real + Indigo for "this device has no white channel at all" — inventing 100 + for it asks its driver to drive something it does not have. A + whiteLevel of *0* is different: the channel exists and is off, and + that IS the blacked-out-room case the default is for; + * **RGB is zeroed alongside.** Matter's ``colorMode`` is one-of, and the + node's push side picks hue/saturation over colour temperature when a + device reports both (``bridge-node/src/endpoints.ts`` ``colorPatch``). + An RGBW lamp left holding stale RGB levels would therefore report a + colour we did not set, and the node would believe it over the + temperature we just wrote. Only touched when the device actually has + RGB channels — an all-zero RGB write to a CT-only driver is its own + way to black out the room. + """ + mireds = args.get(STATE_COLOR_TEMP_MIREDS) + if not isinstance(mireds, (int, float)) or isinstance(mireds, bool) or not mireds: + raise ValueError(f"setColorTemp without usable mireds: {args!r}") + white_level = _number(dev, "whiteLevel") + if white_level is None: + _LOG.debug("setColorTemp skipped: device %s has no white channel", + getattr(dev, "id", "?")) + return + levels: dict[str, int] = { + "whiteLevel": int(_clamp(round(white_level or 100), 0, 100)), + "whiteTemperature": mireds_to_kelvin( + int(_clamp(round(mireds), MIREDS_MIN, MIREDS_MAX))), + } + if _number(dev, "redLevel") is not None: + levels.update(redLevel=0, greenLevel=0, blueLevel=0) + indigo.dimmer.setColorLevels(dev, **levels) + + +class ExtendedColorLightExport(ColorTemperatureLightExport): + """``extendedColorLight`` — adds ``hue``/``saturation`` over Indigo's RGB.""" + + role = export_catalog.ROLE_EXTENDED_COLOR_LIGHT + tolerances = {STATE_HUE: HUE_TOLERANCE_DEGREES} + + def states_for(self, dev: Any) -> dict: + states = super().states_for(dev) + red = _number(dev, "redLevel") + green = _number(dev, "greenLevel") + blue = _number(dev, "blueLevel") + if None not in (red, green, blue): + hue, saturation = rgb_to_hue_saturation(red, green, blue) + states[STATE_SATURATION] = saturation + # Below the floor the recovered hue is noise, not colour — see + # SATURATION_HUE_FLOOR for the measured error table. + if saturation >= SATURATION_HUE_FLOOR: + states[STATE_HUE] = hue + return states + + def commands(self) -> dict[str, Callable[[dict, Any], None]]: + return {**super().commands(), COMMAND_SET_COLOR: self._set_color} + + @staticmethod + def _set_color(args: dict, dev: Any) -> None: + """Set hue/saturation as Indigo RGB levels. + + Ecosystems send ``setColor`` **twice with identical values** (E3a, seen + live) because Matter carries hue and saturation as separate attribute + writes. No de-duplication is attempted: ``setColorLevels`` takes + absolute values, so the second call is a no-op at the lamp, and + suppressing it on "we already believe that" would silently skip the + first call too whenever Indigo's belief has drifted from the hardware. + Idempotent beats clever. + + The white channel is zeroed with the same colour-mode reasoning as + :meth:`ColorTemperatureLightExport._set_color_temp` — an RGBW lamp + holding both a colour and a white level reports both, and the node has + to pick. It picks the colour, so leaving white lit means the lamp and + the ecosystem disagree about what "the colour" is. Only sent when the + device has a white channel to zero. + """ + hue = args.get(STATE_HUE) + saturation = args.get(STATE_SATURATION) + for name, value in ((STATE_HUE, hue), (STATE_SATURATION, saturation)): + if not isinstance(value, (int, float)) or isinstance(value, bool): + raise ValueError(f"setColor without a numeric {name}: {args!r}") + red, green, blue = hue_saturation_to_rgb(hue, saturation) + levels = {"redLevel": red, "greenLevel": green, "blueLevel": blue} + if _number(dev, "whiteLevel") is not None: + levels["whiteLevel"] = 0 + indigo.dimmer.setColorLevels(dev, **levels) + + +# -------------------------------------------------------------------------- +# Registry +# -------------------------------------------------------------------------- +#: role → handler. **E3 roles only** — see the module docstring on why an +#: unimplemented role must be skipped by the caller rather than sent anyway. +HANDLERS: dict[str, ExportHandler] = { + handler.role: handler for handler in ( + OnOffExport(), + OnOffLightExport(), + DimmableLightExport(), + ColorTemperatureLightExport(), + ExtendedColorLightExport(), + ) +} + +#: The roles this plugin can actually bridge today. +BRIDGEABLE_ROLES = frozenset(HANDLERS) + + +def handler_for(role: str) -> Optional[ExportHandler]: + """The handler for ``role``, or ``None`` if v1 cannot bridge it yet.""" + return HANDLERS.get(role) + + +def is_bridgeable(role: str) -> bool: + """True if an allow-list entry with ``role`` can be exported today.""" + return role in HANDLERS diff --git a/indigo-matter.indigoPlugin/Contents/Server Plugin/plugin.py b/indigo-matter.indigoPlugin/Contents/Server Plugin/plugin.py index dd40f3e..66a059a 100644 --- a/indigo-matter.indigoPlugin/Contents/Server Plugin/plugin.py +++ b/indigo-matter.indigoPlugin/Contents/Server Plugin/plugin.py @@ -19,6 +19,7 @@ from concurrent.futures import CancelledError as FuturesCancelledError from concurrent.futures import TimeoutError as FuturesTimeoutError from datetime import datetime, timezone +from typing import Optional import indigo # provided by the Indigo runtime @@ -26,7 +27,9 @@ from async_runtime import AsyncRuntime from commission_jobs import CommissionJobs, node_id_to_str from device_sync import DeviceSync +from export_bridge import ExportBridge import export_catalog +import export_handlers from export_store import ExportEntry, ExportStore, OPTION_INVERT from http_handlers import HttpApi, MatterUnavailable from matter_client import MatterClient @@ -135,6 +138,22 @@ def __init__(self, plugin_id, plugin_display_name, plugin_version, plugin_prefs, # can consult it; None means "the plugin has not started yet", which # every export callback checks rather than assuming. self.exports: ExportStore | None = None + # The outbound export engine (PRD-indigo-matter-export §5.4). Built in + # startup; it owns the bridge client and starts one only when something + # is actually exported (XG5). + self.export_bridge: ExportBridge | None = None + #: The allow-listed device ids, cached as a plain frozenset attribute. + #: ``deviceUpdated`` fires for EVERY device on the server, so its guard + #: has to be one attribute load and one hash lookup — no lock, no + #: rebuild, no allocation. Refreshed only by :meth:`_exports_changed`. + self._exported_ids: frozenset[int] = frozenset() + #: Whether ``indigo.devices.subscribeToChanges()`` has been issued. + self._subscribed_to_devices = False + #: Device ids whose export callback is currently failing. This callback + #: fires on every change of an exported device, so a stuck failure would + #: otherwise write one traceback per dimmer-ramp step; cleared on the + #: first success so a second, later outage is still heard. + self._export_callback_failed: set[int] = set() self._install_thread: threading.Thread | None = None self._stopping = False # When WE restart matter-server (menu / post-install), the client sees a brief @@ -229,6 +248,14 @@ def startup(self) -> None: diagnostics_provider=self._diagnostics_sync, ) + # Export (outbound) — built unconditionally, started only if the + # allow-list is non-empty. Both decisions live in _exports_changed. + self.export_bridge = ExportBridge( + self.exports, self.runtime, self.logger, lambda: self.pluginPrefs, + plugin_version=self._version, plugin_id=self._export_plugin_id(), + ) + self._exports_changed() + run_future = self.runtime.submit(self.matter.run()) # if the run-loop coroutine ever dies, surface it rather than parking the # exception on an unretrieved future. @@ -244,6 +271,128 @@ def _log_run_future(self, fut) -> None: if exc is not None: self.logger.exception(exc) + # ------------------------------------------------------------------ + # Export wiring (PRD-indigo-matter-export §5.4) + # ------------------------------------------------------------------ + def _exports_changed(self) -> None: + """THE seam for "the allow-list changed" — call it after every write. + + Refreshes the hot-path id set, subscribes to device changes if this is + the first export, and lets the bridge start or stop itself (XG5). + """ + self._exported_ids = self.exports.ids() if self.exports is not None else frozenset() + if self._exported_ids: + self._subscribe_to_device_changes() + if self.export_bridge is not None: + self.export_bridge.exports_changed() + + def _subscribe_to_device_changes(self) -> None: + """Ask the server for every device change — once, and only if we need it. + + Three findings settle the shape of this, all from the Indigo docs rather + than from what was convenient: + + * ``indigo.devices.subscribeToChanges()`` subscribes to **every device + on the server**, not ours, and the IOM reference is explicit that it + "causes a significant amount of traffic between IndigoServer and your + plugin". The default posture of this plugin is an empty allow-list + (XG5), so subscribing unconditionally would tax every existing user + who never exports anything — for callbacks that would return on their + first line every single time. + * It is a plain request to the server, not a startup-only registration. + Issuing it from a menu callback the first time a user exports a device + works exactly as it does from ``startup``, which is what makes the + conditional subscription safe: the first export in a session turns it + on, and every later ``deviceUpdated`` arrives. + * There is **no unsubscribe** in the canonical scripting reference (only + ``subscribeToChanges``), so this is a one-way door. We therefore never + try to turn it off when the allow-list empties again; the hot-path + guard below already makes a stale subscription free, and a + "clever" unsubscribe against an undocumented API is exactly the sort + of thing that fails silently on an Indigo upgrade. + + Note there is deliberately **no ``pluginId`` self-loop guard** here (the + usual companion to this subscription). It would be dead code: our own + devices are excluded by the catalog's loop guard, so they can never + reach the allow-list, and the id-set check below already refuses them. + The dispatch→state→push path does not loop either — the node + echo-guards its own writes (§6.4) and a push produces no Indigo change. + """ + if self._subscribed_to_devices: + return + try: + indigo.devices.subscribeToChanges() + except Exception as exc: # noqa: BLE001 + self.logger.error("Matter export: could not subscribe to Indigo device changes — " + "exported accessories will not follow Indigo state. %s", exc) + self.logger.exception(exc) + return + self._subscribed_to_devices = True + self.logger.debug("subscribed to Indigo device changes (export is active)") + + def deviceUpdated(self, origDev, newDev): # noqa: N802 + """Push an exported device's change outward. + + This runs for **every device on the server** (see + :meth:`_subscribe_to_device_changes`), so the second statement is the + whole performance story: a frozenset membership test on an int, against + an attribute the plugin already holds. Nothing is classified, nothing is + locked and nothing is allocated for a device nobody exported. + """ + super().deviceUpdated(origDev, newDev) + if newDev.id not in self._exported_ids: + return + if self.export_bridge is None: + return + try: + self.export_bridge.device_updated(origDev, newDev) + except Exception as exc: # noqa: BLE001 - never let export break Indigo's callback + # Named and rate-limited: a bare traceback here says a device broke + # but not which one, and this callback fires often enough that a + # stuck device would bury the rest of the event log. + if newDev.id not in self._export_callback_failed: + self._export_callback_failed.add(newDev.id) + self.logger.error( + "Matter export: the update of %s (id %s) could not be handed to the bridge " + "— %s. Its accessory will show stale state until this clears.", + getattr(newDev, "name", ""), newDev.id, exc) + self.logger.exception(exc) + else: + self._export_callback_failed.discard(newDev.id) + + def deviceDeleted(self, dev): # noqa: N802 + """A deleted device leaves the allow-list and the bridge (PRD §5.4).""" + super().deviceDeleted(dev) + if dev.id not in self._exported_ids or self.exports is None: + return + try: + self.exports.remove(dev.id) + self.logger.info("Removed Matter export: %s (id %s) — the Indigo device was deleted", + getattr(dev, "name", ""), dev.id) + except Exception as exc: # noqa: BLE001 + # The store rolled back, so the entry survives; the endpoint removal + # below is still right (the device is gone either way) and the + # startup sweep will report the orphan. + self.logger.error("Matter export: removing the deleted device %s from the export " + "list FAILED — %s", dev.id, exc) + self.logger.exception(exc) + self._export_callback_failed.discard(dev.id) + try: + if self.export_bridge is not None: + self.export_bridge.remove(dev.id) + except Exception as exc: # noqa: BLE001 + self.logger.exception(exc) + finally: + # In a finally because the endpoint removal above can raise (the + # socket is the bridge's, not ours) and the id-set cache has no + # other way back in sync: leaving a deleted device in it makes + # deviceUpdated hand a ghost to the bridge on every later change, + # and deviceDeleted will never fire for it again. + try: + self._exports_changed() + except Exception as exc: # noqa: BLE001 + self.logger.exception(exc) + def shutdown(self) -> None: self.logger.debug("%s shutting down", PLUGIN_NAME) # Signal any in-flight background install to skip its post-npm plugin-state @@ -257,6 +406,13 @@ def shutdown(self) -> None: self.runtime.submit(self.matter.close()).result(timeout=4) except Exception as exc: # noqa: BLE001 self.logger.debug("matter close error: %s", exc) + # Same ordering rule as the controller client: close the socket while the + # loop still exists to close it on. The bridge *agent* is deliberately + # left running (PRD §5.4 / PM-B) — a plugin reload must not un-pair + # anyone's ecosystems. + if self.runtime is not None and self.runtime.is_running and self.export_bridge is not None: + self.export_bridge.stop() + self.export_bridge = None if self.runtime is not None: self.runtime.stop() self.runtime = None @@ -386,6 +542,12 @@ def _health_tick(self) -> None: self.logger.debug("matter-server not currently connected") else: self._disconnect_ticks = 0 + # The export side keeps its OWN counter (E1 audit note): the two clients + # talk to different processes and fail independently, so a shared streak + # counter would let a healthy bridge silence a dead matter-server, or + # the reverse. + if self.export_bridge is not None: + self.export_bridge.health_tick() # ------------------------------------------------------------------ # Config @@ -1207,10 +1369,47 @@ def _export_summary(self) -> str: # scratch, and the rebuild's first save overwrites the rescue copy. error = self.exports.load_error if error: - return error if not count else f"{error} {count} device(s) exported." + return error if not count else \ + f"{error} {count} device(s) exported.{self._export_bridge_note()}" if not count: return "Nothing is exported yet." - return f"{count} device(s) exported." + summary = f"{count} device(s) exported." + # An export whose role this version cannot bridge is silently absent + # from every ecosystem otherwise — the dialog is the only place the user + # would ever look for the reason. + pending = sum(1 for entry in self.exports.all() + if not export_handlers.is_bridgeable(entry.role)) + if pending: + summary += (f" {pending} of them use a role this version cannot bridge yet " + "(sensors, locks, coverings and thermostats arrive in a later " + "release) and will not appear in any ecosystem.") + return summary + self._export_bridge_note() + + def _export_bridge_note(self) -> str: + """One sentence when the exports exist but are not actually live. + + "3 device(s) exported." is true and useless while the bridge client is + halted on a version skew: the user is looking at this dialog precisely + because a light is missing from the Home app, and every state below + answers that question. Reported as a suffix so a load error — which is + about rescuing the user's list, and outranks everything — still leads. + """ + bridge = self.export_bridge + if bridge is None or not bridge.active: + # No client is the CORRECT state for an empty allow-list (XG5), and + # the count above already says the list is not empty — so this is a + # plugin still starting, which its own log line covers. + return "" + client = bridge.client + if client.halted: + return (f" Bridge client halted ({client.halted_reason or 'no reason recorded'}) " + "— restart the bridge node.") + if client.recovery: + return (" The bridge node is waiting for an endpoint-map rebuild — exports are not " + "live until it is done.") + if not client.attached: + return " Not connected to the bridge node — exports are not live." + return "" def get_menu_action_config_ui_values(self, menu_id): """Seed the export dialog (menu dialogs never remember their values). @@ -1496,7 +1695,9 @@ def exportAddOrUpdate(self, valuesDict, typeId="", devId=0): options = {} if role == export_catalog.ROLE_WINDOW_COVERING and self._truthy(values.get("exportInvert")): options[OPTION_INVERT] = True - existed = device_id in self.exports + previous = self.exports.get(device_id) + existed = previous is not None + role_changed = existed and previous.role != role try: self.exports.upsert(ExportEntry( indigo_device_id=device_id, role=role, @@ -1513,10 +1714,42 @@ def exportAddOrUpdate(self, valuesDict, typeId="", devId=0): self.logger.info("%s Matter export: %s (id %s) as %s%s", verb, dev.name, device_id, role, f' named "{name_override}"' if name_override else "") + self._nudge_export(device_id, role_changed=role_changed) values["exportStatus"] = f"{verb} {dev.name} as {export_catalog.role_label(role)}. " \ - f"{self._export_summary()}" + f"{self._role_change_warning(role_changed)}{self._export_summary()}" return values + @staticmethod + def _role_change_warning(role_changed: bool) -> str: + """What a role change actually costs the user, said before they find out. + + BRIDGE_PROTOCOL §4.1 rejects changing an existing endpoint's role, so the + plugin removes and re-adds it. Ecosystems treat that as a brand-new + accessory: the name and room it was given in Apple Home are gone. + """ + if not role_changed: + return "" + return ("Changing the role RE-CREATES the accessory, so it loses the name and room " + "you gave it in Apple Home and any other paired ecosystem. ") + + def _nudge_export(self, device_id: int, *, role_changed: bool = False) -> None: + """Tell the bridge about one changed export, without a full reconnect. + + A role change is the one case that cannot be an ``upsert``: §4.1 refuses + it with ``role_change``, so it becomes remove-then-add. + """ + self._exports_changed() + bridge = self.export_bridge + if bridge is None: + return + try: + if role_changed: + bridge.replace(device_id) + else: + bridge.upsert(device_id) + except Exception as exc: # pylint: disable=broad-except + self.logger.exception(exc) + def exportRemove(self, valuesDict, typeId="", devId=0): # pylint: disable=unused-argument """Drop the picked device from the allow-list. Returns values only (see above).""" @@ -1541,6 +1774,15 @@ def exportRemove(self, valuesDict, typeId="", devId=0): dev = self._indigo_device(device_id) name = str(getattr(dev, "name", "") or "") if dev is not None else f"device {device_id}" self.logger.info("Removed Matter export: %s (id %s)", name, device_id) + # XAC7: the accessory has to leave every paired ecosystem, not just the + # allow-list. Order matters — remove the endpoint BEFORE the empty + # allow-list stops the client out from under it. + if self.export_bridge is not None: + try: + self.export_bridge.remove(device_id) + except Exception as exc: # pylint: disable=broad-except + self.logger.exception(exc) + self._exports_changed() values["exportRole"] = "" values["exportName"] = "" values["exportInvert"] = False diff --git a/pyproject.toml b/pyproject.toml index bd1deda..e2416fd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,9 @@ ignore-paths = ["tests", "__pycache__"] max-line-length = 120 [tool.pylint.design] -max-attributes = 20 +# The Plugin class holds one attribute per subsystem it owns plus the +# rate-limit latches its Indigo callbacks need; 21 is what that currently is. +max-attributes = 21 max-args = 8 [tool.pylint."messages control"] diff --git a/tests/conftest.py b/tests/conftest.py index 18051bf..ac891c8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -49,10 +49,31 @@ class _IndigoPluginBaseStub: """Stand-in for ``indigo.PluginBase`` at ``class Plugin`` definition time. Real Indigo's ``PluginBase`` does server-bound init the tests don't need; - subclassing this empty stub lets ``class Plugin(indigo.PluginBase):`` + subclassing this stub lets ``class Plugin(indigo.PluginBase):`` import-succeed under MagicMock test doubles. + + The device callbacks are here because the real base class **does real work** + in them (``deviceStartComm``/``deviceStopComm`` for our own devices), so any + override has to call ``super()`` — the SDK's most-broken rule. Each records + the call in ``base_calls`` so a test can assert the chain-up actually + happened rather than merely not crashing. """ + def _record_base_call(self, name, *args): + calls = getattr(self, "base_calls", None) + if calls is None: + calls = self.base_calls = [] + calls.append((name, *args)) + + def deviceCreated(self, dev): # noqa: N802 + self._record_base_call("deviceCreated", dev) + + def deviceUpdated(self, origDev, newDev): # noqa: N802 + self._record_base_call("deviceUpdated", origDev, newDev) + + def deviceDeleted(self, dev): # noqa: N802 + self._record_base_call("deviceDeleted", dev) + @pytest.fixture def mock_indigo_base(monkeypatch): diff --git a/tests/fakes.py b/tests/fakes.py index 5e80c78..95362a8 100644 --- a/tests/fakes.py +++ b/tests/fakes.py @@ -7,6 +7,7 @@ import asyncio import json +from concurrent.futures import Future from typing import Callable, Optional import protocol @@ -118,6 +119,103 @@ async def returns(value): return value +# --------------------------------------------------------------------------- +# Export (outbound) doubles — the E3 bridge wiring +# --------------------------------------------------------------------------- +class RecordingRuntime: + """Stands in for ``AsyncRuntime``, running each coroutine to completion NOW. + + ``ExportBridge`` deliberately never awaits what it submits (§3.4), so a + runtime that merely queued coroutines would let every assertion pass against + work that never happened. Running them synchronously on the calling thread + keeps the tests deterministic *and* exercises ``_fire``'s done-callback, which + is the only thing that stops a failed state push being silent. + """ + + def __init__(self, running: bool = True): + self.is_running = running + self.submitted: list = [] + + def submit(self, coro): + self.submitted.append(coro) + if not self.is_running: + raise RuntimeError("asyncio runtime is not running") + future: Future = Future() + try: + future.set_result(asyncio.run(coro)) + except Exception as exc: # the client's own errors reach the callback + future.set_exception(exc) + return future + + +class FakeBridgeClient: + """Records what ``ExportBridge`` sends, without a socket or a node. + + Mirrors the slice of :class:`bridge_client.BridgeClient` the export engine + actually touches. ``calls`` is ordered, because frame order is part of the + contract (§1: frames are applied in receipt order). + """ + + def __init__(self, logger=None, prefs=None, **kwargs): + self.logger = logger + self.prefs = dict(prefs or {}) + self.kwargs = kwargs + self.calls: list[tuple] = [] + self.attached = True + self.connected = True + self.halted = False + self.halted_reason = None + self.recovery = False + self.closed = False + self.ran = False + #: The ``timeout`` of each attach, in order. Kept OUT of ``calls`` so the + #: existing tuple assertions stay readable — but recorded, because the + #: deadline an un-export is given is the whole of X1: the empty endpoint + #: list says nothing about how many endpoints the node has to remove. + self.attach_timeouts: list = [] + #: command name → exception to raise instead of succeeding. + self.fail: dict = {} + + # -- the recorded surface ------------------------------------------- + async def run(self): + self.ran = True + + async def attach(self, endpoints=None, *, replace_all=False, timeout=None): + self.attach_timeouts.append(timeout) + return self._record("attach", endpoints, replace_all) + + async def upsert_endpoint(self, spec, timeout=None): + return self._record("upsert_endpoint", spec) + + async def remove_endpoint(self, device_id, timeout=None): + return self._record("remove_endpoint", device_id) + + async def set_state(self, device_id, states): + return self._record("set_state", device_id, dict(states)) + + async def set_reachable(self, device_id, reachable, timeout=None): + return self._record("set_reachable", device_id, reachable) + + async def close(self): + self.closed = True + self._record("close") + + # -- introspection --------------------------------------------------- + def _record(self, name, *args): + self.calls.append((name, *args)) + if name in self.fail: + raise self.fail[name] + return None + + def names(self) -> list[str]: + return [call[0] for call in self.calls] + + def only(self, name: str) -> tuple: + matches = [call for call in self.calls if call[0] == name] + assert len(matches) == 1, f"expected exactly one {name}, got {self.names()}" + return matches[0] + + # --------------------------------------------------------------------------- # Fake Indigo devices for the export catalog / picker (PRD §5.1-§5.2) # --------------------------------------------------------------------------- @@ -143,6 +241,10 @@ def __init__(self, dev_id=1, name="Device", plugin_id=OTHER_PLUGIN_ID, **attrs): self.id = dev_id self.name = name self.pluginId = plugin_id + # Every real Indigo device carries both, and §4.1 `reachable` is derived + # from them (XAC8) — so they are base attributes, not per-class extras. + self.enabled = attrs.pop("enabled", True) + self.configured = attrs.pop("configured", True) self.pluginProps = dict(attrs.pop("pluginProps", None) or {}) self.deviceTypeId = attrs.pop("deviceTypeId", "") self.displayStateValUi = attrs.pop("displayStateValUi", "") @@ -157,16 +259,28 @@ class RelayDevice(FakeIndigoDevice): def __init__(self, *args, **kwargs): self.supportsOnState = kwargs.pop("supportsOnState", True) + self.onState = kwargs.pop("onState", False) super().__init__(*args, **kwargs) class DimmerDevice(RelayDevice): - """indigo.DimmerDevice — a relay that also dims, and maybe colours.""" + """indigo.DimmerDevice — a relay that also dims, and maybe colours. + + The colour attributes default to ``None``, which is what real Indigo reports + on a device with no colour channel — the export handlers have to omit those + §4.2 keys rather than push a fabricated 0. + """ def __init__(self, *args, **kwargs): self.supportsColor = kwargs.pop("supportsColor", False) self.supportsRGB = kwargs.pop("supportsRGB", False) self.supportsWhiteTemperature = kwargs.pop("supportsWhiteTemperature", False) + self.brightness = kwargs.pop("brightness", 0) + self.redLevel = kwargs.pop("redLevel", None) + self.greenLevel = kwargs.pop("greenLevel", None) + self.blueLevel = kwargs.pop("blueLevel", None) + self.whiteLevel = kwargs.pop("whiteLevel", None) + self.whiteTemperature = kwargs.pop("whiteTemperature", None) super().__init__(*args, **kwargs) diff --git a/tests/test_export_bridge.py b/tests/test_export_bridge.py new file mode 100644 index 0000000..3ccf21b --- /dev/null +++ b/tests/test_export_bridge.py @@ -0,0 +1,833 @@ +"""E3: the outbound export engine (`export_bridge`). + +What is pinned here, and why each earns its place: + +* **The endpoint provider re-classifies** — the allow-list is a user declaration + made in the past, so a device that has since been deleted, disabled, taken + over by this plugin, or re-typed must be skipped with a warning rather than + sent as a stale spec (the E2 handover's standing requirement); +* **an E4 role is skipped, not sent** — an unknown role fails the WHOLE attach + on the node (E3a), so one un-bridgeable export would silently un-export every + working one; +* **the attach deadline scales with the endpoint count** — the node paces bulk + removals ~100ms apart, so a fixed 8s deadline times out on exactly the large + databases that most need export to work; +* **the client exists only while something is exported** (XG5), and emptying the + allow-list goes out as the deliberate §3.1 ``replace_all`` attach, not as a + disconnect; +* **nothing is awaited on Indigo's thread** — every push is fire-and-forget, and + a failed one still reaches the log (§3.4). + +References to ``§N`` are ``docs/BRIDGE_PROTOCOL.md``. +""" +from __future__ import annotations + +import importlib + +import pytest + +import bridge_client +import bridge_protocol +import export_catalog +from export_store import ExportEntry, ExportStore + +from conftest import load_bridge_frames +from fakes import ( + OTHER_PLUGIN_ID, + DimmerDevice, + FakeBridgeClient, + FakeIndigoDevices, + RecordingRuntime, + RelayDevice, + SprinklerDevice, +) + +FRAMES = load_bridge_frames() +OURS = export_catalog.DEFAULT_PLUGIN_ID + + +@pytest.fixture +def bridge_mod(mock_indigo_base): + """`export_bridge` (and the handlers it uses) bound to a mocked ``indigo``.""" + import export_handlers + import export_bridge as module + importlib.reload(export_handlers) + importlib.reload(module) + return module + + +class Harness: + """An ExportBridge wired to fakes, plus the knobs the tests need.""" + + def __init__(self, module, mock_logger, devices, entries=()): + self.logger = mock_logger + self.prefs: dict = {} + self.devices = devices + self.store = ExportStore(lambda: self.prefs, mock_logger) + for entry in entries: + self.store.upsert(entry) + self.runtime = RecordingRuntime() + self.clients: list[FakeBridgeClient] = [] + self.bridge = module.ExportBridge( + self.store, self.runtime, mock_logger, lambda: self.prefs, + plugin_version="2026.7.28", plugin_id=OURS, + device_getter=self._device, + client_factory=self._client, + ) + + def _device(self, device_id): + try: + return self.devices[device_id] + except KeyError: + return None + + def _client(self, logger, prefs, **kwargs): + client = FakeBridgeClient(logger, prefs, **kwargs) + self.clients.append(client) + return client + + @property + def client(self) -> FakeBridgeClient: + assert self.clients, "no bridge client was created" + return self.clients[-1] + + def start(self) -> FakeBridgeClient: + self.bridge.start() + return self.client + + +def warnings_of(logger) -> str: + return " ".join(str(call.args[0]) % call.args[1:] if len(call.args) > 1 + else str(call.args[0]) + for call in logger.warning.call_args_list) + + +def errors_of(logger) -> str: + return " ".join(str(call.args[0]) % call.args[1:] if len(call.args) > 1 + else str(call.args[0]) + for call in logger.error.call_args_list) + + +@pytest.fixture +def devices(): + return FakeIndigoDevices([ + RelayDevice(101, "Study Plug", onState=False), + DimmerDevice(102, "Hall Dimmer", onState=True, brightness=40), + SprinklerDevice(104, "Irrigation"), + RelayDevice(105, "Matter Plug", plugin_id=OURS), + ]) + + +# --------------------------------------------------------------------------- +# The attach-timeout formula +# --------------------------------------------------------------------------- +class TestAttachTimeout: + """E3a's pacing×count interaction, answered without a protocol change.""" + + def test_small_sets_keep_the_flat_floor(self): + """The floor is over the count SENT — which is not always the count paced. + + ``attach_timeout_for(0)`` returning the floor is correct arithmetic and + wrong as a deadline for the ONE caller that sends zero endpoints on + purpose: the §3.1 ``replace_all`` un-export sends nothing and makes the + node remove everything. That path must size its own deadline over the + removals — see + ``TestLifecycle.test_the_un_export_deadline_is_sized_by_the_removals``. + """ + assert bridge_client.attach_timeout_for(0) == bridge_client.ATTACH_TIMEOUT + assert bridge_client.attach_timeout_for(20) == bridge_client.ATTACH_TIMEOUT + + def test_a_large_set_gets_more_time_than_its_pacing_costs(self): + # ~100ms per removal (§3.3) means 80 endpoints can spend 8s in pacing + # alone — precisely the flat deadline it would otherwise be given. + assert bridge_client.attach_timeout_for(80) > 80 * 0.1 + assert bridge_client.attach_timeout_for(80) == pytest.approx(14.0) + + def test_the_crossover_off_the_floor_is_where_the_arithmetic_says(self): + """T3: 2.0 + 0.15n passes 8.0 between 40 and 41, and nowhere else. + + Pinned because the crossover is the only observable consequence of the + two constants — tune either and this is the test that notices. + """ + assert bridge_client.attach_timeout_for(40) == bridge_client.ATTACH_TIMEOUT + assert bridge_client.attach_timeout_for(41) > bridge_client.ATTACH_TIMEOUT + assert bridge_client.attach_timeout_for(41) == pytest.approx(8.15) + + def test_it_is_monotonic_and_never_negative(self): + values = [bridge_client.attach_timeout_for(n) for n in range(0, 200, 10)] + assert values == sorted(values) + assert bridge_client.attach_timeout_for(-5) == bridge_client.ATTACH_TIMEOUT + + +# --------------------------------------------------------------------------- +# The endpoint provider (§3.1 reconcile source) +# --------------------------------------------------------------------------- +class TestEndpointProvider: + def test_builds_a_spec_from_the_live_device(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(102, "dimmableLight")]) + (spec,) = h.bridge.endpoint_specs() + assert spec.indigo_device_id == 102 + assert spec.role == "dimmableLight" + assert spec.label == "Hall Dimmer" + assert spec.reachable is True + assert spec.states == {"onOff": True, "level": 40} + + def test_the_name_override_wins_over_the_indigo_name(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(101, "onOffLight", name_override="Desk Lamp")]) + assert h.bridge.endpoint_specs()[0].label == "Desk Lamp" + + def test_options_ride_along(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, []) + h.store.upsert(ExportEntry(102, "dimmableLight", options={"someKey": 1})) + assert h.bridge.endpoint_specs()[0].options == {"someKey": 1} + + def test_a_disabled_device_is_unreachable_not_absent(self, bridge_mod, mock_logger, devices): + """XAC8: greyed out in the ecosystem beats timing out.""" + devices[101].enabled = False + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + assert h.bridge.endpoint_specs()[0].reachable is False + + def test_an_unconfigured_device_is_unreachable(self, bridge_mod, mock_logger, devices): + devices[101].configured = False + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + assert h.bridge.endpoint_specs()[0].reachable is False + + def test_a_deleted_device_is_skipped_with_a_warning(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(999, "onOffLight")]) + assert h.bridge.endpoint_specs() == [] + assert "no longer exists" in warnings_of(mock_logger) + + def test_a_now_excluded_device_is_skipped_with_its_reason(self, bridge_mod, mock_logger, + devices): + """The store is NOT the guard — classify is, on every attach.""" + h = Harness(bridge_mod, mock_logger, devices, []) + # A sprinkler cannot be exported at all, so it can only get in by a + # restored backup or a hand-edited pref — exactly what this covers. + h.store.upsert(ExportEntry(104, "onOffLight")) + assert h.bridge.endpoint_specs() == [] + assert export_catalog.REASON_SPRINKLER in warnings_of(mock_logger) + + def test_our_own_device_never_reaches_the_node(self, bridge_mod, mock_logger, devices): + """XAC6/XNG3 — the loop guard, re-run at endpoint-build time.""" + h = Harness(bridge_mod, mock_logger, devices, []) + h.store.upsert(ExportEntry(105, "onOffLight")) + assert h.bridge.endpoint_specs() == [] + assert export_catalog.REASON_LOOP_GUARD in warnings_of(mock_logger) + + def test_a_role_the_device_no_longer_offers_is_skipped(self, bridge_mod, mock_logger, + devices): + h = Harness(bridge_mod, mock_logger, devices, []) + # A plain relay never offers dimmableLight; the user's dimmer was + # replaced by a relay under the same device id. + h.store.upsert(ExportEntry(101, "dimmableLight")) + assert h.bridge.endpoint_specs() == [] + assert "no longer offers" in warnings_of(mock_logger) + + def test_an_e4_role_is_skipped_rather_than_failing_the_whole_attach( + self, bridge_mod, mock_logger, devices): + """E3a: an unknown role fails the ENTIRE attach with ``internal``.""" + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(101, "doorLock"), ExportEntry(102, "dimmableLight")]) + specs = h.bridge.endpoint_specs() + assert [s.indigo_device_id for s in specs] == [102], "the good export must survive" + assert "cannot be bridged yet" in warnings_of(mock_logger) + + def test_the_same_skip_is_warned_once_not_once_per_reconnect(self, bridge_mod, mock_logger, + devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "doorLock")]) + for _ in range(5): + h.bridge.endpoint_specs() + assert mock_logger.warning.call_count == 1 + + def test_a_changed_skip_reason_is_warned_again(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "doorLock")]) + h.bridge.endpoint_specs() + h.store.upsert(ExportEntry(101, "dimmableLight")) # now a different failure + h.bridge.endpoint_specs() + assert mock_logger.warning.call_count == 2 + + +# --------------------------------------------------------------------------- +# Client lifecycle (XG5) +# --------------------------------------------------------------------------- +class TestLifecycle: + def test_an_empty_allow_list_starts_nothing(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, []) + h.bridge.exports_changed() + assert h.bridge.active is False + assert h.clients == [] + + def test_the_first_export_starts_the_client(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, []) + h.store.upsert(ExportEntry(101, "onOffLight")) + h.bridge.exports_changed() + assert h.bridge.active is True + assert h.client.ran is True + + def test_starting_twice_is_a_no_op(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + h.bridge.start() + h.bridge.start() + assert len(h.clients) == 1 + + def test_emptying_the_allow_list_un_exports_deliberately_then_stops( + self, bridge_mod, mock_logger, devices): + """PRD §7: endpoints go, pairings stay — and it needs the §3.1 opt-in.""" + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + client = h.start() + h.store.remove(101) + h.bridge.exports_changed() + assert client.only("attach") == ("attach", [], True), "must carry replace_all" + assert client.closed is True + assert h.bridge.active is False + + def test_the_un_export_deadline_is_sized_by_the_removals_not_the_empty_send( + self, bridge_mod, mock_logger, devices): + """X1: attach latency is dominated by REMOVALS, and this path removes all. + + The endpoint list sent is ``[]``, so letting the client derive the + deadline from it hands the un-export of a 60-device database the flat 8s + floor — while the node paces those 60 removals ~100ms apart (§3.3). It + times out, warns that accessories "may linger", and then ``close()`` + yanks the socket out from under a reconcile that was going fine. + """ + entries = [ExportEntry(200 + n, "onOffLight") for n in range(60)] + h = Harness(bridge_mod, mock_logger, devices, entries) + client = h.start() + for entry in entries: + h.store.remove(entry.indigo_device_id) + h.bridge.exports_changed() + + assert client.attach_timeouts == [bridge_client.attach_timeout_for(60)] + assert client.attach_timeouts[0] > 60 * 0.1, "must outlast the node's pacing" + assert client.attach_timeouts[0] > bridge_client.ATTACH_TIMEOUT + assert "may linger" not in warnings_of(mock_logger) + + def test_a_small_un_export_still_gets_the_floor(self, bridge_mod, mock_logger, devices): + """Sizing by removals must not make the ordinary case slower to fail.""" + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + client = h.start() + h.store.remove(101) + h.bridge.exports_changed() + assert client.attach_timeouts == [bridge_client.ATTACH_TIMEOUT] + + def test_the_client_is_dropped_even_if_the_final_attach_fails( + self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + client = h.start() + client.fail["attach"] = ConnectionError("node is gone") + h.store.remove(101) + h.bridge.exports_changed() + assert h.bridge.active is False + assert "may linger" in warnings_of(mock_logger) + # F4: the socket must still be released — the client is unreachable from + # here on, so a skipped close() leaks it until the plugin reloads. + assert client.closed is True + + def test_stop_is_idempotent(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + h.start() + h.bridge.stop() + h.bridge.stop() + assert h.bridge.active is False + + def test_the_client_gets_the_ws_port_pref(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + h.prefs[bridge_protocol.PREF_WS_PORT] = "5999" + assert h.start().prefs[bridge_protocol.PREF_WS_PORT] == "5999" + + +# --------------------------------------------------------------------------- +# Indigo → node +# --------------------------------------------------------------------------- +class TestDeviceUpdated: + def _harness(self, bridge_mod, mock_logger, devices, entry): + h = Harness(bridge_mod, mock_logger, devices, [entry]) + h.start() + return h + + def test_a_state_change_becomes_a_set_state(self, bridge_mod, mock_logger, devices): + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "onOffLight")) + before = RelayDevice(101, "Study Plug", onState=False) + after = RelayDevice(101, "Study Plug", onState=True) + h.bridge.device_updated(before, after) + assert h.client.only("set_state") == ("set_state", 101, {"onOff": True}) + + def test_a_level_change_becomes_a_set_state(self, bridge_mod, mock_logger, devices): + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(102, "dimmableLight")) + before = DimmerDevice(102, "Hall Dimmer", onState=True, brightness=40) + after = DimmerDevice(102, "Hall Dimmer", onState=True, brightness=90) + h.bridge.device_updated(before, after) + assert h.client.only("set_state") == ("set_state", 102, {"level": 90}) + + def test_an_unchanged_device_sends_nothing(self, bridge_mod, mock_logger, devices): + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "onOffLight")) + same = RelayDevice(101, "Study Plug", onState=True) + h.bridge.device_updated(same, RelayDevice(101, "Study Plug", onState=True)) + assert h.client.names() == [] + + def test_a_rename_re_sends_the_spec_so_the_label_follows(self, bridge_mod, mock_logger, + devices): + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "onOffLight")) + devices[101].name = "Desk Plug" + h.bridge.device_updated(RelayDevice(101, "Study Plug"), devices[101]) + _name, spec = h.client.only("upsert_endpoint") + assert spec.label == "Desk Plug" + + def test_a_rename_is_ignored_when_the_user_pinned_a_name(self, bridge_mod, mock_logger, + devices): + h = self._harness(bridge_mod, mock_logger, devices, + ExportEntry(101, "onOffLight", name_override="Desk Lamp")) + devices[101].name = "Something Else" + h.bridge.device_updated(RelayDevice(101, "Study Plug"), devices[101]) + assert "upsert_endpoint" not in h.client.names() + + def test_disabling_a_device_sets_reachable_false(self, bridge_mod, mock_logger, devices): + """XAC8 groundwork — the split §3.5 command, not a cluster state.""" + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "onOffLight")) + before = RelayDevice(101, "Study Plug", enabled=True) + after = RelayDevice(101, "Study Plug", enabled=False) + h.bridge.device_updated(before, after) + assert h.client.only("set_reachable") == ("set_reachable", 101, False) + + def test_re_enabling_a_device_sets_reachable_true(self, bridge_mod, mock_logger, devices): + """T3: the other direction of XAC8 — a device that comes back must say so. + + A one-way ``set_reachable`` leaves the accessory greyed out in every + ecosystem forever, which looks exactly like a dead device. + """ + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "onOffLight")) + before = RelayDevice(101, "Study Plug", enabled=False) + after = RelayDevice(101, "Study Plug", enabled=True) + h.bridge.device_updated(before, after) + assert h.client.only("set_reachable") == ("set_reachable", 101, True) + + def test_an_e4_role_update_is_skipped_silently(self, bridge_mod, mock_logger, devices): + """T3: the provider already warned; repeating it per state change is noise.""" + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "doorLock")) + mock_logger.reset_mock() + h.bridge.device_updated(RelayDevice(101, "P", onState=False), + RelayDevice(101, "P", onState=True)) + assert h.client.names() == [] + assert mock_logger.warning.call_count == 0 + + def test_a_device_removed_between_the_guard_and_here_is_dropped(self, bridge_mod, + mock_logger, devices): + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "onOffLight")) + h.store.remove(101) + h.bridge.device_updated(RelayDevice(101, "P", onState=False), + RelayDevice(101, "P", onState=True)) + assert h.client.names() == [] + + def test_pushes_are_never_awaited(self, bridge_mod, mock_logger, devices): + """§3.4 — a state push must not make Indigo's thread wait on Matter.""" + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "onOffLight")) + h.client.fail["set_state"] = ConnectionError("socket died mid-write") + h.bridge.device_updated(RelayDevice(101, "P", onState=False), + RelayDevice(101, "P", onState=True)) + # It failed, it did not raise into Indigo, and it was NOT silent. + assert "set_state dev 101 failed" in warnings_of(mock_logger) + + def test_nothing_is_sent_before_the_attach_completes(self, bridge_mod, mock_logger, devices): + """An incremental frame sent un-attached is refused (§1.1) and pointless.""" + h = self._harness(bridge_mod, mock_logger, devices, ExportEntry(101, "onOffLight")) + h.client.attached = False + h.bridge.device_updated(RelayDevice(101, "P", onState=False), + RelayDevice(101, "P", onState=True)) + h.bridge.upsert(101) + h.bridge.remove(101) + assert h.client.names() == [] + + +class TestIncrementalCrud: + def test_upsert_sends_the_current_spec(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(102, "dimmableLight")]) + h.start() + h.bridge.upsert(102) + _name, spec = h.client.only("upsert_endpoint") + assert spec.indigo_device_id == 102 and spec.role == "dimmableLight" + + def test_upsert_of_an_unbridgeable_export_sends_nothing(self, bridge_mod, mock_logger, + devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "doorLock")]) + h.start() + h.bridge.upsert(101) + assert h.client.names() == [] + + def test_remove_drops_the_endpoint(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + h.start() + h.bridge.remove(101) + assert h.client.only("remove_endpoint") == ("remove_endpoint", 101) + + def test_a_role_change_is_a_remove_then_an_add(self, bridge_mod, mock_logger, devices): + """§4.1 rejects a role change in place — ecosystems cache the type.""" + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffPlugInUnit")]) + h.start() + h.store.upsert(ExportEntry(101, "onOffLight")) + h.bridge.replace(101) + assert h.client.names() == ["remove_endpoint", "upsert_endpoint"] + assert h.client.calls[1][1].role == "onOffLight" + + def test_a_role_change_for_a_vanished_entry_only_removes(self, bridge_mod, mock_logger, + devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(102, "dimmableLight")]) + h.start() + h.store.remove(102) + h.bridge.replace(102) + assert h.client.names() == ["remove_endpoint"] + + +# --------------------------------------------------------------------------- +# Node → Indigo (§5 command events) +# --------------------------------------------------------------------------- +class TestOnCommand: + def _deliver(self, h, frame_name): + data = FRAMES[frame_name]["data"] + h.bridge.on_command(bridge_protocol.parse_command(data)) + + def test_an_on_off_command_reaches_the_device(self, bridge_mod, mock_logger, devices, + mock_indigo_base): + devices.add(RelayDevice(123456789, "Golden Plug")) + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(123456789, "onOffLight")]) + self._deliver(h, "command_on_off") + mock_indigo_base.device.turnOn.assert_called_once_with(devices[123456789]) + + def test_a_set_level_command_reaches_the_device(self, bridge_mod, mock_logger, devices, + mock_indigo_base): + devices.add(DimmerDevice(123456789, "Golden Lamp")) + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(123456789, "dimmableLight")]) + self._deliver(h, "command_set_level") + mock_indigo_base.dimmer.setBrightness.assert_called_once_with( + devices[123456789], value=60) + + def test_a_set_color_temp_command_reaches_the_device(self, bridge_mod, mock_logger, devices, + mock_indigo_base): + devices.add(DimmerDevice(900004, "Golden CT", whiteLevel=70, + supportsWhiteTemperature=True)) + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(900004, "colorTemperatureLight")]) + self._deliver(h, "command_set_color_temp") + _args, kwargs = mock_indigo_base.dimmer.setColorLevels.call_args + assert kwargs["whiteTemperature"] == 3125 # 1e6 / 320 mireds + assert kwargs["whiteLevel"] == 70 + + def test_a_set_color_command_reaches_the_device(self, bridge_mod, mock_logger, devices, + mock_indigo_base): + devices.add(DimmerDevice(900005, "Golden RGB", supportsRGB=True)) + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(900005, "extendedColorLight")]) + self._deliver(h, "command_set_color") + _args, kwargs = mock_indigo_base.dimmer.setColorLevels.call_args + assert set(kwargs) == {"redLevel", "greenLevel", "blueLevel"} + assert kwargs["blueLevel"] == 100 # hue 210, saturation 80 + + def test_a_command_for_an_unexported_device_is_refused_with_a_warning( + self, bridge_mod, mock_logger, devices, mock_indigo_base): + """PRD §7 race row, against the golden frame for exactly this case.""" + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + self._deliver(h, "command_unexported_device") + assert "not exported" in warnings_of(mock_logger) + mock_indigo_base.device.turnOff.assert_not_called() + + def test_a_command_for_a_vanished_device_is_refused(self, bridge_mod, mock_logger, devices, + mock_indigo_base): + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(123456789, "onOffLight")]) # no such Indigo device + self._deliver(h, "command_on_off") + assert "no longer exists" in warnings_of(mock_logger) + mock_indigo_base.device.turnOn.assert_not_called() + + def test_a_command_the_role_does_not_define_is_refused(self, bridge_mod, mock_logger, + devices, mock_indigo_base): + devices.add(RelayDevice(123456789, "Golden Plug")) + h = Harness(bridge_mod, mock_logger, devices, + [ExportEntry(123456789, "onOffLight")]) + self._deliver(h, "command_set_level") # a light that cannot dim + assert "does not define" in warnings_of(mock_logger) + mock_indigo_base.dimmer.setBrightness.assert_not_called() + + def test_a_command_for_an_e4_role_is_refused_not_attempted(self, bridge_mod, mock_logger, + devices, mock_indigo_base): + """The lock seam: E4 owns it, and nothing here may auto-confirm it.""" + lock_id = FRAMES["command_lock"]["data"]["indigoDeviceId"] + devices.add(RelayDevice(lock_id, "Front Door")) + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(lock_id, "doorLock")]) + h.bridge.on_command(bridge_protocol.parse_command(FRAMES["command_lock"]["data"])) + assert "cannot bridge" in warnings_of(mock_logger) + mock_indigo_base.device.lock.assert_not_called() + + def test_a_failing_dispatch_is_logged_not_raised(self, bridge_mod, mock_logger, devices, + mock_indigo_base): + devices.add(RelayDevice(123456789, "Golden Plug")) + mock_indigo_base.device.turnOn.side_effect = RuntimeError("server said no") + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(123456789, "onOffLight")]) + self._deliver(h, "command_on_off") + assert "server said no" in errors_of(mock_logger) + + +# --------------------------------------------------------------------------- +# Failure surfacing +# --------------------------------------------------------------------------- +class TestFailureSurfacing: + def _bridge(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + h.start() + return h + + def test_a_refused_attach_names_the_code(self, bridge_mod, mock_logger, devices): + h = self._bridge(bridge_mod, mock_logger, devices) + h.bridge._on_attach_refused(bridge_protocol.ERR_MASS_REMOVAL_REFUSED, "no intent") + assert bridge_protocol.ERR_MASS_REMOVAL_REFUSED in errors_of(mock_logger) + + def test_an_invalid_endpoint_map_says_what_a_rebuild_costs(self, bridge_mod, mock_logger, + devices): + h = self._bridge(bridge_mod, mock_logger, devices) + h.bridge._on_attach_refused(bridge_protocol.ERR_ENDPOINT_MAP_INVALID, "unreadable") + assert "duplicate accessories" in errors_of(mock_logger) + + def test_version_skew_says_restart_the_agent(self, bridge_mod, mock_logger, devices): + h = self._bridge(bridge_mod, mock_logger, devices) + h.bridge._on_version_skew(bridge_protocol.Hello(2, "9.9.9", "1.0")) + assert "restart the bridge agent" in errors_of(mock_logger) + + def test_drift_is_reported_never_repaired(self, bridge_mod, mock_logger, devices): + h = self._bridge(bridge_mod, mock_logger, devices) + h.bridge._on_drift_detected(bridge_protocol.parse_drift( + FRAMES["drift_detected"]["data"]["drift"])) + assert "DRIFT" in errors_of(mock_logger) + assert h.client.names() == [], "drift must not trigger a repair" + + def test_an_unreachable_node_is_reported_once_per_outage(self, bridge_mod, mock_logger, + devices): + h = self._bridge(bridge_mod, mock_logger, devices) + for attempt in range(1, 6): + h.bridge._on_unreachable(attempt) + assert mock_logger.warning.call_count == 1 + assert "started by hand" in warnings_of(mock_logger) + + +class TestHealthTick: + def _bridge(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + h.start() + return h + + def test_an_inactive_bridge_says_nothing(self, bridge_mod, mock_logger, devices): + h = Harness(bridge_mod, mock_logger, devices, []) + h.bridge.health_tick() + assert mock_logger.warning.call_count == 0 + + def test_an_attached_client_says_nothing(self, bridge_mod, mock_logger, devices): + h = self._bridge(bridge_mod, mock_logger, devices) + for _ in range(10): + h.bridge.health_tick() + assert mock_logger.warning.call_count == 0 + + def test_a_disconnected_client_warns_once_after_about_a_minute(self, bridge_mod, mock_logger, + devices): + h = self._bridge(bridge_mod, mock_logger, devices) + h.client.attached = False + for _ in range(10): + h.bridge.health_tick() + assert mock_logger.warning.call_count == 1 + assert "~1 min" in warnings_of(mock_logger) + + def test_a_halted_client_says_so_once_per_streak(self, bridge_mod, mock_logger, devices): + """Halted is not transient: nothing is coming to fix it on its own. + + F9: which is exactly why it must not be said every 15s tick, forever — + the state never changes, so the repeat carries no new information and + buries everything else in the event log. + """ + h = self._bridge(bridge_mod, mock_logger, devices) + h.client.halted = True + h.client.halted_reason = "version_skew" + for _ in range(20): + h.bridge.health_tick() + assert mock_logger.warning.call_count == 1 + assert "HALTED" in warnings_of(mock_logger) + assert "version_skew" in warnings_of(mock_logger) + + def test_a_recovered_client_can_warn_again(self, bridge_mod, mock_logger, devices): + """Once per STREAK, not once per process: a second outage must be heard.""" + h = self._bridge(bridge_mod, mock_logger, devices) + h.client.halted = True + h.bridge.health_tick() + h.client.halted = False + h.bridge._on_attached(bridge_protocol.StatusReport( + commissioned=True, fabrics=[], endpoint_count=1, endpoints=[], drift=[])) + h.client.halted = True + h.bridge.health_tick() + assert mock_logger.warning.call_count == 2 + + def test_the_recovery_state_says_nothing_is_exported_once(self, bridge_mod, mock_logger, + devices): + h = self._bridge(bridge_mod, mock_logger, devices) + h.client.attached = False + h.client.recovery = True + for _ in range(20): + h.bridge.health_tick() + assert mock_logger.warning.call_count == 1 + assert "rebuild" in warnings_of(mock_logger) + + +# --------------------------------------------------------------------------- +# Nothing that stops export may be silent (the PR #124 silent-failure sweep) +# --------------------------------------------------------------------------- +class TestSilentFailures: + """Every path here used to `return` with no trace of what was dropped.""" + + def _bridge(self, bridge_mod, mock_logger, devices, role="onOffLight"): + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, role)]) + h.start() + return h + + def test_a_drop_while_merely_unattached_names_the_device_at_debug( + self, bridge_mod, mock_logger, devices): + """F1: attach WILL reconcile this, so it is debug — but not silence.""" + h = self._bridge(bridge_mod, mock_logger, devices) + h.client.attached = False + h.bridge.upsert(101) + debug = " ".join(str(call.args[0]) % call.args[1:] if len(call.args) > 1 + else str(call.args[0]) + for call in mock_logger.debug.call_args_list) + assert "101" in debug + assert mock_logger.warning.call_count == 0 + + @pytest.mark.parametrize("state,expected", [ + ("halted", "HALTED (version_skew)"), + ("recovery", "endpoint-map"), + ]) + def test_a_drop_while_halted_or_in_recovery_is_a_warning_with_the_reason( + self, bridge_mod, mock_logger, devices, state, expected): + """F1: the client's own loud path is unreachable from here — replicate it. + + ``_live_client`` gates before ``set_state`` is ever called, so + ``BridgeClient._log_dropped_state_push``'s warning is dead code on this + route: a halted bridge dropped every push in total silence. + """ + h = self._bridge(bridge_mod, mock_logger, devices) + h.client.attached = False + setattr(h.client, state, True) + h.client.halted_reason = "version_skew" + h.bridge.device_updated(RelayDevice(101, "P", onState=False), + RelayDevice(101, "P", onState=True)) + assert expected in warnings_of(mock_logger) + assert "101" in warnings_of(mock_logger) + + def test_the_halted_drop_warning_is_once_per_streak(self, bridge_mod, mock_logger, devices): + h = self._bridge(bridge_mod, mock_logger, devices) + h.client.attached = False + h.client.halted = True + for _ in range(10): + h.bridge.device_updated(RelayDevice(101, "P", onState=False), + RelayDevice(101, "P", onState=True)) + assert mock_logger.warning.call_count == 1 + + def test_a_failed_run_loop_schedule_is_a_warning_not_a_debug_line( + self, bridge_mod, mock_logger, devices): + """F2: if run() never got scheduled, NOTHING is exported, ever.""" + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + h.runtime.is_running = False + h.bridge.start() + assert "bridge client run loop" in warnings_of(mock_logger) + + def test_a_failed_un_export_schedule_is_a_warning(self, bridge_mod, mock_logger, devices): + """F2: the accessories stay in every paired ecosystem, unexplained.""" + h = self._bridge(bridge_mod, mock_logger, devices) + h.runtime.is_running = False + h.store.remove(101) + h.bridge.exports_changed() + assert "un-exporting everything" in warnings_of(mock_logger) + + def test_an_ordinary_push_that_cannot_be_scheduled_stays_at_debug( + self, bridge_mod, mock_logger, devices): + """F2: a set_state is re-delivered by the next attach; it is not a loss.""" + h = self._bridge(bridge_mod, mock_logger, devices) + h.runtime.is_running = False + h.bridge.device_updated(RelayDevice(101, "P", onState=False), + RelayDevice(101, "P", onState=True)) + assert mock_logger.warning.call_count == 0 + assert mock_logger.debug.called + + def test_a_failed_dispatch_corrects_the_ecosystem_back_to_the_truth( + self, bridge_mod, mock_logger, devices, mock_indigo_base): + """F5: otherwise Home shows the light the user asked for and Indigo does not. + + The ecosystem applied the command optimistically the moment it sent it. + Logging and returning leaves those two beliefs permanently split until + something else happens to that device. + """ + devices.add(RelayDevice(123456789, "Golden Plug", onState=False)) + mock_indigo_base.device.turnOn.side_effect = RuntimeError("server said no") + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(123456789, "onOffLight")]) + h.start() + h.bridge.on_command(bridge_protocol.parse_command(FRAMES["command_on_off"]["data"])) + assert h.client.only("set_state") == ("set_state", 123456789, {"onOff": False}) + assert "still shows" in errors_of(mock_logger) + + def test_a_state_read_failure_dedupes_on_the_reason_not_the_message( + self, bridge_mod, mock_logger, devices): + """F8b: a varying ``str(exc)`` in the key defeats the dedupe entirely.""" + h = Harness(bridge_mod, mock_logger, devices, [ExportEntry(101, "onOffLight")]) + calls = {"n": 0} + + def exploding_states_for(_dev): + calls["n"] += 1 + raise RuntimeError(f"transient read error #{calls['n']}") + + handler = bridge_mod.export_handlers.handler_for("onOffLight") + original, handler.states_for = handler.states_for, exploding_states_for + try: + for _ in range(5): + h.bridge.endpoint_specs() + finally: + handler.states_for = original + assert mock_logger.warning.call_count == 1 + assert "transient read error #1" in warnings_of(mock_logger), \ + "the varying detail belongs in the LINE, just not in the key" + + def test_a_non_terminal_attach_refusal_is_reported_once_per_streak( + self, bridge_mod, mock_logger, devices): + """F8c: a transient refusal reconnects on backoff — this fires every cycle.""" + h = self._bridge(bridge_mod, mock_logger, devices) + for _ in range(10): + h.bridge._on_attach_refused(bridge_protocol.ERR_INTERNAL, "node is confused") + assert mock_logger.error.call_count == 1 + + def test_a_terminal_refusal_is_still_said_every_time(self, bridge_mod, mock_logger, devices): + """Terminal refusals do not loop, and each one is a distinct decision.""" + h = self._bridge(bridge_mod, mock_logger, devices) + h.bridge._on_attach_refused(bridge_protocol.ERR_ENDPOINT_MAP_INVALID, "unreadable") + h.bridge._on_attach_refused(bridge_protocol.ERR_ENDPOINT_MAP_INVALID, "unreadable") + assert mock_logger.error.call_count == 2 + + def test_a_reattach_lets_a_refusal_be_reported_again(self, bridge_mod, mock_logger, devices): + h = self._bridge(bridge_mod, mock_logger, devices) + h.bridge._on_attach_refused(bridge_protocol.ERR_INTERNAL, "node is confused") + h.bridge._on_attached(bridge_protocol.StatusReport( + commissioned=True, fabrics=[], endpoint_count=1, endpoints=[], drift=[])) + h.bridge._on_attach_refused(bridge_protocol.ERR_INTERNAL, "node is confused again") + assert mock_logger.error.call_count == 2 + + def test_a_failing_diff_names_the_device_and_repeats_once_per_streak( + self, bridge_mod, mock_logger, devices): + """F6/F7: a bare traceback per state change tells you nothing and never stops.""" + h = self._bridge(bridge_mod, mock_logger, devices) + handler = bridge_mod.export_handlers.handler_for("onOffLight") + original = handler.diff + handler.diff = lambda _o, _n: (_ for _ in ()).throw(RuntimeError("bad device")) + try: + for _ in range(10): + h.bridge.device_updated(RelayDevice(101, "Study Plug", onState=False), + RelayDevice(101, "Study Plug", onState=True)) + finally: + handler.diff = original + assert mock_logger.exception.call_count == 1 + assert "Study Plug" in errors_of(mock_logger) + assert "101" in errors_of(mock_logger) diff --git a/tests/test_export_handlers.py b/tests/test_export_handlers.py new file mode 100644 index 0000000..e9acad5 --- /dev/null +++ b/tests/test_export_handlers.py @@ -0,0 +1,448 @@ +"""E3: the per-role OUTBOUND handler table (`export_handlers`). + +Two layers, deliberately: + +* a **zoo table test** over every implemented role, asserting the invariants + that make PRD §XG7 true — every E3 role has a handler, every state key it + emits is in that role's §4.2 vocabulary, and every §4.2 command that role + declares is dispatchable. Adding a role without its vocabularies fails here, + which is the point; +* per-role behaviour: what ``states_for`` reads off a real-shaped Indigo device, + what ``diff`` decides has changed, and which ``indigo.*`` call ``dispatch`` + actually makes. + +References to ``§N`` are ``docs/BRIDGE_PROTOCOL.md``. +""" +from __future__ import annotations + +import importlib + +import pytest + +import bridge_protocol +from fakes import DimmerDevice, RelayDevice + + +@pytest.fixture +def handlers(mock_indigo_base): + """`export_handlers` bound to a mocked ``indigo`` module. + + The module imports ``indigo`` at top level (it has to — it calls + ``indigo.device.turnOn``), so it must be reloaded after the mock is in + ``sys.modules``. Reload mutates the module object in place, so anything + already holding a reference to it sees the rebound ``indigo`` too. + """ + import export_handlers as module + importlib.reload(module) + return module + + +# --------------------------------------------------------------------------- +# The zoo (invariants over the whole table) +# --------------------------------------------------------------------------- +E3_ROLES = ( + "onOffPlugInUnit", "onOffLight", "dimmableLight", + "colorTemperatureLight", "extendedColorLight", +) + + +def test_every_e3_role_has_a_handler(handlers): + assert set(handlers.HANDLERS) == set(E3_ROLES) + + +def test_no_handler_claims_a_role_outside_the_protocol_enum(handlers): + assert set(handlers.HANDLERS) <= bridge_protocol.ROLES + + +@pytest.mark.parametrize("role", E3_ROLES) +def test_the_handler_is_registered_under_its_own_role(handlers, role): + assert handlers.HANDLERS[role].role == role + + +@pytest.mark.parametrize("role", E3_ROLES) +def test_state_keys_are_a_subset_of_the_roles_4_2_vocabulary(handlers, role): + """A key the node does not know for this role is dropped on the floor.""" + dev = _fully_populated_device() + produced = set(handlers.HANDLERS[role].states_for(dev)) + allowed = set(bridge_protocol.ROLE_STATE_KEYS[role]) + assert produced <= allowed, f"{role} emits {produced - allowed}" + + +@pytest.mark.parametrize("role", E3_ROLES) +def test_a_capable_device_produces_the_whole_vocabulary(handlers, role): + """The other direction: a device that CAN answer every key must answer it. + + Subset-only would pass for a handler that silently stopped reporting + brightness, which an ecosystem renders as a light stuck at its last level. + """ + dev = _fully_populated_device() + assert set(handlers.HANDLERS[role].states_for(dev)) == \ + set(bridge_protocol.ROLE_STATE_KEYS[role]) + + +@pytest.mark.parametrize("role", E3_ROLES) +def test_dispatch_covers_every_command_the_role_declares(handlers, role): + assert set(handlers.HANDLERS[role].commands()) == set(bridge_protocol.ROLE_COMMANDS[role]) + + +@pytest.mark.parametrize("role", sorted(bridge_protocol.ROLES - set(E3_ROLES))) +def test_an_e4_role_is_explicitly_unbridgeable(handlers, role): + """E4 roles must answer ``None``, not raise and not half-work. + + The allow-list can already hold them — the §5.1 dialog offers doorLock, + windowCovering and the sensors as roles — and an unknown role fails the + WHOLE attach on the node (E3a), so the caller has to be able to ask. + """ + assert handlers.handler_for(role) is None + assert not handlers.is_bridgeable(role) + + +def _fully_populated_device(): + """A dimmer that can answer every E3 state key.""" + return DimmerDevice(1, "Zoo Lamp", onState=True, brightness=60, + redLevel=100, greenLevel=0, blueLevel=0, + whiteLevel=80, whiteTemperature=2700, + supportsRGB=True, supportsWhiteTemperature=True) + + +# --------------------------------------------------------------------------- +# onOff (plug + light) +# --------------------------------------------------------------------------- +class TestOnOff: + def test_states_read_on_state(self, handlers): + handler = handlers.handler_for("onOffPlugInUnit") + assert handler.states_for(RelayDevice(1, "Plug", onState=True)) == {"onOff": True} + assert handler.states_for(RelayDevice(1, "Plug", onState=False)) == {"onOff": False} + + def test_diff_is_empty_when_nothing_moved(self, handlers): + handler = handlers.handler_for("onOffLight") + before = RelayDevice(1, "Lamp", onState=True) + after = RelayDevice(1, "Lamp", onState=True) + assert handler.diff(before, after) == {} + + def test_diff_reports_the_flip(self, handlers): + handler = handlers.handler_for("onOffLight") + assert handler.diff(RelayDevice(1, "L", onState=False), + RelayDevice(1, "L", onState=True)) == {"onOff": True} + + def test_dispatch_true_turns_on(self, handlers, mock_indigo_base): + dev = RelayDevice(1, "Plug") + assert handlers.handler_for("onOffPlugInUnit").dispatch("onOff", {"value": True}, dev) + mock_indigo_base.device.turnOn.assert_called_once_with(dev) + mock_indigo_base.device.turnOff.assert_not_called() + + def test_dispatch_false_turns_off(self, handlers, mock_indigo_base): + dev = RelayDevice(1, "Plug") + assert handlers.handler_for("onOffPlugInUnit").dispatch("onOff", {"value": False}, dev) + mock_indigo_base.device.turnOff.assert_called_once_with(dev) + mock_indigo_base.device.turnOn.assert_not_called() + + def test_an_unknown_command_is_refused_not_guessed(self, handlers, mock_indigo_base): + dev = RelayDevice(1, "Plug") + assert handlers.handler_for("onOffPlugInUnit").dispatch("setLevel", {"level": 5}, dev) \ + is False + mock_indigo_base.device.turnOn.assert_not_called() + + +# --------------------------------------------------------------------------- +# dimmableLight +# --------------------------------------------------------------------------- +class TestDimmable: + def test_states_carry_on_off_and_level(self, handlers): + dev = DimmerDevice(1, "Lamp", onState=True, brightness=42) + assert handlers.handler_for("dimmableLight").states_for(dev) == \ + {"onOff": True, "level": 42} + + def test_level_is_omitted_when_the_device_cannot_answer(self, handlers): + """A relay exported as a dimmable light has no ``brightness`` at all.""" + dev = RelayDevice(1, "Plug", onState=True) + assert handlers.handler_for("dimmableLight").states_for(dev) == {"onOff": True} + + def test_diff_reports_only_the_changed_key(self, handlers): + handler = handlers.handler_for("dimmableLight") + before = DimmerDevice(1, "Lamp", onState=True, brightness=40) + after = DimmerDevice(1, "Lamp", onState=True, brightness=75) + assert handler.diff(before, after) == {"level": 75} + + def test_dispatch_sets_brightness_on_the_same_0_100_scale(self, handlers, mock_indigo_base): + dev = DimmerDevice(1, "Lamp") + assert handlers.handler_for("dimmableLight").dispatch("setLevel", {"level": 60}, dev) + mock_indigo_base.dimmer.setBrightness.assert_called_once_with(dev, value=60) + + def test_dispatch_clamps_an_out_of_range_level(self, handlers, mock_indigo_base): + dev = DimmerDevice(1, "Lamp") + handlers.handler_for("dimmableLight").dispatch("setLevel", {"level": 140}, dev) + mock_indigo_base.dimmer.setBrightness.assert_called_once_with(dev, value=100) + + def test_dispatch_of_level_zero_is_an_off_not_a_missing_level(self, handlers, + mock_indigo_base): + """T3: 0 is falsy, and the guard has to be a type check, not a truth test.""" + dev = DimmerDevice(1, "Lamp") + assert handlers.handler_for("dimmableLight").dispatch("setLevel", {"level": 0}, dev) + mock_indigo_base.dimmer.setBrightness.assert_called_once_with(dev, value=0) + + def test_dispatch_without_a_level_raises_rather_than_guessing(self, handlers): + with pytest.raises(ValueError): + handlers.handler_for("dimmableLight").dispatch("setLevel", {}, DimmerDevice(1, "L")) + + +# --------------------------------------------------------------------------- +# colorTemperatureLight +# --------------------------------------------------------------------------- +class TestColorTemperature: + def test_kelvin_becomes_mireds(self, handlers): + dev = DimmerDevice(1, "Lamp", onState=True, brightness=50, whiteTemperature=2700) + states = handlers.handler_for("colorTemperatureLight").states_for(dev) + assert states["colorTempMireds"] == 370 # 1e6 / 2700 + + def test_mireds_stay_inside_the_declared_domain(self, handlers): + """§4.2 declares 153-500. A 10000K device converts to 100.""" + dev = DimmerDevice(1, "Lamp", onState=True, brightness=50, whiteTemperature=10000) + states = handlers.handler_for("colorTemperatureLight").states_for(dev) + assert states["colorTempMireds"] == 153 + + def test_an_unset_temperature_is_omitted_not_zeroed(self, handlers): + for value in (None, 0): + dev = DimmerDevice(1, "Lamp", onState=True, brightness=50, whiteTemperature=value) + assert "colorTempMireds" not in \ + handlers.handler_for("colorTemperatureLight").states_for(dev) + + def test_dispatch_preserves_the_white_level(self, handlers, mock_indigo_base): + dev = DimmerDevice(1, "Lamp", whiteLevel=40) + handlers.handler_for("colorTemperatureLight").dispatch( + "setColorTemp", {"colorTempMireds": 370}, dev) + mock_indigo_base.dimmer.setColorLevels.assert_called_once_with( + dev, whiteLevel=40, whiteTemperature=2703) + + def test_a_1800k_lamp_is_clamped_to_the_declared_maximum(self, handlers): + """T3: the other end of the domain. 1800K → 556 mireds → clamped to 500. + + The MIREDS_MIN side has a test above; without this one the ``_clamp`` + call could lose its upper bound and nothing would notice until a warm + lamp pushed an out-of-domain value the node then had to clamp for us. + """ + dev = DimmerDevice(1, "Lamp", onState=True, brightness=50, whiteTemperature=1800) + states = handlers.handler_for("colorTemperatureLight").states_for(dev) + assert states["colorTempMireds"] == handlers.MIREDS_MAX == 500 + + def test_a_device_with_no_white_channel_is_skipped_not_guessed_at( + self, handlers, mock_indigo_base): + """X4: ``whiteLevel is None`` means there is no white channel at all. + + Inventing ``whiteLevel=100`` for such a device asks its driver to drive a + channel it does not have — and the previous "default to full" rule was + written for a device that HAS the channel and reports it off. + """ + dev = DimmerDevice(1, "Lamp", whiteLevel=None) + handlers.handler_for("colorTemperatureLight").dispatch( + "setColorTemp", {"colorTempMireds": 250}, dev) + mock_indigo_base.dimmer.setColorLevels.assert_not_called() + + def test_a_white_channel_reporting_off_still_gets_full_level( + self, handlers, mock_indigo_base): + """A colour tweak must never be the thing that blacks out the room.""" + dev = DimmerDevice(1, "Lamp", whiteLevel=0) + handlers.handler_for("colorTemperatureLight").dispatch( + "setColorTemp", {"colorTempMireds": 250}, dev) + _args, kwargs = mock_indigo_base.dimmer.setColorLevels.call_args + assert kwargs["whiteLevel"] == 100 + + @pytest.mark.parametrize("mireds,expected_kelvin", [ + (1, 6536), # → clamped up to MIREDS_MIN 153 + (10000, 2000), # → clamped down to MIREDS_MAX 500 + ]) + def test_out_of_domain_mireds_are_clamped_before_the_indigo_write( + self, handlers, mock_indigo_base, mireds, expected_kelvin): + """X2: the node clamps too — but our Indigo write must not depend on it. + + Unclamped, 1 mired writes 1,000,000 K and 10000 mireds writes 100 K. + Indigo's own whiteTemperature domain is 1200–15000, so both are values + the driver has to reject or silently mangle, in the OTHER process. + """ + dev = DimmerDevice(1, "Lamp", whiteLevel=40) + handlers.handler_for("colorTemperatureLight").dispatch( + "setColorTemp", {"colorTempMireds": mireds}, dev) + _args, kwargs = mock_indigo_base.dimmer.setColorLevels.call_args + assert kwargs["whiteTemperature"] == expected_kelvin + assert 1200 <= kwargs["whiteTemperature"] <= 15000 + + def test_fractional_mireds_round_rather_than_truncate(self, handlers, mock_indigo_base): + """X2: ``int()`` on 369.9 is 369, which is a different colour.""" + dev = DimmerDevice(1, "Lamp", whiteLevel=40) + handlers.handler_for("colorTemperatureLight").dispatch( + "setColorTemp", {"colorTempMireds": 369.9}, dev) + _args, kwargs = mock_indigo_base.dimmer.setColorLevels.call_args + assert kwargs["whiteTemperature"] == 2703 # 1e6 / 370, not 1e6 / 369 + + def test_setting_a_temperature_zeroes_the_rgb_channels(self, handlers, mock_indigo_base): + """X4: Matter ``colorMode`` is one-of, and the node picks hue/sat over CT. + + An RGBW lamp left with live RGB levels alongside a new white temperature + reports both, and the node's push side then believes the stale colour. + """ + dev = DimmerDevice(1, "Lamp", whiteLevel=40, redLevel=100, greenLevel=0, blueLevel=0) + handlers.handler_for("colorTemperatureLight").dispatch( + "setColorTemp", {"colorTempMireds": 370}, dev) + mock_indigo_base.dimmer.setColorLevels.assert_called_once_with( + dev, whiteLevel=40, whiteTemperature=2703, + redLevel=0, greenLevel=0, blueLevel=0) + + +# --------------------------------------------------------------------------- +# extendedColorLight +# --------------------------------------------------------------------------- +class TestExtendedColor: + def test_rgb_becomes_hue_and_saturation(self, handlers): + dev = DimmerDevice(1, "Lamp", onState=True, brightness=50, + redLevel=100, greenLevel=0, blueLevel=0) + states = handlers.handler_for("extendedColorLight").states_for(dev) + assert states["hue"] == 0 and states["saturation"] == 100 + dev.redLevel, dev.greenLevel, dev.blueLevel = 0, 0, 100 + states = handlers.handler_for("extendedColorLight").states_for(dev) + assert states["hue"] == 240 and states["saturation"] == 100 + + def test_colour_keys_are_omitted_when_a_channel_is_unset(self, handlers): + dev = DimmerDevice(1, "Lamp", onState=True, brightness=50, + redLevel=100, greenLevel=0, blueLevel=None) + states = handlers.handler_for("extendedColorLight").states_for(dev) + assert "hue" not in states and "saturation" not in states + + def test_a_one_degree_hue_wobble_is_not_a_change(self, handlers): + """E3a: hue round-trips through Matter's 0-254 scale ±1°. + + Without the tolerance every ecosystem-driven colour change echoes a + spurious ``set_state`` straight back out. + """ + handler = handlers.handler_for("extendedColorLight") + before = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=2, blueLevel=0) + after = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=4, blueLevel=0) + assert handler.states_for(before)["hue"] == 1 + assert handler.states_for(after)["hue"] == 2 + assert handler.states_for(before)["saturation"] == \ + handler.states_for(after)["saturation"], "only hue may move in this fixture" + assert handler.diff(before, after) == {} + + def test_a_real_hue_move_is_still_reported(self, handlers): + handler = handlers.handler_for("extendedColorLight") + before = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=0, blueLevel=0) + after = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=0, greenLevel=0, blueLevel=100) + assert handler.diff(before, after)["hue"] == 240 + + def test_saturation_has_no_tolerance(self, handlers): + """0-100 → 0-254 → 0-100 round-trips exactly, so ±1 IS a real change.""" + handler = handlers.handler_for("extendedColorLight") + before = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=0, blueLevel=0) + after = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=1, blueLevel=1) + assert handler.diff(before, after) == {"saturation": 99} + + def test_hue_is_omitted_below_the_saturation_floor(self, handlers): + """X3: at sat 5 the integer-RGB round trip moves hue by up to 6°. + + The ±1° tolerance cannot absorb that, so every pastel nudge pushed a + ``hue`` the ecosystem had not asked for and the Home wheel jumped. + """ + handler = handlers.handler_for("extendedColorLight") + dev = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=95, blueLevel=95) + states = handler.states_for(dev) + assert states["saturation"] == 5 + assert "hue" not in states + + def test_a_pastel_hue_wobble_pushes_nothing(self, handlers): + handler = handlers.handler_for("extendedColorLight") + before = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=95, blueLevel=95) # hue 0 + after = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=95, blueLevel=100) # hue 300 + assert handler.states_for(before)["saturation"] == \ + handler.states_for(after)["saturation"] == 5, "only hue may move here" + assert handler.diff(before, after) == {} + + def test_a_fully_desaturated_device_reports_no_hue(self, handlers): + """At sat 0 hue is not merely noisy, it is undefined — RGB says 0 always.""" + handler = handlers.handler_for("extendedColorLight") + dev = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=100, blueLevel=100) + assert handler.states_for(dev) == {"onOff": True, "level": 50, "saturation": 0} + + def test_at_the_floor_itself_hue_is_reported_again(self, handlers): + """sat 20 is where the measured error falls to the ±1° tolerance.""" + handler = handlers.handler_for("extendedColorLight") + dev = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=80, blueLevel=80) + states = handler.states_for(dev) + assert states["saturation"] == handlers.SATURATION_HUE_FLOOR == 20 + assert states["hue"] == 0 + + def test_a_saturated_hue_move_is_unaffected_by_the_floor(self, handlers): + handler = handlers.handler_for("extendedColorLight") + before = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=80, blueLevel=80) # sat 20, hue 0 + after = DimmerDevice(1, "L", onState=True, brightness=50, + redLevel=100, greenLevel=80, blueLevel=100) # sat 20, hue 300 + assert handler.diff(before, after) == {"hue": 300} + + def test_setting_a_colour_zeroes_the_white_channel(self, handlers, mock_indigo_base): + """X4: the mirror of the CT path — colourMode is one-of.""" + dev = DimmerDevice(1, "Lamp", whiteLevel=80) + handlers.handler_for("extendedColorLight").dispatch( + "setColor", {"hue": 240, "saturation": 100}, dev) + mock_indigo_base.dimmer.setColorLevels.assert_called_once_with( + dev, redLevel=0, greenLevel=0, blueLevel=100, whiteLevel=0) + + def test_dispatch_writes_rgb_levels(self, handlers, mock_indigo_base): + dev = DimmerDevice(1, "Lamp") + handlers.handler_for("extendedColorLight").dispatch( + "setColor", {"hue": 240, "saturation": 100}, dev) + mock_indigo_base.dimmer.setColorLevels.assert_called_once_with( + dev, redLevel=0, greenLevel=0, blueLevel=100) + + def test_the_duplicate_set_color_is_dispatched_again_not_swallowed( + self, handlers, mock_indigo_base): + """E3a: ecosystems send ``setColor`` twice with identical pairs. + + ``setColorLevels`` takes absolute values, so the repeat is a no-op at the + lamp — and de-duplicating on "we already believe that" would skip the + FIRST call too whenever Indigo's belief has drifted from the hardware. + """ + dev = DimmerDevice(1, "Lamp") + args = {"hue": 120, "saturation": 50} + handler = handlers.handler_for("extendedColorLight") + handler.dispatch("setColor", dict(args), dev) + handler.dispatch("setColor", dict(args), dev) + assert mock_indigo_base.dimmer.setColorLevels.call_count == 2 + first, second = mock_indigo_base.dimmer.setColorLevels.call_args_list + assert first == second + + def test_dispatch_without_a_numeric_pair_raises(self, handlers): + handler = handlers.handler_for("extendedColorLight") + with pytest.raises(ValueError): + handler.dispatch("setColor", {"hue": 120}, DimmerDevice(1, "L")) + + +# --------------------------------------------------------------------------- +# Colour conversion (pure) +# --------------------------------------------------------------------------- +@pytest.mark.parametrize("rgb,expected", [ + ((100, 0, 0), (0, 100)), + ((0, 100, 0), (120, 100)), + ((0, 0, 100), (240, 100)), + ((100, 100, 100), (0, 0)), + ((0, 0, 0), (0, 0)), +]) +def test_rgb_to_hue_saturation(handlers, rgb, expected): + assert handlers.rgb_to_hue_saturation(*rgb) == expected + + +@pytest.mark.parametrize("hue", [0, 45, 120, 200, 300, 359]) +def test_hue_survives_a_round_trip_through_rgb(handlers, hue): + """The conversion pair must not drift more than the declared tolerance.""" + red, green, blue = handlers.hue_saturation_to_rgb(hue, 100) + back, _saturation = handlers.rgb_to_hue_saturation(red, green, blue) + assert abs(back - hue) <= handlers.HUE_TOLERANCE_DEGREES diff --git a/tests/test_export_menu.py b/tests/test_export_menu.py index 538437a..69917ce 100644 --- a/tests/test_export_menu.py +++ b/tests/test_export_menu.py @@ -75,6 +75,11 @@ def plug(plugin_mod, devices): # noqa: ARG001 - devices installs indigo.devices p.pluginId = OURS p.pluginPrefs = {} p.exports = ExportStore(lambda: p.pluginPrefs, p.logger) + # Post-startup shape: the dialog callbacks nudge the export bridge and + # refresh the deviceUpdated fast-path id set (E3b). + p.export_bridge = None + p._exported_ids = frozenset() + p._subscribed_to_devices = False return p @@ -765,6 +770,10 @@ def build(prefs=None): p = plugin_mod.Plugin.__new__(plugin_mod.Plugin) p.logger = Mock() p.pluginId = OURS + p._version = "2026.0.1" + p._subscribed_to_devices = False + p._exported_ids = frozenset() + p.export_bridge = None p.pluginPrefs = {} if prefs is None else prefs p.proto = object() p.registry = object() diff --git a/tests/test_export_wiring.py b/tests/test_export_wiring.py new file mode 100644 index 0000000..efc7493 --- /dev/null +++ b/tests/test_export_wiring.py @@ -0,0 +1,508 @@ +"""E3: the plugin-side export wiring — subscription, callbacks, dialog nudges. + +``export_bridge`` is tested on its own in ``test_export_bridge.py``; this file +pins the four things only ``plugin.py`` can get wrong: + +* **the ``deviceUpdated`` fast path.** The subscription is server-wide, so this + callback fires for every device change on the whole Indigo database. If it + ever does more than a set lookup for a device nobody exported, every user who + exports one lamp pays for it on every state change in their house; +* **when the subscription is issued at all.** Default posture is inert (XG5), so + a plugin with an empty allow-list must not ask Indigo for the firehose; +* **the delete path** — a deleted device leaves the allow-list *and* the bridge; +* **the dialog nudges** — a role change re-creates the accessory (§4.1), which + costs the user their Home-app name and room, so it is a different call and a + different message from an ordinary update. +""" +from __future__ import annotations + +import importlib +from unittest.mock import Mock + +import pytest + +import export_catalog +from export_store import ExportEntry, ExportStore +from fakes import FakeIndigoDevices, RelayDevice + +OURS = export_catalog.DEFAULT_PLUGIN_ID + + +@pytest.fixture +def plugin_mod(mock_indigo_base): + import plugin as plugin_module + importlib.reload(plugin_module) + return plugin_module + + +@pytest.fixture +def devices(mock_indigo_base): + collection = FakeIndigoDevices([ + RelayDevice(101, "Study Plug", onState=False), + RelayDevice(102, "Porch Light", onState=False), + ]) + mock_indigo_base.devices = collection + return collection + + +@pytest.fixture +def plug(plugin_mod, devices): # noqa: ARG001 - devices installs indigo.devices + """A plugin in its post-startup shape, with the bridge mocked out.""" + p = plugin_mod.Plugin.__new__(plugin_mod.Plugin) + p.logger = Mock() + p.pluginId = OURS + p._version = "2026.7.28" + p.pluginPrefs = {} + p.exports = ExportStore(lambda: p.pluginPrefs, p.logger) + p.export_bridge = Mock() + # The fixture's allow-list is empty, so XG5 says there is no client — and a + # bare Mock would otherwise answer `halted` truthily and add F10's note to + # every status line in this file. + p.export_bridge.active = False + p._exported_ids = frozenset() + p._subscribed_to_devices = False + p._export_callback_failed = set() + p.runtime = None + return p + + +class _ExplodingStore: + """A store that fails the test on ANY attribute access (T2). + + ``deviceUpdated`` fires for every device on the server. Reading so much as + ``exports.ids`` there is a cost every user pays on every state change in + their house, forever — so the guard has to be "nothing was touched", not + "nothing was called". + """ + + def __getattr__(self, name): + raise AssertionError(f"store attribute {name!r} was read on the fast path") + + +def _values(**kwargs): + base = {"exportFilter": "", "exportDevice": "0", "exportRole": "", + "exportName": "", "exportInvert": False, "exportStatus": ""} + base.update(kwargs) + return base + + +# --------------------------------------------------------------------------- +# startup (T1) +# --------------------------------------------------------------------------- +class _FakeRuntime: + is_running = True + + def start(self): + pass + + def submit(self, coro): + if hasattr(coro, "close"): + coro.close() + return Mock() + + +def _started_plugin(plugin_mod, monkeypatch, prefs): + """Run the real ``Plugin.startup()`` with every I/O collaborator faked.""" + class FakeMatter: + def __init__(self, *a, **k): + pass + + def run(self): + return None + + monkeypatch.setattr(plugin_mod, "MatterClient", FakeMatter) + monkeypatch.setattr(plugin_mod, "AsyncRuntime", lambda logger: _FakeRuntime()) + monkeypatch.setattr(plugin_mod, "CommissionJobs", lambda *a, **k: Mock()) + monkeypatch.setattr(plugin_mod, "HttpApi", lambda *a, **k: Mock()) + # MUST be patched: an unpatched local-mode startup builds a real + # ServerProcess against the real $HOME (see test_plugin_behaviour.py). + monkeypatch.setattr(plugin_mod, "ServerProcess", lambda *a, **k: Mock()) + + p = plugin_mod.Plugin.__new__(plugin_mod.Plugin) + p.logger = Mock() + p.pluginId = OURS + p._version = "2026.7.28" + p._subscribed_to_devices = False + p.pluginPrefs = prefs + p.proto = object() + p.registry = object() + p.device_sync = Mock() + p.runtime = None + p.server_process = None + p.exports = None + p.export_bridge = None + p._exported_ids = frozenset() + p.startup() + return p + + +def _prefs_with_exports(*entries): + """pluginPrefs carrying a persisted allow-list, written by the real store.""" + prefs: dict = {} + store = ExportStore(lambda: prefs, Mock()) + for entry in entries: + store.upsert(entry) + return prefs + + +class TestStartupWithExistingExports: + """T1: the startup half of ``_exports_changed`` had no test at all. + + Everything below is reachable only through the ``_exports_changed()`` call + at the end of ``startup``. Delete that one line and the plugin still starts, + still logs "N device(s) exported", still builds the bridge — and exports + nothing, forever, for every user who already had an allow-list. Which is + every user who restarts Indigo. + """ + + def test_a_persisted_allow_list_is_live_after_startup(self, plugin_mod, monkeypatch, + mock_indigo_base, devices): + subscribe = Mock() + mock_indigo_base.devices.subscribeToChanges = subscribe + built: list = [] + monkeypatch.setattr(plugin_mod, "ExportBridge", + lambda *a, **k: built.append(Mock()) or built[-1]) + + p = _started_plugin(plugin_mod, monkeypatch, + _prefs_with_exports(ExportEntry(101, "onOffLight"), + ExportEntry(102, "onOffPlugInUnit"))) + + assert p._exported_ids == frozenset({101, 102}), "the hot-path guard must be primed" + subscribe.assert_called_once_with() + assert len(built) == 1, "the bridge is built unconditionally" + built[0].exports_changed.assert_called_once_with() + + def test_an_empty_allow_list_starts_inert(self, plugin_mod, monkeypatch, mock_indigo_base, + devices): + """XG5 — the same seam must NOT ask for the firehose on a fresh install.""" + subscribe = Mock() + mock_indigo_base.devices.subscribeToChanges = subscribe + monkeypatch.setattr(plugin_mod, "ExportBridge", lambda *a, **k: Mock()) + + p = _started_plugin(plugin_mod, monkeypatch, {}) + + assert p._exported_ids == frozenset() + subscribe.assert_not_called() + assert p._subscribed_to_devices is False + + +# --------------------------------------------------------------------------- +# The subscription decision +# --------------------------------------------------------------------------- +class TestSubscription: + def test_an_empty_allow_list_never_asks_for_the_firehose(self, plug, mock_indigo_base): + """XG5: a fresh install must cost an existing user nothing.""" + plug._exports_changed() + mock_indigo_base.devices.subscribeToChanges = Mock() + plug._exports_changed() + mock_indigo_base.devices.subscribeToChanges.assert_not_called() + + def test_the_first_export_subscribes(self, plug, mock_indigo_base): + subscribe = Mock() + mock_indigo_base.devices.subscribeToChanges = subscribe + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug._exports_changed() + subscribe.assert_called_once_with() + assert plug._subscribed_to_devices is True + + def test_it_subscribes_exactly_once(self, plug, mock_indigo_base): + subscribe = Mock() + mock_indigo_base.devices.subscribeToChanges = subscribe + plug.exports.upsert(ExportEntry(101, "onOffLight")) + for _ in range(5): + plug._exports_changed() + assert subscribe.call_count == 1 + + def test_it_stays_subscribed_after_the_list_empties(self, plug, mock_indigo_base): + """There is no documented unsubscribe — so this is a one-way door. + + Turning it off through an undocumented API is exactly the sort of thing + that fails silently on an Indigo upgrade; the fast-path guard makes a + stale subscription free anyway. + """ + mock_indigo_base.devices.subscribeToChanges = Mock() + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug._exports_changed() + plug.exports.remove(101) + plug._exports_changed() + assert plug._subscribed_to_devices is True + assert plug._exported_ids == frozenset() + + def test_a_failed_subscription_is_loud_and_not_fatal(self, plug, mock_indigo_base): + mock_indigo_base.devices.subscribeToChanges = Mock(side_effect=RuntimeError("nope")) + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug._exports_changed() + assert plug._subscribed_to_devices is False + assert plug.logger.error.called + + +# --------------------------------------------------------------------------- +# deviceUpdated +# --------------------------------------------------------------------------- +class TestDeviceUpdated: + def test_a_non_exported_device_touches_nothing(self, plug, monkeypatch, plugin_mod): + """THE hot path. It fires for every device change on the server. + + Anything beyond the set lookup — a classify, a store read, a handler + lookup — is paid by every user for every device they never exported. + """ + monkeypatch.setattr(plugin_mod.export_catalog, "classify", + Mock(side_effect=AssertionError("classified on the fast path"))) + monkeypatch.setattr(plugin_mod.export_handlers, "handler_for", + Mock(side_effect=AssertionError("handler looked up on the fast path"))) + # A Mock() answers every attribute happily, so it only catches a CALL. + # This catches the ACCESS — including `exports.lock` or `exports.ids`, + # either of which would put a lock or a set rebuild on the hot path. + plug.exports = _ExplodingStore() + plug._exported_ids = frozenset({999}) + + plug.deviceUpdated(RelayDevice(101, "Study Plug", onState=False), + RelayDevice(101, "Study Plug", onState=True)) + + plug.export_bridge.device_updated.assert_not_called() + + def test_the_base_class_still_gets_its_callback(self, plug): + """The SDK's most-broken rule: the base does real work in here.""" + plug._exported_ids = frozenset() + before, after = RelayDevice(101, "P"), RelayDevice(101, "P") + plug.deviceUpdated(before, after) + assert plug.base_calls == [("deviceUpdated", before, after)] + + def test_an_exported_device_is_handed_to_the_bridge(self, plug): + plug._exported_ids = frozenset({101}) + before = RelayDevice(101, "Study Plug", onState=False) + after = RelayDevice(101, "Study Plug", onState=True) + plug.deviceUpdated(before, after) + plug.export_bridge.device_updated.assert_called_once_with(before, after) + + def test_a_failing_bridge_never_breaks_indigos_callback(self, plug): + plug._exported_ids = frozenset({101}) + plug.export_bridge.device_updated.side_effect = RuntimeError("boom") + plug.deviceUpdated(RelayDevice(101, "P"), RelayDevice(101, "P")) + assert plug.logger.exception.called + + def test_the_failure_names_the_device_and_repeats_once_per_streak(self, plug): + """F6: a bare traceback per state change names nothing and never stops. + + This callback fires on every change of an exported device — a lamp on a + dimmer ramp produces one per step. A stuck failure would write the same + anonymous traceback into the event log tens of times a minute. + """ + plug._exported_ids = frozenset({101}) + plug.export_bridge.device_updated.side_effect = RuntimeError("boom") + for _ in range(10): + plug.deviceUpdated(RelayDevice(101, "Study Plug"), RelayDevice(101, "Study Plug")) + assert plug.logger.exception.call_count == 1 + errors = " ".join(str(c.args[0]) % c.args[1:] if len(c.args) > 1 else str(c.args[0]) + for c in plug.logger.error.call_args_list) + assert "Study Plug" in errors and "101" in errors + + def test_a_recovered_device_can_report_again(self, plug): + plug._exported_ids = frozenset({101}) + bridge = plug.export_bridge + bridge.device_updated.side_effect = RuntimeError("boom") + plug.deviceUpdated(RelayDevice(101, "P"), RelayDevice(101, "P")) + bridge.device_updated.side_effect = None + plug.deviceUpdated(RelayDevice(101, "P"), RelayDevice(101, "P")) + bridge.device_updated.side_effect = RuntimeError("boom again") + plug.deviceUpdated(RelayDevice(101, "P"), RelayDevice(101, "P")) + assert plug.logger.exception.call_count == 2 + + def test_it_survives_a_bridge_that_does_not_exist_yet(self, plug): + plug.export_bridge = None + plug._exported_ids = frozenset({101}) + plug.deviceUpdated(RelayDevice(101, "P"), RelayDevice(101, "P")) # must not raise + + +# --------------------------------------------------------------------------- +# deviceDeleted +# --------------------------------------------------------------------------- +class TestDeviceDeleted: + def test_a_non_exported_device_is_ignored_but_the_base_still_runs(self, plug): + dev = RelayDevice(101, "Study Plug") + plug.deviceDeleted(dev) + plug.export_bridge.remove.assert_not_called() + assert plug.base_calls == [("deviceDeleted", dev)] + + def test_an_exported_device_leaves_the_list_and_the_bridge(self, plug): + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug._exports_changed() + plug.deviceDeleted(RelayDevice(101, "Study Plug")) + assert plug.exports.ids() == frozenset() + plug.export_bridge.remove.assert_called_once_with(101) + assert plug._exported_ids == frozenset() + + def test_a_failed_store_write_still_removes_the_endpoint(self, plug, monkeypatch): + """The device is gone either way — the accessory must not outlive it.""" + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug._exports_changed() + monkeypatch.setattr(plug.exports, "remove", + Mock(side_effect=RuntimeError("prefs are read-only"))) + plug.deviceDeleted(RelayDevice(101, "Study Plug")) + plug.export_bridge.remove.assert_called_once_with(101) + assert plug.logger.error.called + + def test_a_raising_endpoint_removal_still_refreshes_the_cache(self, plug): + """F9a: otherwise ``_exported_ids`` keeps an id whose device is gone. + + ``deviceUpdated`` would then hand a deleted device to the bridge on + every later change, and ``deviceDeleted`` would never fire for it again + — the cache has no other way back in sync until the plugin reloads. + """ + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug._exports_changed() + plug.export_bridge.remove.side_effect = RuntimeError("socket died") + plug.deviceDeleted(RelayDevice(101, "Study Plug")) + assert plug._exported_ids == frozenset() + assert plug.logger.exception.called + + +# --------------------------------------------------------------------------- +# Dialog integration +# --------------------------------------------------------------------------- +class TestDialogNudges: + def test_adding_an_export_upserts_the_endpoint(self, plug): + plug.exportAddOrUpdate( + _values(exportDevice="101", exportRole="onOffPlugInUnit"), "manageMatterExports") + plug.export_bridge.upsert.assert_called_once_with(101) + plug.export_bridge.replace.assert_not_called() + assert plug._exported_ids == frozenset({101}) + + def test_changing_only_the_name_is_still_an_upsert(self, plug): + plug.exportAddOrUpdate( + _values(exportDevice="101", exportRole="onOffPlugInUnit"), "manageMatterExports") + plug.export_bridge.upsert.reset_mock() + plug.exportAddOrUpdate( + _values(exportDevice="101", exportRole="onOffPlugInUnit", exportName="Desk"), + "manageMatterExports") + plug.export_bridge.upsert.assert_called_once_with(101) + plug.export_bridge.replace.assert_not_called() + + def test_a_role_change_recreates_the_accessory(self, plug): + """§4.1 refuses a role change in place, so it is remove-then-add.""" + plug.exportAddOrUpdate( + _values(exportDevice="101", exportRole="onOffPlugInUnit"), "manageMatterExports") + values = plug.exportAddOrUpdate( + _values(exportDevice="101", exportRole="onOffLight"), "manageMatterExports") + plug.export_bridge.replace.assert_called_once_with(101) + assert "RE-CREATES" in values["exportStatus"] + assert "Apple Home" in values["exportStatus"] + + def test_an_ordinary_update_does_not_threaten_the_user(self, plug): + values = plug.exportAddOrUpdate( + _values(exportDevice="101", exportRole="onOffPlugInUnit"), "manageMatterExports") + assert "RE-CREATES" not in values["exportStatus"] + + def test_a_refused_add_nudges_nothing(self, plug): + plug.exportAddOrUpdate(_values(exportDevice="0"), "manageMatterExports") + plug.export_bridge.upsert.assert_not_called() + plug.export_bridge.replace.assert_not_called() + + def test_removing_an_export_removes_the_endpoint(self, plug): + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug._exports_changed() + plug.export_bridge.reset_mock() + plug.exportRemove(_values(exportDevice="101"), "manageMatterExports") + plug.export_bridge.remove.assert_called_once_with(101) + assert plug._exported_ids == frozenset() + + def test_removing_something_unexported_nudges_nothing(self, plug): + plug.exportRemove(_values(exportDevice="101"), "manageMatterExports") + plug.export_bridge.remove.assert_not_called() + + +class TestStatusSummary: + def test_an_e4_role_is_called_out_in_the_dialog(self, plug): + """Otherwise the accessory is simply absent, with no visible cause.""" + plug.exports.upsert(ExportEntry(101, "doorLock")) + summary = plug._export_summary() + assert "cannot bridge yet" in summary + + def test_bridgeable_exports_get_no_such_note(self, plug): + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug.export_bridge.active = False + assert "cannot bridge yet" not in plug._export_summary() + + def _bridge_in(self, plug, **state): + """A bridge whose client is in some non-serving state.""" + client = Mock(halted=False, halted_reason=None, recovery=False, attached=True) + for key, value in state.items(): + setattr(client, key, value) + plug.export_bridge.active = True + plug.export_bridge.client = client + + def test_a_halted_bridge_is_named_in_the_dialog(self, plug): + """F10: "2 device(s) exported." over a halted bridge is a lie of omission. + + The dialog is the only surface a user looks at to answer "why is my + light not in Home?" — and every one of these states answers it. + """ + plug.exports.upsert(ExportEntry(101, "onOffLight")) + self._bridge_in(plug, halted=True, halted_reason="version skew", attached=False) + summary = plug._export_summary() + assert summary.startswith("1 device(s) exported.") + assert "halted" in summary and "version skew" in summary + assert "restart the bridge node" in summary + + def test_an_endpoint_map_rebuild_is_named(self, plug): + plug.exports.upsert(ExportEntry(101, "onOffLight")) + self._bridge_in(plug, recovery=True, attached=False) + assert "endpoint-map" in plug._export_summary() + + def test_a_never_attached_bridge_says_exports_are_not_live(self, plug): + plug.exports.upsert(ExportEntry(101, "onOffLight")) + self._bridge_in(plug, attached=False) + assert "Not connected to the bridge node" in plug._export_summary() + assert "exports are not live" in plug._export_summary() + + def test_a_healthy_bridge_adds_nothing(self, plug): + plug.exports.upsert(ExportEntry(101, "onOffLight")) + self._bridge_in(plug) + assert plug._export_summary() == "1 device(s) exported." + + def test_a_load_error_still_leads(self, plug): + """The rescue copy is the thing a user must not be talked out of.""" + plug.exports.upsert(ExportEntry(101, "onOffLight")) + plug.exports.load_error = "Could not read the export list." + self._bridge_in(plug, halted=True, attached=False) + summary = plug._export_summary() + assert summary.startswith("Could not read the export list.") + assert "halted" in summary + + +# --------------------------------------------------------------------------- +# Lifecycle +# --------------------------------------------------------------------------- +class TestLifecycleWiring: + def test_shutdown_closes_the_bridge_before_the_loop_it_runs_on(self, plug): + order = [] + plug.export_bridge.stop.side_effect = lambda *a, **k: order.append("bridge") + plug.runtime = Mock(is_running=True, stop=lambda *a, **k: order.append("runtime")) + plug.matter = None + plug._install_thread = None + plug._stopping = False + plug.shutdown() + assert order == ["bridge", "runtime"] + + def test_shutdown_leaves_the_agent_running(self, plug): + """PM-B: a plugin reload must not un-pair anyone's ecosystems.""" + bridge = plug.export_bridge + plug.runtime = Mock(is_running=True) + plug.matter = None + plug._install_thread = None + plug._stopping = False + plug.shutdown() + bridge.stop.assert_called_once() + assert not hasattr(bridge, "uninstall") or not bridge.uninstall.called + + def test_the_watchdog_ticks_the_bridge(self, plug): + plug.runtime = Mock(is_running=True) + plug.matter = None + plug._health_tick() + plug.export_bridge.health_tick.assert_called_once_with() + + def test_the_watchdog_skips_a_bridge_that_does_not_exist(self, plug): + plug.export_bridge = None + plug.runtime = Mock(is_running=True) + plug.matter = None + plug._health_tick() # must not raise diff --git a/tests/test_plugin_behaviour.py b/tests/test_plugin_behaviour.py index 461e97d..ab33ee3 100644 --- a/tests/test_plugin_behaviour.py +++ b/tests/test_plugin_behaviour.py @@ -37,6 +37,9 @@ def plug(plugin_mod): p.http = Mock() p.jobs = None p.pluginPrefs = {} + p.export_bridge = None + p._exported_ids = frozenset() + p._subscribed_to_devices = False return p @@ -423,6 +426,8 @@ def submit(self, coro): seen.clear() p = plugin_mod.Plugin.__new__(plugin_mod.Plugin) p.logger = Mock() + p._version = "2026.0.1" + p._subscribed_to_devices = False p.pluginPrefs = {"serverLocation": "local", "enableTestNetDcl": value} p.proto = object() p.registry = object() @@ -474,6 +479,8 @@ def submit(self, coro): p = plugin_mod.Plugin.__new__(plugin_mod.Plugin) p.logger = Mock() + p._version = "2026.0.1" + p._subscribed_to_devices = False p.pluginPrefs = {} p.proto = object() p.registry = object() diff --git a/tests/test_ws_json_client.py b/tests/test_ws_json_client.py index aa49e7b..e124060 100644 --- a/tests/test_ws_json_client.py +++ b/tests/test_ws_json_client.py @@ -343,7 +343,9 @@ def test_an_unanswered_attach_names_itself_in_the_log(self, mock_logger, monkeyp # The handshake's attach is pumped inline, so its deadline is a bare # asyncio.TimeoutError with an empty str() — "connection lost: " was all # the log said about the single most diagnostic failure in §2. - monkeypatch.setattr("bridge_client.ATTACH_TIMEOUT", 0.05) + # The deadline is derived from the endpoint count (E3b), so pin the + # formula rather than its floor constant. + monkeypatch.setattr("bridge_client.attach_timeout_for", lambda _count: 0.05) async def scenario(): delays = []