Replace #5138 with upstream NeoPixelBus fix

[willmmiles]

Makuna/Neopixelbus#894 was merged! Use that instead of a local shim.

Summary by CodeRabbit

Release Notes

• Refactor
  • Updated NeoPixelBus library dependency to source from a specific upstream git commit instead of a fixed version number for improved version control.
  • Simplified ESP8266 hardware-specific DMA support by removing local workaround code and consolidating with upstream library implementations.

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

[coderabbitai]

Walkthrough

Remove local ESP8266 DMA 3-step encoding workaround. Delete NeoEsp8266DmaMethodFix header and library manifest. Update NeoPixelBus dependency from fixed version 2.8.3 to specific git commit. Remove NeoESP8266DMAFix from platformio.ini and bus_wrapper.h includes.

Changes

Cohort / File(s)	Summary
Local ESP8266 DMA workaround removal lib/NeoESP8266DMAFix/include/NeoEsp8266DmaMethodFix.h, lib/NeoESP8266DMAFix/library.json	Deleted template class NeoEsp8266Dma3StepEncodeFixed<T_PATTERN> and its specializations for DMA encoding. Removed library manifest declaring dependency on NeoPixelBus 2.8.3.
Dependency update platformio.ini	Updated NeoPixelBus from fixed version 2.8.3 to git-based reference (commit a0919d1c10696614625978dd6fb750a1317a14ce). Removed NeoESP8266DMAFix from esp8266 library dependencies.
Bus wrapper configuration wled00/bus_wrapper.h	Removed conditional include of NeoEsp8266DmaMethodFix.h guarded by #ifdef ARDUINO_ARCH_ESP8266.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify that the updated NeoPixelBus commit includes the 3-step DMA encoding fix previously provided locally
• Confirm no remaining code dependencies on NeoEsp8266DmaMethodFix symbols
• Check that removing the library from platformio.ini does not break any build configurations

Possibly related PRs

• Fix ESP8266 DMA off-by-one #5138: Inverse operation that adds NeoEsp8266DmaMethodFix header, library manifest, and includes to implement the ESP8266 DMA encoding workaround.

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 'Replace #5138 with upstream NeoPixelBus fix' accurately describes the primary change: removing the local NeoESP8266DMAFix workaround and adopting the upstream NeoPixelBus fix instead.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 ae37f42 and 7b9d643.

📒 Files selected for processing (4)

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

💤 Files with no reviewable changes (3)

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

🧰 Additional context used

📓 Path-based instructions (1)

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 (4)

📓 Common learnings

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: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.913Z
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: 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.

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.

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

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.

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

• platformio.ini

📚 Learning: 2025-08-29T00:26:15.808Z

Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.

Applied to files:

• platformio.ini

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

• 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). (15)

• GitHub Check: wled_build / Build Environments (usermods)
• GitHub Check: wled_build / Build Environments (esp32dev_debug)
• GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
• GitHub Check: wled_build / Build Environments (esp32S3_wroom2)
• GitHub Check: wled_build / Build Environments (esp32_eth)
• GitHub Check: wled_build / Build Environments (lolin_s2_mini)
• GitHub Check: wled_build / Build Environments (esp01_1m_full)
• GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
• GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
• GitHub Check: wled_build / Build Environments (esp8266_2m_160)
• GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
• GitHub Check: wled_build / Build Environments (esp32_wrover)
• GitHub Check: wled_build / Build Environments (esp32dev)
• GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
• GitHub Check: wled_build / Build Environments (nodemcuv2_160)

🔇 Additional comments (1)

platformio.ini (1)

163-163: Consider using a release tag instead of git commit hash for long-term maintainability.

Pinning NeoPixelBus to a specific commit hash (a0919d1c10696614625978dd6fb750a1317a14ce) works but introduces reproducibility concerns for CI/CD pipelines. Per the established pattern of maintaining consistency with upstream approaches, verify whether the upstream fix has been released as a version tag. If available, switching to a release tag (e.g., #v3.x.x) would be more sustainable and align with standard dependency management practices. If the commit represents an unreleased upstream fix, document the expectation for when this transitions to a release version.

---

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

[willmmiles]

Also the same bug was present on ESP32s using I2S ... now fixed.