Add Chase Race usermod effect #5085
Conversation
WalkthroughThis PR introduces a new Chase Race effect usermod for WLED, featuring three colored cars racing around LED segments. It includes the effect implementation with position tracking and color animation, supporting build configuration files, and modifies the build system for improved usermod dependency path resolution. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
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
🧹 Nitpick comments (4)
wled00/const.h (1)
210-210: Chase Race usermod ID definition is consistent with existing IDs
USERMOD_ID_CHASE_RACEfits the existing sequential ID pattern and matches howChaseRaceUsermod::getId()uses it; no functional issues here. If you want to stay aligned with the pattern some usermods follow (keeping IDs local to avoid touching core and given plans to eventually de-emphasize numeric IDs), you could alternatively hardcode the ID in the usermod itself, but what you have is consistent with most of the table. Based on learningsusermods/chase_race/library.json (1)
1-7: Manifest wiring looks good; consider updating the descriptionThe manifest is structurally fine and
libArchive=falsematches the enforcement inload_usermods.py. The only nit is the"Scaffolding"wording, which undersells the now fully implemented effect—consider updating the description to something like “Three‑car Chase Race WLED effect usermod” for clarity.pio-scripts/load_usermods.py (1)
51-61: Include-path caching and recursion guard look sound
cached_add_includesde-duplicates deps via thedep_cacheset and only recurses into non-usermods (usermod_dir not in Path(dep.src_dir).parents), which avoids unnecessary cycles while still collecting transitive include dirs. That’s a reasonable balance between correctness and performance for the dependency graph.usermods/chase_race/const.h (1)
1-4: Avoid duplicating coreconst.hunder usermods; rely onwled00/const.h(or isolate mod‑specific bits)This header appears to be a near‑copy of the core
wled00/const.h(global limits, IDs, DMX modes, etc.) withUSERMOD_ID_CHASE_RACEadded, and it reuses the same include guard macroWLED_CONST_H. That combination is fragile:
- If both headers are visible on the include path, whichever is included first will “win” and the other will be silently skipped due to the shared guard, making future divergence between the two very hard to debug.
- Duplicating all core constants under
usermods/chase_raceadds maintenance burden; any future changes to core ranges, error codes, or usermod IDs must now be kept in sync in two places.Given that wled00/const.h is already updated to define
USERMOD_ID_CHASE_RACE, the usual pattern would be:
- Keep the single source of truth in
wled00/const.h.- In
chase_race_usermod.cpp, includewled.h(which pulls in the core consts) and useUSERMOD_ID_CHASE_RACEfrom there.- If you need a local header for chase‑race‑specific definitions, give it a unique name/guard and limit it to mod‑specific constants only.
I’d strongly recommend removing this duplicate
const.h(or trimming it down to only chase‑race‑specific definitions with a distinct guard) and relying on the core header instead.Please double‑check how
USERMOD_ID_CHASE_RACEis currently obtained inchase_race_usermod.cppand whether any translation unit includes this usermodconst.hinstead of the core one; if so, consider switching those includes towled.h/wled00/const.hto keep a single authoritative definition.Also applies to: 148-211
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
pio-scripts/load_usermods.py(2 hunks)usermods/chase_race/chase_race_usermod.cpp(1 hunks)usermods/chase_race/const.h(1 hunks)usermods/chase_race/library.json(1 hunks)usermods/chase_race/load_usermods.py(1 hunks)usermods/chase_race/readme.md(1 hunks)wled00/const.h(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/const.h
🧠 Learnings (16)
📓 Common learnings
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.
📚 Learning: 2025-03-29T01:22:54.617Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Applied to files:
wled00/const.husermods/chase_race/chase_race_usermod.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/const.husermods/chase_race/chase_race_usermod.cppusermods/chase_race/const.h
📚 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:
usermods/chase_race/chase_race_usermod.cppusermods/chase_race/const.h
📚 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:
usermods/chase_race/chase_race_usermod.cpp
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
usermods/chase_race/const.h
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
usermods/chase_race/const.h
📚 Learning: 2025-09-16T18:08:42.848Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.
Applied to files:
usermods/chase_race/const.h
📚 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:
usermods/chase_race/const.h
📚 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:
usermods/chase_race/const.h
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
usermods/chase_race/const.h
📚 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:
usermods/chase_race/const.h
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
usermods/chase_race/const.h
📚 Learning: 2025-09-12T17:29:43.826Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4923
File: wled00/FX.cpp:4883-4901
Timestamp: 2025-09-12T17:29:43.826Z
Learning: In WLED’s web UI, only one slider value (e.g., SEGMENT.intensity or SEGMENT.custom1) changes at a time; code relying on this may use simplified change guards, though presets/JSON can still update multiple fields atomically.
Applied to files:
usermods/chase_race/const.h
📚 Learning: 2025-09-01T10:26:17.959Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.
Applied to files:
usermods/chase_race/const.h
📚 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:
usermods/chase_race/const.h
🧬 Code graph analysis (3)
wled00/const.h (1)
usermods/chase_race/chase_race_usermod.cpp (1)
USERMOD_ID_CHASE_RACE(100-100)
usermods/chase_race/chase_race_usermod.cpp (1)
usermods/usermod_v2_animartrix/usermod_v2_animartrix.cpp (1)
SEGENV(64-76)
usermods/chase_race/const.h (1)
usermods/chase_race/chase_race_usermod.cpp (1)
USERMOD_ID_CHASE_RACE(100-100)
🪛 Clang (14.0.6)
usermods/chase_race/chase_race_usermod.cpp
[error] 1-1: 'wled.h' file not found
(clang-diagnostic-error)
usermods/chase_race/const.h
[error] 9-9: unknown type name 'constexpr'
(clang-diagnostic-error)
[error] 9-9: expected ';' after top level declarator
(clang-diagnostic-error)
[error] 10-10: unknown type name 'constexpr'
(clang-diagnostic-error)
[error] 10-10: expected ';' after top level declarator
(clang-diagnostic-error)
[error] 11-11: unknown type name 'constexpr'
(clang-diagnostic-error)
[error] 11-11: expected ';' after top level declarator
(clang-diagnostic-error)
[error] 12-12: unknown type name 'constexpr'
(clang-diagnostic-error)
[error] 12-12: expected ';' after top level declarator
(clang-diagnostic-error)
[error] 62-62: 'driver/ledc.h' file not found
(clang-diagnostic-error)
🪛 LanguageTool
usermods/chase_race/readme.md
[grammar] ~30-~30: Ensure spelling is correct
Context: ...els to honor the requested spacing, the usermod shrinks the gaps automatically (down ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.14.4)
usermods/chase_race/load_usermods.py
1-1: Undefined name Import
(F821)
8-8: Undefined name env
(F821)
27-27: Avoid specifying long messages outside the exception class
(TRY003)
37-37: Undefined name env
(F821)
48-48: Undefined name env
(F821)
48-48: Undefined name env
(F821)
48-48: Undefined name env
(F821)
65-65: Undefined name env
(F821)
109-109: Undefined name env
(F821)
pio-scripts/load_usermods.py
8-8: Undefined name env
(F821)
🔇 Additional comments (7)
usermods/chase_race/readme.md (1)
1-36: Readme matches the implementation and build flowThe build instructions and control descriptions line up with the usermod code (Pace/speed, Car length/intensity, Gap size/custom1, car color slots) and the PlatformIO env setup. No changes needed from a code/doc consistency standpoint.
pio-scripts/load_usermods.py (3)
8-9: Project/usermod path resolution is clear and efficientComputing
project_dirandusermod_dironce at import time viaPath(...).resolve()is straightforward and avoids repeated env lookups, while also handling symlinks and relative paths cleanly.
29-33: Improved WLED-module detection via resolved source rootsResolving
dep.src_dirbefore checking whether it lives underusermod_dirand falling back to a name prefix ("wled-") gives a simple, robust test for WLED modules (including symlinked usermods). This should behave better across platforms than raw string checks.
68-105: Wrapper around ConfigureProjectLibBuilder correctly wires WLED usermods; just watch PlatformIO changesThe wrapper clones the original
ConfigureProjectLibBuilder, walks itsdepbuilders, builds a sharedextra_include_dirsdeque, and then:
- Identifies WLED modules via
is_wled_module.- Prepends the WLED project src dir and all collected dependency include dirs to each WLED module’s CPPPATH.
- Enforces
dep.lib_archive == False, emitting a clear error if any usermod forgets to setlibArchive=false.- Exposes the final WLED dependency list via
WLED_MODULESfor downstream use.This is a clean way to centralize usermod wiring for modules like
chase_race. Given this depends on PlatformIO’s internal builder API and ondep.src_dirbeing consistently populated, it’s worth re-validating after PlatformIO upgrades to ensure the hook still fires anddepbuildershas the expected shape.Also applies to: 108-109
usermods/chase_race/chase_race_usermod.cpp (3)
21-30: Effect metadata string and control mapping align with WLED conventionsThe
_data_FX_MODE_CHASE_RACEdescriptor cleanly exposes Pace, Car length, Gap size, and three car color slots, matching both the comments and the Readme. The format (Name@sliders;colors;;) and the unused fourth slider slot via,,,follow existing WLED patterns, so UI/JSON integration should be straightforward.
31-88: Race logic is bounded and degrades gracefully on small segmentsThe implementation of
mode_chase_race:
- Uses
SEGENV.step/strip.nowto derive a speed-dependent interval and advances the head position modulosegLen.- Computes
carLenMaxfromsegLenso three cars plus minimal gaps can fit where possible, and clampscarLenaccordingly.- Derives
maxGapand clampsgapLen, shrinking automatically down to zero when the strip is too short—matching the documented behaviour.- Clears the segment and draws up to three cars using modular indexing, so wrapping and small-strip cases don’t cause out-of-bounds access.
Overall the math is defensive and well-contained; no obvious off-by-one or overflow issues for typical WLED segment sizes.
92-105: Effect registration and usermod ID usage look correct; consider legacy ID compatibility
ChaseRaceUsermod::setup()follows the usual pattern of registering the effect viastrip.addEffect, andgetId()correctly returnsUSERMOD_ID_CHASE_RACEas added inwled00/const.h, so usermod identification is consistent.One thing to double‑check: passing
255as the first argument toaddEffectlikely means “append at the end”. If the intent is to preserve the original Chase Race effect ID for existing presets or API integrations, you may want to ensure this ID matches whatever the legacyFX_MODE_*ID was. If you’re fine with the mode moving to a new ID whenever the usermod is enabled, the current approach is OK.
| def wrapped_ConfigureProjectLibBuilder(xenv): | ||
| # Call the wrapped function | ||
| result = old_ConfigureProjectLibBuilder.clone(xenv)() | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Fix ConfigureProjectLibBuilder wrapper: .clone(xenv)() will likely break builds
old_ConfigureProjectLibBuilder is a bound SCons/PlatformIO method, not an object with a .clone method. Calling old_ConfigureProjectLibBuilder.clone(xenv)() will almost certainly raise AttributeError at build time when PlatformIO calls env.ConfigureProjectLibBuilder().
PlatformIO’s usual monkey‑patch pattern is to save the old bound method and call it with no arguments (the env is already bound), e.g. project = oldLibBuilder().
Recommend changing the wrapper to invoke the original directly:
-def wrapped_ConfigureProjectLibBuilder(xenv):
- # Call the wrapped function
- result = old_ConfigureProjectLibBuilder.clone(xenv)()
+def wrapped_ConfigureProjectLibBuilder(xenv):
+ # Call the original ConfigureProjectLibBuilder to resolve dependencies
+ result = old_ConfigureProjectLibBuilder()The rest of the function (using xenv for PROJECT_SRC_DIR, setting WLED_MODULES, etc.) can stay as is.
Please re‑run a few representative pio run environments (including ones without custom_usermods) after applying this change to confirm the wrapper behaves correctly with your PlatformIO version.
I'll now verify the review comment by inspecting the codebase and checking PlatformIO documentation.
[/run_scripts]
[/web_search]
🏁 Script executed:
#!/bin/bash
# Find and inspect the file in question
find . -name "load_usermods.py" -type f 2>/dev/null | head -5Length of output: 69
🏁 Script executed:
#!/bin/bash
# Search for ConfigureProjectLibBuilder usage patterns in the codebase
rg -n "ConfigureProjectLibBuilder" -C 3 --type pyLength of output: 3106
🏁 Script executed:
#!/bin/bash
# Look for any clone() method usage in Python files to understand context
rg -n "\.clone\(" --type py -C 2 | head -40Length of output: 678
🌐 Web query:
PlatformIO ConfigureProjectLibBuilder API documentation
💡 Result:
The ConfigureProjectLibBuilder API is implemented in PlatformIO Core; see:
- Source code (function and related lib-builder classes) — platformio/builder/tools/piolib.py (PlatformIO Core repository). [1]
- PlatformIO build/scripting documentation (Advanced Scripting / Build System API and Library Builder docs) — PlatformIO documentation. [2]
References
- platformio/platformio-core — platformio/builder/tools/piolib.py (ConfigureProjectLibBuilder implementation). [GitHub]. [1]
- PlatformIO — Advanced Scripting / Build System API and Library builder documentation. [Docs]. [2]
I need more specific information about how ConfigureProjectLibBuilder is typically called. Let me search for examples and SCons documentation.
[/web_search]
🌐 Web query:
SCons environment clone method Python
💡 Result:
env.Clone(...) returns a separate copy of an SCons Construction Environment; any keyword args you pass override values in the copy. It accepts the same kinds of construction-variable assignments as Environment(...) and also supports tools, toolpath, parse_flags, and (since 4.8.0) variables. Example:
env = Environment(CC='gcc')
opt = env.Clone(CCFLAGS='-O2') # opt is a separate Environment with CC='gcc' and CCFLAGS='-O2'
dbg = env.Clone(CCFLAGS='-g') # dbg is independent from env and opt
You can also pass tools or parse_flags:
env2 = env.Clone(tools=['msvc', MyTool], parse_flags='!wx-config --cflags')
Docs: SCons user guide and API reference. [1][2]
Sources:
[1] SCons User Guide — Making Copies of Construction Environments (env.Clone).
[2] SCons API / Functions and Environment Methods (env.Clone details).
🌐 Web query:
PlatformIO ConfigureProjectLibBuilder example usage call
💡 Result:
Short answer — call it from a PlatformIO extra Python build script (the SCons/PlatformIO env). Example:
- In library.json (or platformio.ini) point to an extra script:
{
"build": { "extraScript": "extra_script.py" }
} - extra_script.py:
Import("env")
project_lib_builder = env.ConfigureProjectLibBuilder()then use project_lib_builder as needed (PlatformIO's library builder will
configure/install/search libs for the project during the build)
References: PlatformIO extraScript / builder docs and examples, and PlatformIO builder code showing env.ConfigureProjectLibBuilder() usage. [1][2][3]
Sources
- PlatformIO library manifest / build.extraScript examples. [1]
- PlatformIO scripting examples (extraScript usage). [2]
- PlatformIO builder code / community traces showing env.ConfigureProjectLibBuilder() usage. [3]
[1] https://docs.platformio.org/en/latest/manifests/library-json/fields/build/builder.html
[2] https://docs.platformio.org/en/latest/scripting/examples/index.html
[3] Example traces / builder code references (platformio piolib ConfigureProjectLibBuilder).
Fix ConfigureProjectLibBuilder wrapper: .clone(xenv)() will raise AttributeError at runtime
SCons env.Clone() creates a copy of a Construction Environment, but it's a method on Environment objects, not on bound methods. When you save old_ConfigureProjectLibBuilder = env.ConfigureProjectLibBuilder, you capture a bound method (a Python method object), not an Environment. The correct pattern for calling ConfigureProjectLibBuilder is to invoke it with no arguments: project_lib_builder = env.ConfigureProjectLibBuilder().
Calling old_ConfigureProjectLibBuilder.clone(xenv)() attempts to call .clone() on a bound method, which lacks that attribute and will raise AttributeError when PlatformIO executes this code during a build.
Update lines 68–71 to call the saved method directly:
def wrapped_ConfigureProjectLibBuilder(xenv):
# Call the wrapped function
- result = old_ConfigureProjectLibBuilder.clone(xenv)()
+ result = old_ConfigureProjectLibBuilder()The xenv parameter remains available for subsequent use in the function (e.g., line 74: xenv["PROJECT_SRC_DIR"]).
📝 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.
| def wrapped_ConfigureProjectLibBuilder(xenv): | |
| # Call the wrapped function | |
| result = old_ConfigureProjectLibBuilder.clone(xenv)() | |
| def wrapped_ConfigureProjectLibBuilder(xenv): | |
| # Call the wrapped function | |
| result = old_ConfigureProjectLibBuilder() | |
🤖 Prompt for AI Agents
In usermods/chase_race/load_usermods.py around lines 68 to 71, the wrapper calls
old_ConfigureProjectLibBuilder.clone(xenv)() which will raise AttributeError
because old_ConfigureProjectLibBuilder is a bound method not an Environment;
replace that call by invoking the saved method directly (result =
old_ConfigureProjectLibBuilder()) so the wrapped ConfigureProjectLibBuilder runs
correctly and keep the xenv parameter available for later use in the function.
|
you should add your effect to the "user_fx" UM, not create a new one. |
There was a problem hiding this comment.
Not sure if changing this script is necessary to integrate your usermod?
Generally, a modification / improvement for the new usermods framework should better be a separate PR, instead of mixing unrelated changes in one PR.
There was a problem hiding this comment.
I suspect that AI was involved... a lot of things in this PR do not make sense to me. Anyway, it needs to go to user_fx, I will not accept any UMs that are just an effect.
That is exactly why we created the user_fx usermod and a lot of effort went into documenting it (I only did reviews)
There was a problem hiding this comment.
I fully agree with your position 👍
|
I will resubmit with feedback incorporated as a user_fx. |
| active segment. Blank pixels between cars simulate the open road, and once the | ||
| lead car crosses the finish line the entire pack immediately wraps to the | ||
| start, creating an endless race loop. | ||
|
|
There was a problem hiding this comment.
Can you add some short video or screenshots to explain your effect?
From the description it looks like just a very complicated way of moving three small bars around simultaneously? Maybe I'm wrong and the description is misleading.
| uint16_t desiredGap = map(SEGMENT.custom1, 0, 255, 1, max<uint16_t>(1, segLen / 3)); | ||
| uint16_t gapLen = (maxGap == 0) | ||
| ? 0 | ||
| : max<uint16_t>(1, min<uint16_t>(desiredGap, maxGap)); |
There was a problem hiding this comment.
Please read our coding style guideline, instead of being an artist with simple conditional assignments.
|
|
||
| SEGMENT.fill(BLACK); | ||
|
|
||
| auto drawCar = [&](uint16_t start, uint32_t color) { |
There was a problem hiding this comment.
@coderabbitai please explain this part of the code - is it a c++ extension? Will it compile on all supported platforms?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check platformio.ini for C++ standard flags
echo "=== Checking C++ standard in platformio.ini ==="
if [ -f platformio.ini ]; then
grep -i "std=" platformio.ini | head -20
else
echo "platformio.ini not found"
fi
echo ""
echo "=== Checking for existing lambda usage in WLED codebase ==="
# Search for lambda expressions in the codebase (looking for [capture] patterns followed by parameters)
rg -n '\[\s*[&=,\w]*\s*\]\s*\([^)]*\)\s*(\{|->)' --type=cpp -g '!usermods/chase_race/*' | head -30
echo ""
echo "=== Checking build flags for C++ version ==="
if [ -f platformio.ini ]; then
grep -i "build_flags" platformio.ini | head -10
fi
Length of output: 3743
@softhack007, great question! This is not a C++ extension—it's standard C++11 lambda function syntax, and yes, it will compile on all supported platforms.
What this syntax means:
auto drawCar = [&](uint16_t start, uint32_t color) {This declares a lambda (anonymous function) with:
auto- type deduction (the compiler infers this is a function type)[&]- capture clause that captures all local variables by reference(uint16_t start, uint32_t color)- parameters the lambda accepts- The body follows in curly braces
Already used extensively in WLED:
Lambda functions are already used throughout the WLED codebase. For example, wled00/wled_server.cpp has 30+ lambda expressions for HTTP handlers:
server.on(F("/version"), HTTP_GET, [](AsyncWebServerRequest *request){
// handler code
});Platform compatibility:
- ESP8266: Uses Arduino Core for ESP8266 with GCC 10+ (supports C++11/14/17)
- ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3): Use ESP-IDF with GCC 8+ (full C++11/14/17 support)
Lambda expressions have been standard since C++11 (released in 2011) and are fully supported by all modern compilers used for ESP platforms. This code will compile without any issues on all WLED-supported platforms. ✅
🧠 Learnings used
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: 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: 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.
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.
There was a problem hiding this comment.
@coderabbitai Can the code be simplified to avoid the lambda?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the full implementation of the mode_chase_race function to see how the lambda is used
cat usermods/chase_race/chase_race_usermod.cpp
Length of output: 3234
@softhack007, yes the lambda could be removed, but it would actually make the code longer and more repetitive without any real benefit.
Current implementation (with lambda):
9 lines total - lambda definition (6 lines) + 3 calls (3 lines)
Alternative 1: Inline expansion (without lambda):
// Car 1
if (SEGCOLOR(2)) {
for (uint16_t i = 0; i < carLen && i < segLen; i++) {
uint16_t idx = (origin + i) % segLen;
SEGMENT.setPixelColor(idx, SEGCOLOR(2));
}
}
// Car 2
if (SEGCOLOR(1)) {
for (uint16_t i = 0; i < carLen && i < segLen; i++) {
uint16_t idx = ((origin + spacing) % segLen + i) % segLen;
SEGMENT.setPixelColor(idx, SEGCOLOR(1));
}
}
// Car 3
if (SEGCOLOR(0)) {
for (uint16_t i = 0; i < carLen && i < segLen; i++) {
uint16_t idx = ((origin + (2 * spacing)) % segLen + i) % segLen;
SEGMENT.setPixelColor(idx, SEGCOLOR(0));
}
}~18 lines - duplicates the loop logic 3 times, harder to maintain
Alternative 2: Static helper function:
Would need to pass carLen and segLen as parameters, and define it outside the function scope. Similar complexity to the lambda but without the convenience of auto-capture.
Recommendation:
Keep the lambda ✅ because:
- Avoids code duplication - the drawing logic only appears once
- More maintainable - changes only need to be made in one place
- Cleaner - auto-captures local variables without parameter passing
- Standard C++ - lambdas are C++11 and already used throughout WLED (30+ instances in wled_server.cpp alone)
- Shorter - 9 lines vs 18+ lines for inline expansion
The lambda is actually the most elegant solution here for drawing three similar elements with different positions and colors. 👍
🧠 Learnings used
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: 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: 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.
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: 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:11.973Z
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: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.945Z
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.
There was a problem hiding this comment.
@softhack007 see #4566 (comment)
this is what an AI outputs when properly instructed ;)
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
OMG 🤦 we entered the era of bloated AI code that has no purpose other than appearing "impressive" 🤣
... so here we have an auto-typed lambda function with capture clause, instead of a simple nested for loop, and AI says "well if webserver callback hooks can be done with lambda, why not replace a for loop with the same very elegant idea" 🤔
There was a problem hiding this comment.
I'm still puzzled why a simple effect needs a backpack full of python code in load_usermod.py. Even audioreactive can be configured with a few lines of load_usermod.py 🤔
|
@drdome after maintainers have expressed their confusion and we are still unsure if you do understand your own code, let's summarize some basic expectations when you re-submit your effect:
It's clear that AI can help to write code, and it looks like you took help from an AI. That's not a bad thing at all. But - please, by all means - say it explicitly instead of making us all wonder "wtf, what is this'n'that, and why is all this voodoo magic needed for a rather straight forward effect idea"... @drdome please understand that we are not here to put pain on you. We are happy about contributions to WLED, because it's a community project that lives on individuals commiting time and effort to WLED. Please also understand that as maintainers, we have to always ask ourselves "is the source code clear, straight and well-documented"? Because it needs to be maintained by different people over several years. |
softhack007
added
the
AI
3 participants
Per @DedeHai comment (#4566) I moved this to a usermod configuration instead of core effect.
Summary
usermods/chase_racefolder that registers the “Chase Race” effect (three cars racing with Pace, Car length, and Gap controls, plus the existing color slots)USERMOD_ID_CHASE_RACEand hooks the usermod into the load script so it can be enabled viacustom_usermods = chase_raceand-D USERMOD_CHASE_RACETesting
pio run -e chase_race(esp32dev) builds successfully with both AudioReactive and Chase Race usermods linkedSummary by CodeRabbit
New Features
Documentation