DMX improvements and configuration option. #5216
Open
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
…of fixtures configuration option to the DMX settings.
Contributor
WalkthroughAdds DMX fixture-count support (DMXNumFixtures) across config, UI, and runtime; restricts DMX output to that fixture range and adds per-frame rate/early-return guards; removes SparkFunDMX backend and introduces an ESP32 esp_dmx-based DMX output backend. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)wled00/**/*.cpp📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (22)📓 Common learnings📚 Learning: 2025-10-10T18:34:06.550ZApplied to files:
📚 Learning: 2025-11-16T19:40:46.260ZApplied to files:
📚 Learning: 2025-08-28T08:09:20.630ZApplied to files:
📚 Learning: 2025-09-01T10:26:17.959ZApplied to files:
📚 Learning: 2025-12-28T14:06:48.772ZApplied to files:
📚 Learning: 2025-11-14T13:37:11.994ZApplied to files:
📚 Learning: 2025-08-26T11:51:21.817ZApplied to files:
📚 Learning: 2025-11-14T13:37:30.955ZApplied to files:
📚 Learning: 2025-11-27T06:33:11.436ZApplied to files:
📚 Learning: 2026-01-13T03:07:13.582ZApplied to files:
📚 Learning: 2025-04-26T19:19:07.600ZApplied to files:
📚 Learning: 2025-06-15T09:59:52.720ZApplied to files:
📚 Learning: 2025-11-22T12:12:53.486ZApplied to files:
📚 Learning: 2025-04-28T20:51:29.773ZApplied to files:
📚 Learning: 2025-09-16T18:08:42.848ZApplied to files:
📚 Learning: 2026-01-01T13:18:14.738ZApplied to files:
📚 Learning: 2025-09-02T19:42:57.475ZApplied to files:
📚 Learning: 2025-08-31T03:38:14.114ZApplied to files:
📚 Learning: 2025-10-20T09:38:51.997ZApplied to files:
📚 Learning: 2026-01-03T16:42:26.035ZApplied to files:
📚 Learning: 2025-06-07T15:58:42.579ZApplied to files:
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting 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: 1
🧹 Nitpick comments (2)
wled00/data/settings_dmx.htm (1)
66-67: Consider using MAX_LEDS constant instead of hardcoded 1500.Line 66 uses
max="1500"for the DMX start LED input, but this should ideally reference theMAX_LEDSconstant to stay consistent with the configured LED limit.Based on learnings, magic numbers like this can be addressed in context.
Note: The
max="512"for number of fixtures is appropriate as it matches the DMX universe limit.wled00/dmx_output.cpp (1)
38-45: Remove redundant strip.getBrightness() call.Line 41 calls
strip.getBrightness()again, butbrightnesswas already assigned from this function at line 30. Use the cached variable instead:🔎 Proposed fix
// Change detection: only update if strip data has changed since last update static unsigned long lastStripShow = 0; unsigned long currentShow = strip.getLastShow(); - if (currentShow == lastStripShow && brightness == strip.getBrightness() && brightness != 0) { + if (currentShow == lastStripShow && brightness != 0) { return; }Note: The
brightness != 0check in the condition is also somewhat redundant since line 36 already returns whenbrightness == 0 && lastSentBrightness == 0, but keeping it doesn't hurt and makes the intent clearer.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
wled00/cfg.cpp(2 hunks)wled00/data/settings_dmx.htm(1 hunks)wled00/dmx_output.cpp(3 hunks)wled00/set.cpp(1 hunks)wled00/src/dependencies/dmx/SparkFunDMX.cpp(1 hunks)wled00/xml.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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_dmx.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_dmx.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_dmx.htm
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/set.cppwled00/xml.cppwled00/dmx_output.cppwled00/cfg.cppwled00/src/dependencies/dmx/SparkFunDMX.cpp
🧠 Learnings (15)
📓 Common learnings
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: 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: 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.
📚 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/data/settings_dmx.htmwled00/set.cppwled00/xml.cppwled00/dmx_output.cppwled00/cfg.cppwled00/src/dependencies/dmx/SparkFunDMX.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/data/settings_dmx.htmwled00/set.cppwled00/xml.cppwled00/dmx_output.cppwled00/cfg.cpp
📚 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_dmx.htm
📚 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/set.cppwled00/xml.cppwled00/dmx_output.cppwled00/cfg.cpp
📚 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/set.cppwled00/xml.cppwled00/dmx_output.cppwled00/cfg.cpp
📚 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/set.cppwled00/dmx_output.cppwled00/cfg.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/set.cppwled00/dmx_output.cppwled00/cfg.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/set.cpp
📚 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/set.cppwled00/dmx_output.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/set.cppwled00/dmx_output.cppwled00/cfg.cpp
📚 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/set.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Applied to files:
wled00/dmx_output.cpp
📚 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/dmx_output.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/dmx_output.cpp
🧬 Code graph analysis (1)
wled00/xml.cpp (1)
wled00/util.cpp (4)
printSetFormValue(114-116)printSetFormValue(114-114)printSetFormValue(121-123)printSetFormValue(121-121)
🔇 Additional comments (11)
wled00/xml.cpp (1)
639-639: LGTM!The NF field is correctly emitted alongside other DMX settings fields.
wled00/set.cpp (1)
643-646: LGTM!The NF parameter validation and assignment are correctly implemented with appropriate bounds checking (1-512, matching the DMX universe limit).
wled00/cfg.cpp (2)
737-737: LGTM!The deserialization of the
num-fixturesfield is correctly implemented using the CJSON macro pattern, consistent with other DMX configuration fields.
1239-1239: LGTM!The serialization of the
num-fixturesfield is correctly implemented and follows the same pattern as other DMX configuration fields.wled00/dmx_output.cpp (7)
15-15: LGTM!The DMX rate limit of 44 Hz correctly matches the DMX512 theoretical maximum update rate.
22-23: LGTM!Good defensive check to ensure segments exist before accessing strip data.
25-28: LGTM!Rate limiting implementation correctly limits DMX updates to the configured maximum rate.
30-36: LGTM!The brightness tracking correctly allows one final DMX update when brightness transitions to 0, then skips subsequent updates when the strip remains off.
56-62: LGTM!The optimization to process only the configured number of fixtures (DMXNumFixtures) instead of all LEDs is correctly implemented and will provide the performance improvement described in the PR objectives.
63-63: LGTM!The loop bounds are correctly updated to respect the configured fixture limit.
100-100: LGTM!Correctly updates the brightness tracking variable after sending DMX data.
1 task
Collaborator
|
looks good to me, except the rabbit's remark: the break signal should be low. |
netmindz
requested changes
Dec 21, 2025
Member
netmindz
left a comment
netmindz
left a comment
There was a problem hiding this comment.
DMX is meant to be a constant stream of data, not only on state change. Fixtures will consider it a failure state of they do not receive updates
Member
|
If we are doing anything with DMX output we should really be ditching the sparkfun implementation and using the esp_dmx as is used for input as it appears to be more complaint |
Author
Good point, bit of an oversight on my part. I've resolved it.
I've been testing a bit with the esp_dmx, and seem to get much better performance out of it. It seems to not influence the frame rate at all, which was the case with the sparkfun implementation. I will push my suggested changes soon However the library documentation explains that input and output concurrently is not possible. I don't see any code that would prevent anyone from doing so. Wouldn't it be best to prevent compilation with the DMX output + input flag defined. As it most certainly would cause issues, even before the switch over to esp_dmx for output? |
…erformance of the DMX output.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
wled00/src/dependencies/dmx/EspDmxOutput.cpp (3)
17-18: Document the rationale for defaultMax = 32.The magic number 32 for
defaultMaxwould benefit from a comment explaining why this specific default was chosen.🔎 Suggested comment addition
#define dmxMaxChannel 512 -#define defaultMax 32 +#define defaultMax 32 // Default channel count when initWrite receives invalid input
30-32: Verify RS485 transceiver requirements for rtsPin.The
rtsPinis set toDMX_PIN_NO_CHANGE, but most RS485 transceivers require direction control (DE/RE pins). Users with typical RS485 hardware will need to set this to a valid GPIO.Consider adding a more prominent note in comments or documentation about this requirement.
🔎 Suggested comment enhancement
static const int txPin = 2; // transmit DMX data over this pin (default is GPIO2) static const int rxPin = DMX_PIN_NO_CHANGE; // RX unused for DMX output -static const int rtsPin = DMX_PIN_NO_CHANGE; // RS485 DE/RE pin (UART RTS). Set to a GPIO to control transceiver direction. +static const int rtsPin = DMX_PIN_NO_CHANGE; // RS485 DE/RE pin (UART RTS). IMPORTANT: Set this to a valid GPIO for RS485 transceivers that require direction control (most do).
80-88: Clarify misleading comment about break signal.The comment at line 83 says "Always send the break signal first", but
dmx_write_slot(dmxPort, 0, DMX_SC)writes the start code to slot 0, not the break signal. The actual DMX break signal (timing mark) is generated internally bydmx_send()at line 87.🔎 Suggested comment correction
- // Always send the break signal first - dmx_write_slot(dmxPort, 0, DMX_SC); + // Ensure NULL start code is set before transmission + dmx_write_slot(dmxPort, 0, DMX_SC); - // Send the frame currently staged in the driver buffer. + // Send the frame (dmx_send generates break signal and transmits data) dmx_send(dmxPort, txSize);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
wled00/dmx_output.cpp(2 hunks)wled00/src/dependencies/dmx/EspDmxOutput.cpp(1 hunks)wled00/src/dependencies/dmx/EspDmxOutput.h(1 hunks)wled00/src/dependencies/dmx/SparkFunDMX.cpp(0 hunks)wled00/src/dependencies/dmx/SparkFunDMX.h(0 hunks)
💤 Files with no reviewable changes (2)
- wled00/src/dependencies/dmx/SparkFunDMX.h
- wled00/src/dependencies/dmx/SparkFunDMX.cpp
🧰 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/src/dependencies/dmx/EspDmxOutput.h
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/src/dependencies/dmx/EspDmxOutput.cppwled00/dmx_output.cpp
🧠 Learnings (13)
📓 Common learnings
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: 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: 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: 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.
📚 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/src/dependencies/dmx/EspDmxOutput.hwled00/src/dependencies/dmx/EspDmxOutput.cppwled00/dmx_output.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/src/dependencies/dmx/EspDmxOutput.cppwled00/dmx_output.cpp
📚 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/src/dependencies/dmx/EspDmxOutput.cppwled00/dmx_output.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/dmx_output.cpp
📚 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/dmx_output.cpp
📚 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/dmx_output.cpp
📚 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/dmx_output.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/dmx_output.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/dmx_output.cpp
📚 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/dmx_output.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/dmx_output.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/dmx_output.cpp
🧬 Code graph analysis (1)
wled00/src/dependencies/dmx/EspDmxOutput.h (1)
wled00/src/dependencies/dmx/EspDmxOutput.cpp (6)
initWrite(42-62)initWrite(42-42)write(64-78)write(64-64)update(80-88)update(80-80)
🔇 Additional comments (8)
wled00/src/dependencies/dmx/EspDmxOutput.h (1)
1-16: LGTM! Clean minimal interface.The header provides a clean, minimal interface for ESP32 DMX output. The three methods (initWrite, write, update) align with WLED's DMX API requirements and match the implementation in EspDmxOutput.cpp.
wled00/dmx_output.cpp (6)
6-10: LGTM! Documentation updated for new library.The comments correctly reference the new esp_dmx library and point to the appropriate source file for pin configuration on ESP32/S3.
15-16: LGTM! Rate limiting correctly implements 44 Hz DMX refresh.The rate limiting logic correctly implements a maximum 44 Hz update rate using ceiling division to ensure the limit is never exceeded. The timing is appropriate for DMX512 specification.
Also applies to: 25-28, 35-35
22-23: LGTM! Appropriate safety check.Checking for segment count prevents potential issues when accessing strip data with no configured segments.
46-47: LGTM! Appropriate guard against zero-length strip.This check prevents unnecessary processing when the strip has no LEDs configured.
48-53: Define a constant for DMX channel/fixture limit instead of hardcoding 512.The optimization correctly limits processing to configured fixtures with proper bounds checking. DMXNumFixtures is validated in
wled00/set.cpp(range 1-512). However, the hardcoded value512appears throughout the codebase (inset.cpp,dmx_output.cpp, anddmx_input.cpp). Define a constant likeDMX_MAX_FIXTURESor similar in a header file and use it consistently to improve maintainability and reduce risk of inconsistencies.⛔ Skipped due to learnings
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-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.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: 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: softhack007 Repo: wled/WLED PR: 0 File: :0-0 Timestamp: 2025-11-14T13:37:11.994Z Learning: In WLED code reviews, file operations (especially file.open()) should be checked to ensure they respect LittleFS filename limitations. The default LittleFS filename limit is 255 bytes (LFS_NAME_MAX). Reviews should assume default WLED configuration defines and not extreme edge-case values (e.g., WLED_MAX_SEGNAME_LEN = 512 would not be standard). File paths should be validated to stay within the 255-byte limit.Learnt from: softhack007 Repo: wled/WLED PR: 0 File: :0-0 Timestamp: 2025-11-14T13:37:30.955Z Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.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.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.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.
32-33: The brightness==0 check is a reasonable optimization with proper documentation.When the LED strip is off (brightness=0), skipping DMX writes avoids unnecessary bus traffic. The rate limiting (44Hz max) and shutter channel mechanism (if configured) already handle brightness control appropriately. This behavior is intentional and aligns with the recent performance improvements made in the DMX output refactor.
Likely an incorrect or invalid review comment.
wled00/src/dependencies/dmx/EspDmxOutput.cpp (1)
64-78: LGTM! Write method correctly handles bounds and size tracking.The write method properly validates inputs, tracks transmit size growth, and maintains both local and driver buffers. The clamping logic at lines 73-74 ensures size stays within all applicable bounds.
Note: Channel 0 writes are allowed but will be overwritten by
update()to enforce the NULL start code, as documented in the comment.
softhack007
reviewed
Jan 14, 2026
softhack007
reviewed
Jan 14, 2026
softhack007
reviewed
Jan 14, 2026
Updated comment to clarify rounding behavior for dmxFrameTime calculation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/dmx_output.cpp`:
- Line 27: The comment for the dmxFrameTime calculation is incorrect: the
expression "const unsigned long dmxFrameTime = (1000UL + MAX_DMX_RATE - 1) /
MAX_DMX_RATE;" uses addition of (MAX_DMX_RATE - 1) to perform ceiling division,
not subtraction; update the comment next to dmxFrameTime (and optionally mention
MAX_DMX_RATE) to explain that adding (MAX_DMX_RATE - 1) implements ceil(1000 /
MAX_DMX_RATE) (i.e., rounds up) rather than saying "Subtract 1 to round up."
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/dmx_output.cpp
🧰 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/dmx_output.cpp
🧠 Learnings (18)
📓 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: 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: 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: 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: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
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: 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: 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.
📚 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/dmx_output.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/dmx_output.cpp
📚 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/dmx_output.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/dmx_output.cpp
📚 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/dmx_output.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/dmx_output.cpp
📚 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/dmx_output.cpp
📚 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/dmx_output.cpp
📚 Learning: 2025-12-28T14:06:48.772Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
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/dmx_output.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/dmx_output.cpp
📚 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/dmx_output.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/dmx_output.cpp
📚 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/dmx_output.cpp
📚 Learning: 2025-04-28T20:51:29.773Z
Learnt from: KrX3D
Repo: wled/WLED PR: 4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:430-435
Timestamp: 2025-04-28T20:51:29.773Z
Learning: In WLED, `bri` is a global variable used for brightness control.
Applied to files:
wled00/dmx_output.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/dmx_output.cpp
📚 Learning: 2026-01-03T16:42:26.035Z
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T16:42:26.035Z
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/dmx_output.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/dmx_output.cpp
🔇 Additional comments (4)
wled00/dmx_output.cpp (4)
15-16: LGTM!Good addition of a named constant for the DMX rate limit. The 44 Hz value correctly reflects the DMX512 theoretical maximum frame rate.
22-23: LGTM!Good defensive check to ensure segments exist before accessing strip data.
30-35: LGTM!Good optimization to skip DMX output when brightness is zero. The placement of
last_dmx_timeupdate after the brightness check means DMX will be sent immediately when the strip turns back on, which is desirable behavior.
45-53: LGTM! Good optimization to limit DMX processing.This correctly limits DMX output to the configured fixture range rather than iterating all LEDs, addressing the performance issue described in the PR objectives.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Member
|
Looks like some of our development effort is overlapping @Stijn-Jacobs #5287 |
Author
|
@netmindz Yea we can't really merge them both, would you be willing to add your changes to my PR? The runtime pin change is a nice addition. I've already tested all of my changes with working hardware. Combined would be the best of both. I also think we should do something when the user enables both DMX input and output at the same time. Since this isn't supported by the underlying library. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
As already reported in my #5133 , I found out that the current DMX implementation was negatively impacting performance. It previously tried to push all of the led strips registered onto the DMX bus.
With this PR I've added a new option to configure how many DMX fixtures worth of output should be sent.
I've also added rate limiting to the DMX code, to make sure it is updated at 44hz (DMX theoretical maximum speed).
I've tested these changes on my own hardware, which has DMX output added to it.
Update:
I've changed the implementation from the old sparkfun one, to use esp_dmx instead. The sparkfun implementation created (and destroyed) 2 UART serials for each loop cycle which caused major performance issues. It seems to no longer have any negative impact at all on the frame rate, where previously I've seen about 20 frames per second, now >100 is possible. Also removed the legacy code from sparkfun implementation where it was reading DMX, this has since been moved.
Summary by CodeRabbit
New Features
Improvements
Removed
✏️ Tip: You can customize this high-level summary in your review settings.