Fix ESP8266 DMA off-by-one

[willmmiles]

Shim in Makuna/NeoPixelBus#894 until approved by upstream. Fixes #4906 and #5136.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Fixed DMA handling on ESP8266 boards for improved compatibility and performance.

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

[coderabbitai]

Walkthrough

This pull request adds an ESP8266 DMA encoding fix library to address a memory buffer overrun issue in NeoPixelBus v2.8.1's 3-bit I2S cadence implementation. Changes include a new templated DMA encoding header, library manifest, build configuration update, and conditional integration into the bus wrapper.

Changes

Cohort / File(s)	Summary
ESP8266 DMA Fix Library lib/NeoESP8266DMAFix/include/NeoEsp8266DmaMethodFix.h, lib/NeoESP8266DMAFix/library.json	Added templated DMA encoding strategy class NeoEsp8266Dma3StepEncodeFixed<T_PATTERN> with static methods for DMA buffer filling and bit-packing logic. Added library manifest specifying espressif8266 platform, build.libArchive false, and NeoPixelBus 2.8.3 dependency.
Build Configuration platformio.ini	Added NeoESP8266DMAFix to ESP8266 lib_deps section.
Bus Integration wled00/bus_wrapper.h	Added conditional include for NeoEsp8266DmaMethodFix.h when ARDUINO_ARCH_ESP8266 is defined.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• DMA encoding template logic in NeoEsp8266DmaMethodFix.h may require verification of bit-packing correctness and boundary condition handling
• Dependency compatibility should be confirmed (NeoPixelBus 2.8.3 and build integration)
• Pattern specialization for NeoEsp8266DmaNormalPattern and NeoEsp8266DmaInvertedPattern inheritance chain

Suggested reviewers

