Skip to content

Switch to out of tree AudioReactive Usermod#5311

Draft
netmindz wants to merge 8 commits intowled:mainfrom
netmindz:AR-Usermod-out-of-tree
Draft

Switch to out of tree AudioReactive Usermod#5311
netmindz wants to merge 8 commits intowled:mainfrom
netmindz:AR-Usermod-out-of-tree

Conversation

@netmindz
Copy link
Member

@netmindz netmindz commented Jan 23, 2026

The AudioReactive usermod is maintained by the MoonModules team, so rather than trying to maintain in one repo and then endless PRs to keep other forks up to date, use the new out-of-tree usermod support to take it out of the tree and WLED-MM can just add the repo to the include path for the v14 style

Summary by CodeRabbit

  • Refactor

    • Reorganized Audio Reactive usermod dependency structure for ESP32 variants; removed default Audio Reactive usermod activation from multiple environments.
    • Removed Audio Reactive usermod interface and library files.
  • New Features

    • Added error codes for hardware setting changes requiring reboot or power-cycle.
    • Enhanced settings UI with improved pin configuration helpers.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

Walkthrough

This PR extracts the embedded audio source implementation from the audioreactive usermod into an external library (wled-audioreactive). It removes audio source header and implementation files, updates PlatformIO configuration to reference the external library, adds error codes for hardware reboot/poweroff scenarios, and introduces new UI helpers for managing analog pin configurations in settings.

Changes

Cohort / File(s) Summary
Audio Source Extraction
usermods/audioreactive/audio_source.h, usermods/audioreactive/library.json, usermods/audioreactive/override_sqrt.py, usermods/audioreactive/readme.md
Removed embedded audio source implementation, build configurations, and documentation. AudioSource base class and all derived classes (I2SSource, ES7243, ES8388Source, I2SAdcSource, SPH0654) deleted.
Dependency Management
platformio.ini
Updated AR_lib_deps to reference external wled-audioreactive library; removed audioreactive from custom_usermods defaults across multiple ESP32 and ESP8266 environments.
Error Codes
wled00/const.h
Added ERR_REBOOT_NEEDED (98) and ERR_POWEROFF_NEEDED (99) for hardware configuration changes.
Settings UI & Helpers
wled00/data/settings_um.htm, wled00/fcn_declare.h
Added analog pin selection helpers (aOpt, rOpt, xOpt, dOpt, dRO); initialized d.a_pins property; added oappendi template function for logging.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • willmmiles
  • softhack007
  • DedeHai
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Switch to out of tree AudioReactive Usermod' accurately captures the main objective and primary change in the PR, which involves migrating the AudioReactive usermod from in-tree to an external dependency.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@netmindz netmindz changed the title Ar usermod out of tree Switch to out of tree AudioReactive Usermod Jan 24, 2026
@netmindz
Copy link
Member Author

netmindz commented Jan 31, 2026

aOpt is missing in upstream

@netmindz
Copy link
Member Author

Screenshot From 2026-01-31 17-52-43

@netmindz
Copy link
Member Author

Screenshot From 2026-01-31 17-54-20

@netmindz netmindz marked this pull request as ready for review January 31, 2026 17:54
@netmindz netmindz requested a review from softhack007 January 31, 2026 17:55
protected:
// Shim for oappend(), which used to exist in utils.cpp
template<typename T> static inline void oappend(const T& t) { oappend_shim->print(t); };
template<typename T> static inline void oappendi(const T& t) { oappend_shim->print(t); };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willmmiles - possibly not needed, just change the usermod

WLED-MM has

bool oappendi(int i)
{
  char s[16];               // WLEDMM max 32bit integer needs 11 chars (sign + 10) not 10
  snprintf(s, 15, "%d", i); // WLEDMM
  return oappend(s);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the usual API drift between forks ;)

Yes, in a perfect world I'd suggest a local shim in the usermod. That said, I wouldn't be opposed to a "MM usermod compatibility" PR, since we wanted to add global name support anyways.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I'm out of my depth with C when it comes to templates, so I have no idea if what I have written is valid, I was just trying to get it to compile. The comment suggests that oappend is kinda legacy, so is there something better the usermod should use?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent was to change to a new API, passing an Arduino Print& directly to the usermod code so we could completely eliminate buffer size limitations. Updated usermods implement void appendConfigData(Print& uiScript) instead. The relevant PR was #4152 .

The quick shim would be to do something like this in audio_reactive.h:

#ifndef WLEDMM   // or whatever other define is unique to MM
#define oappendi oppend
#endif

... since the current oappend shim here is guaranteed to be length safe for all integer values.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Line 395: The change sets custom_usermods to empty which removes the
AudioReactive usermod for generic builds; update the PR/release notes and add a
migration guide explaining how affected users can restore AudioReactive:
instruct them to build with the env named "env:usermods", or copy the
AudioReactive example from platformio_override.sample.ini into their
platformio.ini, or enable the feature via build flags (e.g., -D
UM_AUDIOREACTIVE_ENABLE plus required I2S pin flags); optionally add a new
example build environment demonstrating external AudioReactive library usage and
clearly mark this as a breaking change in the CHANGELOG with these migration
steps.
- Line 284: The AR_lib_deps entry currently lacks version pinning and has an
extra space after the first '=', so update the AR_lib_deps line that references
wled-audioreactive (https://github.com/MoonModules/WLED-AudioReactive-Usermod)
to pin to a specific commit hash or branch (e.g., append #<commit-hash> or
#<branch-name>) to match the project's dependency pattern and remove the stray
space after '='; ensure the updated value follows the same format used elsewhere
in platformio.ini (URL#commit or user/repo @ version).

In `@wled00/data/settings_um.htm`:
- Around line 288-305: The aOpt function assumes d.a_pins exists and uses
d.a_pins.length; guard against missing initialization by treating d.a_pins as an
empty array when undefined (e.g., create a local const pins =
Array.isArray(d.a_pins) ? d.a_pins : [];), then use pins in place of d.a_pins
for the membership checks inside aOpt (referencing function aOpt and variable
d.a_pins) so the loop and comparison (currently using d.a_pins[jj]) never throws
if S() hasn’t run.
🧹 Nitpick comments (2)
wled00/fcn_declare.h (1)

335-335: Consider adding a brief comment explaining this alias.

oappendi is identical to oappend (line 334). This appears to be a compatibility shim for external usermods (like the MoonModules AudioReactive usermod) that call oappendi. A short inline comment would help future maintainers understand why both exist.

📝 Suggested documentation
     template<typename T> static inline void oappend(const T& t) { oappend_shim->print(t); };
-    template<typename T> static inline void oappendi(const T& t) { oappend_shim->print(t); };
+    template<typename T> static inline void oappendi(const T& t) { oappend_shim->print(t); };  // alias for external usermod compatibility
wled00/data/settings_um.htm (1)

288-375: Normalize el to a single element to prevent silent no-ops.

When el is omitted or passed as an element, select stays an HTMLCollection and the helpers return early. Consider defaulting to the first match and supporting element inputs for all five helpers.

♻️ Example tweak (apply similarly to rOpt/xOpt/dOpt/dRO)
-	var select = obj;
-	if (obj[el]) select = obj[el];
+	let select = (el && el.nodeType === 1) ? el : (obj[el] || obj[0]);
+	if (!select) return;

# additional build flags for audioreactive - must be applied globally
AR_build_flags = ;; -fsingle-precision-constant ;; forces ArduinoFFT to use float math (2x faster)
AR_lib_deps = ;; for pre-usermod-library platformio_override compatibility
AR_lib_deps = wled-audioreactive = https://github.com/MoonModules/WLED-AudioReactive-Usermod
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's check the repository status and available versions
# We'll also examine the platformio.ini file to see how other dependencies are specified

# Check if we can find platformio.ini and examine it
git ls-files | grep platformio.ini

Repository: wled/WLED

Length of output: 67


🌐 Web query:

WLED-AudioReactive-Usermod GitHub releases versions MoonModules

💡 Result:

  • “WLED-AudioReactive-Usermod” isn’t a separate GitHub project with its own release stream anymore—the AudioReactive usermod is part of upstream WLED (available since 0.14.0-beta1, and included in official WLED releases since 0.15.0). (mm.kno.wled.ge)
  • Latest upstream WLED release (GitHub Releases): WLED 0.15.3 (timestamped 2025-12-04 in the release assets). (github.com)

MoonModules (WLED-MM) releases (GitHub Releases):

  • Latest stable: WLED-MM v14.7.1 (released 2026-01-13). (github.com)
  • Latest nightly shown on the releases page: Nightly mdev Build 20260131 (dated 2026-01-31). (github.com)

Citations:


🏁 Script executed:

#!/bin/bash
# Let's examine the platformio.ini file to see the dependency patterns
head -n 300 platformio.ini | tail -n 50

Repository: wled/WLED

Length of output: 2421


🏁 Script executed:

#!/bin/bash
# Search for all library dependency patterns in platformio.ini
rg 'lib_deps|_lib_deps' platformio.ini -A 2 -B 1

Repository: wled/WLED

Length of output: 4387


Specify a commit hash or branch for the AudioReactive library to match the version pinning pattern used throughout platformio.ini.

All other dependencies in this file pin to specific versions, commits, or tags (e.g., fastled/FastLED @ 3.6.0, https://github.com/Aircoookie/GifDecoder.git#bc3af189b6b1e06946569f6b4287f0b79a860f8e). The AR_lib_deps lacks any pinning, making builds non-reproducible and inconsistent with the file's standards.

Example with commit hash
-AR_lib_deps =  wled-audioreactive = https://github.com/MoonModules/WLED-AudioReactive-Usermod
+AR_lib_deps = wled-audioreactive = https://github.com/MoonModules/WLED-AudioReactive-Usermod.git#<commit_hash>

(Also remove the extra space after the first =.)

🤖 Prompt for AI Agents
In `@platformio.ini` at line 284, The AR_lib_deps entry currently lacks version
pinning and has an extra space after the first '=', so update the AR_lib_deps
line that references wled-audioreactive
(https://github.com/MoonModules/WLED-AudioReactive-Usermod) to pin to a specific
commit hash or branch (e.g., append #<commit-hash> or #<branch-name>) to match
the project's dependency pattern and remove the stray space after '='; ensure
the updated value follows the same format used elsewhere in platformio.ini
(URL#commit or user/repo @ version).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@netmindz, that makes perfect sense! During parallel development when both repositories are evolving together, pinning to a specific commit would indeed create unnecessary friction and make testing difficult.

For the active development phase, keeping the dependency unpinned is the pragmatic approach. However, I'd recommend pinning to a specific commit hash before this PR is merged to main—that way, the release builds remain reproducible and users get a tested, stable version of the AudioReactive usermod.

Feel free to update the pinning when you're ready to finalize the PR. Thanks for the clarification! 👍


🧠 Learnings used
Learnt from: netmindz
Repo: wled/WLED PR: 5311
File: platformio.ini:395-395
Timestamp: 2026-01-31T18:19:28.320Z
Learning: In WLED, usermods can be referenced as out-of-tree library dependencies (e.g., via lib_deps pointing to external repositories) while the `custom_usermods` setting in platformio.ini controls which in-tree usermods are enabled. Setting `custom_usermods = ` (empty) only disables in-tree usermods; out-of-tree usermods pulled via library dependencies remain active.

Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.840Z
Learning: In WLED, board JSON files in the boards/ directory may reference partition files that don't exist or have slightly different names than actual files in tools/. This is intentional because platformio.ini or platformio_override.sample.ini explicitly override the partition file using the board_build.partitions setting, which takes precedence over the board JSON partition reference. This pattern exists in multiple local board definitions and should not be flagged as an issue.

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 platformio.ini : Use platformio.ini as the single source of truth for hardware build targets and settings

Learnt from: Jason2866
Repo: wled/WLED PR: 4167
File: platformio.ini:427-430
Timestamp: 2025-07-06T16:15:24.051Z
Learning: In espressif Arduino framework, `-fno-lto` is set as a standard default in the `platformio-build.py` script. Therefore, using `-fno-lto` in `build_unflags` is not only valid but necessary to remove this default setting before enabling LTO with `-flto=auto` or `-flto=4` in `build_flags`.

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.

Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.

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: In WLED projects, ArduinoJson.h is not directly included via `#include` <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding `#include` <ArduinoJson.h> is not necessary and may not work.

Learnt from: Jason2866
Repo: wled/WLED PR: 4167
File: platformio.ini:427-430
Timestamp: 2025-07-06T16:11:05.896Z
Learning: In PlatformIO, using `-fno-lto` in `build_unflags` is a valid pattern to remove potentially existing `-fno-lto` flags from the build system before adding `-flto=auto` in `build_flags`. This ensures that LTO can be enabled without conflicts from default build system flags that might disable LTO.

Learnt from: DedeHai
Repo: wled/WLED PR: 4750
File: usermods/audioreactive/audio_reactive.cpp:311-321
Timestamp: 2025-06-27T06:04:44.652Z
Learning: In WLED's AudioReactive usermod, FFT buffer memory (valFFT, windowFFT) is allocated once during initialization and never deallocated, following typical embedded system design where core functionality buffers persist for the application lifetime. Raw buffer pointers for aligned allocations don't need to be tracked in the current implementation since deallocation is never performed.

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: 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: 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.

Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.

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: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR `#5134`, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai Yes you are correct, this is the pattern we use, however this PR is going side by side with the creation of that repo, so we don't want to keep updating this PR after every commit to the usermod, as the only way to test the usermod is through it's usage here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@netmindz, understood! I see you're confirming the parallel development approach. That makes complete sense for your current workflow—keeping it unpinned allows you to iterate freely while both repositories evolve together.

As mentioned earlier, feel free to pin to a specific commit when you're ready to finalize before merging to main. No action needed for now. 👍


🧠 Learnings used
Learnt from: netmindz
Repo: wled/WLED PR: 5311
File: platformio.ini:395-395
Timestamp: 2026-01-31T18:19:28.320Z
Learning: In WLED, usermods can be referenced as out-of-tree library dependencies (e.g., via lib_deps pointing to external repositories) while the `custom_usermods` setting in platformio.ini controls which in-tree usermods are enabled. Setting `custom_usermods = ` (empty) only disables in-tree usermods; out-of-tree usermods pulled via library dependencies remain active.

Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.840Z
Learning: In WLED, board JSON files in the boards/ directory may reference partition files that don't exist or have slightly different names than actual files in tools/. This is intentional because platformio.ini or platformio_override.sample.ini explicitly override the partition file using the board_build.partitions setting, which takes precedence over the board JSON partition reference. This pattern exists in multiple local board definitions and should not be flagged as an issue.

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 platformio.ini : Use platformio.ini as the single source of truth for hardware build targets and settings

Learnt from: Jason2866
Repo: wled/WLED PR: 4167
File: platformio.ini:427-430
Timestamp: 2025-07-06T16:15:24.051Z
Learning: In espressif Arduino framework, `-fno-lto` is set as a standard default in the `platformio-build.py` script. Therefore, using `-fno-lto` in `build_unflags` is not only valid but necessary to remove this default setting before enabling LTO with `-flto=auto` or `-flto=4` in `build_flags`.

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.

Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.

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: In WLED projects, ArduinoJson.h is not directly included via `#include` <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding `#include` <ArduinoJson.h> is not necessary and may not work.

Learnt from: Jason2866
Repo: wled/WLED PR: 4167
File: platformio.ini:427-430
Timestamp: 2025-07-06T16:11:05.896Z
Learning: In PlatformIO, using `-fno-lto` in `build_unflags` is a valid pattern to remove potentially existing `-fno-lto` flags from the build system before adding `-flto=auto` in `build_flags`. This ensures that LTO can be enabled without conflicts from default build system flags that might disable LTO.

Learnt from: DedeHai
Repo: wled/WLED PR: 4750
File: usermods/audioreactive/audio_reactive.cpp:311-321
Timestamp: 2025-06-27T06:04:44.652Z
Learning: In WLED's AudioReactive usermod, FFT buffer memory (valFFT, windowFFT) is allocated once during initialization and never deallocated, following typical embedded system design where core functionality buffers persist for the application lifetime. Raw buffer pointers for aligned allocations don't need to be tracked in the current implementation since deallocation is never performed.

Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.

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: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR `#5134`, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:167-171
Timestamp: 2026-01-13T03:07:20.560Z
Learning: In WLED effects (usermods/user_fx/user_fx.cpp and similar), when storing future timestamps, assignments like `stored_time = now + delay` are correct and handle uint32_t wraparound properly. Only comparison statements need to use subtraction-based elapsed-time checks (e.g., `(now - stored_time) >= delay`). Do not flag assignment statements as needing fixes for wraparound handling.

build_flags = ${common.build_flags} ${esp8266.build_flags} -D WLED_RELEASE_NAME=\"ESP8266_160\" #-DWLED_DISABLE_2D
-D WLED_DISABLE_PARTICLESYSTEM2D
custom_usermods = audioreactive
custom_usermods =
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are any references to the audioreactive usermod in the codebase
# that might break without the in-tree version

echo "=== Searching for references to audioreactive in source files ==="
rg -i "audioreactive" --type cpp --type c --type h -C 2 || echo "No references found"

echo -e "\n=== Checking for audio-related configurations that might depend on the usermod ==="
rg -i "USERMOD.*AUDIO|AUDIO.*USERMOD" --type cpp --type c --type h -C 2 || echo "No config references found"

echo -e "\n=== Looking for example platformio_override.ini configurations ==="
fd -type f \( -name "*platformio*override*" -o -name "*example*" \) | head -20

Repository: wled/WLED

Length of output: 11506


🏁 Script executed:

#!/bin/bash
# Check the platformio.ini file around the mentioned lines
echo "=== Checking platformio.ini around line 395 ==="
sed -n '390,400p' platformio.ini

echo -e "\n=== Checking other mentioned lines ==="
for line_num in 423 453 460 479 495 512 527 564 586 607 648 668; do
  echo "Line $line_num:"
  sed -n "${line_num}p" platformio.ini
done

echo -e "\n=== Checking if any environment has 'audioreactive' in custom_usermods ==="
rg "custom_usermods.*audioreactive" platformio.ini -i

echo -e "\n=== Checking for migration guides or documentation files ==="
fd -type f \( -name "*.md" -o -name "*.txt" -o -name "CHANGELOG*" \) | xargs grep -l "audioreactive\|migration" -i 2>/dev/null | head -10

Repository: wled/WLED

Length of output: 1188


🏁 Script executed:

#!/bin/bash
# Check git history to see if these lines changed
echo "=== Checking git diff/log for platformio.ini changes ==="
git log --oneline platformio.ini | head -5

echo -e "\n=== Checking for recent changes to custom_usermods ==="
git log -p --follow -S "custom_usermods" platformio.ini | head -100

echo -e "\n=== Searching for README or documentation files ==="
find . -maxdepth 2 -type f \( -name "README*" -o -name "CHANGELOG*" -o -name "*.md" \) | head -20

echo -e "\n=== Checking for audioreactive documentation ==="
find . -type f -name "*.md" -exec grep -l "audioreactive\|AudioReactive" {} \; 2>/dev/null | head -10

echo -e "\n=== Looking for example/sample platformio configurations ==="
find . -type f -name "*platformio*" | grep -v ".git"

Repository: wled/WLED

Length of output: 1230


🏁 Script executed:

#!/bin/bash
# Check CHANGELOG for AudioReactive mentions
echo "=== Checking CHANGELOG for AudioReactive mentions ==="
grep -A 5 -B 5 -i "audioreactive" CHANGELOG.md | head -50

echo -e "\n=== Checking platformio_override.usermods.ini ==="
cat ./usermods/platformio_override.usermods.ini

echo -e "\n=== Checking platformio_override.sample.ini ==="
cat ./platformio_override.sample.ini

echo -e "\n=== Checking if env:usermods exists and its custom_usermods setting ==="
grep -A 10 "\[env:usermods\]" platformio.ini

Repository: wled/WLED

Length of output: 29853


Document the breaking change for AudioReactive usermod users.

Setting custom_usermods = (empty) removes the AudioReactive usermod from most generic build environments, breaking builds for users who relied on it being included by default (as per CHANGELOG Build 2409100). Note that specialized environments (HUB75 variants, Adafruit MatrixPortal, etc.) retain AudioReactive support, and env:usermods still includes all usermods via custom_usermods = *.

Consider:

  1. Adding a migration guide in the PR description or release notes explaining that users can:
    • Use env:usermods build environment
    • Copy the AudioReactive configuration example from platformio_override.sample.ini
    • Enable via build flags (-D UM_AUDIOREACTIVE_ENABLE with I2S pin configuration)
  2. Creating a dedicated build environment that demonstrates external AudioReactive library usage if applicable
  3. Ensuring release notes clearly document this as a breaking change with migration instructions
🤖 Prompt for AI Agents
In `@platformio.ini` at line 395, The change sets custom_usermods to empty which
removes the AudioReactive usermod for generic builds; update the PR/release
notes and add a migration guide explaining how affected users can restore
AudioReactive: instruct them to build with the env named "env:usermods", or copy
the AudioReactive example from platformio_override.sample.ini into their
platformio.ini, or enable the feature via build flags (e.g., -D
UM_AUDIOREACTIVE_ENABLE plus required I2S pin flags); optionally add a new
example build environment demonstrating external AudioReactive library usage and
clearly mark this as a breaking change in the CHANGELOG with these migration
steps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not being removed, rather it's using the ability to reference out-of-tree usermods so wled-audioreactive being added as a library instead means that it is still included. We might look to rename custom_usermods to something like custom_usermods_enabled to help clarify this relates to in-tree mods. Do you have any other suggestions for naming?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@netmindz, thank you for the clarification! You're absolutely right, and I apologize for mischaracterizing this change. I now understand that AudioReactive functionality is preserved—it's being migrated from an in-tree usermod to an out-of-tree library dependency via the AR_lib_deps mechanism, which still makes it available to all ESP32 builds through ${esp32.AR_lib_deps} in the esp32_all_variants section.

Regarding naming suggestions for clarifying that custom_usermods relates to in-tree mods, here are some options to consider:

More explicit about scope:

  • intree_usermods - Clear and concise
  • local_usermods - Indicates locally-available mods
  • builtin_usermods - Suggests mods built into the tree

Emphasizing the "enabled" aspect (your suggestion):

  • custom_usermods_enabled - Your suggestion, makes the intent clear
  • enabled_usermods - Shorter alternative

Following existing PlatformIO patterns:

  • usermods_intree - Matches the lib_deps pattern (noun_descriptor)

My recommendation would be intree_usermods as it's:

  • Short and clear
  • Parallels the out-of-tree terminology
  • Follows PlatformIO conventions (noun_descriptor)
  • Self-documenting

Alternatively, local_usermods is also strong if you want to avoid explicitly mentioning "tree" structure.


✏️ Learnings added
Learnt from: netmindz
Repo: wled/WLED PR: 5311
File: platformio.ini:395-395
Timestamp: 2026-01-31T18:19:28.320Z
Learning: In WLED, usermods can be referenced as out-of-tree library dependencies (e.g., via lib_deps pointing to external repositories) while the `custom_usermods` setting in platformio.ini controls which in-tree usermods are enabled. Setting `custom_usermods = ` (empty) only disables in-tree usermods; out-of-tree usermods pulled via library dependencies remain active.

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: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to platformio.ini : Use platformio.ini as the single source of truth for hardware build targets and settings

Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.840Z
Learning: In WLED, board JSON files in the boards/ directory may reference partition files that don't exist or have slightly different names than actual files in tools/. This is intentional because platformio.ini or platformio_override.sample.ini explicitly override the partition file using the board_build.partitions setting, which takes precedence over the board JSON partition reference. This pattern exists in multiple local board definitions and should not be flagged as an issue.

Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai unfortunately, I think it may be a platformio restriction that we must name our value with custom_ as the prefix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🌐 Web query:

PlatformIO custom variable naming conventions requirements

💡 Result:

PlatformIO has two different “custom variable” concepts, with different naming rules:

1) Custom configuration options in platformio.ini (to read from extra scripts)

  • Requirement: option names must start with custom_ or board_ (otherwise PlatformIO warns that the option is unknown/ignored). [1]
    Example: custom_my_flag = 1

2) “Custom variables” via interpolation (dynamic variables) like ${section.option}

  • Format: ${<section>.<option>}. [2][3]
  • Section naming restriction: platformio and env are reserved section names and cannot be used as your own custom section for these shared values. [2][4]
    • Use something like [common], [global], [custom], etc. [2][4]
  • (Separately) Environment names in [env:NAME] must match: lowercase a-z, digits 0-9, _ and -. [5]

Sources:
[1] PlatformIO docs: “Custom options in platformio.ini” [1]
[2] PlatformIO docs/manpage: dynamic variables ${section.option} + reserved sections [2][4]
[5] PlatformIO docs: environment name character set [5]


@netmindz, you're absolutely right! The web search confirms that PlatformIO requires custom configuration options in platformio.ini to start with either custom_ or board_ prefix, otherwise PlatformIO will warn that the option is unknown or ignored.

Given this constraint, here are revised naming suggestions that maintain the required prefix while clarifying the in-tree scope:

Recommended options:

  • custom_usermods_intree - Clearly indicates in-tree scope, follows descriptor-last pattern
  • custom_intree_usermods - Emphasizes "in-tree" upfront
  • custom_usermods_enabled - Your original suggestion, emphasizes enabled/disabled state
  • custom_local_usermods - Simpler alternative suggesting locally-available mods
  • custom_builtin_usermods - Suggests mods built into the tree

My top recommendation would be custom_usermods_intree because it:

  • Maintains the base custom_usermods name for easier migration/grep
  • Adds the clarifying _intree suffix
  • Reads naturally: "custom usermods (that are) in-tree"

However, custom_usermods_enabled (your suggestion) is also excellent if you want to emphasize the enabled/disabled toggle aspect rather than the location aspect.

Would either of these work for your use case?


🧠 Learnings used
Learnt from: netmindz
Repo: wled/WLED PR: 5311
File: platformio.ini:395-395
Timestamp: 2026-01-31T18:19:28.320Z
Learning: In WLED, usermods can be referenced as out-of-tree library dependencies (e.g., via lib_deps pointing to external repositories) while the `custom_usermods` setting in platformio.ini controls which in-tree usermods are enabled. Setting `custom_usermods = ` (empty) only disables in-tree usermods; out-of-tree usermods pulled via library dependencies remain active.

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 platformio.ini : Use platformio.ini as the single source of truth for hardware build targets and settings

Learnt from: softhack007
Repo: wled/WLED PR: 5107
File: boards/adafruit_matrixportal_esp32s3.json:1-66
Timestamp: 2025-11-22T20:33:14.840Z
Learning: In WLED, board JSON files in the boards/ directory may reference partition files that don't exist or have slightly different names than actual files in tools/. This is intentional because platformio.ini or platformio_override.sample.ini explicitly override the partition file using the board_build.partitions setting, which takes precedence over the board JSON partition reference. This pattern exists in multiple local board definitions and should not be flagged as an issue.

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.

Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts on naming @willmmiles ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, yes, platformio does require names to start with custom_ ... I spent entirely too long trying to find some way to hack around that since I though it was so obnoxious.

I have no strong objections to changing the property name: it's easy enough to carry forward support for the old one as well for some period of time. That said, I'd been experimenting with going the other way: finding some way to make custom_usermods accept lib_deps syntax so we could just put any or all modules there. Would that be better?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds better, it would be much nicer to have a single field that lists all the usermods, with just a different format of name = url for the external

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll get that done, then.

Comment on lines +288 to +305
function aOpt(name,el) {
let obj = d.getElementsByName(name);
if (!obj || obj.length === 0) return; // No elements found

var select = obj;
if (obj[el]) select = obj[el];

// Check if it's actually a select element with options
if (!select.options || !select.options.length) return;

for (let i=0; i<select.options.length; i++) {
let c = select.options[i];
let found = false;
for (let jj=0; jj<d.a_pins.length; jj++) if (d.a_pins[jj] == c.value) found = true; //value -1 or analog pins
if (c.value != -1 && !found) {
select.removeChild(c);
i--; //decrease i by one because the index has been adjusted
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid crash if d.a_pins isn’t initialized before aOpt runs.

aOpt iterates d.a_pins.length; if it’s invoked before S() initializes or if another script omits it, this throws. Add a defensive default.

💡 Suggested fix
 function aOpt(name,el) {
+	if (!d.a_pins) d.a_pins = [];
 	let obj = d.getElementsByName(name);
 	if (!obj || obj.length === 0) return; // No elements found
📝 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.

Suggested change
function aOpt(name,el) {
let obj = d.getElementsByName(name);
if (!obj || obj.length === 0) return; // No elements found
var select = obj;
if (obj[el]) select = obj[el];
// Check if it's actually a select element with options
if (!select.options || !select.options.length) return;
for (let i=0; i<select.options.length; i++) {
let c = select.options[i];
let found = false;
for (let jj=0; jj<d.a_pins.length; jj++) if (d.a_pins[jj] == c.value) found = true; //value -1 or analog pins
if (c.value != -1 && !found) {
select.removeChild(c);
i--; //decrease i by one because the index has been adjusted
}
function aOpt(name,el) {
if (!d.a_pins) d.a_pins = [];
let obj = d.getElementsByName(name);
if (!obj || obj.length === 0) return; // No elements found
var select = obj;
if (obj[el]) select = obj[el];
// Check if it's actually a select element with options
if (!select.options || !select.options.length) return;
for (let i=0; i<select.options.length; i++) {
let c = select.options[i];
let found = false;
for (let jj=0; jj<d.a_pins.length; jj++) if (d.a_pins[jj] == c.value) found = true; //value -1 or analog pins
if (c.value != -1 && !found) {
select.removeChild(c);
i--; //decrease i by one because the index has been adjusted
🤖 Prompt for AI Agents
In `@wled00/data/settings_um.htm` around lines 288 - 305, The aOpt function
assumes d.a_pins exists and uses d.a_pins.length; guard against missing
initialization by treating d.a_pins as an empty array when undefined (e.g.,
create a local const pins = Array.isArray(d.a_pins) ? d.a_pins : [];), then use
pins in place of d.a_pins for the membership checks inside aOpt (referencing
function aOpt and variable d.a_pins) so the loop and comparison (currently using
d.a_pins[jj]) never throws if S() hasn’t run.

@netmindz netmindz added the usermod usermod related label Jan 31, 2026
@DedeHai
Copy link
Collaborator

DedeHai commented Feb 2, 2026

I gave this a go and it does compile and most importantly, the tested I2S mic works. Did not poke around much though.

  • when switching to this branch then back to main, I need to do a full clean or it won't compile (complains about missing UM's as well as arduinoFFT)
  • it adds a hefty 11k to the flash as well as a few hundered bytes of static RAM use - definitely need to slim it down for AC by making a lot of the advanced MM stuff optional (mic filters, FFT windows, ...) @softhack007 do you happen to know which features are the most flash heavy?

@softhack007 softhack007 marked this pull request as draft February 2, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

usermod usermod related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants