feat(export): E3a — bridge-node endpoint CRUD for relay/dimmer/colour roles - #123
Merged
Conversation
… roles
The bridge node's endpoint set is now driven entirely by the protocol
(BRIDGE_PROTOCOL §3.1-§3.5). The hard-coded E0 endpoint (indigo-999001) is
gone: a node nobody has attached to serves an empty aggregator. The E0
pairing on jarvis survives because matter.js persists endpoint numbers
against `Endpoint.id`, not against presence in the running set — verified by
a restart smoke test that re-attaches the same devices in a different order
and gets the same numbers back.
Implements the five E3 roles only — onOffPlugInUnit, onOffLight,
dimmableLight, colorTemperatureLight, extendedColorLight. Sensors,
thermostat, doorLock and windowCovering are E4 and are refused with an
`internal` error naming the gap, rather than silently skipped: `unknown_role`
would be a lie (they are in the v1 enum) and a dropped export the user
selected is worse than a loud one.
New modules:
- src/endpoints.ts — role → device type, the §4.2 unit converters next to
the clusters they feed, the state writers and the cluster-change
listeners.
- src/reconcile.ts — matter.js-free arg parsing and the reconcile planner,
including the §3.1 mass-removal guard. The test double reuses it, so the
guard and the role rules are decided in one place.
- src/registry.ts — the live endpoint set: reconcile, upsert, remove,
set_state, set_reachable, ~100ms bulk-removal pacing (injectable).
Three matter.js 0.17.8 findings, all pinned in comments:
- ExtendedColorLightDevice is built with `ColorControlServer.with("Xy",
"ColorTemperature")` — no HueSaturation — so §4.2's hue/saturation
vocabulary had no attributes to write. The Matter spec makes HS mandatory
for device type 0x010D, so the override restores conformance.
- ColorControl refuses to initialise without colorMode, enhancedColorMode,
colorCapabilities, coupleColorTempToLevelMinMireds and
startUpColorTemperatureMireds, none of which the typings mark required.
- `currentLevel` is constrained to 1-254 on all four lighting device types,
so 0% is written as 1 — lossless, because it converts back to 0.
Golden frames: attach_with_endpoints, attach_replace_all,
attach_mass_removal_refused, upsert_endpoint, upsert_endpoint_role_change,
remove_endpoint, remove_endpoint_absent, set_state, set_state_unknown_device
and set_reachable move out of "pending" into real deepEqual assertions on
both sides.
Two frames had to change to be honourable, per §7:
- attach_with_endpoints' second endpoint was a doorLock, which E3 cannot
build. It is now a dimmableLight (same device id, same endpoint number);
rebuild_endpoint_map follows, and command_lock moves to 900007, the
doorLock that attach_all_roles already declares.
- get_status carried the E0 999001 endpoint, which nothing creates any
more. It now carries the same StatusReport attach_with_endpoints returns
— §6.2's invariant is that attach answers with exactly what get_status
would, so sharing the payload is the contract.
The Python suites' PENDING lookups become name lookups across both sections:
which section a frame sits in is a statement about the node, and says nothing
about the plugin-side client those tests drive.
bridge-node 0.1.0 → 0.2.0; PluginVersion 2026.7.26 → 2026.7.27. No Python
source changes — the plugin side is the next PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
📝 WalkthroughWalkthroughThe bridge now supports multiple dynamic Matter endpoints. It validates endpoint specifications, reconciles endpoint sets, manages supported roles, applies state and reachability updates, emits ecosystem command events, and exposes the behavior through WebSocket commands. ChangesDynamic endpoint lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant WebSocketClient
participant BridgeWsServer
participant BridgeNode
participant EndpointRegistry
participant MatterEndpoint
WebSocketClient->>BridgeWsServer: attach endpoint specifications
BridgeWsServer->>BridgeNode: reconcile validated endpoints
BridgeNode->>EndpointRegistry: create or update endpoints
EndpointRegistry->>MatterEndpoint: apply role and initial state
MatterEndpoint-->>EndpointRegistry: endpoint status
EndpointRegistry-->>BridgeNode: reconciliation status
BridgeNode-->>BridgeWsServer: status report
BridgeWsServer-->>WebSocketClient: attach response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…atch
Silent-failure findings from the three reviews, with fail-before regression
tests for each:
- closeOne evicted the endpoint before awaiting close(), so a throwing close
left an accessory live in every ecosystem that the registry denied existed;
the retry then answered {removed: false}, which §3.3 defines as success.
Close first, restore the listeners on failure, surface `internal`.
- No mutex on the registry: a superseding attach on a new socket (the
plugin-crash-restart window) planned its diff from a snapshot the incumbent's
paced reconcile was still mutating. All mutating facade calls now run on one
promise chain, and a queued operation says so.
- A reconcile that failed after mutation started skipped the batched
ConfigurationVersion bump and answered a bare `internal`. It now bumps if
anything changed and logs how far it got and what is live.
- set_state answered success for keys the role does not speak, so a whole
role's writes could go nowhere unnoticed (the plugin does not await it).
Keys given but none consumed is now malformed_args naming them; an empty
states stays a lawful no-op, and a partly-understood one still applies.
- A missing $Changed observable was skipped silently, which would leave a
command family permanently dead with no evidence; it now throws at
construction. The handler body is guarded so a throw cannot escape into
matter.js's commit path and take the process down.
- managedTransitionTimeHandling pinned false on the level-bearing roles:
managed transition steps arrive as offline contexts and the echo guard would
eat every brightness change.
Also: dropped events and non-OPEN sends are logged; a result that cannot be
serialised still gets a response (§1); StatusReport gains `driftChecked` so an
empty `drift` is not read as an all-clear; label drift compares productLabel
too; clamps log at debug. The stub bridge now retains endpoint numbers as
matter.js does, upsert_endpoint_unknown_role graduates to a live golden frame,
and set_state_bad_keys joins it on both suites.
Tests: node 193 (173 pass, 20 skipped), python 1633.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
bridge-node/src/endpoints.ts (1)
651-657: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider guarding each
offcall in the teardown.If one
offthrows, the loop stops and the remaining handlers stay subscribed. A removed endpoint then keeps emittingcommandevents for a device the plugin no longer exports, which is the exact failure the teardown exists to prevent. matter.js is not expected to throw here, so this is defensive only.♻️ Proposed guard
return () => { for (const off of teardown) { - off(); + try { + off(); + } catch (error) { + log(`Failed to detach a listener for endpoint ${spec.indigoDeviceId}: ${describeErrorWithStack(error)}`); + } } teardown.length = 0; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bridge-node/src/endpoints.ts` around lines 651 - 657, Update the teardown function to invoke each handler in teardown defensively so one off call throwing does not prevent later handlers from being unsubscribed. Preserve clearing teardown after attempting all removals, and keep the existing endpoint teardown behavior unchanged.bridge-node/src/reconcile.ts (2)
108-116: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider refusing an unrecognized
intentstring.An
intentsuch as"replace-all"returnsfalsehere. The attach then fails later withmass_removal_refused, which names the correct literal but not the actual typo. A direct refusal reports the real mistake at the parse boundary. The current behavior fails safe, so this is optional.♻️ Proposed refusal
if (typeof intent !== "string") { throw new ProtocolError(ErrorCode.malformedArgs, "intent must be a string"); } - return intent === INTENT_REPLACE_ALL; + if (intent !== INTENT_REPLACE_ALL) { + throw new ProtocolError(ErrorCode.malformedArgs, `intent must be ${INTENT_REPLACE_ALL} when present`); + } + return true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bridge-node/src/reconcile.ts` around lines 108 - 116, Optionally update parseReplaceAll to reject non-empty, unrecognized intent strings instead of returning false, while preserving undefined as false and INTENT_REPLACE_ALL as true. Use the existing ProtocolError malformedArgs pattern so typos are reported at the parsing boundary.
40-45: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider narrowing
parseDeviceIdto a positive safe integer.
Number.isIntegeraccepts0, negative values, and magnitudes such as1e21. Each value flows intoendpointIdForanduniqueIdForinbridge-node/src/endpoints.ts, which build the persisted identity key by string interpolation. That yields ids such asindigo--5orindigo-1e+21. Indigo device ids are positive, so refusing the rest here keeps the persisted identity domain tight.♻️ Proposed stricter gate
export function parseDeviceId(value: unknown): number { - if (typeof value !== "number" || !Number.isInteger(value)) { - throw new ProtocolError(ErrorCode.malformedArgs, "indigoDeviceId must be an integer"); + if (typeof value !== "number" || !Number.isSafeInteger(value) || value <= 0) { + throw new ProtocolError(ErrorCode.malformedArgs, "indigoDeviceId must be a positive integer"); } return value; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bridge-node/src/reconcile.ts` around lines 40 - 45, Update parseDeviceId to accept only positive safe integers: retain the number and integer checks, and additionally require Number.isSafeInteger(value) with value greater than zero before returning it. Keep the existing ProtocolError and malformedArgs behavior for all invalid values.docs/BRIDGE_PROTOCOL.md (1)
252-256: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a
statesbullet to the §4.1 field list.The bullet list describes
indigoDeviceId,role,label,reachable, andoptions, but it skipsstates, which the JSON example above carries. One line that points at §4.2 for the role vocabulary makes the field list complete.📝 Proposed addition
- `reachable` — Bridged Device Basic Information `Reachable`. **Omitting it means `true`.** An absent flag says nothing about availability, and the other reading — an accessory that greys itself out in every ecosystem because the plugin left a field off — is the worse default by far. +- `states` — the role's state keys and their Indigo-natural units (§4.2). An + omitted or empty object means "no state asserted"; unknown keys are refused. - `options` — role-specific extras (e.g. window-covering polarity).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/BRIDGE_PROTOCOL.md` around lines 252 - 256, Add a `states` bullet to the §4.1 field list, describing it as role-specific state data and directing readers to §4.2 for the supported role vocabulary. Place it alongside the existing `options` entry so the documented fields match the JSON example.bridge-node/test/units.test.ts (1)
55-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a non-finite input case.
The clamp suite covers out-of-range finite values. The production guards in
bridge-node/src/endpoints.tsaccept anytypeof value === "number", which admitsNaN.clamppropagatesNaN, sopercentToMatter(NaN)returnsNaNtoday. Pinning the current behavior here records whetherNaNmust be clamped or must reach matter.js validation.💚 Proposed additional assertions
it("clamps out-of-range inputs rather than producing illegal Matter values", () => { assert.equal(percentToMatter(-5), 0); assert.equal(percentToMatter(500), MATTER_LEVEL_MAX); assert.equal(matterToPercent(-1), 0); assert.equal(matterToPercent(9999), 100); }); + + it("pins the behaviour for a non-finite percentage", () => { + // `levelPatch` admits any `typeof number`, so NaN reaches the converter. + assert.ok(Number.isNaN(percentToMatter(Number.NaN))); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bridge-node/test/units.test.ts` around lines 55 - 60, Extend the clamp test in the percentToMatter/matterToPercent suite with a NaN assertion that captures the intended current behavior, and update the conversion or validation logic only if needed to satisfy that contract. Use the existing percentToMatter and matterToPercent symbols and preserve finite-value clamping behavior.tests/test_bridge_protocol_frames.py (1)
36-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDetect a bare-name collision when building
BY_NAME.
_exchanges()returns live keys first andpending:-prefixed keys second. Stripping the prefix therefore lets a pending entry silently overwrite a live entry with the same bare name, because the pending key is inserted last. No name collides today, because a frame moves out of the pending section when the node implements it. A graduation commit that copies a frame instead of moving it would leave both copies, and everyBY_NAMEassertion would then test the stale pending shape without failing.Assert uniqueness so the duplicate fails loudly.
♻️ Proposed guard
-BY_NAME = {name.removeprefix("pending:"): exchange for name, exchange in EXCHANGES.items()} +_BARE_NAMES = [name.removeprefix("pending:") for name in EXCHANGES] +assert len(_BARE_NAMES) == len(set(_BARE_NAMES)), \ + f"a frame exists both live and pending: {sorted({n for n in _BARE_NAMES if _BARE_NAMES.count(n) > 1})}" +BY_NAME = {name.removeprefix("pending:"): exchange for name, exchange in EXCHANGES.items()}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_bridge_protocol_frames.py` around lines 36 - 39, Update the BY_NAME construction to detect duplicate bare names after removing the pending: prefix, raising an assertion or equivalent failure instead of silently overwriting an existing live entry. Preserve the current mapping for unique names and ensure duplicate live/pending entries fail loudly during test setup.bridge-node/test/protocol.test.ts (1)
288-295: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep the follow-up assertions: the
incumbent.closedcheck alone is racy.
incumbent.closedis set from the socket'close'event on a different socket than the refusal response. If supersession ever moved before parsing, the server would callincumbent.close()and the close frame could still be in flight when Line 289 runs, so that assertion alone could pass.The assertions at Lines 293-295 are what make the test sound: the incumbent still receives a
commandevent and still answersget_status. Add a note so a later cleanup does not remove them and leave only the flag check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bridge-node/test/protocol.test.ts` around lines 288 - 295, Preserve the follow-up assertions after the incumbent.closed check in the protocol test: keep cold.emitCommand(golden.command_on_off.data as never) with the incumbent.next() assertion and the incumbent.request(golden.get_status.request) response assertion. Add a concise comment explaining that these event and command-response checks are required because incumbent.closed alone is racy.bridge-node/test/integration.test.ts (1)
44-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: share
ecosystemWritewithregistry.test.ts.
ecosystemWriteand theChangeObservableinterface are duplicated inbridge-node/test/registry.test.ts(Lines 739-764). Both copies emit on the real$Changedobservable with{ offline: false }. If the matter.js observable signature changes, both copies must change together.Move the helper into a shared test module, for example
bridge-node/test/matter-writes.ts, and import it in both files. Process isolation per test file is unaffected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bridge-node/test/integration.test.ts` around lines 44 - 60, Extract the duplicated ChangeObservable interface and ecosystemWrite helper from integration.test.ts and registry.test.ts into a shared test module, then import and use that shared helper in both tests. Preserve the existing behavior of locating the `${attribute}$Changed` observable, asserting its presence, and emitting with offline: false.bridge-node/src/ws-server.ts (1)
355-373: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNarrow the doc comment:
truemeans queued, not delivered.
socket.sendfromwsaccepts the write and defers network failures outside thistry/catch. The return value should describe what this helper can actually prove: the frame passed serialization and was accepted for sending.♻️ Proposed comment change
/** - * Write one frame, reporting whether it went. + * Queue one frame, reporting whether it was accepted for sending. * - * Never throws: a serialisation failure here would otherwise escape into - * whatever was mid-flight (an observable, a handler chain) rather than into - * the response the caller is trying to send. + * Never throws: a serialisation failure would otherwise escape into + * whatever was mid-flight (an observable, a handler chain) rather than into + * the response the caller is trying to send. A frame that fails *after* + * being queued surfaces on the socket `error` listener instead. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bridge-node/src/ws-server.ts` around lines 355 - 373, Update the doc comment for the private send method to state that true means the frame serialized successfully and was accepted or queued by socket.send, not that it was delivered; preserve the existing never-throws and false-for-unavailable-socket behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bridge-node/src/endpoints.ts`:
- Around line 183-194: The colorControlDefaults function always reports xy
support, including for color-temperature-only devices. Add an xy parameter to
colorControlDefaults, use it in colorCapabilities, and update each caller so
ColorTemperatureLightDevice passes false while ExtendedColorLightDevice passes
true.
---
Nitpick comments:
In `@bridge-node/src/endpoints.ts`:
- Around line 651-657: Update the teardown function to invoke each handler in
teardown defensively so one off call throwing does not prevent later handlers
from being unsubscribed. Preserve clearing teardown after attempting all
removals, and keep the existing endpoint teardown behavior unchanged.
In `@bridge-node/src/reconcile.ts`:
- Around line 108-116: Optionally update parseReplaceAll to reject non-empty,
unrecognized intent strings instead of returning false, while preserving
undefined as false and INTENT_REPLACE_ALL as true. Use the existing
ProtocolError malformedArgs pattern so typos are reported at the parsing
boundary.
- Around line 40-45: Update parseDeviceId to accept only positive safe integers:
retain the number and integer checks, and additionally require
Number.isSafeInteger(value) with value greater than zero before returning it.
Keep the existing ProtocolError and malformedArgs behavior for all invalid
values.
In `@bridge-node/src/ws-server.ts`:
- Around line 355-373: Update the doc comment for the private send method to
state that true means the frame serialized successfully and was accepted or
queued by socket.send, not that it was delivered; preserve the existing
never-throws and false-for-unavailable-socket behavior.
In `@bridge-node/test/integration.test.ts`:
- Around line 44-60: Extract the duplicated ChangeObservable interface and
ecosystemWrite helper from integration.test.ts and registry.test.ts into a
shared test module, then import and use that shared helper in both tests.
Preserve the existing behavior of locating the `${attribute}$Changed`
observable, asserting its presence, and emitting with offline: false.
In `@bridge-node/test/protocol.test.ts`:
- Around line 288-295: Preserve the follow-up assertions after the
incumbent.closed check in the protocol test: keep
cold.emitCommand(golden.command_on_off.data as never) with the incumbent.next()
assertion and the incumbent.request(golden.get_status.request) response
assertion. Add a concise comment explaining that these event and
command-response checks are required because incumbent.closed alone is racy.
In `@bridge-node/test/units.test.ts`:
- Around line 55-60: Extend the clamp test in the
percentToMatter/matterToPercent suite with a NaN assertion that captures the
intended current behavior, and update the conversion or validation logic only if
needed to satisfy that contract. Use the existing percentToMatter and
matterToPercent symbols and preserve finite-value clamping behavior.
In `@docs/BRIDGE_PROTOCOL.md`:
- Around line 252-256: Add a `states` bullet to the §4.1 field list, describing
it as role-specific state data and directing readers to §4.2 for the supported
role vocabulary. Place it alongside the existing `options` entry so the
documented fields match the JSON example.
In `@tests/test_bridge_protocol_frames.py`:
- Around line 36-39: Update the BY_NAME construction to detect duplicate bare
names after removing the pending: prefix, raising an assertion or equivalent
failure instead of silently overwriting an existing live entry. Preserve the
current mapping for unique names and ensure duplicate live/pending entries fail
loudly during test setup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 398e9084-0444-42a5-b1b8-7f4fa3436345
📒 Files selected for processing (21)
bridge-node/package.jsonbridge-node/src/endpoints.tsbridge-node/src/node.tsbridge-node/src/protocol.tsbridge-node/src/reconcile.tsbridge-node/src/registry.tsbridge-node/src/storage.tsbridge-node/src/ws-server.tsbridge-node/test/fixture-shapes.tsbridge-node/test/fixtures.test.tsbridge-node/test/integration.test.tsbridge-node/test/protocol.test.tsbridge-node/test/reconcile.test.tsbridge-node/test/registry.test.tsbridge-node/test/stub-bridge.tsbridge-node/test/units.test.tsdocs/BRIDGE_PROTOCOL.mdindigo-matter.indigoPlugin/Contents/Info.plisttests/fixtures/bridge_protocol/frames.jsontests/test_bridge_client.pytests/test_bridge_protocol_frames.py
Comment on lines
+183
to
+194
| function colorControlDefaults(hueSaturation: boolean): Record<string, unknown> { | ||
| return { | ||
| colorMode: ColorControl.ColorMode.ColorTemperatureMireds, | ||
| enhancedColorMode: ColorControl.EnhancedColorMode.ColorTemperatureMireds, | ||
| colorCapabilities: { colorTemperature: true, xy: true, hueSaturation }, | ||
| colorTempPhysicalMinMireds: MIREDS_MIN, | ||
| colorTempPhysicalMaxMireds: MIREDS_MAX, | ||
| coupleColorTempToLevelMinMireds: MIREDS_MIN, | ||
| startUpColorTemperatureMireds: null, | ||
| ...(hueSaturation ? { currentHue: 0, currentSaturation: 0 } : {}), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check which ColorControl features matter.js 0.17.8 gives each colour device type.
set -uo pipefail
fd -t d '^main$' bridge-node/node_modules/@matter -d 3 || fd -t d '`@matter`' bridge-node/node_modules -d 2
for name in color-temperature-light extended-color-light; do
echo "=== $name ==="
fd -t f "${name}\.(d\.ts|js)$" bridge-node/node_modules/@matter --exec rg -n -C6 'ColorControlServer|ColorControl\.Cluster|\.with\(' {}
done
# Cross-check the declared capability field and its conformance.
fd -t f 'color-control.*\.d\.ts$' bridge-node/node_modules/@matter --exec rg -n -C4 'colorCapabilities|ColorCapabilities' {}Repository: simons-plugins/indigo-matter
Length of output: 50384
Report xy capabilities only for XY ColorControl servers.
ColorTemperatureLightDevice uses ColorControlServer.with("ColorTemperature"), so advertises xy: false; ExtendedColorLightDevice uses ColorControlServer.with("Xy", "ColorTemperature"), so accepts the existing xy: true. Make colorControlDefaults accept an xy flag and pass false for the CT-only role to avoid reporting unsupported ColorControl attributes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bridge-node/src/endpoints.ts` around lines 183 - 194, The
colorControlDefaults function always reports xy support, including for
color-temperature-only devices. Add an xy parameter to colorControlDefaults, use
it in colorCapabilities, and update each caller so ColorTemperatureLightDevice
passes false while ExtendedColorLightDevice passes true.
This was referenced Aug 5, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First half of milestone E3: the bridge node's endpoint set becomes fully attach-driven for the five lighting/plug roles (onOffPlugInUnit, onOffLight, dimmableLight, colorTemperatureLight, extendedColorLight).
endpoints.ts— role→device-type factory (.with(BridgedDeviceBasicInformationServer)), §4.2 unit converters beside their clusters (0-100↔0-254 round-half-up with 0↔off exact, mireds clamp, hue/sat), cluster-change listeners with teardown,ctx.offlineecho guardreconcile.ts— matter.js-free arg parsing + reconcile planner incl. the §3.1 mass-removal guard (intent: replace_all)registry.ts— reconcile/upsert/remove/setState/setReachable, ~100ms paced bulk removals, ConfigurationVersion bumpsattachis a full reconcile;role_changerefused on upsert per §4.1 (attach recreates, logged)999001in get_status) — Python fixture suite green on the same bytesmatter.js 0.17.8 findings (pinned in comments)
ExtendedColorLightDeviceomits the spec-mandatory HueSaturation feature (restored via.with()); ColorControl needs five 'optional' attributes seeded or fails opaquely;currentLevelis min-1 (0% writes as 1, lossless); bridgedincreaseConfigurationVersionthrows unless seeded.Verification
TS: 174 tests (153 pass, 0 fail, 21 pending-skips). Python: 1630 green on shared fixtures. Live smoke: attach/upsert/set_state/remove/mass-removal-guard all verified over WS, plus endpoint numbers stable across restart with a reordered endpoint set (persisted id map, not creation order).
For E3b (plugin side)
E4 roles gate the whole attach with
internal(not partial-apply) until implemented; role change via attach = recreate (accessory loses name/room — plugin should gate); hue round-trips ±1°;setColorfires twice per moveToHueAndSaturation (idempotent pair).🤖 Generated with Claude Code
https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
Summary by CodeRabbit
New Features
Documentation
Chores