Random colors via JSON API in Segment object like "col":["r","r","r"] #4996 #5000
Conversation
WalkthroughDeserializeSegment in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/colors.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/colors.cpp
wled00/colors.cpp
Outdated
| if (in[0] == 'r') { // Random colors via JSON API in Segment object like col=["r","r","r"] · Issue #4996 | ||
| setRandomColor(rgb); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Tighten the string comparison to match exactly "r".
The current check if (in[0] == 'r') is overly permissive—it treats any string starting with 'r' (e.g., "random", "red", "r12345") as a request for random colors. This could cause unintended behavior if a user passes a malformed hex string or expects different handling.
Apply this diff to ensure only the exact string "r" triggers random color generation:
- if (in[0] == 'r') { // Random colors via JSON API in Segment object like col=["r","r","r"] · Issue #4996
+ if (in[0] == 'r' && in[1] == '\0') { // Random colors via JSON API in Segment object like col=["r","r","r"] · Issue #4996
setRandomColor(rgb);
return true;
}Additionally, verify that this pattern is consistent with how "r" is handled for palette and effect settings elsewhere in the codebase.
Run the following script to check how "r" shorthand is implemented for palette (pal) and effect (fx) settings:
🏁 Script executed:
#!/bin/bash
# Description: Find how "r" shorthand is handled for palette and effect settings
# to ensure consistency across the codebase.
# Search for palette randomization logic
echo "=== Palette randomization patterns ==="
rg -n -C3 'pal.*==.*["\']r["\']' --type=cpp
# Search for effect randomization logic
echo -e "\n=== Effect randomization patterns ==="
rg -n -C3 'fx.*==.*["\']r["\']' --type=cpp
# Search for other "r" string comparisons that might set random values
echo -e "\n=== Other 'r' comparisons for randomization ==="
rg -n -C3 '\[0\]\s*==\s*["\']r["\']' --type=cppLength of output: 108
🏁 Script executed:
#!/bin/bash
# Search for palette randomization logic
echo "=== Palette randomization patterns ==="
rg -n -C3 "pal.*==.*['\"]r['\"]" --type=cpp
# Search for effect randomization logic
echo -e "\n=== Effect randomization patterns ==="
rg -n -C3 "fx.*==.*['\"]r['\"]" --type=cpp
# Search for other "[0] == 'r'" comparisons
echo -e "\n=== Other 'r' comparisons for randomization ==="
rg -n -C3 "\[0\]\s*==\s*['\"]r['\"]" --type=cppLength of output: 1086
Restrict random‐color shorthand to exact "r".
The current if (in[0] == 'r') matches any string starting with 'r' (e.g., "red", "random"). Only the lone "r" should trigger random colors. Apply:
- if (in[0] == 'r') { // Random colors via JSON API in Segment object like col=["r","r","r"] · Issue #4996
+ if (in[0] == 'r' && in[1] == '\0') { // Random colors via JSON API in Segment object like col=["r","r","r"] · Issue #4996
setRandomColor(rgb);
return true;
}No other "r" shorthand cases were found in palette or effect code.
📝 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.
| if (in[0] == 'r') { // Random colors via JSON API in Segment object like col=["r","r","r"] · Issue #4996 | |
| setRandomColor(rgb); | |
| return true; | |
| } | |
| if (in[0] == 'r' && in[1] == '\0') { // Random colors via JSON API in Segment object like col=["r","r","r"] · Issue #4996 | |
| setRandomColor(rgb); | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In wled00/colors.cpp around lines 495 to 498, the condition uses a prefix check
(in[0] == 'r') which matches strings like "red" or "random"; change it to
require the input be exactly the single-character string "r" (e.g., check that
in[0] == 'r' and the next character is the string terminator or that strlen(in)
== 1, or use strcmp/interned-string equality) so only the lone "r" triggers
setRandomColor and returns true.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/json.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/json.cpp
🧬 Code graph analysis (1)
wled00/json.cpp (1)
wled00/colors.cpp (2)
setRandomColor(134-138)setRandomColor(134-134)
|
Thanks for contributing. What happens if I want [255, 0, r]? |
Hello DedeHai, |
AlexeyMal
changed the title
|
Hello Blaz, |
AlexeyMal
changed the title
I would not like to steal credit from you. |
|
Hello Blaz, |
3 participants
Resolves my feature fequest
#4996
Dear Developers,
this pull request adds the option to set Random colors via JSON API in Segment object like col=["r","r","r"].
As an alternative to setting the colors to the fix values.
Similar to the already impleneted "r" for palette pal and effect fx.
Why? Many people scoll through all effects and palettes using a playlist with one random effect and palette. But some effects use the 1 to 3 colors instead of the palette, so for them the colors can not be set randomly.
Best regards
Summary by CodeRabbit