Add support for TM1815 #5269
Open
Add support for TM1815 #5269
+351
−143
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tested this, timings are not perfect on all protocols but are just within specification
Contributor
WalkthroughAdds TM1815 support: new NeoPixelBus timing classes and typedefs, a new LED type constant, bus initialization and reporting updates, TM identifier renames/variants, and UI/comment changes to treat TM1815 like TM1814. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
wled00/data/settings_leds.htm (1)
292-292: Consider usingmustR(t)instead of hardcoded type values.The condition
t == 31 || t == 35uses magic numbers for TYPE_TM1814 and TYPE_TM1815. SincemustR(t)already exists (line 25) to check the mandatory refresh capability bit fromledTypes, consider using it here for better maintainability:-gId("rf"+n).checked = (gId("rf"+n).checked || t == 31 || t == 35); // LEDs require data in off state (mandatory for TM1814/TM1815) +gId("rf"+n).checked = (gId("rf"+n).checked || mustR(t)); // LEDs require data in off state (mandatory for TM1814/TM1815)This would automatically handle any future LED types that require mandatory refresh without needing to update this line. Based on learnings, replacing magic numbers with appropriate constants/functions improves maintainability.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
wled00/FX.hwled00/bus_customNPBtiming.hwled00/bus_manager.cppwled00/bus_manager.hwled00/bus_wrapper.hwled00/const.hwled00/data/settings_leds.htm
🧰 Additional context used
📓 Path-based instructions (5)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/FX.hwled00/bus_manager.hwled00/const.hwled00/bus_wrapper.hwled00/bus_customNPBtiming.h
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings_leds.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings_leds.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings_leds.htm
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/bus_manager.cpp
🧠 Learnings (32)
📓 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: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Learnt from: 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.
📚 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.hwled00/const.hwled00/bus_wrapper.h
📚 Learning: 2025-12-28T14:06:48.764Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/FX.hwled00/bus_manager.hwled00/data/settings_leds.htmwled00/bus_wrapper.hwled00/bus_manager.cppwled00/bus_customNPBtiming.h
📚 Learning: 2025-11-16T19:40:46.260Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.
Applied to files:
wled00/FX.hwled00/const.hwled00/bus_wrapper.hwled00/bus_manager.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.hwled00/const.hwled00/data/settings_leds.htmwled00/bus_manager.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.hwled00/const.hwled00/bus_wrapper.h
📚 Learning: 2025-11-27T06:33:11.436Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Applied to files:
wled00/FX.h
📚 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.hwled00/const.hwled00/data/settings_leds.htm
📚 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/FX.hwled00/bus_manager.hwled00/bus_wrapper.hwled00/bus_manager.cppwled00/bus_customNPBtiming.h
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/FX.hwled00/const.hwled00/data/settings_leds.htmwled00/bus_wrapper.h
📚 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/bus_manager.hwled00/bus_wrapper.hwled00/bus_manager.cppwled00/bus_customNPBtiming.h
📚 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 bus management code, dead code branches in error handling (like unreachable else breaks) are intentionally preserved to accommodate potential future API changes, even when currently unreachable.
Applied to files:
wled00/bus_manager.hwled00/bus_wrapper.hwled00/bus_manager.cppwled00/bus_customNPBtiming.h
📚 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/bus_manager.hwled00/const.hwled00/data/settings_leds.htmwled00/bus_wrapper.hwled00/bus_manager.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/const.hwled00/data/settings_leds.htmwled00/bus_wrapper.hwled00/bus_manager.cpp
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
wled00/const.hwled00/data/settings_leds.htm
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
wled00/const.h
📚 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/const.hwled00/data/settings_leds.htmwled00/bus_wrapper.h
📚 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/data/settings_leds.htmwled00/bus_manager.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/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-09-12T17:29:43.826Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4923
File: wled00/FX.cpp:4883-4901
Timestamp: 2025-09-12T17:29:43.826Z
Learning: In WLED’s web UI, only one slider value (e.g., SEGMENT.intensity or SEGMENT.custom1) changes at a time; code relying on this may use simplified change guards, though presets/JSON can still update multiple fields atomically.
Applied to files:
wled00/data/settings_leds.htm
📚 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.hwled00/bus_customNPBtiming.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.hwled00/bus_customNPBtiming.h
📚 Learning: 2026-01-03T16:42:26.025Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.025Z
Learning: In WLED PR #5267, the bus allocation strategy for single I2S mode was changed: ESP32 now uses single I2S on bus 8 (the last bus, digitalCount=9) instead of bus 0; ESP32-S2 uses single I2S on bus 4 (digitalCount=5). Memory calculation logic must account for which specific bus uses I2S in single mode, not just check digitalCount <= 8.
Applied to files:
wled00/bus_wrapper.hwled00/bus_customNPBtiming.h
📚 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.hwled00/bus_customNPBtiming.h
📚 Learning: 2026-01-03T16:42:26.025Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.025Z
Learning: In WLED, both parallel I2S and single I2S modes require additional DMA buffers (4× the pixel buffer size). The memory calculation must account for which specific buses use I2S hardware, not just check digitalCount <= 8, because single I2S mode places the I2S bus at the END (ESP32 bus 8 with digitalCount=9, S2 bus 4 with digitalCount=5).
Applied to files:
wled00/bus_wrapper.hwled00/bus_customNPBtiming.h
📚 Learning: 2025-09-13T13:13:35.817Z
Learnt from: netmindz
Repo: wled/WLED PR: 3777
File: wled00/bus_manager.cpp:941-947
Timestamp: 2025-09-13T13:13:35.817Z
Learning: For HUB75 matrix panels in WLED, pins are physically dedicated to the matrix hardware and don't need to be deallocated on driver initialization failure since they cannot be reused for other purposes without hardware changes.
Applied to files:
wled00/bus_wrapper.h
📚 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/bus_wrapper.h
📚 Learning: 2025-11-22T12:12:53.486Z
Learnt from: DedeHai
Repo: wled/WLED PR: 5105
File: wled00/FX.h:968-968
Timestamp: 2025-11-22T12:12:53.486Z
Learning: In WLED's WS2812FX class (wled00/FX.h), the _pixels array stores LED colors indexed by unmapped (logical) LED positions, not physical positions. The getPixelColor(n) function intentionally checks if the logical LED at index n maps to a valid physical position via getMappedPixelIndex(n), then accesses _pixels[n] with the unmapped index. If the mapping is invalid (e.g., -1 for gaps in LED maps, or out of bounds), it returns 0 (black). This allows peek preview to correctly show gaps in LED mappings as black pixels. The mapping from logical to physical positions is only applied when writing to the hardware bus, not when reading from the pixel buffer.
<!-- [add_learning]
Applied to files:
wled00/bus_wrapper.h
📚 Learning: 2025-12-15T20:25:25.815Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-15T20:25:25.815Z
Learning: The CSS palette preview in wled00/data/index.js genPalPrevCss() function uses raw RGB values in CSS linear-gradient() without applying WLED's gamma correction, while actual LED output goes through NeoGammaWLEDMethod gamma correction. This causes inherent discrepancies between the web UI palette preview and actual LED colors, especially noticeable with different gamma settings.
Applied to files:
wled00/bus_wrapper.h
📚 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 finalizeInit() bus creation (wled00/FX_fcn.cpp), digitalCount is intentionally incremented even for placeholder buses to maintain consistent resource mapping across all bus configurations.
Applied to files:
wled00/bus_wrapper.hwled00/bus_manager.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/bus_wrapper.h
📚 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_customNPBtiming.h
⏰ 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). (19)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (nodemcuv2_160)
- GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
- GitHub Check: wled_build / Build Environments (esp32dev_debug)
- GitHub Check: wled_build / Build Environments (esp32dev)
- GitHub Check: wled_build / Build Environments (nodemcuv2)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
- GitHub Check: wled_build / Build Environments (esp8266_2m_160)
- GitHub Check: wled_build / Build Environments (esp01_1m_full)
🔇 Additional comments (15)
wled00/FX.h (1)
936-936: LGTM!The comment update correctly documents TM1815 support alongside TM1814 for the off-refresh requirement, aligning with the broader TM1815 additions in this PR.
wled00/const.h (1)
304-308: LGTM!The new
TYPE_TM1815constant is correctly defined with value 35, within the valid digital type range. The comment clearly describes its relationship to TM1814 as a half-speed variant.wled00/bus_manager.h (2)
174-181: LGTM!The
hasWhite()function correctly includesTYPE_TM1815alongsideTYPE_TM1814, as both are RGBW chipsets. The formatting is consistent with the existing code style.
196-196: LGTM!The
mustRefresh()function correctly returnstrueforTYPE_TM1815, as the half-speed TM1814 variant has the same refresh requirement to prevent falling back to demo mode.wled00/bus_manager.cpp (1)
375-375: LGTM!The TM1815 LED type is correctly added to the
getLEDTypes()list with the appropriate "D" (digital) type indicator and descriptive name.wled00/bus_wrapper.h (6)
7-7: LGTM: New include for custom NeoPixelBus timing.This properly includes the new timing definitions header that defines TM1815 timing classes and methods.
79-83: LGTM: TM1815 bus type constants properly defined.The new constants for TM1815 follow the existing numbering scheme without conflicts.
Also applies to: 96-103
289-292: LGTM: TM1815 bus definitions correctly reuse TM1814 feature class.The comment accurately documents that TM1815 is a half-speed variant of TM1814, so reusing
NeoWrgbTm1814Featureis appropriate since the LED driver capabilities are identical—only timing differs.
387-388: Good documentation of the current limit value.The comment explaining the 22.5 mA setting (max 38mA, safe value chosen) with reference to issue #1905 provides helpful context for this "magic number." Based on learnings about documenting such values in WLED.
418-421: LGTM: TM1815 initialization correctly reuses TM1814 template.Since TM1815 shares the same initialization requirements as TM1814 (pixel settings, current limits), reusing the
beginTM1814template is the right approach.
1399-1401: LGTM: getI() properly maps TYPE_TM1815 to internal bus types.The mapping correctly returns the appropriate internal type identifiers for both ESP8266 and ESP32 platforms.
Also applies to: 1464-1466
wled00/bus_customNPBtiming.h (4)
1-10: LGTM: Well-structured header for custom NeoPixelBus timing.Good file organization with proper header guards and necessary includes. The comment explains the purpose clearly.
30-54: LGTM: RMT timing class correctly implements half-speed TM1814.The timing values (2.5µs per bit vs TM1814's ~1.25µs) and the use of inverted base class are appropriate. The
DRAM_ATTRandIRAM_ATTRattributes are correctly applied for RMT operation.
82-88: LGTM: I2S/DMA timing class correctly positioned and configured.Placing
NeoBitsSpeedTm1815outside platform guards makes it available to both ESP32 I2S methods and ESP8266 DMA method. The 2500ns (2.5µs) bit time is consistent with other TM1815 timing definitions.
90-121: LGTM: Method typedefs correctly structured for all platforms.The platform-specific I2S method aliases (X8Tm1815Method) are properly configured:
- ESP32-S3 uses LCD method
- ESP32-S2 uses I2S0
- Classic ESP32 uses I2S1
All I2S methods correctly use
NeoBitsInvertedto match TM1815's inverted signal protocol.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
wled00/bus_customNPBtiming.h (1)
67-73: Typo in comment: "400mhz" should be "400khz".The baud rate 1600000 / 4 = 400 kbps, not MHz.
Suggested fix
- static const uint32_t UartBaud = 1600000; // 400mhz, 4 serial bytes per NeoByte + static const uint32_t UartBaud = 1600000; // 400khz, 4 serial bytes per NeoByte
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/bus_customNPBtiming.h
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/bus_customNPBtiming.h
🧠 Learnings (12)
📓 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: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
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.
📚 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_customNPBtiming.h
📚 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_customNPBtiming.h
📚 Learning: 2025-12-28T14:06:48.764Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/bus_customNPBtiming.h
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/bus_customNPBtiming.h
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/bus_customNPBtiming.h
📚 Learning: 2025-11-27T06:33:11.436Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Applied to files:
wled00/bus_customNPBtiming.h
📚 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_customNPBtiming.h
📚 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/bus_customNPBtiming.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_customNPBtiming.h
📚 Learning: 2026-01-03T16:42:26.025Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.025Z
Learning: In WLED, both parallel I2S and single I2S modes require additional DMA buffers (4× the pixel buffer size). The memory calculation must account for which specific buses use I2S hardware, not just check digitalCount <= 8, because single I2S mode places the I2S bus at the END (ESP32 bus 8 with digitalCount=9, S2 bus 4 with digitalCount=5).
Applied to files:
wled00/bus_customNPBtiming.h
📚 Learning: 2026-01-03T16:42:26.025Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.025Z
Learning: In WLED PR #5267, the bus allocation strategy for single I2S mode was changed: ESP32 now uses single I2S on bus 8 (the last bus, digitalCount=9) instead of bus 0; ESP32-S2 uses single I2S on bus 4 (digitalCount=5). Memory calculation logic must account for which specific bus uses I2S in single mode, not just check digitalCount <= 8.
Applied to files:
wled00/bus_customNPBtiming.h
⏰ 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). (7)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32S3_wroom2)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
🔇 Additional comments (2)
wled00/bus_customNPBtiming.h (2)
1-20: LGTM: Header structure is well-organized.The header guards, includes, and platform-specific RMT driver selection follow WLED conventions correctly.
90-122: LGTM: Method typedefs are comprehensive and correct.The platform-specific method typedefs correctly instantiate NeoPixelBus templates with TM1815 timing classes, following established patterns for ESP32 (RMT, I2S, LCD) and ESP8266 (UART, DMA, BitBang) drivers.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wled00/bus_customNPBtiming.hwled00/bus_manager.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/bus_manager.cpp
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/bus_customNPBtiming.h
🧠 Learnings (17)
📓 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: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:173-180
Timestamp: 2025-09-02T01:56:43.841Z
Learning: willmmiles prefers to maintain consistency with upstream NeoPixelBus patterns (like unchecked malloc in construct() methods) rather than diverging until improvements are made upstream first, to minimize maintenance burden and keep the codebase aligned.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
📚 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_customNPBtiming.h
📚 Learning: 2025-12-28T14:06:48.764Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/bus_customNPBtiming.h
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/bus_customNPBtiming.h
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/bus_customNPBtiming.h
📚 Learning: 2025-11-27T06:33:11.436Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Applied to files:
wled00/bus_customNPBtiming.h
📚 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/**/*.cpp : Use 2-space indentation for C++ source files (.cpp)
Applied to files:
wled00/bus_customNPBtiming.h
📚 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 : Use 2-space indentation for non-generated C++ header files (.h)
Applied to files:
wled00/bus_customNPBtiming.h
📚 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/bus_customNPBtiming.h
📚 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/bus_customNPBtiming.h
📚 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_customNPBtiming.h
📚 Learning: 2026-01-01T13:18:10.072Z
Learnt from: softhack007
Repo: wled/WLED PR: 5251
File: usermods/user_fx/user_fx.cpp:225-227
Timestamp: 2026-01-01T13:18:10.072Z
Learning: In WLED effects, when computing time differences between unsigned long timestamps (e.g., `strip.now`, `lastBumpUpdate`), always cast the subtraction result to `int` before converting to `float` to ensure correct behavior on ESP32-C3 (RISC-V). Use pattern: `float(int(timestamp1 - timestamp2))` instead of `float(timestamp1 - timestamp2)`.
Applied to files:
wled00/bus_customNPBtiming.h
📚 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_customNPBtiming.h
📚 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/bus_customNPBtiming.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_customNPBtiming.h
📚 Learning: 2026-01-03T16:42:26.025Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.025Z
Learning: In WLED, both parallel I2S and single I2S modes require additional DMA buffers (4× the pixel buffer size). The memory calculation must account for which specific buses use I2S hardware, not just check digitalCount <= 8, because single I2S mode places the I2S bus at the END (ESP32 bus 8 with digitalCount=9, S2 bus 4 with digitalCount=5).
Applied to files:
wled00/bus_customNPBtiming.h
📚 Learning: 2026-01-03T16:42:26.025Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.025Z
Learning: In WLED PR #5267, the bus allocation strategy for single I2S mode was changed: ESP32 now uses single I2S on bus 8 (the last bus, digitalCount=9) instead of bus 0; ESP32-S2 uses single I2S on bus 4 (digitalCount=5). Memory calculation logic must account for which specific bus uses I2S in single mode, not just check digitalCount <= 8.
Applied to files:
wled00/bus_customNPBtiming.h
⏰ 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). (4)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
🔇 Additional comments (4)
wled00/bus_customNPBtiming.h (4)
1-11: LGTM!Clean header structure with appropriate includes. The dual include guard pattern (
#pragma once+ traditional guards) is a valid defensive approach.
12-20: LGTM!The RMT driver selection logic correctly excludes RISC-V (ESP32-C3) from the HI path, consistent with the project's approach to handling C3 interrupt issues. Based on learnings from PR #4890.
82-88: LGTM!The I2S/DMA timing class is correctly placed outside platform-specific blocks, making it available for both ESP32 and ESP8266. Timing values are consistent with TM1815's 400kHz speed.
90-124: LGTM!The method typedefs follow established NeoPixelBus patterns with correct platform-specific selection:
- ESP32 RMT HI/NT methods selected via
USE_NEOPIXELBUS_RMT_HI- I2S/LCD methods appropriately differentiated for S3, S2, and classic ESP32
- ESP32-C3 correctly excluded from I2S methods
- ESP8266 UART/DMA/BitBang variants properly defined
This is consistent with the project's approach to maintaining alignment with upstream NeoPixelBus patterns. Based on learnings from PR #4890.
Comment on lines
+45
to
+54
| void NeoEsp32RmtSpeedTm1815::Translate(const void* src, | ||
| rmt_item32_t* dest, | ||
| size_t src_size, | ||
| size_t wanted_num, | ||
| size_t* translated_size, | ||
| size_t* item_num) | ||
| { | ||
| _translate(src, dest, src_size, wanted_num, translated_size, item_num, | ||
| RmtBit0, RmtBit1, RmtDurationReset); | ||
| } |
Contributor
There was a problem hiding this comment.
Add IRAM_ATTR to the function definition.
The IRAM_ATTR attribute is present on the declaration (line 37) but missing from the definition. For ESP-IDF, the attribute should be on the definition to ensure the function is placed in IRAM for interrupt-safe execution.
🔎 Proposed fix
-void NeoEsp32RmtSpeedTm1815::Translate(const void* src,
+void IRAM_ATTR NeoEsp32RmtSpeedTm1815::Translate(const void* src,
rmt_item32_t* dest,
size_t src_size,
size_t wanted_num,
size_t* translated_size,
size_t* item_num)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void NeoEsp32RmtSpeedTm1815::Translate(const void* src, | |
| rmt_item32_t* dest, | |
| size_t src_size, | |
| size_t wanted_num, | |
| size_t* translated_size, | |
| size_t* item_num) | |
| { | |
| _translate(src, dest, src_size, wanted_num, translated_size, item_num, | |
| RmtBit0, RmtBit1, RmtDurationReset); | |
| } | |
| void IRAM_ATTR NeoEsp32RmtSpeedTm1815::Translate(const void* src, | |
| rmt_item32_t* dest, | |
| size_t src_size, | |
| size_t wanted_num, | |
| size_t* translated_size, | |
| size_t* item_num) | |
| { | |
| _translate(src, dest, src_size, wanted_num, translated_size, item_num, | |
| RmtBit0, RmtBit1, RmtDurationReset); | |
| } |
🤖 Prompt for AI Agents
In wled00/bus_customNPBtiming.h around lines 45 to 54, the function definition
for NeoEsp32RmtSpeedTm1815::Translate is missing the IRAM_ATTR attribute present
on its declaration; update the definition to include IRAM_ATTR (i.e., add
IRAM_ATTR to the function signature so it matches the declaration) so the
routine is placed in IRAM for interrupt-safe execution, rebuild to confirm no
mismatched declaration/definition warnings.
Comment on lines
+75
to
+80
| class NeoWrgbTm1815Feature : | ||
| public Neo4ByteFeature<ColorIndexW, ColorIndexR, ColorIndexG, ColorIndexB>, | ||
| public NeoElementsTm1814Settings<ColorIndexW, ColorIndexR, ColorIndexG, ColorIndexB> | ||
| { | ||
| }; | ||
| #endif |
Contributor
There was a problem hiding this comment.
NeoWrgbTm1815Feature is only defined for ESP8266 but is needed for ESP32 as well.
The NeoWrgbTm1815Feature class is inside the #else // ESP8266 block, so it won't be compiled for ESP32. However, ESP32 also needs this feature class to instantiate complete TM1815 methods (combining method + feature in bus_wrapper.h). Move this class definition outside the platform-specific block.
🔎 Proposed fix
Move NeoWrgbTm1815Feature outside the #ifdef ARDUINO_ARCH_ESP32 ... #else ... #endif block so it's available for both platforms:
};
#endif
+class NeoWrgbTm1815Feature :
+ public Neo4ByteFeature<ColorIndexW, ColorIndexR, ColorIndexG, ColorIndexB>,
+ public NeoElementsTm1814Settings<ColorIndexW, ColorIndexR, ColorIndexG, ColorIndexB>
+{
+};
+
// I2S / DMA custom timing
class NeoBitsSpeedTm1815 : public NeoBitsSpeedBaseAnd remove the class from inside the ESP8266 block (lines 75-79).
🤖 Prompt for AI Agents
In wled00/bus_customNPBtiming.h around lines 75-80, the NeoWrgbTm1815Feature
class is currently defined only inside the ESP8266-specific branch which
prevents ESP32 from compiling it; move the NeoWrgbTm1815Feature class definition
out of the #ifdef ARDUINO_ARCH_ESP32 / #else / #endif block so it is declared
unconditionally for both platforms, and remove the duplicate definition from the
ESP8266 branch to avoid redeclaration.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds custom NPB timings and methods to support TM1815.
@willmmiles this also paves the way to add more custom timings. What do you think of this approach and the file structure I chose?
Summary by CodeRabbit
New Features
User Interface
Settings
Chores
✏️ Tip: You can customize this high-level summary in your review settings.