• netmindz
• blazoncek

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix ESP8266 DMA off-by-one' directly and concisely describes the main change: correcting an off-by-one error in ESP8266 DMA buffer handling, which is the primary objective of this PR.
Linked Issues check	✅ Passed	The PR implements a shim to fix an off-by-one error in ESP8266 DMA buffer translation [#4906] by introducing NeoEsp8266Dma3StepEncodeFixed template class and specializations that correct the buffer size calculation and prevent memory overruns.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the ESP8266 DMA off-by-one error: new DMA encoding helper header, library manifest, dependency addition, and conditional include. No unrelated modifications detected.

✨ 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 b5f13e4 and 4fa4bc8.

📒 Files selected for processing (4)

• lib/NeoESP8266DMAFix/include/NeoEsp8266DmaMethodFix.h (1 hunks)
• lib/NeoESP8266DMAFix/library.json (1 hunks)
• platformio.ini (1 hunks)
• wled00/bus_wrapper.h (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

wled00/**/!(html_*)*.h

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

Use 2-space indentation for non-generated C++ header files (.h)

Files:

• wled00/bus_wrapper.h

platformio.ini

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

Use platformio.ini as the single source of truth for hardware build targets and settings

Files:

• platformio.ini

🧠 Learnings (8)

📓 Common learnings

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: 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: 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: 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: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:87-91
Timestamp: 2025-09-02T02:15:44.324Z
Learning: ESP_ERROR_CHECK_WITHOUT_ABORT_SILENT_TIMEOUT is part of the NeoPixelBus library API and can be safely depended upon when NeoPixelBus is a declared dependency.

Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.

📚 Learning: 2025-09-02T01:45:58.047Z

Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.

Applied to files:

• wled00/bus_wrapper.h
• platformio.ini

📚 Learning: 2025-09-18T03:17:30.107Z

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 BusManager (wled00/FX_fcn.cpp), direct access to BusManager::busses is part of the intended public API design, not an encapsulation violation. The wrapper accessor methods are considered unnecessary obfuscation that should be removed in future refactoring.

Applied to files:

• wled00/bus_wrapper.h

📚 Learning: 2025-09-02T02:15:44.324Z

Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:87-91
Timestamp: 2025-09-02T02:15:44.324Z
Learning: ESP_ERROR_CHECK_WITHOUT_ABORT_SILENT_TIMEOUT is part of the NeoPixelBus library API and can be safely depended upon when NeoPixelBus is a declared dependency.

Applied to files:

• wled00/bus_wrapper.h
• lib/NeoESP8266DMAFix/library.json
• lib/NeoESP8266DMAFix/include/NeoEsp8266DmaMethodFix.h
• platformio.ini

📚 Learning: 2025-09-02T01:48:16.409Z

Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:72-85
Timestamp: 2025-09-02T01:48:16.409Z
Learning: In NeoPixelBus driver code, ESP_ERROR_CHECK (which can abort) is used in destructors when driver uninstall failures indicate hardware is in an unsafe configuration, as such errors are considered unrecoverable hardware safety issues rather than typical software errors.

Applied to files:

• wled00/bus_wrapper.h
• lib/NeoESP8266DMAFix/include/NeoEsp8266DmaMethodFix.h

📚 Learning: 2025-09-02T01:56:43.841Z

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.

Applied to files:

• wled00/bus_wrapper.h
• lib/NeoESP8266DMAFix/include/NeoEsp8266DmaMethodFix.h

📚 Learning: 2025-02-19T12:43:34.199Z

Learnt from: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.

Applied to files:

• lib/NeoESP8266DMAFix/library.json

📚 Learning: 2025-08-29T01:34:34.358Z

Learnt from: willmmiles
Repo: wled/WLED PR: 4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.358Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.

Applied to files:

• lib/NeoESP8266DMAFix/include/NeoEsp8266DmaMethodFix.h

🔇 Additional comments (7)

lib/NeoESP8266DMAFix/library.json (1)

1-12: LGTM! Library manifest is well-formed.

The manifest correctly declares this as a header-only library (libArchive: false) targeting ESP8266, with a matching NeoPixelBus 2.8.3 dependency.

wled00/bus_wrapper.h (1)

7-9: LGTM! Conditional include is correctly placed.

The DMA fix header is properly included after NeoPixelBus.h and guarded for ESP8266 only, ensuring the template specializations can overlay the base DMA encoder classes.

lib/NeoESP8266DMAFix/include/NeoEsp8266DmaMethodFix.h (4)

4-5: Track upstream merge for future removal.

This shim references the pending upstream PR (Makuna/NeoPixelBus#894). Once that PR is merged and WLED updates to the fixed NeoPixelBus version, this entire library should be removed to reduce maintenance burden.

Consider adding a TODO comment or tracking issue to monitor the upstream PR status and schedule removal of this temporary fix.

---

48-115: Core DMA encoding logic appears correct.

The FillBuffers implementation handles 3-bit cadence encoding with proper boundary conditions:

• Fast path (lines 71-81) when sufficient bits remain in the current DMA word
• Boundary case (lines 82-107) correctly splits bits across DMA words
• Line 97's if (bitSplit) check prevents unnecessary writes when destBitsLeft == 3
• Line 114 flushes any remaining bits at the end

This logic should address the off-by-one buffer overrun described in issues #4906 and #5136.

---

119-120: Explicit specializations successfully overlay upstream classes.

These specializations replace NeoPixelBus's NeoEsp8266Dma3StepEncode implementations for both normal and inverted patterns, ensuring the fixed FillBuffers method is used for all ESP8266 DMA operations.

---

36-36: This include is an established pattern already accepted in prior work.

The internal header dependency (internal/methods/NeoEsp8266DmaMethod.h) follows the same pattern as other NeoPixelBus integrations in WLED (e.g., PR 4890), where internal API dependencies are acceptable when extending upstream driver classes. No fragility concerns—this is a stable extension point maintained by NeoPixelBus for driver implementations.

Likely an incorrect or invalid review comment.

platformio.ini (1)

223-223: Confirm compat builds don't require NeoESP8266DMAFix.

The NeoPixelBus 2.7.9 used in compat builds (lines 238–255) predates v2.8.1 (Sep 1, 2024), where the ESP8266 DMA bug was introduced. Compat builds are unaffected and correctly exclude this fix from their lib_deps.

---

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

[netmindz]

Thanks for the quick turnaround @willmmiles

I'll do a merge and refresh today's nightly build to make it easier for people to test