New smooth effect: Color Clouds #5268
Open
+80
−1
Conversation
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
ColorClouds: Random start points for clouds and color ColorClouds: new config option 'More red'
Contributor
WalkthroughAdds a new FX mode "Color Clouds" (Perlin-like, cloud-based color/brightness noise), its control metadata, registers it in the FX registry, and inserts a new mode constant, incrementing the FX mode count. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wled00/FX.cppwled00/FX.h
🧰 Additional context used
📓 Path-based instructions (2)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/FX.cpp
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/FX.h
🧠 Learnings (9)
📓 Common learnings
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Learnt from: DedeHai
Repo: wled/WLED PR: 5105
File: wled00/FX.h:968-968
Timestamp: 2025-11-22T12:12:53.486Z
Learning: In WLED's WS2812FX class (wled00/FX.h), the _pixels array stores LED colors indexed by unmapped (logical) LED positions, not physical positions. The getPixelColor(n) function intentionally checks if the logical LED at index n maps to a valid physical position via getMappedPixelIndex(n), then accesses _pixels[n] with the unmapped index. If the mapping is invalid (e.g., -1 for gaps in LED maps, or out of bounds), it returns 0 (black). This allows peek preview to correctly show gaps in LED mappings as black pixels. The mapping from logical to physical positions is only applied when writing to the hardware bus, not when reading from the pixel buffer.
<!-- [add_learning]
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
wled00/FX.cppwled00/FX.h
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Applied to files:
wled00/FX.cppwled00/FX.h
📚 Learning: 2025-11-27T06:33:11.436Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-11-16T19:40:46.260Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.
Applied to files:
wled00/FX.cppwled00/FX.h
📚 Learning: 2025-12-28T14:06:48.764Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-08-08T17:22:37.374Z
Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:22:37.374Z
Learning: In WLED FX.cpp, um_data returned by getAudioData() is intentionally untyped because different usermods provide different payloads, and simulateSound() may supply dummy data when the AudioReactive usermod isn’t built. Strongly typed accessors are not feasible; prefer explicit casts (e.g., samplePeak as uint8_t at u_data[3]) or small helpers that encapsulate these casts.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Applied to files:
wled00/FX.cppwled00/FX.h
🧬 Code graph analysis (1)
wled00/FX.cpp (1)
wled00/fcn_declare.h (3)
hw_random16(434-434)hw_random16(435-435)hw_random16(436-436)
🔇 Additional comments (2)
wled00/FX.h (1)
382-383: LGTM!The new
FX_MODE_COLORCLOUDSconstant follows the correct sequential numbering pattern (218, afterFX_MODE_PARTICLEGALAXYat 217), andMODE_COUNTis properly updated to 219 (highest ID + 1). The changes align with established conventions and the mode registration mentioned in the AI summary.wled00/FX.cpp (1)
11115-11115: Color Clouds registration wiring looks correctThe new registry entry
addEffect(FX_MODE_COLORCLOUDS, &mode_ColorClouds, _data_FX_MODE_COLORCLOUDS);correctly ties the new mode ID to the implementation and metadata, and is placed alongside other 1D non‑audio FX. Assuming
FX_MODE_COLORCLOUDSandMODE_COUNTwere updated inFX.has indicated, this registration is good to go.
Collaborator
|
thanks for contributing, I quite like the effect. Just a few notes:
|
Collaborator
|
@coderabbitai when reviewing code, please check that the function aliases inoise8, inoise16 and similar fastled functions are not used and the perlin8/perlin16 etc are used instead. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. @DedeHai, I'll remember these guidelines for future code reviews. I notice that the current PR #5268 (Color Clouds effect) actually contains both of these issues:
These should be corrected in this PR before merging. ✏️ Learnings added
🧠 Learnings used |
Author
|
@DedeHai, thanks a lot for your feedback; glad to hear that you like the effect. However, this comment astonished me:
The effect uses neither palettes nor fixed colors; it is based on HSV color calculations. IIRC, in older versions of WLED (where i started developing this effect), the color sliders were not shown. If you're interested, I'll search in my old playgrounds. |
Collaborator
that is what I mean by "fixed colors" you really should use palettes - or use colorwheel() which uses HSV but still supports palettes but you can just make the default the "rainbow" palette and you do have your HSV equivalent in palette form. |
Author
|
OK, got it and I'll give it a try. I can remember that there have recently been changes regarding these rainbow colors and the color wheel. Unfortunately I didn't understand all the details from the discussion of that PR, so I got lost again with too many options. Trying to learn from examples of |
Collaborator
|
instead of then adjust the metadata string, see any other effect that uses palette colors, shimmer for example. |
- Support for color palettes - Use perlin16() instead of inoise16() - Use cos8_t() instead of cos8()
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wled00/FX.cpp (1)
5035-5107: Color Clouds effect logic, ranges, and palette/noise usage look solid
- Perlin usage (
perlin16) and trig (cos8_t) follow the WLED math/noise wrappers; there are no remaining FastLED aliases (inoise*,cos8()) in this effect.volCutoff = 12500 + SEGMENT.custom3 * 900withSEGMENT.custom3in 0–31 keepsvolCutoffsafely belowvolSaturate = 52000, somap(vol, volCutoff, volSaturate, 0, 255)remains monotonic across the full Distance slider without needing extra clamps.- Brightness and hue domains (the
volX/hueXspatial scales andvolT/hueTtemporal scales) are well withinuint32_tlimits for realisticSEGLEN, so there’s no risk of overflow-related artifacts beyond the intentional wraparound of the Perlin time axis.- Palette integration is appropriate: when a palette is active,
SEGMENT.color_from_palette(hue, false, true, 0, vol)drives the colors; otherwise the HSV branch viahsv2rgb(CHSV32(hue, 255, vol), pixel)gives a consistent standalone rainbow. The “More red” shaping viacos8_t(128 + hue / 2)is localized and doesn’t break palette behavior.- The dark-pixel suppression and initial seeding via
SEGENV.aux0/aux1follow existing FX patterns and won’t interact badly with 1D vs 2D segments.If you like, you could very slightly clarify intent by casting the mixed-width offsets explicitly, e.g.
uint8_t hueOffset0 = uint8_t(SEGENV.aux0 + SEGENV.aux1);, but that’s purely cosmetic.Based on learnings, SEGMENT.custom3’s 0–31 range makes the current
volCutoffmath safe without extra clamping.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/FX.cpp
🧰 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/FX.cpp
🧠 Learnings (18)
📓 Common learnings
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-11-27T06:33:11.436Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-11-16T19:40:46.260Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-12-28T14:06:48.772Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-08-08T17:22:37.374Z
Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:22:37.374Z
Learning: In WLED FX.cpp, um_data returned by getAudioData() is intentionally untyped because different usermods provide different payloads, and simulateSound() may supply dummy data when the AudioReactive usermod isn’t built. Strongly typed accessors are not feasible; prefer explicit casts (e.g., samplePeak as uint8_t at u_data[3]) or small helpers that encapsulate these casts.
Applied to files:
wled00/FX.cpp
📚 Learning: 2026-01-01T07:19:40.244Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5251
File: usermods/user_fx/user_fx.cpp:146-148
Timestamp: 2026-01-01T07:19:40.244Z
Learning: In the WLED Ants effect (usermods/user_fx/user_fx.cpp), the color alternation logic `antIndex % 3 == 1` is intentional and creates a 1/3 vs 2/3 distribution between SEGCOLOR(0) and SEGCOLOR(2). This unbalanced alternation is by design and should not be changed to a balanced mod 2 alternation.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/FX.cpp
📚 Learning: 2026-01-03T14:38:09.949Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T14:38:09.949Z
Learning: In WLED code reviews, check that FastLED noise function aliases (`inoise8`, `inoise16`, etc.) are not used. These should be replaced with the proper WLED functions (`perlin8`, `perlin16`, etc.) as the `inoise*` functions are only aliases kept for compatibility reasons.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, file operations (especially file.open()) should be checked to ensure they respect LittleFS filename limitations. The default LittleFS filename limit is 255 bytes (LFS_NAME_MAX). Reviews should assume default WLED configuration defines and not extreme edge-case values (e.g., WLED_MAX_SEGNAME_LEN = 512 would not be standard). File paths should be validated to stay within the 255-byte limit.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-10-20T09:38:51.997Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5223-5226
Timestamp: 2025-10-20T09:38:51.997Z
Learning: WLED matrices: each dimension (SEG_W, SEG_H) is limited to ≤255; 256 or larger per side is not supported/feasible on ESP32, so effects should assume per-dimension max 255.
Applied to files:
wled00/FX.cpp
🧬 Code graph analysis (1)
wled00/FX.cpp (3)
wled00/util.cpp (6)
perlin16(1111-1113)perlin16(1111-1111)perlin16(1115-1117)perlin16(1115-1115)perlin16(1119-1121)perlin16(1119-1119)wled00/wled_math.cpp (2)
cos8_t(88-90)cos8_t(88-88)wled00/colors.cpp (2)
hsv2rgb(296-325)hsv2rgb(296-296)
🔇 Additional comments (1)
wled00/FX.cpp (1)
11119-11120: Effect registration is consistent and correctly wiredThe
addEffect(FX_MODE_COLORCLOUDS, &mode_ColorClouds, _data_FX_MODE_COLORCLOUDS);entry matches surrounding registrations and slots Color Clouds appropriately among 1D non‑audio FX.
Author
|
Implementing the usage of color palettes was a bit tricky, because their colors - and the ones from color_from_palette() - are much brighter than the colors from FastLED's CHSV(). That required quite some experimenting again with the constants, but finally the colors and clouds are behaving again as expected. |
Author
|
@DedeHai - Is there still anything missing in this PR? |
DedeHai
requested changes
Jan 23, 2026
wled00/FX.cpp
Outdated
|
|
||
| // Put more emphasis on the red'ish colors when true. | ||
| const bool moreRed = SEGMENT.check3; | ||
| const long volCutoff = 12500 + SEGMENT.custom3 * 900; |
Collaborator
There was a problem hiding this comment.
avoid long (also in other places), use either int or better yet int32_t
Contributor
There was a problem hiding this comment.
FYI long is guaranteed to be at least 32 bits by C and C++ standards. int is only guaranteed to be at least 16 bits (most often it is "native" or 32 bit sized).
Collaborator
There was a problem hiding this comment.
that is true and the reason why I almost exclusively use uintxx_t as those are well defined.
Contributor
There was a problem hiding this comment.
Both long and int are well defined. However, their actual bit depth allows misuse in certain situations.
If you need a counter with at least 32 bit precision then you need to use long to be compatible on all platforms or, as you say, int32_t on compilers that support it (#include <cstdint.h>).
Author
There was a problem hiding this comment.
No problem; I usually also prefer the more descriptive variants. The long probably remainded, because the map() function below is declared with longs.
wled00/FX.cpp
Outdated
|
|
||
| // Suppress extremely dark pixels to avoid flickering of plain r/g/b. | ||
| // Unfortunately this doesn't work anymore when gamma correction for color is enabled. | ||
| // Unfortunately this doesn't always work properly when gamma correction for color is enabled. |
Collaborator
There was a problem hiding this comment.
fix it, you have to assume gamma correction is always enabled.
Author
There was a problem hiding this comment.
Removed the comment. The problem vanished when the palettes were integrated.
wled00/FX.cpp
Outdated
| return FRAMETIME; | ||
| } | ||
| static const char _data_FX_MODE_COLORCLOUDS[] PROGMEM = "Color Clouds@Cloud speed,Color speed,Clouds,Colors,Distance,,,More red;;;;sx=24,ix=32,c1=48,c2=64,c3=12"; | ||
| static const char _data_FX_MODE_COLORCLOUDS[] PROGMEM = "Color Clouds@Cloud speed,Color speed,Clouds,Colors,Distance,,,More red;;!;;sx=24,ix=32,c1=48,c2=64,c3=12,pal=0"; |
Collaborator
There was a problem hiding this comment.
use "!" instead of Cloud speed. also calling it "more red" makes no sense with palettes.
Author
There was a problem hiding this comment.
Calling it now "cozy" and adjusted the corresponding comment.
However, the "Cloud speed" should remain as is - to express the correlation between the Sliders "Cloud Speed & Clouds" and "Color Speed & Colors".
Collaborator
There was a problem hiding this comment.
since the effect is called "Colored Clouds", what else would "Speed" mean (using ! will default to "speed")
Author
There was a problem hiding this comment.
I've set both labels to default; "effect intensity" for "Color Speed" also makes sense somehow.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/FX.cpp`:
- Around line 5094-5096: The code passes an 8-bit hue into CHSV32 causing hue
collapse; when palette is disabled (SEGMENT.palette false) update the hsv2rgb
call to convert the 8-bit hue to 16-bit (e.g., use hue << 8) before constructing
CHSV32 so hsv2rgb receives a full-range hue; change the branch that currently
calls hsv2rgb(CHSV32(hue, 255, vol), pixel) to use scaled hue while keeping the
rest (saturation/value) the same.
Comment on lines
+5094
to
+5096
| uint32_t pixel; | ||
| if(SEGMENT.palette) { pixel = SEGMENT.color_from_palette(hue, false, true, 0, vol); } | ||
| else { hsv2rgb(CHSV32(hue, 255, vol), pixel); } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find mode_ColorClouds function and surrounding context
cd /tmp && find . -name "FX.cpp" -path "*/wled00/*" 2>/dev/null | head -1 | xargs wc -lRepository: wled/WLED
Length of output: 54
🏁 Script executed:
# Read the mode_ColorClouds function implementation
rg -n "mode_ColorClouds" wled00/FX.cpp -A 100 | head -120Repository: wled/WLED
Length of output: 5871
🏁 Script executed:
# Find CHSV32 definition and hsv2rgb function signature
rg -n "struct CHSV32|class CHSV32" wled00/ -A 5Repository: wled/WLED
Length of output: 362
🏁 Script executed:
# Search for hsv2rgb function definition
rg -n "hsv2rgb.*CHSV32" wled00/colors.cpp -B 2 -A 10Repository: wled/WLED
Length of output: 982
Scale hue to 16‑bit before CHSV32 conversion.
Line 5096 passes an 8‑bit hue into CHSV32, but the struct expects a 16‑bit hue (see colors.h:74). When palette is disabled, this collapses all hue values to the red region because hsv2rgb() divides the hue by 10923 (65536/6 for the six color regions), and 255 / 10923 = 0 always. Use hue << 8 to scale the 8‑bit value to the full 16‑bit range.
🔧 Suggested fix (scale to 16‑bit hue)
- else { hsv2rgb(CHSV32(hue, 255, vol), pixel); }
+ else {
+ const uint16_t hue16 = uint16_t(hue) << 8; // scale 0..255 -> 0..65535
+ hsv2rgb(CHSV32(hue16, 255, vol), pixel);
+ }🤖 Prompt for AI Agents
In `@wled00/FX.cpp` around lines 5094 - 5096, The code passes an 8-bit hue into
CHSV32 causing hue collapse; when palette is disabled (SEGMENT.palette false)
update the hsv2rgb call to convert the 8-bit hue to 16-bit (e.g., use hue << 8)
before constructing CHSV32 so hsv2rgb receives a full-range hue; change the
branch that currently calls hsv2rgb(CHSV32(hue, 255, vol), pixel) to use scaled
hue while keeping the rest (saturation/value) the same.
Author
There was a problem hiding this comment.
The constructor takes 3 uint8_t - see colors.h:88
Contributor
There was a problem hiding this comment.
@JoaDick You’re right. CHSV32 has a ctor that takes (uint8_t h, uint8_t s, uint8_t v) and handles the upconversion internally, so hsv2rgb(CHSV32(hue, 255, vol), pixel) is correct as written. No change needed on that line. If you like, we can drop a short comment there to avoid future confusion from automated checks.
✏️ Learnings added
Learnt from: JoaDick
Repo: wled/WLED PR: 5268
File: wled00/FX.cpp:5094-5096
Timestamp: 2026-01-24T22:01:15.416Z
Learning: WLED wled00/colors.h: CHSV32 provides a constructor taking uint8_t h,s,v and internally upconverts the 8‑bit hue to its 16‑bit representation. Passing an 8‑bit hue to CHSV32 is correct; no manual h<<8 is required.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:149-151
Timestamp: 2026-01-24T18:51:02.052Z
Learning: In WLED's Spinning Wheel effect (usermods/user_fx/user_fx.cpp), the color scale calculation (SEGENV.aux1) must match the position modulo range. When position uses `pos = (pos_fixed >> 16) % SEGLEN`, the color scale must be `aux1 = (255 << 8) / SEGLEN` to distribute the palette evenly across the full position range. Using vstripLen for color scaling while position uses SEGLEN causes palette colors to cycle too quickly and appear incomplete.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-13T21:23:35.514Z
Learning: In WLED, the global `paletteBlend` variable (wled.h:603) and the `WS2812FX::paletteBlend` member (FX.h:940) are duplicates without synchronization code. The global is loaded/saved in cfg.cpp and set via UI in set.cpp, but never copied to the strip member. This is the only such case in the codebase; other settings are either strip-only members (autoSegments, correctWB, cctFromRgb, isMatrix) or global-only (gammaCorrectCol/Bri/Val, blendingStyle).
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR `#4798`, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-15T20:25:25.815Z
Learning: The CSS palette preview in wled00/data/index.js genPalPrevCss() function uses raw RGB values in CSS linear-gradient() without applying WLED's gamma correction, while actual LED output goes through NeoGammaWLEDMethod gamma correction. This causes inherent discrepancies between the web UI palette preview and actual LED colors, especially noticeable with different gamma settings.
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
This PR adds a new effect called "Color Clouds", which is a very smooth effect. With its slowly moving clouds of rainbow color, it is very suitable for ambient lights and discreet table lamps.
It offers a variety of configuration options for the user:
The algorithm is very simple and uses perlin noise to generate the brightness- and color-gradients.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.