rob/CampervanControlSystems
Watch
1
0
Fork
You've already forked CampervanControlSystems
 0
Code Issues Pull requests 2 Projects Releases 86 Packages 1 Wiki Activity Actions

Make the kitchen strip a WS2805 RGBCCT colour light #107

Merged
rob merged 13 commits from feat/kitchen-cct into main 2026-10-13 20:24:18 +00:00
Conversation 5 Commits 13 Files changed 35 +1599 -48
Claude commented 2026-10-12 08:38:43 +00:00
Collaborator

Makes the kitchen strip an RGBCCT colour light (EP_COLOUR_LIGHT) on a 12 V WS2805 strip. Adds the CMD_COLOUR / STATE_COLOUR codecs and shared vectors, a ColourEndpoint that turns each change into one target colour, and two libraries: lib/led-strips (RMT frame-buffer driver) and lib/strip-animations (uniform fade and middle-out, picked by id). Kitchen uses 26 pixels on D1 with middle-out and QuadIn easing; touch is a plain toggle. Ticket CCS-LLA-5, reworked under CCS-LLA-29.

Behaviour: brightness 0-100 %, colour temperature in K (clamped to 2700-6500), hue 0-359 degrees and saturation 0-100 %. Saturation 0 is pure white at the colour temperature, 100 is the pure hue, and values between blend the two. In CMD_COLOUR, 0xFF / 0xFFFF leaves a field unchanged. Out-of-range values clamp and never become a sentinel. The light takes CMD_COLOUR and CMD_SWITCH; CMD_LEVEL is ignored.

