Bugfix in ledmap generation

[DedeHai]

• Fix off-by-one bug: was leading to crashes due to out of bounds access
• Remove unnecessary 16384 restriction: ledmap is unsigned, no need to restrict it here

also improved debug outputs

Summary by CodeRabbit

Bug Fixes

• Extended LED array size support from 16,384 to 65,535 LEDs, enabling larger installations.
• Improved custom LED mapping validation with refined boundary checking for better robustness.
• Enhanced debug output for improved troubleshooting of invalid LED mappings.

[coderabbitai]

Walkthrough

The change modifies LED map validation and debug output in FX_fcn.cpp, increasing the index upper bound check from 16384 to 65535, adjusting the custom mapping size boundary check condition, adding LED map dimension debug output, and improving invalid mapping display.

Changes

Cohort / File(s)	Change Summary
LED Map Validation and Debug Output wled00/FX_fcn.cpp	Increased index upper bound validation to 65535, adjusted customMappingSize break condition from > to >=, added debug print for LED map width/height, and changed invalid mapping display from 0xFFFF to -1 in debug output

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Boundary value changes are localized and straightforward
• Debug output adjustments are minor formatting improvements
• Single file affected with no structural complexity
• Changes follow clear intent patterns (validation relaxation and readability improvement)

Possibly related PRs

• fix timing issue when changing 1D <-> 2D credits to @blazoncek #5023: Shares modifications to LED map deserialization/validation logic and index bounds checking in FX_fcn.cpp

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Bugfix in ledmap generation' directly relates to the main changes: fixing off-by-one bugs and removing unnecessary restrictions in LED map handling.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd933ff and a666f07.

📒 Files selected for processing (1)

• wled00/FX_fcn.cpp (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

wled00/**/*.cpp

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use 2-space indentation for C++ source files (.cpp)

Files:

• wled00/FX_fcn.cpp

🧠 Learnings (13)

📓 Common learnings

Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.

Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.

Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.

Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.

Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.

Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.

Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.

Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), digitalCount is intentionally incremented even for placeholder buses to maintain consistent resource mapping across all bus configurations.

Learnt from: DedeHai
Repo: wled/WLED PR: 4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.

📚 Learning: 2025-08-26T11:51:21.817Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-08-31T03:38:14.114Z

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-09-16T18:08:42.848Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-08-28T08:09:20.630Z

Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-10-10T18:34:06.550Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-06-15T09:59:52.720Z

Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-10-05T15:24:05.545Z

Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-09-01T10:26:17.959Z

Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-10-20T09:38:51.997Z

Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5223-5226
Timestamp: 2025-10-20T09:38:51.997Z
Learning: WLED matrices: each dimension (SEG_W, SEG_H) is limited to ≤255; 256 or larger per side is not supported/feasible on ESP32, so effects should assume per-dimension max 255.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-10-20T09:41:50.374Z

Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5209-5212
Timestamp: 2025-10-20T09:41:50.374Z
Learning: In WLED 2D effects, SEGMENT.length() returns the physical pixel count (upper bound), while SEG_W * SEG_H can be smaller depending on mapping (mirroring/grouping). For per-cell state buffers (e.g., mode_2Dgameoflife in wled00/FX.cpp), allocate using SEGMENT.length() to ensure sufficient capacity and avoid reallocations, while bounding indexing by cols*rows.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-05-09T18:43:15.355Z

Learnt from: DedeHai
Repo: wled/WLED PR: 4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.

Applied to files:

• wled00/FX_fcn.cpp

📚 Learning: 2025-06-07T15:58:42.579Z

Learnt from: willmmiles
Repo: wled/WLED PR: 4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.

Applied to files:

• wled00/FX_fcn.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). (8)

• GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
• GitHub Check: wled_build / Build Environments (lolin_s2_mini)
• GitHub Check: wled_build / Build Environments (usermods)
• GitHub Check: wled_build / Build Environments (esp32_wrover)
• GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
• GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
• GitHub Check: wled_build / Build Environments (esp32dev_debug)
• GitHub Check: wled_build / Build Environments (esp01_1m_full_160)

🔇 Additional comments (4)

wled00/FX_fcn.cpp (4)

2000-2000: LGTM - Helpful debug addition.

The debug output for LED map dimensions will aid in troubleshooting mapping issues.

---

2024-2024: Correct fix - expanded valid range to full uint16_t.

The change from 16384 to 65535 correctly allows the full range of uint16_t values as mentioned in the PR description. The comment could be slightly clearer: this validates that the index fits within uint16_t range (0-65535) and marks out-of-range values as invalid (0xFFFF), rather than preventing wrap-around per se.

---

2026-2026: Critical off-by-one fix - prevents buffer overflow.

This change correctly prevents writing beyond the allocated buffer. With the old condition >, when customMappingSize reached getLengthTotal(), line 2025's post-increment would write to customMappingTable[getLengthTotal()] (out of bounds) before the check triggered. The new >= condition breaks immediately after writing to the last valid index [getLengthTotal()-1], preventing the crash mentioned in the PR description.

---

2036-2036: LGTM - Improved debug readability.

Displaying invalid mappings (0xFFFF) as -1 makes the debug output more intuitive and easier to spot gaps in the LED map.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[blazoncek]

Changing the index limit from 16384 to 65535 adds no real value. I have yet to see a WLED installation with more than 16384 LEDs without modifying code and custom compiling.
My suggestion is to replace this value with MAX_LEDS constant instead which will make maintenance easier in the future.

However, the actual fix is ok.

[DedeHai]

Changing the index limit from 16384 to 65535 adds no real value. I have yet to see a WLED installation with more than 16384 LEDs without modifying code and custom compiling.

that is correct, but there is also no good reason to restrict it if this limit is ever increased (P4 coming up). It should just accept any value that is allowed, restrictions should be applied elsewhere.