Added Chase Race Effect to WLED #4566
Conversation
WalkthroughThe changes introduce a new effect mode called "Chase Race" along with several additional effect modes utilizing various animation techniques. The implementation file has been updated to support these effects, and the header file has been modified to redefine the total number of effect modes, incrementing the mode count accordingly. Additionally, the Changes
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
wled00/FX.cpp (2)
932-995: Consider refactoring repeated logic and clarifying thedo_paletteparameter.
- The repeated sequence of for-loops filling color1, color2, and color3 could be more compact and easier to maintain if consolidated in a helper method or by iterating over an array of colors.
- The
bool do_paletteargument is never used in this function body. If palette-based color selection isn’t planned, removing this parameter reduces confusion and complexity. Otherwise, consider implementing palette-based logic.Below is one possible approach to eliminate the unused parameter and move the color-filling logic into a simple loop:
-uint16_t chase_race(uint32_t color1, uint32_t color2, uint32_t color3, bool do_palette) +uint16_t chase_race(uint32_t color1, uint32_t color2, uint32_t color3) { uint16_t counter = strip.now * ((SEGMENT.speed >> 2) + 1); // ... existing code to compute a, b, c, d, e, f - // if (do_palette) { ... } // <— not currently implemented SEGMENT.fill(0); // Example of refactoring repeated loops uint32_t colors[3] = {color1, color2, color3}; uint16_t fillStarts[3] = {a, c, e}; uint16_t fillEnds[3] = {b, d, f}; for (int idx = 0; idx < 3; idx++) { if (fillStarts[idx] < fillEnds[idx]) { for (unsigned i = fillStarts[idx]; i < fillEnds[idx]; i++) { SEGMENT.setPixelColor(i, colors[idx]); } } else { for (unsigned i = fillStarts[idx]; i < SEGLEN; i++) { SEGMENT.setPixelColor(i, colors[idx]); } for (unsigned i = 0; i < fillEnds[idx]; i++) { SEGMENT.setPixelColor(i, colors[idx]); } } } return FRAMETIME; }
1006-1008: Fix spelling in documentation.
The comment “Chace Race with 3 color strips” contains a minor spelling mistake. Consider changing “Chace” to “Chase” to maintain clarity.-* Chace Race with 3 color strips +* Chase Race with 3 color stripswled00/FX.h (1)
354-355: Consider adding documentation for the new effect mode.Since this is a specialized effect for automotive racing displays, it would be helpful to add a comment describing its purpose and behavior, similar to how other effects are documented (e.g., line 179's comment for FX_MODE_TETRIX).
Add a comment like:
+// Chase Race effect simulates three colored "cars" chasing each other with a black background #define FX_MODE_CHASE_RACE 215 #define MODE_COUNT 216
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wled00/FX.cpp(3 hunks)wled00/FX.h(1 hunks)
🔇 Additional comments (6)
wled00/FX.cpp (2)
928-931: Good documentation on the feature intent.
This doc comment succinctly outlines the purpose of the new Chase Race function and how it operates with three separate color bands.
10272-10272: Verify the effect ID assignment.
Ensure thatFX_MODE_CHASE_RACEdoes not conflict with existing effect IDs and thatMODE_COUNThas been incremented accordingly in the header file. If the new effect ID is already in use, it would cause unexpected behavior.wled00/FX.h (4)
354-355: LGTM! New effect mode added correctly.The new Chase Race effect mode is properly defined with a unique identifier (215) and MODE_COUNT is correctly incremented to 216.
1023-1026: Verify vector capacity for new effect mode.The
_modeand_modeDatavectors need to accommodate the new effect. The code already handles this in the constructor (lines 839-842) by reserving capacity and falling back to a single mode if allocation fails.
354-355:✅ Verification successful
Verify effect mode number uniqueness.
Let's verify that no other effect is using the same mode number to prevent conflicts.
🏁 Script executed:
#!/bin/bash # Search for any other occurrences of mode number 215 rg --type cpp "215" --no-filename | grep -v "FX_MODE_CHASE_RACE"Length of output: 1231
Effect mode number 215 is confirmed unique.
The search output shows that the literal “215” appears in various parts of the code (comments, array parameters, EEPROM accesses, etc.), but among the effect mode definitions, it’s used only for
FX_MODE_CHASE_RACE(withMODE_COUNTimmediately following at 216). There are no other macro definitions assigning 215 to an effect mode.
75-87:❓ Verification inconclusive
Verify memory constraints for new effect.
Since each segment uses significant memory (82 bytes) and there are platform-specific limits, let's ensure adding a new effect doesn't push memory usage beyond safe limits.
🏁 Script executed:
#!/bin/bash # Look for memory-related issues or warnings in the codebase rg -A 3 "insufficient memory|memory allocation failed|out of memory" --no-filenameLength of output: 1165
Ensure new effect respects existing memory constraints
- The current macro definitions in
wled00/FX.hset platform-specific limits (e.g., 16 segments for ESP8266, 20 for ESP32S2, 32 for others), with each segment consuming roughly 82 bytes.- Our grep search confirms that warnings (e.g., "insufficient memory", "out of memory") are already in place to catch memory allocation issues elsewhere in the code.
- When integrating the new effect, please verify that its memory footprint does not exceed the limits implied by
MAX_NUM_SEGMENTSandMAX_SEGMENT_DATA. Ensure that, on low-memory platforms, adding the effect won’t push memory usage into unsafe territory.- Further testing on the target hardware (especially on constrained devices like ESP8266) is recommended to confirm that memory remains within safe bounds.
|
thanks for your contribution. consider this: in my brief checking it looks almost the same but improved gaps, no overlapping and 40% less code size. one more thing: the colors should be named 1, 2, 3 (see "Chase 3" effect) instead of Fx, Bg, Cs an option to add a palette background with a checkmark would also be a nice addition |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
wled00/FX.cpp (3)
928-931: Clarify documentation to reflect absence of palette usage
The doc block states that three colored bands race along with black in between, but it does not mention that the background is forced to black and that thedo_paletteparameter remains unused. Consider elaborating on this to ensure users know the effect doesn’t honor palette-based backgrounds.
933-970: Validate arithmetic for wrapping and segment sizes
The arithmetic, especially aroundsizeandgap, correctly uses modular wrapping but can become large ifSEGLENand the intensity setting are near their maximum. Although the modulo math infillSegmentmitigates out-of-bounds writes, you may wish to clamp or explicitly validate sizes for extreme cases. Additionally, watch for scenarios where(end == start)can create unintended edge conditions.
980-987: Consider optional palette background support
This mode currently sets three colors and forces a black background. If you want to leverage existing palette features or add a user-selectable background, consider accepting abool do_paletteparameter or adopting a palette-based fill approach to improve consistency with other multi-color effects.platformio.ini (1)
13-14: Default Environments Revision ImpactYou've replaced the extensive list of default environments with a single target (
esp32dev). This change streamlines the build process—likely to focus on testing the new Chase Race effect on a specific platform—but it reduces the diversity of environments tested by default. Please verify that this narrowing is intentional and that alternative configurations are tested elsewhere if multi-board compatibility remains a requirement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
platformio.ini(1 hunks)wled00/FX.cpp(3 hunks)
🔇 Additional comments (1)
wled00/FX.cpp (1)
10246-10246: Effect registration looks good
Registering the new “Chase Race” effect withaddEffect()is straightforward and consistent with the approach used for other modes. No issues here.
|
This feels a bit niche to be a core effect and I think should be a usermod instead |
|
@netmindz if done right, its about the same as the other chase effects so I see no reason to have them but not this version. |
|
@DedeHai cherry-pick from my fork. Already done that part. |
Added a new effect, "Chase Race," based on the existing Chase effect.
Chase Race is designed for automotive racing displays, simulating three colored "cars" chasing each other in LED sequences. Unlike Chase, it accepts three input colors, with a fixed black background. This can be integrated with Home Assistant, allowing real-time race data to dynamically adjust colors and sequencing. Initial testing has been conducted using F1 race data.
Summary by CodeRabbit
esp32dev.