Wire change, approved by the owner: rebased onto main after the origin and lockout removal (#116), so nothing here carries either. EP_CCT_LIGHT, CMD_CCT and STATE_CCT become EP_COLOUR_LIGHT, CMD_COLOUR and STATE_COLOUR with the same numeric values and new 8-byte layouts; the valid-flag byte is gone. Every node needs reflashing and the Home Assistant integration needs updating together. Shared vectors are regenerated.

Tested: pio test -e native (580 cases, pass), pio run for all five envs. Not re-tested on the bench strip since the colour rework.

Makes the kitchen strip an RGBCCT colour light (`EP_COLOUR_LIGHT`) on a 12 V WS2805 strip. Adds the `CMD_COLOUR` / `STATE_COLOUR` codecs and shared vectors, a `ColourEndpoint` that turns each change into one target colour, and two libraries: `lib/led-strips` (RMT frame-buffer driver) and `lib/strip-animations` (uniform fade and middle-out, picked by id). Kitchen uses 26 pixels on D1 with middle-out and QuadIn easing; touch is a plain toggle. Ticket CCS-LLA-5, reworked under CCS-LLA-29. Behaviour: brightness 0-100 %, colour temperature in K (clamped to 2700-6500), hue 0-359 degrees and saturation 0-100 %. Saturation 0 is pure white at the colour temperature, 100 is the pure hue, and values between blend the two. In `CMD_COLOUR`, `0xFF` / `0xFFFF` leaves a field unchanged. Out-of-range values clamp and never become a sentinel. The light takes `CMD_COLOUR` and `CMD_SWITCH`; `CMD_LEVEL` is ignored. Wire change, approved by the owner: rebased onto main after the origin and lockout removal (#116), so nothing here carries either. `EP_CCT_LIGHT`, `CMD_CCT` and `STATE_CCT` become `EP_COLOUR_LIGHT`, `CMD_COLOUR` and `STATE_COLOUR` with the same numeric values and new 8-byte layouts; the valid-flag byte is gone. Every node needs reflashing and the Home Assistant integration needs updating together. Shared vectors are regenerated. Tested: `pio test -e native` (580 cases, pass), `pio run` for all five envs. Not re-tested on the bench strip since the colour rework.
Make the kitchen strip a WS2805 CCT light
All checks were successful
Build and test / build (pull_request) Successful in 1m32s
Details
Build and test / release (pull_request) Has been skipped
Details
b59e645d44
Claude reviewed 2026-10-12 08:39:52 +00:00
Claude left a comment

Findings only.

  1. src/node_kitchen/main.cpp: Ease::QuintIn is now applied to an 8-bit output (cct_endpoint.cpp applyOutput). x^5 * 255 rounds to 0 below roughly 26% brightness, then steps 0, 1, 1, 2. The bottom quarter of the slider is dark and coarse. The old PWM had far more bits. Use a milder curve (or Linear), or dither/gamma at higher precision. Please confirm on the strip.
  2. docs/can-protocol.md is untouched. CMD_CCT and STATE_CCT say only "Colour temperature, uint16". The code assumes Kelvin, clamps silently to the secondary range, and ignores the ramp byte for colour temperature (it jumps). CLAUDE.md requires the spec to say this in the same PR, in as few words as possible.
  3. docs/hardware.md line 40 still says "D1 | PWM, under-cabinet strip". It is now the WS2805 data line.
  4. main.cpp: stripPixelCount = 60 is a guess, flagged by a "not yet counted" comment. Either use the real count or leave the placeholder out of a merged PR. A comment saying "set this" is a TODO.
  5. cct_endpoint.cpp applyOutput: if warmKelvin == coldKelvin, coldShare divides by zero and the float to uint8_t cast is UB. Cheap guard, or reject the config.
  6. ws2805.cpp begin(): items_ is allocated before rmtInit and never checked. If new or rmtInit fails, service only guards rmt_, so a failed items_ allocation would crash. Guard both.
  7. Tests: nothing drives a CMD_CCT frame through Node dispatch; all go straight to onCommand. Nothing covers a command carrying both flags under an active lockout. emitState with Origin::Failsafe and the failsafe flag is also untested. The pixel-timing and RMT code in ws2805.cpp has no test (the file is empty on native), so its bit timings are unchecked until it runs on hardware.
  8. Test style: 0 | CctFlag::Brightness is a needless idiom. Pass the flag alone, or build the combined flags the way the codec tests do.
Findings only. 1. src/node_kitchen/main.cpp: `Ease::QuintIn` is now applied to an 8-bit output (cct_endpoint.cpp `applyOutput`). x^5 * 255 rounds to 0 below roughly 26% brightness, then steps 0, 1, 1, 2. The bottom quarter of the slider is dark and coarse. The old PWM had far more bits. Use a milder curve (or Linear), or dither/gamma at higher precision. Please confirm on the strip. 2. docs/can-protocol.md is untouched. CMD_CCT and STATE_CCT say only "Colour temperature, uint16". The code assumes Kelvin, clamps silently to the secondary range, and ignores the ramp byte for colour temperature (it jumps). CLAUDE.md requires the spec to say this in the same PR, in as few words as possible. 3. docs/hardware.md line 40 still says "D1 | PWM, under-cabinet strip". It is now the WS2805 data line. 4. main.cpp: `stripPixelCount = 60` is a guess, flagged by a "not yet counted" comment. Either use the real count or leave the placeholder out of a merged PR. A comment saying "set this" is a TODO. 5. cct_endpoint.cpp `applyOutput`: if `warmKelvin == coldKelvin`, `coldShare` divides by zero and the float to uint8_t cast is UB. Cheap guard, or reject the config. 6. ws2805.cpp `begin()`: `items_` is allocated before `rmtInit` and never checked. If `new` or `rmtInit` fails, `service` only guards `rmt_`, so a failed `items_` allocation would crash. Guard both. 7. Tests: nothing drives a CMD_CCT frame through `Node` dispatch; all go straight to `onCommand`. Nothing covers a command carrying both flags under an active lockout. `emitState` with `Origin::Failsafe` and the failsafe flag is also untested. The pixel-timing and RMT code in ws2805.cpp has no test (the file is empty on native), so its bit timings are unchecked until it runs on hardware. 8. Test style: `0 | CctFlag::Brightness` is a needless idiom. Pass the flag alone, or build the combined flags the way the codec tests do.
Document the Kelvin unit and the strip's data pin
All checks were successful
Build and test / build (pull_request) Successful in 1m39s
Details
Build and test / release (pull_request) Has been skipped
Details
218223c5b1
Use a gentler brightness curve and 1.25 us bit period on the strip
All checks were successful
Build and test / build (pull_request) Successful in 1m31s
Details
Build and test / release (pull_request) Has been skipped
Details
dc46b67df8
rob reviewed 2026-10-12 08:43:40 +00:00
lib/node-runtime/src/ws2805.cpp Outdated
@ -0,0 +13,4 @@
constexpr uint8_t bitsPerPixel = channelsPerPixel * 8;
constexpr float tickNs = 50;
constexpr uint32_t rmtItem(uint32_t highTicks, uint32_t lowTicks) { return highTicks | (1u << 15) | (lowTicks << 16); }
rob commented 2026-10-12 08:43:40 +00:00
Owner

What does rmtItem mean?

What does `rmtItem` mean?
Claude commented 2026-10-12 08:44:58 +00:00
Author
Collaborator

Replaced rmtItem with a pulse() that sets the core's own rmt_data_t fields (level0, duration0, level1, duration1). Timings unchanged.

Replaced `rmtItem` with a `pulse()` that sets the core's own `rmt_data_t` fields (`level0`, `duration0`, `level1`, `duration1`). Timings unchanged.
Build WS2805 bits from the core's rmt_data_t fields
All checks were successful
Build and test / build (pull_request) Successful in 1m40s
Details
Build and test / release (pull_request) Has been skipped
Details
f9a082bc6f
Run the kitchen CCT light through the strip animations
Some checks failed
Build and test / release (pull_request) Has been cancelled
Details
Build and test / build (pull_request) Has been cancelled
Details
168c7e1124
Claude reviewed 2026-10-12 11:17:36 +00:00
Claude left a comment

Findings only.

  1. src/node_kitchen/main.cpp, onTouch: leftover debug Serial.println("AAAA"). Remove it and the braces.
  2. lib/node-runtime/src/cct_endpoint.cpp, loop(): spec 8.4 says ramp progress is reported at 5 Hz and the resting value always. This only marks dirty when the animation ends. A long ramp sends one frame with Ramping set, then silence. Copy the lastRampReportMs_ throttle from LevelEndpoint::loop, or change the spec in this PR.
  3. cct_endpoint.cpp, onCommand: CmdLevel StopRamp, RampUp and RampDown fall to default: return true and do nothing. LevelEndpoint handles StopRamp. Either handle it or say in the spec that CCT ignores these.
  4. cct_endpoint.cpp, colour()/transition(): a low level on an eased curve rounds to a dark pixel (level 1 with QuadIn gives 0). The strip goes dark while the endpoint reports On. MiddleOut also treats it as "off" via target.dark(). Clamp the lit channel to at least 1 when level > 0.
  5. cct_endpoint.h, selectAnimation(): the old animation keeps running_ == true and is never updated again. Switching mid-ramp leaves it stale. Call transitionTo(colour(), 0, now) on the new one, or stop the old one.
  6. Tests: nothing covers finding 2 (ramp progress frames), 3, or selectAnimation mid-ramp. test_state_cct... only checks the Ramping flag at the start and end.
  7. CONTRIBUTING.md: the new "native environment compiles ..." sentence runs on as one over-long line and breaks the file's wrapping. Rewrap it.
  8. CLAUDE.md: the text "neither depends on the runtime" is true, but node-runtime/library.json now depends on both libraries. Say so, otherwise the "boundary is load-bearing" paragraph reads as if the runtime is at the bottom.
Findings only. 1. src/node_kitchen/main.cpp, onTouch: leftover debug `Serial.println("AAAA")`. Remove it and the braces. 2. lib/node-runtime/src/cct_endpoint.cpp, loop(): spec 8.4 says ramp progress is reported at 5 Hz and the resting value always. This only marks dirty when the animation ends. A long ramp sends one frame with Ramping set, then silence. Copy the `lastRampReportMs_` throttle from LevelEndpoint::loop, or change the spec in this PR. 3. cct_endpoint.cpp, onCommand: CmdLevel `StopRamp`, `RampUp` and `RampDown` fall to `default: return true` and do nothing. LevelEndpoint handles StopRamp. Either handle it or say in the spec that CCT ignores these. 4. cct_endpoint.cpp, colour()/transition(): a low level on an eased curve rounds to a dark pixel (level 1 with QuadIn gives 0). The strip goes dark while the endpoint reports On. MiddleOut also treats it as "off" via `target.dark()`. Clamp the lit channel to at least 1 when level > 0. 5. cct_endpoint.h, selectAnimation(): the old animation keeps `running_ == true` and is never updated again. Switching mid-ramp leaves it stale. Call `transitionTo(colour(), 0, now)` on the new one, or stop the old one. 6. Tests: nothing covers finding 2 (ramp progress frames), 3, or `selectAnimation` mid-ramp. `test_state_cct...` only checks the Ramping flag at the start and end. 7. CONTRIBUTING.md: the new "native environment compiles ..." sentence runs on as one over-long line and breaks the file's wrapping. Rewrap it. 8. CLAUDE.md: the text "neither depends on the runtime" is true, but node-runtime/library.json now depends on both libraries. Say so, otherwise the "boundary is load-bearing" paragraph reads as if the runtime is at the bottom.
Remove debug print from the kitchen touch handler and rewrap docs
All checks were successful
Build and test / build (pull_request) Successful in 1m45s
Details
Build and test / release (pull_request) Has been skipped
Details
7c0e181c17
rob force-pushed feat/kitchen-cct from 7c0e181c17
All checks were successful
Build and test / build (pull_request) Successful in 1m45s
Details
Build and test / release (pull_request) Has been skipped
Details
to f6a4e868fd
Some checks failed
Build and test / release (pull_request) Has been cancelled
Details
Build and test / build (pull_request) Has been cancelled
Details
2026-10-13 19:23:32 +00:00 Compare
Claude changed title from Make the kitchen strip a WS2805 CCT light to Make the kitchen strip a WS2805 RGBCCT colour light 2026-10-13 19:23:44 +00:00
Claude reviewed 2026-10-13 19:24:41 +00:00
Claude left a comment

Three small things, none blocking.

Three small things, none blocking.
CONTRIBUTING.md Outdated
@ -85,1 +83,3 @@
It links the SocketCAN backend so `Node` has a `CanBus` to resolve against, and
The `native` environment compiles `lib/can-protocol`, `lib/node-runtime`,
`lib/led-strips` and `lib/strip-animations`, minus the endpoints that drive
hardware, which compile to nothing off Arduino. It links the SocketCAN backend so `Node` has a `CanBus` to resolve against, and
Claude commented 2026-10-13 19:24:41 +00:00
Author
Collaborator

The rewrap is half done: the It links the SocketCAN backend... line now runs past the wrap width. Rewrap the paragraph.

The rewrap is half done: the `It links the SocketCAN backend...` line now runs past the wrap width. Rewrap the paragraph.
docs/can-protocol.md Outdated
Claude commented 2026-10-13 19:24:41 +00:00
Author
Collaborator

Stale: open question 2 still says "Addressable WWA strips". The kitchen strip is now an RGBCCT colour light. Reword it, or drop it if the per-segment question is settled.

Stale: open question 2 still says "Addressable WWA strips". The kitchen strip is now an RGBCCT colour light. Reword it, or drop it if the per-segment question is settled.
test/test_endpoint_meta/test_endpoint_meta.cpp
@ -13,3 +13,3 @@
constexpr EndpointType allTypes[] = {
EndpointType::Switch, EndpointType::Dimmer, EndpointType::CctLight, EndpointType::Fan, EndpointType::Valve, EndpointType::Motor,
EndpointType::Switch, EndpointType::Dimmer, EndpointType::ColourLight, EndpointType::Fan, EndpointType::Valve, EndpointType::Motor,
Claude commented 2026-10-13 19:24:41 +00:00
Author
Collaborator

The rename widened allTypes and the rows no longer match clang-format (it flags lines 16-18). Run clang-format -i on this file.

The rename widened `allTypes` and the rows no longer match clang-format (it flags lines 16-18). Run `clang-format -i` on this file.
Fix the review nits on formatting and stale wording
Some checks failed
Build and test / release (pull_request) Has been cancelled
Details
Build and test / build (pull_request) Has been cancelled
Details
3be947e040
Finish rewrapping the native environment paragraph
All checks were successful
Build and test / build (pull_request) Successful in 5m36s
Details
Build and test / release (pull_request) Has been skipped
Details
a9f7c81a00
Drop CMD_LEVEL from the colour light
All checks were successful
Build and test / build (pull_request) Successful in 1m30s
Details
Build and test / release (pull_request) Has been skipped
Details
5bc62e7bf2
Give the colour light its own PK_COLOUR presentation kind
All checks were successful
Build and test / build (pull_request) Successful in 1m28s
Details
Build and test / release (pull_request) Has been skipped
Details
1a784fb1de
rob merged commit 17768cd349 into main 2026-10-13 20:24:18 +00:00
rob deleted branch feat/kitchen-cct 2026-10-13 20:24:18 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/CampervanControlSystems!107
No description provided.