Add per-bus white-LED color temperature for accurate auto-white#5654
Open
NerdyGriffin wants to merge 12 commits into
Open
Add per-bus white-LED color temperature for accurate auto-white#5654NerdyGriffin wants to merge 12 commits into
NerdyGriffin wants to merge 12 commits into
Conversation
The Auto-Calculate White "Accurate" mode subtracts the W channel value equally from R, G, B — which implicitly assumes the physical W LED emits RGB(255, 255, 255), i.e. the sRGB white point near D65 / ~6500 K. For 2700 K WW or 5000 K CW strips this shifts the resulting color visibly. Add a per-bus configurable white-LED color temperature (Kelvin) that feeds into autoWhiteCalc, so the W LED's actual R/G/B contribution is computed via colorKtoRGB and used to (a) cap the W channel without overflowing any RGB channel and (b) subtract the correct per-channel amount in ACCURATE mode. The feature is opt-in per bus via a UI checkbox; when off (the default, wk = 0) autoWhiteCalc behaves as before, so existing configs render identically. UI lives next to the per-bus "Auto-calculate W channel from RGB" selector and is only shown when AW mode is Brighter or Accurate. The Kelvin number input is disabled (and not submitted) until the user checks the enable box, at which point it defaults to 6500 K — matching the implicit legacy reference white point. https://claude.ai/code/session_019b31kdwp79ouA3gD5Tox9A Co-authored-by: Claude <noreply@anthropic.com>
DUAL mode (RGBW_MODE_DUAL) falls through to the per-channel-cap branch of autoWhiteCalc whenever the caller hasn't set the manual white value (w == 0) — the same path used by BRIGHTER and ACCURATE. The UI gate was only revealing the WKE checkbox and Kelvin input for modes 1 and 2, so users on DUAL had no way to configure the W-LED color temperature even though their output was affected by _wR/_wG/_wB. Include awv === 3 in the visibility condition and update the comment to reflect the actual code path in bus_manager.cpp. Co-authored-by: Claude <noreply@anthropic.com>
The per-channel-cap branch added in the prior commit ran three integer divisions per pixel even when _whiteKelvin == 0 (the default), because the cached _wR/_wG/_wB values were always read as locals — the compiler could not constant-fold them. For RGBW strips this is a measurable hot- path regression vs the original min(r,g,b) implementation, paid by every user regardless of whether they enabled the feature. Split the else-branch in two: - _whiteKelvin == 0 (feature off, default): identical math to the pre-feature WLED code (w = min RGB, equal subtraction in ACCURATE). - _whiteKelvin > 0 (feature on): the per-channel-cap path that uses the cached W-LED RGB equivalent. Documents the underflow argument (floor division composes back through the subtraction) and the _wB == 0 case near 1900 K explicitly in the comments. Behavior is unchanged for both paths. Co-authored-by: Claude <noreply@anthropic.com>
Rename the enable checkbox label to "Tune RGB to W channel color temperature" so it mirrors the adjacent "Auto-calculate W channel from RGB" selector and reads as a refinement of it. Move the Kelvin number input out of the checkbox line into its own wrapper div (dig<n>wkv) with a dedicated "W channel color temperature:" label, matching the dominant WLED pattern of a checkbox revealing a sub-options block on the following line. UI() now toggles the wrapper's visibility instead of the bare input. Co-authored-by: Claude <noreply@anthropic.com>
Very warm white LEDs can read as neutral even when fed a fairly warm RGB mask, so users need to dial the configured temperature below the previous 1900 K floor to compensate. colorKtoRGB() is well-defined down to 1000 K (blue already clamps to 0 below 1900 K, green stays positive -> 255,68,0), and the per-channel-cap path's existing zero guards already handle the resulting zero channels. Lower the bound in the form validator (set.cpp), the number input's min attribute and the re-enable seed threshold (settings_leds.htm), and update the zero-guard comment in bus_manager.cpp to note that blue is zero across the whole sub-1900 K range now, not just near 1900 K. Co-authored-by: Claude <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds per-bus white-channel color temperature (Kelvin) support: Bus stores a whiteKelvin and cached RGB coefficients, autoWhiteCalc uses either a neutral fast path or per-channel capped subtraction, and the value is persisted, parsed from settings, emitted to the UI JSON, and exposed in the settings page UI/JS. ChangesPer-Bus White Kelvin Feature
Sequence DiagramsequenceDiagram
participant BusManager
participant Bus
participant autoWhiteCalc
BusManager->>Bus: setWhiteKelvin(k)
Bus->>Bus: colorKtoRGB(k) -> cache _wR/_wG/_wB
Note over Bus: _whiteKelvin stored
autoWhiteCalc->>Bus: read _whiteKelvin & cached _wR/_wG/_wB
alt _whiteKelvin == 0
autoWhiteCalc->>autoWhiteCalc: fast path (darkest channel)
else _whiteKelvin > 0
autoWhiteCalc->>autoWhiteCalc: compute per-channel caps
autoWhiteCalc->>autoWhiteCalc: subtract scaled RGB contributions
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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. 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 |
softhack007
added
the
AI
Author
|
I plan to submit a PR to |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
wled00/bus_manager.cpp (1)
101-115: ⚡ Quick winAdd attribution for the AI-generated blocks.
These sections are marked as AI-generated, but the required source/inspiration attribution is still missing.
As per coding guidelines "Document attribution of inspiration / knowledge / sources used in AI-generated code, e.g. link to GitHub repositories or other websites".
Also applies to: 139-158
🤖 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 `@wled00/bus_manager.cpp` around lines 101 - 115, The AI-generated blocks (including the setWhiteKelvin function and the other block at the section covering lines 139-158) need an attribution comment added immediately above each AI-marked region: include a brief "Generated with assistance from [AI tool]" line plus links or identifiers for any external sources or repositories used to inspire the implementation (e.g., the AI model name and any GitHub/URL references), and summarize what was taken from the source (e.g., colorKtoRGB usage/logic). Update the comments around the unique symbols Bus::setWhiteKelvin and the other AI-labeled block to contain that attribution text so reviewers can trace provenance.wled00/data/settings_leds.htm (1)
824-830: 💤 Low valueUse the standard AI block markers for consistency.
The other two new AI blocks in this file (Lines 208/223 and 388/408) use
// AI: below section was generated by an AI…// AI: end. This loadCfg block uses a single inline// AI:instead, deviating from the convention.♻️ Align with the AI marker convention
- { // AI: derive WKE checkbox + WK seed from stored wk (0 = feature off) + // AI: below section was generated by an AI + // derive WKE checkbox + WK seed from stored wk (0 = feature off) + { const wkChkEl = d.getElementsByName("WKE"+i)[0]; const wkEl = d.getElementsByName("WK"+i)[0]; const wkv = parseInt(v.wk) | 0; if (wkChkEl) wkChkEl.checked = wkv > 0; if (wkEl) wkEl.value = wkv > 0 ? wkv : 6500; } + // AI: endAs per coding guidelines: "Mark AI-generated source code blocks with
// AI: below section was generated by an AI/// AI: endcomments".🤖 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 `@wled00/data/settings_leds.htm` around lines 824 - 830, Replace the single inline AI comment in the loadCfg block that handles WKE/WK (the block creating wkChkEl, wkEl and computing wkv) with the standard AI block markers: add a starting comment "// AI: below section was generated by an AI" immediately before the block and an ending comment "// AI: end" after it, so the block wrapping the wkChkEl/wkEl/wkv logic matches the other AI-marked sections; keep the existing code unchanged and follow the same spacing/comment style as the other AI blocks.
🤖 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 `@wled00/bus_manager.cpp`:
- Around line 105-113: The setter Bus::setWhiteKelvin allows any non-zero k
through and thus can accept out-of-range kelvin values; clamp/normalize non-zero
k to the supported 1000–10000K range before storing and using it: if k==0 keep
legacy behavior, otherwise bound k to [1000,10000], assign the normalized value
to _whiteKelvin, then call colorKtoRGB(normalizedK, rgb) and update _wR/_wG/_wB
accordingly so runtime behavior matches the shared contract.
In `@wled00/data/settings_leds.htm`:
- Around line 213-222: The comment says wkChk should seed WK when re-enabling
from a blank or sub-min value but parseInt("") yields NaN so the current test
misses blanks; update function wkChk to parse wk.value with an explicit radix
(parseInt(wk.value, 10)) and change the condition to treat empty or non-numeric
values as needing seeding (e.g., if (!wk.value || isNaN(parsed) || parsed <
1000) wk.value = 6500), keeping the existing checks for the WKE checkbox
(d.Sf["WKE"+n]) and the UI() call.
---
Nitpick comments:
In `@wled00/bus_manager.cpp`:
- Around line 101-115: The AI-generated blocks (including the setWhiteKelvin
function and the other block at the section covering lines 139-158) need an
attribution comment added immediately above each AI-marked region: include a
brief "Generated with assistance from [AI tool]" line plus links or identifiers
for any external sources or repositories used to inspire the implementation
(e.g., the AI model name and any GitHub/URL references), and summarize what was
taken from the source (e.g., colorKtoRGB usage/logic). Update the comments
around the unique symbols Bus::setWhiteKelvin and the other AI-labeled block to
contain that attribution text so reviewers can trace provenance.
In `@wled00/data/settings_leds.htm`:
- Around line 824-830: Replace the single inline AI comment in the loadCfg block
that handles WKE/WK (the block creating wkChkEl, wkEl and computing wkv) with
the standard AI block markers: add a starting comment "// AI: below section was
generated by an AI" immediately before the block and an ending comment "// AI:
end" after it, so the block wrapping the wkChkEl/wkEl/wkv logic matches the
other AI-marked sections; keep the existing code unchanged and follow the same
spacing/comment style as the other AI blocks.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46aeac26-38de-4191-b2cd-81f03787a86f
📒 Files selected for processing (6)
wled00/bus_manager.cppwled00/bus_manager.hwled00/cfg.cppwled00/data/settings_leds.htmwled00/set.cppwled00/xml.cpp
wkChk() promised to reseed the Kelvin field to 6500 K "from a blank or
sub-min value", but parseInt("") is NaN and NaN < 1000 is false, so a
cleared field was never reseeded — contradicting the comment. Invert the
test to !(parsed >= 1000) so NaN (blank/non-numeric) also seeds, and add
an explicit radix per the review.
Spotted by CodeRabbit on wled#5654.
Co-authored-by: Claude <noreply@anthropic.com>
"Tune RGB to W channel color temperature" implied the setting adjusts RGB, but it primarily changes how the W value is calculated (in Brighter/Accurate/Dual) and only modifies RGB in Accurate mode — in Brighter/Dual the RGB channels are left untouched. Rename to "Correct auto-white for W channel color temperature", which is accurate across all modes and matches the sibling "Auto-calculate W channel from RGB". Co-authored-by: Claude <noreply@anthropic.com>
The block that derives the WKE checkbox / WK seed from stored wk used a single inline "// AI:" comment instead of the start/end markers the other AI-generated sections use. Wrap it with the standard "// AI: below section was generated by an AI" / "// AI: end" pair for consistency. Comment-only; no behavior change. Spotted by CodeRabbit on wled#5654. Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
What this does
Adds an opt-in, per-bus "white LED color temperature" setting that makes the
auto-white calculation account for the actual color of a strip's W LED.
Why
WLED's "Accurate" auto-white mode subtracts the white channel value equally
from R, G and B. That implicitly assumes the physical W LED emits neutral
white — RGB(255,255,255), i.e. the sRGB white point near D65 / ~6500 K. Real
strips often use 2700 K warm-white or 5000 K+ cool-white LEDs, so the equal
subtraction shifts the resulting color visibly. This lets the calculation use
the W LED's true per-channel contribution instead.
How it works
RGB" (shown for Brighter / Accurate / Dual modes). Disabled by default.
colorKtoRGB()) to the W LED's RGB equivalent and cached on the bus.autoWhiteCalc, instead ofw = min(r,g,b)and an equal subtract, itpicks the largest W whose per-channel RGB contribution won't overflow any
channel, then (ACCURATE mode) subtracts that correct per-channel amount.
Floor division composes back through the subtract so it can't underflow;
per-channel zero guards handle channels that are 0 (blue is 0 at/below
1900 K).
wk = 0)autoWhiteCalctakesa fast path that is identical to current behavior, so existing configs
render unchanged and there's no added per-pixel cost.
wkin the LED config; range 1000–10000 K.Testing
esp32dev; flashed via OTA to a QuinLED Dig-Uno (ESP32 + RGBW).power-cycle; Accurate-mode saturation with smooth color transitions and no
regressions; low end down to the 1000 K floor with no underflow artifacts.
Notes / limitations
colorKtoRGB()is a blackbody-curve approximation; real phosphor LEDsdiffer from a true blackbody, so the temperature is a user-tunable value
rather than a fixed per-LED-type table. (In my own testing a 3000 K-rated
strip looked when set to 2700 K or 2500 K instead.)
hot path.
AI assistance
This change was developed with AI assistance (Claude). AI-generated sections
are marked with
// AI:comments per the contribution guidelines. All codeand this description were reviewed and proof-read by @NerdyGriffin, who takes
responsibility for the contribution.
Summary by CodeRabbit
New Features
Improvements