rename CW LED descriptions to more commonly used CCT#5612
Conversation
|
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)
WalkthroughLED type display names and inline bus-method comments were updated to use CCT-style channel naming (e.g., “RGBCCT”, “GRBCCT”) and some RGB/RGBW descriptors were reformatted across two files. ChangesLED Type Naming Standardization
🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels: Suggested reviewers:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Oh, I am 100% for that! Any term other than RGBCCT or RGB+CCT is confusing in my opinion. WW is generally used for "Warm White" and CW is often used for "Cold White", that's fine. Things we're okay up until RGBW but then people starting calling it RGBWW or RGBCW sometimes, denoting the particular color temperature of that strip but there things went wrong, sometimes using RGBCW as referring to dual white also since the C and W is in there, very confusing. I've also seen it written as RGBWWCW, less confusing but still a bit meh in my opinion. CCT is clear what it means, dual white color temperature RGBW stays as is, R+G+B with a single white (whatever temperature) I sometimes use the distinction of RGBCCT and RGB+CCT if the white is integrated into the same package, or external, but for the software side, that really doesn't matter, CCT and RGBCCT is the correct way to do it in my opinion. |
| {TYPE_APA106, "D", PSTR("APA106/PL9823")}, | ||
| {TYPE_TM1914, "D", PSTR("TM1914")}, | ||
| {TYPE_FW1906, "D", PSTR("FW1906 GRBCW")}, | ||
| {TYPE_FW1906, "D", PSTR("RGBCCT FW1906/WS2811")}, |
There was a problem hiding this comment.
(Minor) this entry uses "RGBCCT" as prefix to the chip name, but the other entries have that as postfix (after the chip name). Was this intended?
There was a problem hiding this comment.
yes, as it applies to multiple strips, could also add this to the others that are frequently used, i.e.
- RGB WS281x
- RGB 400kHz
- RGBW SK6812/WS2814
- RGBW UCS8904
- RGBCCT WS2805 (note: uses slightly faster timing)
- RGBCCT SM16825
etc.
There was a problem hiding this comment.
Makes sense for me 👍
Same as @intermittech says, I remember there were different namings around when RGB strips got white as an extra LED. CCT seems to be the term that "survived".
To be discussed: maybe we could always use the RGB+" naming?
- RGB (no white)
- RGB+W (single white - either cold or warm white)
- RGB+CCT (both warm and cold white)
- White (single white)
- CCT (cold+warm)
Not sure what to do with "CCT + Amber" strips that have three white channels?
|
Oh that is a good one @softhack007, there is a SK6812 WWA which has Warm, Cold and Amber White, I'd say it's so much of an exception (and the only one of that sort as far as I am aware) that it shouldn't define the naming for the rest. So maybe we just keep it as an exception to the rule, I think that's fine. It seems the PWM naming was done by another developer and that it's 5 channel mode is called RGB+CCT, maybe just clean it all up and make it the same and I'd personally choose for RGBCCT then. |
this is a suggestion to rename "CW" into "CCT"
@intermittech what do you think? Open for discussion
Summary by CodeRabbit