fix(color): XY fallback for lights without the optional HueSaturation feature (#60)#61
Conversation
…#60) HueSaturation is OPTIONAL per the Matter spec — an Extended Color Light need only implement XY + ColorTemperature — but the RGB path sent MoveToHueAndSaturation unconditionally, so XY-only lights (including matter.js's ExtendedColorLightDevice on the rig) rejected every colour command with UNSUPPORTED_COMMAND (live repro 2026-06-12). - Subscribe + cache ColorCapabilities (0x400A) in a new colorCapabilities device state; RGB set-colour picks MoveToHueAndSaturation when HS is supported, MoveToColor (sRGB→CIE xy) when the device is XY-only, and keeps historical HS behaviour while capabilities are unknown. - Receive side: CurrentX/CurrentY (0x0003/0x0004) updates recompute the Indigo RGB states via xy→sRGB, with raw values kept in new colorX/colorY states so one-axis reports stay coherent. Pre-fix devices without the new states degrade quietly. - docs/TESTING.md: how the plugin is tested — unit suite, device zoo, the matter.js virtual fleet (incl. the rig rules), and the live validation method, with the real-hardware validation table. 756 tests. Closes #60. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 11 minutes and 5 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR implements capability-aware Extended Color Light support to resolve issue ChangesExtended Color Light Feature
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Live deploy of the XY fix surfaced it: Indigo builds a device's state list at creation and never re-reads Devices.xml, so fielded colour devices lacked the new colorCapabilities state and Indigo logged "state key colorCapabilities not defined" on every report. deviceStartComm now calls stateListOrDisplayStateIdChanged() (the documented refresh), and the capabilities write degrades quietly when the state is still missing, same as the other guarded states. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s creation Live evidence (issue #62): a device whose type was changed via Indigo's Type menu is left configured=False, drops out of iter("self"), and never receives deviceStartComm — so the index loses it, _unique_name can't see it, and reconcile's recreate attempt fails NameNotUniqueError every pass as a raw traceback. Both #58 guards are structurally blind to this state. The collision now logs the remedy (delete the device, reload) instead of a traceback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Live-verified on jarvis: the previously-failing XY-only rig light received and ACCEPTED |
![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
🤖 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 `@tests/test_device_sync.py`:
- Line 1544: Replace the unused unpacked variable in the test by changing the
assignment "indigo_mock, devices = indigo_env" to use the dummy variable pattern
(e.g., "indigo_mock, _ = indigo_env") so that "devices" is not left unused;
update the line where the indigo_env fixture is unpacked (referencing
indigo_mock and devices) accordingly.
🪄 Autofix (Beta)
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
Run ID: 2791a7b7-b2ab-4027-a008-490a8ba53b34
📒 Files selected for processing (10)
README.mddocs/TESTING.mdindigo-matter.indigoPlugin/Contents/Info.plistindigo-matter.indigoPlugin/Contents/Server Plugin/Devices.xmlindigo-matter.indigoPlugin/Contents/Server Plugin/device_sync.pyindigo-matter.indigoPlugin/Contents/Server Plugin/matter_handlers/color_control.pyindigo-matter.indigoPlugin/Contents/Server Plugin/plugin.pytests/test_device_sync.pytests/test_dimmer_color.pytests/test_plugin_module.py
| """A NameNotUniqueError on create (issue #62: type-edited device invisible | ||
| to iter('self') still holds the name server-side) must log the actionable | ||
| remedy, not a raw traceback, and must not abort the batch.""" | ||
| indigo_mock, devices = indigo_env |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix unused variable flagged by Ruff.
The devices variable is unpacked but never used in this test. Replace it with _ to follow the dummy variable pattern.
🧹 Proposed fix
- indigo_mock, devices = indigo_env
+ indigo_mock, _ = indigo_env📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| indigo_mock, devices = indigo_env | |
| indigo_mock, _ = indigo_env |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 1544-1544: Unpacked variable devices is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 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_device_sync.py` at line 1544, Replace the unused unpacked variable
in the test by changing the assignment "indigo_mock, devices = indigo_env" to
use the dummy variable pattern (e.g., "indigo_mock, _ = indigo_env") so that
"devices" is not left unused; update the line where the indigo_env fixture is
unpacked (referencing indigo_mock and devices) accordingly.
Source: Linters/SAST tools
Two long-read pages in the Domio site's Aizome direction (indigo dye + warm paper, Fraunces/General Sans, letterpress detailing) but as their own engineering gazette: numbered sections, drop caps, rubber-stamp validation marks, the share-model pipeline as a real diagram, and the four test layers drawn as strata. Leaf accent for the landscape page, ochre for the proving ground; plus a small landing index and a shared stylesheet. Scroll-reveal is progressive-enhancement only (no-JS, print and screenshot tools always get visible content). Serve from GitHub Pages → main /docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) Live testing found the W slider (whiteLevel) dead: Matter colour lights have no separate white channel, so the handler ignored the channel entirely and, with no Matter attribute to echo it, the Indigo slider snapped back even when RGB worked. - whiteLevel now maps to its Matter meaning: MoveToColorTemperature at the requested/stored white temperature plus MoveToLevelWithOnOff at the slider's intensity (W=0 → off). Handlers may now return a LIST of actions; the plugin sends each sequentially, individually acked. - whiteLevel (and the RGB-side W zeroing) is echoed optimistically via updateStatesOnServer — cosmetic-only, guarded, since no device report will ever carry it. 765 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Closes #60.
What
Clicking an RGB colour in Indigo failed with
Unsupported command (code 129)on the rig's colour light. Root cause:ColorControlHandlersentMoveToHueAndSaturationunconditionally, but HueSaturation is optional per the Matter spec — even an Extended Color Light is only required to implement XY + ColorTemperature, and matter.js'sExtendedColorLightDevice(and real XY-only bulbs) reject HS commands outright.How
ColorCapabilities(0x400A) is subscribed and cached in a newcolorCapabilitiesdevice state (primed from the node snapshot at reconcile).MoveToColorwith a standard sRGB→CIE-xy conversion (D65, clamped to the spec's 0xFEFF); capabilities unknown (pre-fix device / not yet primed) → historical HS behaviour, no guessing.CurrentX/CurrentYreports recompute Indigo's RGB states (xy→sRGB, brightness frombrightnessLevel); raw values kept in newcolorX/colorYstates so single-axis reports combine correctly. Devices created before the fix lack the new states and degrade quietly (same SDK guard pattern aswhiteTemperature).Also in this PR
docs/TESTING.md(requested by Simon): the four-layer test story for users and plugin devs — unit suite, the device zoo (with its track record and "add your weird device to ZOO" workflow), the matter.js virtual fleet (ports/discriminators/relaunch rules), and the live-validation method, plus the real-hardware validation table (Tapo P110M Wi-Fi, Aqara FP300 Thread).Testing
756 tests (+8): XY-only command selection with CIE-value assertions, HS-capable and unknown-capability paths, capability caching, X/Y report recombination into RGB, pre-fix-device degradation, conversion round-trips, black→white-point. Live verification on jarvis with the XY-only rig light to follow on this branch build.
Version: 2026.2.22 → 2026.2.23 (patch).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores