(WIP) Integrate larger and smaller fonts from old upsteam PRs #285
Conversation
only the minimal adjustments to make files compatible with the new "extended ASCII" framework
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds optional compile-time font toggles, new font assets, expands SEGMENT.custom2→font-size mapping, validates fonts early, and implements multi-byte (wide) glyph rendering in the 2D text draw path while preserving a legacy mapping branch behind a guard. Changes
Sequence Diagram(s)sequenceDiagram
participant SEG as SEGMENT
participant FX as FX (2D)
participant Fonts as Font Registry
participant Draw as drawCharacter
rect rgb(240,248,255)
SEG->>FX: request scrolling text (read SEGMENT.custom2)
FX->>Fonts: compute max_font & map custom2 -> font index
Fonts-->>FX: fontInfo (raw, width_bytes, progmem, bounds)
end
rect rgb(245,255,240)
FX->>Draw: drawCharacter(glyph, fontInfo, x,y)
alt font.raw == nullptr
Draw-->>FX: return (no-op)
else font valid
Draw->>Draw: for offset in 0..width_bytes-1
Draw->>Draw: read bytes at (glyph + offset*stride)
Draw->>Draw: compute bits, x0, numBits
Draw->>Draw: paint pixels & shadow per-offset
Draw-->>FX: glyph painted
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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: 3
🧹 Nitpick comments (3)
wled00/wled.h (1)
50-50: Add descriptive comment for the feature flag.The comment is empty and doesn't explain what
WLED_ENABLE_LARGE_FONTSdoes or its impact on memory/flash usage. Consider adding a description similar to other flags in this section (e.g., "enables large font support, uses X kb").wled00/src/font/clear_font_3x5.h (1)
244-244: Add newline at end of file.The file ends without a trailing newline after the closing brace. While not critical, it's a best practice to end source files with a newline character, as some tools and workflows expect this.
wled00/wled_fonts.hpp (1)
12-17: Consider making font feature flags configurable.These testing flags are currently hardcoded and will enable fonts unconditionally in all builds. Before merging to production, consider either making them configurable via build flags or adding them to a configuration system where users can opt-in.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
wled00/FX.cpp(1 hunks)wled00/FX_2Dfcn.cpp(4 hunks)wled00/src/font/clear_font_3x5.h(1 hunks)wled00/wled.h(1 hunks)wled00/wled_fonts.hpp(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.{cpp,h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ firmware source and headers in wled00
Files:
wled00/wled.hwled00/src/font/clear_font_3x5.hwled00/FX.cppwled00/FX_2Dfcn.cpp
🧠 Learnings (9)
📓 Common learnings
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR #270, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
📚 Learning: 2025-10-12T21:55:59.766Z
Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-12T21:55:59.766Z
Learning: Applies to wled00/html_*.h : Do not edit generated embedded web header files: wled00/html_*.h
Applied to files:
wled00/wled.hwled00/wled_fonts.hppwled00/src/font/clear_font_3x5.h
📚 Learning: 2025-11-02T22:07:15.689Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:07:15.689Z
Learning: In MoonModules/WLED-MM, WLEDMM_FASTPATH will become the default code path. Code inside "#ifndef WLEDMM_FASTPATH" blocks represents deprecated paths that are being phased out and should be ignored during reviews.
Applied to files:
wled00/wled.hwled00/wled_fonts.hpp
📚 Learning: 2025-11-02T22:17:26.380Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:17:26.380Z
Learning: In wled00/FX.cpp, the `#ifndef WLED_DISABLE_2D` guard around the Akemi 2D fallback in the `mode_oops()` function (lines 142-146) is intentionally kept. This guard is necessary because when users build with `-D WLED_DISABLE_2D` to remove all 2D effects from the build, the Akemi effect is also not compiled, so the guard prevents calling a non-existent function.
Applied to files:
wled00/wled.hwled00/FX.cppwled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-12T21:55:59.766Z
Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-12T21:55:59.766Z
Learning: Applies to wled00/**/*.{cpp,h} : Use spaces (2 per level) for C++ firmware source and headers in wled00
Applied to files:
wled00/wled.hwled00/wled_fonts.hpp
📚 Learning: 2025-07-02T23:22:57.175Z
Learnt from: netmindz
Repo: MoonModules/WLED-MM PR: 248
File: platformio.ini:1613-1613
Timestamp: 2025-07-02T23:22:57.175Z
Learning: In WLED platformio.ini, the particle system disable flags (WLED_DISABLE_PARTICLESYSTEM1D and WLED_DISABLE_PARTICLESYSTEM2D) are intentionally varied across different build environments based on platform memory constraints. More memory-limited platforms (like ESP8266, ESP32-C3, ESP32-S2) disable both 1D and 2D particle systems, while platforms with more available memory (like esp32_4MB_V4_S with 4MB flash) may only disable the 1D version to preserve flash space while keeping 2D functionality available.
Applied to files:
wled00/wled.h
📚 Learning: 2025-06-30T17:53:37.232Z
Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 248
File: usermods/audioreactive/audio_reactive.h:199-200
Timestamp: 2025-06-30T17:53:37.232Z
Learning: In WLED AudioReactive usermod, when using ArduinoFFT (enabled via UM_AUDIOREACTIVE_USE_ARDUINO_FFT), it's acceptable and necessary to redefine the global sqrt macro to sqrtf within the conditional compilation block for performance optimization. This is a specific requirement for ArduinoFFT optimization that can provide 10-50% performance improvement on ESP32.
Applied to files:
wled00/wled.hwled00/wled_fonts.hpp
📚 Learning: 2025-11-14T13:55:44.442Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-14T13:55:44.442Z
Learning: In MoonModules/WLED-MM, always flag Variable Length Arrays (VLA) during code reviews, especially when they could overflow available stack space (typically 2K-4K on ESP32/ESP8266). VLAs based on dynamic parameters like segment dimensions, particle counts, or pixel buffers should be highlighted and require justification from the author. Prefer fixed-size arrays, heap allocation, or bounded VLAs with explicit maximum size constraints.
Applied to files:
wled00/wled.hwled00/wled_fonts.hpp
📚 Learning: 2025-10-22T21:50:25.839Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR #270, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
Applied to files:
wled00/wled_fonts.hppwled00/FX.cppwled00/FX_2Dfcn.cpp
🪛 Clang (14.0.6)
wled00/src/font/clear_font_3x5.h
[error] 50-50: unknown type name 'constexpr'
(clang-diagnostic-error)
[error] 50-50: expected ';' after top level declarator
(clang-diagnostic-error)
[error] 51-51: unknown type name 'constexpr'
(clang-diagnostic-error)
[error] 51-51: expected ';' after top level declarator
(clang-diagnostic-error)
[error] 58-58: expected ';' after top level declarator
(clang-diagnostic-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Builds (esp32_4MB_V4_S)
- GitHub Check: Builds (athom_music_esp32_4MB_M)
- GitHub Check: Builds (esp8266_4MB_M)
- GitHub Check: Builds (esp8266pro_16MB_M)
- GitHub Check: Builds (esp8266_2MB_S)
- GitHub Check: Builds (esp32S3_WROOM-2_M)
- GitHub Check: Builds (esp32c3dev_2MB_M)
- GitHub Check: Builds (seeed_esp32c3_4MB_S)
- GitHub Check: Builds (esp32_4MB_V4_HUB75_forum)
- GitHub Check: Builds (esp32c3dev_4MB_M)
- GitHub Check: Builds (esp32S3_8MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_S)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_16MB_PSRAM_M_HUB75)
- GitHub Check: Builds (esp32S3_8MB_M)
- GitHub Check: Builds (esp32S3_4MB_S)
- GitHub Check: Builds (esp32_4MB_PSRAM_S)
- GitHub Check: Builds (esp32_16MB_V4_S)
- GitHub Check: Builds (esp32_16MB_V4_M)
- GitHub Check: Builds (esp32_16MB_V4_M)
🔇 Additional comments (5)
wled00/src/font/clear_font_3x5.h (1)
49-56: Verify C++ compilation mode for this header.Static analysis is reporting errors about
constexprbeing an unknown type and missing semicolons, suggesting the file might be analyzed as C instead of C++. Since this is inwled00/src/and WLED is a C++ project, ensure this.hfile is properly recognized as C++. Theconstexprkeyword (C++11) andstatic const ... PROGMEMsyntax are valid C++.If the static analysis configuration is correct and these are false positives, they can be safely ignored.
wled00/FX.cpp (1)
6808-6815: Extended font-size selection logic looks correct and safe
- Mapping
SEGMENT.custom2from[0,255]to1..6ensures the switch value is always in-range, anddefaultcorrectly falls through tocase 1.- The new
case 6(5x12) integrates cleanly with the existing pattern; the only behavioural change is allowing a larger glyph height, which may clip on very short matrices but won’t affect stability or control flow.No changes needed here from a correctness or robustness standpoint.
wled00/wled_fonts.hpp (1)
97-139: LGTM - Font definitions are correct.The font size calculations (3×5=15, 12×16=192, 12×24=288, 16×32=512, 25×57=1425) and
width_bytesvalues (1 byte for ≤8px width, 2 bytes for 9-16px, 4 bytes for 17-32px) are mathematically correct and properly guarded by feature flags.wled00/FX_2Dfcn.cpp (2)
893-893: LGTM - Good defensive null checks.These null checks prevent crashes when a font is not found or invalid, improving robustness of the text rendering system.
Also applies to: 923-923
924-925: Verify wide font rendering before merging.The restriction on
width_bytes > 1has been removed to enable wide font support. However, the implementation has critical bugs in font indexing and pixel positioning (see previous comments). Please ensure wide fonts render correctly after fixing these issues, and consider adding test cases for 12x16, 12x24, 16x32, and 25x57 fonts.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
wled00/FX_2Dfcn.cpp (1)
942-947: Critical issues from previous review remain unfixed.The loop variable now has a type declaration (
int xoffset) which is good, but the critical indexing bugs identified in the previous review are still present:
Line 943:
pixels_offset = xoffset * wis incorrect. Each byte in font data represents 8 pixels, notwpixels. Should bexoffset * 8.Lines 946-947: The font data indexing is missing the row multiplier. For multi-byte fonts where each row has
width_bytesbytes, the correct index should include(i * font.width_bytes):
- Line 946 should be:
(chr * h * font.width_bytes) + (i * font.width_bytes) + xoffset- Line 947 should be:
(chr * h * font.width_bytes) + ((i-1) * font.width_bytes) + xoffsetApply this diff to fix the indexing:
- int pixels_offset = xoffset * w; // pixel offset inside row + int pixels_offset = xoffset * 8; // each byte covers 8 pixels // get 8 pixels (byte) from raw font data - bits = pgm_read_byte_near(&font.raw[(chr * h * font.width_bytes) + i + xoffset]); - if ((i>0) && drawShadow) bits_up = pgm_read_byte_near(&font.raw[(chr * h * font.width_bytes) + i + xoffset -font.width_bytes]); + bits = pgm_read_byte_near(&font.raw[(chr * h * font.width_bytes) + (i * font.width_bytes) + xoffset]); + if ((i>0) && drawShadow) bits_up = pgm_read_byte_near(&font.raw[(chr * h * font.width_bytes) + ((i-1) * font.width_bytes) + xoffset]);
🧹 Nitpick comments (1)
wled00/FX_2Dfcn.cpp (1)
952-966: Consider bounds checking for the last byte in wide fonts.For wide fonts, the last byte may not use all 8 bits. For example, a 12-pixel wide character uses 2 bytes (8 + 4 pixels), so the second byte only has 4 valid bits. The inner loop currently processes all
wpixels for each byte, which could read beyond valid bit positions in the last byte.Consider computing the valid bit count per byte to avoid processing padding bits:
for (int j = 0; j<w; j++) { // paint character - single row of pixels (width) // Compute how many valid bits remain in this byte int remaining_pixels = w - pixels_offset; int valid_bits = min(8, remaining_pixels); if (j >= valid_bits) break; // Skip padding bits in last byte int x0 = x + (w-1) - j + pixels_offset; // ... rest of the logic }Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/FX_2Dfcn.cpp(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.{cpp,h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ firmware source and headers in wled00
Files:
wled00/FX_2Dfcn.cpp
🧠 Learnings (6)
📓 Common learnings
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
📚 Learning: 2025-11-02T22:17:26.380Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:17:26.380Z
Learning: In wled00/FX.cpp, the `#ifndef WLED_DISABLE_2D` guard around the Akemi 2D fallback in the `mode_oops()` function (lines 142-146) is intentionally kept. This guard is necessary because when users build with `-D WLED_DISABLE_2D` to remove all 2D effects from the build, the Akemi effect is also not compiled, so the guard prevents calling a non-existent function.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-14T13:55:44.442Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-14T13:55:44.442Z
Learning: In MoonModules/WLED-MM, always flag Variable Length Arrays (VLA) during code reviews, especially when they could overflow available stack space (typically 2K-4K on ESP32/ESP8266). VLAs based on dynamic parameters like segment dimensions, particle counts, or pixel buffers should be highlighted and require justification from the author. Prefer fixed-size arrays, heap allocation, or bounded VLAs with explicit maximum size constraints.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T21:59:24.331Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T22:24:46.163Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/json.cpp:295-304
Timestamp: 2025-10-22T22:24:46.163Z
Learning: When deriving an upper-bound index from a count in WLED-MM PR #270, use a safe guard: last = count ? count - 1 : 0. Avoid min/max with 0 on unsigned types and never pass (count-1) directly to helpers (e.g., updateVal16, relativeChange, random8) without guarding. Effect IDs are 16-bit; palette IDs remain 8-bit but should use the same guard when count can be 0.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T21:50:25.839Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR #270, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
Applied to files:
wled00/FX_2Dfcn.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Builds (esp32_4MB_PSRAM_S)
- GitHub Check: Builds (esp32_4MB_V4_S)
- GitHub Check: Builds (esp32_4MB_V4_S)
- GitHub Check: Builds (seeed_esp32c3_4MB_S)
- GitHub Check: Builds (esp8266_2MB_S)
- GitHub Check: Builds (esp32S3_WROOM-2_M)
- GitHub Check: Builds (esp32c3mini_dio_4MB_M)
- GitHub Check: Builds (esp32s2_PSRAM_M)
- GitHub Check: Builds (athom_music_esp32_4MB_M)
- GitHub Check: Builds (esp32c3dev_4MB_M)
- GitHub Check: Builds (esp32c3dev_2MB_M)
- GitHub Check: Builds (esp32_4MB_PSRAM_S)
- GitHub Check: Builds (esp32S3_4MB_S)
- GitHub Check: Builds (esp32_4MB_V4_HUB75_forum)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_8MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_16MB_PSRAM_M_HUB75)
- GitHub Check: Builds (esp32_16MB_V4_S)
- GitHub Check: Builds (esp32_16MB_V4_M)
- GitHub Check: Builds (esp32S3_8MB_M)
🔇 Additional comments (3)
wled00/FX_2Dfcn.cpp (3)
893-893: LGTM: Good defensive check.The null check for invalid font data prevents potential null pointer dereference.
923-923: LGTM: Good defensive check.The null check for invalid font data prevents potential null pointer dereference.
925-925: Verify: Relaxed gating enables wide-font support.Removing the
font.width_bytes > 1condition now allows wide fonts to be processed. Ensure the wide-font handling logic in lines 942-968 is correct before enabling this feature.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/FX_2Dfcn.cpp(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.{cpp,h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ firmware source and headers in wled00
Files:
wled00/FX_2Dfcn.cpp
🧠 Learnings (7)
📓 Common learnings
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR #270, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
📚 Learning: 2025-11-21T23:25:44.646Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 285
File: wled00/src/font/clear_font_3x5.h:49-56
Timestamp: 2025-11-21T23:25:44.646Z
Learning: In wled00/src/font/clear_font_3x5.h, the conditional compilation block around clear_font_3x5_first and clear_font_3x5_last (lines 49-56) intentionally duplicates identical values in both branches. This is a deliberate placeholder pattern highlighting that special handling will be needed once extended glyphs are added to the font, as the current font does not provide all extended characters.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-02T22:17:26.380Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:17:26.380Z
Learning: In wled00/FX.cpp, the `#ifndef WLED_DISABLE_2D` guard around the Akemi 2D fallback in the `mode_oops()` function (lines 142-146) is intentionally kept. This guard is necessary because when users build with `-D WLED_DISABLE_2D` to remove all 2D effects from the build, the Akemi effect is also not compiled, so the guard prevents calling a non-existent function.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-14T13:55:44.442Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-14T13:55:44.442Z
Learning: In MoonModules/WLED-MM, always flag Variable Length Arrays (VLA) during code reviews, especially when they could overflow available stack space (typically 2K-4K on ESP32/ESP8266). VLAs based on dynamic parameters like segment dimensions, particle counts, or pixel buffers should be highlighted and require justification from the author. Prefer fixed-size arrays, heap allocation, or bounded VLAs with explicit maximum size constraints.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T21:59:24.331Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T22:24:46.163Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/json.cpp:295-304
Timestamp: 2025-10-22T22:24:46.163Z
Learning: When deriving an upper-bound index from a count in WLED-MM PR #270, use a safe guard: last = count ? count - 1 : 0. Avoid min/max with 0 on unsigned types and never pass (count-1) directly to helpers (e.g., updateVal16, relativeChange, random8) without guarding. Effect IDs are 16-bit; palette IDs remain 8-bit but should use the same guard when count can be 0.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T21:50:25.839Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR #270, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
Applied to files:
wled00/FX_2Dfcn.cpp
🧬 Code graph analysis (1)
wled00/FX_2Dfcn.cpp (1)
wled00/FX.h (8)
setPixelColorXY(794-794)setPixelColorXY(796-796)setPixelColorXY(797-797)setPixelColorXY(841-841)setPixelColorXY(842-842)setPixelColorXY(844-844)setPixelColorXY(845-845)setPixelColorXY(1195-1195)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Builds (esp32_16MB_V4_S)
- GitHub Check: Builds (esp32_4MB_PSRAM_S)
- GitHub Check: Builds (adafruit_matrixportal_esp32s3_tinyUF2)
- GitHub Check: Builds (athom_music_esp32_4MB_M)
- GitHub Check: Builds (esp32S3_8MB_S)
- GitHub Check: Builds (esp32S3_WROOM-2_M)
- GitHub Check: Builds (esp32s2_PSRAM_S)
- GitHub Check: Builds (esp8266_4MB_M)
- GitHub Check: Builds (esp32S3_8MB_M)
- GitHub Check: Builds (esp32s2_PSRAM_M)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_S)
- GitHub Check: Builds (esp32c3dev_4MB_M)
- GitHub Check: Builds (esp32c3mini_dio_4MB_M)
- GitHub Check: Builds (esp32c3dev_2MB_M)
- GitHub Check: Builds (esp32S3_16MB_PSRAM_M_HUB75)
- GitHub Check: Builds (esp32S3_4MB_S)
- GitHub Check: Builds (esp32_16MB_V4_M)
- GitHub Check: Builds (esp32_16MB_V4_S)
- GitHub Check: Builds (esp32_4MB_V4_HUB75_forum)
🔇 Additional comments (2)
wled00/FX_2Dfcn.cpp (2)
893-893: LGTM: Font validity checks.The added null checks on
font.rawprevent crashes when fonts are missing or invalid. Splitting the condition on lines 924-925 improves clarity by separating font validity from PROGMEM requirement.Also applies to: 923-925
961-963: Verify shadow rendering for multi-byte fonts.The shadow logic checks adjacent bits to determine blank regions, but the bounds checking
j < (numBits-1)andj > 0only operates within the current byte. For multi-byte fonts, the shadow at byte boundaries (wherej==numBits-1in one byte andj==0in the next) may not render correctly.Consider testing with a wide font (e.g., 12-pixel width) that has shadow enabled to ensure the shadow renders correctly at the 8-pixel boundary between bytes. The current logic may create gaps in the shadow at these boundaries.
need to sleep over all these math hacks, looks to complicated
another "w" that escaped
not final - I need to find a way to make the value of the "font size" slider robust against different font counts. - need to investigate speed optimizations
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
wled00/FX_2Dfcn.cpp (1)
934-964: Still mixing glyph width with shadow width in multi-byte fontsUsing
winnumBitsafterwhas been incremented for shadow (if (drawShadow && ((w % 8) != 0)) w++;) still couples glyph bit decoding to the rendering width rather than the actual glyph width. For fonts wherew % 8 != 0(e.g. 3x5, 12‑wide, 25‑wide) anddrawShadow == true, the last byte ends up withnumBitsone larger than the true number of glyph pixels, so an extra bit/column may be processed. This can show up as a stray extra column on the right if any unused bit in that last byte is non‑zero.To fully decouple glyph decoding from the shadow extension while keeping your current per-byte logic, store the original glyph width before modifying
wand basenumBitson that original width per byte:- //if (w<5 || w>6 || h!=8) return; - if (drawShadow && ((w % 8) != 0)) w++; // one more column for shadow on right side + //if (w<5 || w>6 || h!=8) return; + const uint8_t charWidth = w; // glyph width in pixels (from font) + if (drawShadow && ((w % 8) != 0)) w++; // one more column for shadow on right side (rendering only) for (int i = 0; i<h; i++) { // // paint character - top down by row (height) @@ - uint8_t bits_up = 0; // WLEDMM this is the previous line: font[(chr * h) + i -1] - - for(int xoffset=0; xoffset < font.width_bytes; xoffset++) { // handle wide fonts - int pixels_offset = xoffset * 8; // pixel offset inside row -> 8 bits per byte - - // get 8 pixels (byte) from raw font data - bits = pgm_read_byte_near(&font.raw[(chr * h*font.width_bytes) + i*font.width_bytes + xoffset]); - if ((i>0) && drawShadow) bits_up = pgm_read_byte_near(&font.raw[(chr * h*font.width_bytes) + i*font.width_bytes + xoffset -font.width_bytes]); + uint8_t bits_up = 0; // WLEDMM this is the previous line: font[(chr * h) + i -1] + + for (int xoffset = 0; xoffset < font.width_bytes; xoffset++) { // handle wide fonts + int pixels_offset = xoffset * 8; // pixel offset inside row -> 8 bits per byte + + // remaining glyph width for this byte; skip bytes completely beyond the glyph + int remaining = int(charWidth) - pixels_offset; + if (remaining <= 0) break; + + // get 8 pixels (byte) from raw font data + bits = pgm_read_byte_near(&font.raw[(chr * h * font.width_bytes) + i * font.width_bytes + xoffset]); + if ((i > 0) && drawShadow) { + bits_up = pgm_read_byte_near(&font.raw[(chr * h * font.width_bytes) + i * font.width_bytes + xoffset - font.width_bytes]); + } @@ - int numBits = (xoffset+1 < font.width_bytes) ? 8: (w - 8*(font.width_bytes-1) ); // 8 for full bytes, remaining bits for last partial byte - for (int j = 0; j<numBits; j++) { // paint character - single row of pixels (width) + int numBits = (remaining >= 8) ? 8 : remaining; // 8 for full bytes, remaining bits for last partial byte + for (int j = 0; j < numBits; j++) { // paint character - single row of pixels (width) - int x0 = x + (numBits-1) - j + pixels_offset; + int x0 = x + (numBits - 1) - j + pixels_offset; @@ - if ((bits>>(j+(8-numBits))) & 0x01) { // bit set & drawing on-screen + if ((bits >> (j + (8 - numBits))) & 0x01) { // bit set & drawing on-screen @@ - if ((j < (numBits-1)) && (bits>>(j+(8-numBits) +1)) & 0x01) setPixelColorXY(x0, y0, bgCol); // blank when pixel to the right is set - else if ((j > 0) && (bits>>(j+(8-numBits) -1)) & 0x01) setPixelColorXY(x0, y0, bgCol);// blank when pixel to the left is set - else if ((bits_up>>(j+(8-numBits))) & 0x01) setPixelColorXY(x0, y0, bgCol); // blank when pixel above is set + if ((j < (numBits - 1)) && ((bits >> (j + (8 - numBits) + 1)) & 0x01)) setPixelColorXY(x0, y0, bgCol); // blank when pixel to the right is set + else if ((j > 0) && ((bits >> (j + (8 - numBits) - 1)) & 0x01)) setPixelColorXY(x0, y0, bgCol); // blank when pixel to the left is set + else if ((bits_up >> (j + (8 - numBits))) & 0x01) setPixelColorXY(x0, y0, bgCol); // blank when pixel above is set @@ - } - } + } + } + }This keeps
wfree for spacing/shadow while making the per-byte bit decoding depend only on the true glyph width andfont.width_bytes, which matches your requirement thatw/font.width_bytescorrespond to an actual font definition.
🧹 Nitpick comments (2)
wled00/FX_2Dfcn.cpp (1)
551-573: VLAs on ESP stack are risky for larger matrices
box_blur()(uint32_t out[dim1], in[dim1];),moveX()(newPxCol[cols];), andmoveY()(newPxCol[rows];) all use variable-length arrays sized fromvirtualWidth()/virtualHeight(). On ESP32/ESP8266 the available stack is small, and larger 2D matrices (or deep call stacks) can cause stack overflows here.Consider migrating these buffers to:
- A bounded static/heap buffer with an explicit maximum dimension, or
- A shared scratch buffer managed at strip/segment level, or
std::vector/malloc/freewith size checks and graceful failure paths.This isn’t introduced by this PR, but it’s worth tracking as a follow-up cleanup.
Also applies to: 575-599, 593-609
wled00/FX.cpp (1)
6793-6799: Consider moving font feature toggles to a central config/headerThe hardcoded
#define WLED_ENABLE_TINY_FONT,WLED_ENABLE_LARGE_FONTS, and conditionalWLED_ENABLE_XXXL_FONThere are marked “preliminary – for testing”, but if they ship like this they effectively enable all fonts for every build of this file. That can surprise downstream configuration and impact flash/RAM, especially on smaller targets.Suggest:
- Defining these in a shared config header or via build flags (e.g.
platformio.ini) instead of inFX.cpp.- Leaving
FX.cppto just consume the macros so per-target font selections stay centralized and easier to audit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
wled00/FX.cpp(3 hunks)wled00/FX_2Dfcn.cpp(3 hunks)wled00/wled_fonts.hpp(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/wled_fonts.hpp
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.{cpp,h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ firmware source and headers in wled00
Files:
wled00/FX.cppwled00/FX_2Dfcn.cpp
🧠 Learnings (13)
📓 Common learnings
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 285
File: wled00/src/font/clear_font_3x5.h:49-56
Timestamp: 2025-11-21T23:25:44.646Z
Learning: In wled00/src/font/clear_font_3x5.h, the conditional compilation block around clear_font_3x5_first and clear_font_3x5_last (lines 49-56) intentionally duplicates identical values in both branches. This is a deliberate placeholder pattern highlighting that special handling will be needed once extended glyphs are added to the font, as the current font does not provide all extended characters.
📚 Learning: 2025-11-02T22:07:15.689Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:07:15.689Z
Learning: In MoonModules/WLED-MM, WLEDMM_FASTPATH will become the default code path. Code inside "#ifndef WLEDMM_FASTPATH" blocks represents deprecated paths that are being phased out and should be ignored during reviews.
Applied to files:
wled00/FX.cppwled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-02T22:17:26.380Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:17:26.380Z
Learning: In wled00/FX.cpp, the `#ifndef WLED_DISABLE_2D` guard around the Akemi 2D fallback in the `mode_oops()` function (lines 142-146) is intentionally kept. This guard is necessary because when users build with `-D WLED_DISABLE_2D` to remove all 2D effects from the build, the Akemi effect is also not compiled, so the guard prevents calling a non-existent function.
Applied to files:
wled00/FX.cppwled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T21:50:25.839Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR #270, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
Applied to files:
wled00/FX.cppwled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-21T23:25:44.646Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 285
File: wled00/src/font/clear_font_3x5.h:49-56
Timestamp: 2025-11-21T23:25:44.646Z
Learning: In wled00/src/font/clear_font_3x5.h, the conditional compilation block around clear_font_3x5_first and clear_font_3x5_last (lines 49-56) intentionally duplicates identical values in both branches. This is a deliberate placeholder pattern highlighting that special handling will be needed once extended glyphs are added to the font, as the current font does not provide all extended characters.
Applied to files:
wled00/FX.cppwled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-14T13:55:44.442Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-14T13:55:44.442Z
Learning: In MoonModules/WLED-MM, always flag Variable Length Arrays (VLA) during code reviews, especially when they could overflow available stack space (typically 2K-4K on ESP32/ESP8266). VLAs based on dynamic parameters like segment dimensions, particle counts, or pixel buffers should be highlighted and require justification from the author. Prefer fixed-size arrays, heap allocation, or bounded VLAs with explicit maximum size constraints.
Applied to files:
wled00/FX.cppwled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-12T21:55:59.766Z
Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-12T21:55:59.766Z
Learning: Applies to wled00/**/*.{cpp,h} : Use spaces (2 per level) for C++ firmware source and headers in wled00
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-06-30T17:53:37.232Z
Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 248
File: usermods/audioreactive/audio_reactive.h:199-200
Timestamp: 2025-06-30T17:53:37.232Z
Learning: In WLED AudioReactive usermod, when using ArduinoFFT (enabled via UM_AUDIOREACTIVE_USE_ARDUINO_FFT), it's acceptable and necessary to redefine the global sqrt macro to sqrtf within the conditional compilation block for performance optimization. This is a specific requirement for ArduinoFFT optimization that can provide 10-50% performance improvement on ESP32.
Applied to files:
wled00/FX.cppwled00/FX_2Dfcn.cpp
📚 Learning: 2025-07-02T23:22:57.175Z
Learnt from: netmindz
Repo: MoonModules/WLED-MM PR: 248
File: platformio.ini:1613-1613
Timestamp: 2025-07-02T23:22:57.175Z
Learning: In WLED platformio.ini, the particle system disable flags (WLED_DISABLE_PARTICLESYSTEM1D and WLED_DISABLE_PARTICLESYSTEM2D) are intentionally varied across different build environments based on platform memory constraints. More memory-limited platforms (like ESP8266, ESP32-C3, ESP32-S2) disable both 1D and 2D particle systems, while platforms with more available memory (like esp32_4MB_V4_S with 4MB flash) may only disable the 1D version to preserve flash space while keeping 2D functionality available.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-10-22T21:59:24.331Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T22:24:46.163Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/json.cpp:295-304
Timestamp: 2025-10-22T22:24:46.163Z
Learning: When deriving an upper-bound index from a count in WLED-MM PR #270, use a safe guard: last = count ? count - 1 : 0. Avoid min/max with 0 on unsigned types and never pass (count-1) directly to helpers (e.g., updateVal16, relativeChange, random8) without guarding. Effect IDs are 16-bit; palette IDs remain 8-bit but should use the same guard when count can be 0.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-24T00:15:38.046Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-24T00:15:38.046Z
Learning: In MoonModules/WLED-MM, when reading ESP32/ESP32-S3 eFuse blocks using esp_efuse_read_field_blob(): EFUSE_BLK0 has only 7 × 32-bit registers (224 bits total), not 8 registers (256 bits). Reading more than 7 words from EFUSE_BLK0 triggers assertion failure in esp_efuse_utility_read_reg(). Use maximum 7 words when reading EFUSE_BLK0 to avoid crashes. This applies to all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3).
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-07-12T04:20:14.546Z
Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 253
File: usermods/audioreactive/audio_source.h:1256-1261
Timestamp: 2025-07-12T04:20:14.546Z
Learning: In WLED AudioReactive usermod, DC offset removal optimization in DMAadcSource::getSamples() should be implemented after PR #248 merges, which will change the sample buffer from float to int16. This eliminates the need for temporary integer buffers and allows direct integer arithmetic on the native buffer type for better memory efficiency on resource-constrained MCUs like C3 and S2.
Applied to files:
wled00/FX_2Dfcn.cpp
🔇 Additional comments (3)
wled00/FX_2Dfcn.cpp (1)
891-906: Early font validity checks look goodThe added
font.raw == nullptrguards in bothdrawText()anddrawCharacter()cleanly handle invalid/missing font mappings fromgetFontInfo()and avoid null dereferences without impacting existing call sites. This is a solid safety improvement.Also applies to: 923-926
wled00/FX.cpp (2)
6815-6857: Dynamic font index mapping looks consistent across feature combinationsThe new
max_font/map2(SEGMENT.custom2, 0, 255, 0, max_font)plusswitchcovers all indices cleanly:
- For each combination of
WLED_ENABLE_TINY_FONT,WLED_ENABLE_LARGE_FONTS, andWLED_ENABLE_XXXL_FONT, the range0..max_fonthas a correspondingcase(or falls through intentionally when tiny is disabled).- The
default: case 5safely handles out-of-range values without skipping any explicitly-defined large/XXXL cases.- Using
map2instead ofmapis fine here and still guarantees0..max_fontinclusive.Behavior for existing sizes (cases 1–5) is preserved, and extended sizes are only reachable when their feature macros are enabled.
6859-6859: Updated y-offset correctly recenters text for varying font heightsUsing
const int yoffset = map2(SEGMENT.intensity, 0, 255, -rows/2, rows/2) + (rows - letterHeight)/2;correctly:
- Centers text based on
letterHeight, so different fonts don’t drift vertically.- Still lets the “Y Offset” slider (
SEGMENT.intensity) move the text symmetrically around center.No overflow or out-of-range issues given typical matrix sizes; clipping is left to the drawing helpers as before.
sorry you are just too small.
remove 1D PARTICLESYSTEM to stay below flash limits
not sure if ARDUINOJSON_DECODE_UNICODE 1 is really needed - adds 1.2KB flash.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
platformio.ini (1)
110-110: Ensure esp01_1MB_S is consistently treated throughout the file.Line 110 marks the environment as dead with insufficient flash, but lines 1707-1708 still define the
[env:esp01_1MB_S]configuration with additional disable flags. Verify this is intentional: either the environment should be fully removed or kept as a valid (but constrained) build option. The comment placement suggests it's being deliberately deprecated but kept for reference.As per retrieved learnings, the WLED_DISABLE_LOXONE and WLED_DISABLE_MQTT flags appropriately reduce flash usage for constrained 1MB boards, but the dual marking (dead + defined) is ambiguous.
Also applies to: 1707-1708
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platformio.ini(6 hunks)wled00/wled.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/wled.h
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 285
File: wled00/src/font/clear_font_3x5.h:49-56
Timestamp: 2025-11-21T23:25:44.646Z
Learning: In wled00/src/font/clear_font_3x5.h, the conditional compilation block around clear_font_3x5_first and clear_font_3x5_last (lines 49-56) intentionally duplicates identical values in both branches. This is a deliberate placeholder pattern highlighting that special handling will be needed once extended glyphs are added to the font, as the current font does not provide all extended characters.
📚 Learning: 2025-07-02T23:22:57.175Z
Learnt from: netmindz
Repo: MoonModules/WLED-MM PR: 248
File: platformio.ini:1613-1613
Timestamp: 2025-07-02T23:22:57.175Z
Learning: In WLED platformio.ini, the particle system disable flags (WLED_DISABLE_PARTICLESYSTEM1D and WLED_DISABLE_PARTICLESYSTEM2D) are intentionally varied across different build environments based on platform memory constraints. More memory-limited platforms (like ESP8266, ESP32-C3, ESP32-S2) disable both 1D and 2D particle systems, while platforms with more available memory (like esp32_4MB_V4_S with 4MB flash) may only disable the 1D version to preserve flash space while keeping 2D functionality available.
Applied to files:
platformio.ini
📚 Learning: 2025-11-02T22:07:15.689Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:07:15.689Z
Learning: In MoonModules/WLED-MM, WLEDMM_FASTPATH will become the default code path. Code inside "#ifndef WLEDMM_FASTPATH" blocks represents deprecated paths that are being phased out and should be ignored during reviews.
Applied to files:
platformio.ini
📚 Learning: 2025-11-24T00:15:38.046Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-24T00:15:38.046Z
Learning: In MoonModules/WLED-MM, when reading ESP32/ESP32-S3 eFuse blocks using esp_efuse_read_field_blob(): EFUSE_BLK0 has only 7 × 32-bit registers (224 bits total), not 8 registers (256 bits). Reading more than 7 words from EFUSE_BLK0 triggers assertion failure in esp_efuse_utility_read_reg(). Use maximum 7 words when reading EFUSE_BLK0 to avoid crashes. This applies to all ESP32 variants (ESP32, ESP32-S2, ESP32-S3, ESP32-C3).
Applied to files:
platformio.ini
📚 Learning: 2025-11-02T22:17:26.380Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:17:26.380Z
Learning: In wled00/FX.cpp, the `#ifndef WLED_DISABLE_2D` guard around the Akemi 2D fallback in the `mode_oops()` function (lines 142-146) is intentionally kept. This guard is necessary because when users build with `-D WLED_DISABLE_2D` to remove all 2D effects from the build, the Akemi effect is also not compiled, so the guard prevents calling a non-existent function.
Applied to files:
platformio.ini
📚 Learning: 2025-11-14T13:55:44.442Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-14T13:55:44.442Z
Learning: In MoonModules/WLED-MM, always flag Variable Length Arrays (VLA) during code reviews, especially when they could overflow available stack space (typically 2K-4K on ESP32/ESP8266). VLAs based on dynamic parameters like segment dimensions, particle counts, or pixel buffers should be highlighted and require justification from the author. Prefer fixed-size arrays, heap allocation, or bounded VLAs with explicit maximum size constraints.
Applied to files:
platformio.ini
📚 Learning: 2025-10-12T21:55:59.766Z
Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-12T21:55:59.766Z
Learning: Applies to wled00/html_*.h : Do not edit generated embedded web header files: wled00/html_*.h
Applied to files:
platformio.ini
📚 Learning: 2025-10-12T21:55:59.766Z
Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-12T21:55:59.766Z
Learning: Applies to wled00/**/*.{cpp,h} : Use spaces (2 per level) for C++ firmware source and headers in wled00
Applied to files:
platformio.ini
📚 Learning: 2025-06-30T17:53:37.232Z
Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 248
File: usermods/audioreactive/audio_reactive.h:199-200
Timestamp: 2025-06-30T17:53:37.232Z
Learning: In WLED AudioReactive usermod, when using ArduinoFFT (enabled via UM_AUDIOREACTIVE_USE_ARDUINO_FFT), it's acceptable and necessary to redefine the global sqrt macro to sqrtf within the conditional compilation block for performance optimization. This is a specific requirement for ArduinoFFT optimization that can provide 10-50% performance improvement on ESP32.
Applied to files:
platformio.ini
📚 Learning: 2025-05-04T10:32:30.457Z
Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 237
File: wled00/FXparticleSystem.cpp:678-743
Timestamp: 2025-05-04T10:32:30.457Z
Learning: In the Particle System implementation for WLED, stack allocation is preferred over heap allocation for temporary rendering buffers because there's enough stack memory available and heap allocations can cause memory fragmentation.
Applied to files:
platformio.ini
📚 Learning: 2025-10-22T21:50:25.839Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR #270, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
Applied to files:
platformio.ini
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Builds (esp8266_2MB_S)
- GitHub Check: Builds (esp32S3_16MB_PSRAM_M_HUB75)
- GitHub Check: Builds (esp32c3mini_dio_4MB_M)
- GitHub Check: Builds (seeed_esp32c3_4MB_S)
- GitHub Check: Builds (esp32s2_PSRAM_S)
- GitHub Check: Builds (esp32_16MB_V4_M_debug)
- GitHub Check: Builds (esp32_4MB_V4_S)
- GitHub Check: Builds (esp32_4MB_V4_HUB75_forum)
- GitHub Check: Builds (esp32c3mini_dio_4MB_M)
- GitHub Check: Builds (athom_music_esp32_4MB_M)
- GitHub Check: Builds (esp8266_4MB_M)
- GitHub Check: Builds (seeed_esp32c3_4MB_S)
- GitHub Check: Builds (esp32c3dev_2MB_M)
- GitHub Check: Builds (esp32S3_8MB_PSRAM_M)
- GitHub Check: Builds (esp32s2_PSRAM_S)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_S)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_4MB_S)
- GitHub Check: Builds (esp32_16MB_V4_M_debug)
- GitHub Check: Builds (esp32_4MB_V4_HUB75_forum)
🔇 Additional comments (2)
platformio.ini (2)
373-373: Monitor filter additions enhance ESP32 debugging consistency.Adding
monitor_filters = esp32_exception_decoderto ESP32-based environments improves crash-reporting ergonomics by automatically decoding exception details. This is applied consistently across:
[esp32](line 373) – applies to all downstream configs[esp32s2](line 487)[esp32c3](line 514)[esp32s3](line 542)[env:nodemcuv2](line 557) – ESP8266 variant[env:esp32_4MB_V4_HUB75_forum](line 1837)[env:esp32S3_8MB_PSRAM_M](line 2184)This is a pure hygiene improvement with no functional impact on builds.
Also applies to: 487-487, 514-514, 542-542, 557-557, 1837-1837, 2184-2184
2874-2875: Properly disable board-incompatible features in athom_music_esp32_4MB_M.Lines 2874–2875 uncomment build flags to explicitly disable DMXin and HUB75 features (with justification: "no pins"), and lines 2895 and 2898 uncomment corresponding
lib_ignoreentries. This approach:
- Correctly prevents compilation of unusable features
- Saves flash by avoiding unnecessary library dependencies
- Aligns with constraints documented in comments on the Athom music board
The dual approach (build flags + lib_ignore) is consistent with how other constrained boards in this file are configured.
Also applies to: 2895-2895, 2898-2898
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/FX_2Dfcn.cpp(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.{cpp,h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ firmware source and headers in wled00
Files:
wled00/FX_2Dfcn.cpp
🧠 Learnings (10)
📓 Common learnings
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 285
File: wled00/src/font/clear_font_3x5.h:49-56
Timestamp: 2025-11-21T23:25:44.646Z
Learning: In wled00/src/font/clear_font_3x5.h, the conditional compilation block around clear_font_3x5_first and clear_font_3x5_last (lines 49-56) intentionally duplicates identical values in both branches. This is a deliberate placeholder pattern highlighting that special handling will be needed once extended glyphs are added to the font, as the current font does not provide all extended characters.
📚 Learning: 2025-11-21T23:25:44.646Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 285
File: wled00/src/font/clear_font_3x5.h:49-56
Timestamp: 2025-11-21T23:25:44.646Z
Learning: In wled00/src/font/clear_font_3x5.h, the conditional compilation block around clear_font_3x5_first and clear_font_3x5_last (lines 49-56) intentionally duplicates identical values in both branches. This is a deliberate placeholder pattern highlighting that special handling will be needed once extended glyphs are added to the font, as the current font does not provide all extended characters.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-02T22:17:26.380Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:17:26.380Z
Learning: In wled00/FX.cpp, the `#ifndef WLED_DISABLE_2D` guard around the Akemi 2D fallback in the `mode_oops()` function (lines 142-146) is intentionally kept. This guard is necessary because when users build with `-D WLED_DISABLE_2D` to remove all 2D effects from the build, the Akemi effect is also not compiled, so the guard prevents calling a non-existent function.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T21:59:24.331Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR #270, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue #272; do not address it within PR #270.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-14T13:55:44.442Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-14T13:55:44.442Z
Learning: In MoonModules/WLED-MM, always flag Variable Length Arrays (VLA) during code reviews, especially when they could overflow available stack space (typically 2K-4K on ESP32/ESP8266). VLAs based on dynamic parameters like segment dimensions, particle counts, or pixel buffers should be highlighted and require justification from the author. Prefer fixed-size arrays, heap allocation, or bounded VLAs with explicit maximum size constraints.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T22:24:46.163Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/json.cpp:295-304
Timestamp: 2025-10-22T22:24:46.163Z
Learning: When deriving an upper-bound index from a count in WLED-MM PR #270, use a safe guard: last = count ? count - 1 : 0. Avoid min/max with 0 on unsigned types and never pass (count-1) directly to helpers (e.g., updateVal16, relativeChange, random8) without guarding. Effect IDs are 16-bit; palette IDs remain 8-bit but should use the same guard when count can be 0.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-10-22T21:50:25.839Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR #270, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-11-02T22:07:15.689Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:07:15.689Z
Learning: In MoonModules/WLED-MM, WLEDMM_FASTPATH will become the default code path. Code inside "#ifndef WLEDMM_FASTPATH" blocks represents deprecated paths that are being phased out and should be ignored during reviews.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-07-12T04:20:14.546Z
Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 253
File: usermods/audioreactive/audio_source.h:1256-1261
Timestamp: 2025-07-12T04:20:14.546Z
Learning: In WLED AudioReactive usermod, DC offset removal optimization in DMAadcSource::getSamples() should be implemented after PR #248 merges, which will change the sample buffer from float to int16. This eliminates the need for temporary integer buffers and allows direct integer arithmetic on the native buffer type for better memory efficiency on resource-constrained MCUs like C3 and S2.
Applied to files:
wled00/FX_2Dfcn.cpp
📚 Learning: 2025-06-30T17:53:37.232Z
Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 248
File: usermods/audioreactive/audio_reactive.h:199-200
Timestamp: 2025-06-30T17:53:37.232Z
Learning: In WLED AudioReactive usermod, when using ArduinoFFT (enabled via UM_AUDIOREACTIVE_USE_ARDUINO_FFT), it's acceptable and necessary to redefine the global sqrt macro to sqrtf within the conditional compilation block for performance optimization. This is a specific requirement for ArduinoFFT optimization that can provide 10-50% performance improvement on ESP32.
Applied to files:
wled00/FX_2Dfcn.cpp
🧬 Code graph analysis (1)
wled00/FX_2Dfcn.cpp (2)
wled00/FX_fcn.cpp (4)
i(723-733)i(723-723)i(734-740)i(734-734)wled00/FX.h (8)
setPixelColorXY(794-794)setPixelColorXY(796-796)setPixelColorXY(797-797)setPixelColorXY(841-841)setPixelColorXY(842-842)setPixelColorXY(844-844)setPixelColorXY(845-845)setPixelColorXY(1195-1195)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Builds (esp8266_2MB_S)
- GitHub Check: Builds (esp32_16MB_V4_M_debug)
- GitHub Check: Builds (esp32S3_8MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_16MB_PSRAM_M_HUB75)
- GitHub Check: Builds (esp32_4MB_V4_S)
- GitHub Check: Builds (esp32S3_4MB_S)
- GitHub Check: Builds (esp32_4MB_PSRAM_S)
- GitHub Check: Builds (esp32dev_compat)
- GitHub Check: Builds (esp32_4MB_V4_HUB75_forum)
- GitHub Check: Builds (athom_music_esp32_4MB_M)
- GitHub Check: Builds (esp8266_4MB_M)
- GitHub Check: Builds (adafruit_matrixportal_esp32s3_tinyUF2)
- GitHub Check: Builds (esp32s2_PSRAM_S)
- GitHub Check: Builds (esp32c3dev_2MB_M)
- GitHub Check: Builds (seeed_esp32c3_4MB_S)
- GitHub Check: Builds (esp32S3_8MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_WROOM-2_M)
- GitHub Check: Builds (esp32_4MB_PSRAM_S)
- GitHub Check: Builds (esp32_16MB_V4_S)
- GitHub Check: Builds (esp32_16MB_V4_M)
🔇 Additional comments (1)
wled00/FX_2Dfcn.cpp (1)
893-893: LGTM: Early font validation.The null checks prevent null pointer dereferences and the PROGMEM validation ensures only supported font types are processed.
Also applies to: 923-925
based on upstream implementation, with some fixes and improvements: * correct centering when text scrolls vertically (-90 and 90 degrees) * minimal changes in DrawCharacter => drawing logic statys the same; only the reference X/Y coodinate system is reversed / transposed * replaced switch cases with cleaner logic: every rotation is a combination of transpose, invertX, and invertY * complementary to segment transpose and reverse => not the same effect * still ToDo: "inverted" scrolling = text scrolls up or down, by changing offsetY instead of OffsetX --> could be added by simply "extending" the rotation slider; rot=3 => scroll up; rot=-3 => scroll down
added map0() that should be used when the input value range is starting at 0. fast and very accurate, much better than map() or map2().
experimental
Summary by CodeRabbit
New Features
Bug Fixes
Other
✏️ Tip: You can customize this high-level summary in your review settings.