Non-blocking, unified, dependency-less DMXOutput#5522
Non-blocking, unified, dependency-less DMXOutput#5522
Conversation
This increases clarity that WLED_ENABLE_DMX_INPUT is not a subset of WLED_ENABLE_DMX but independent.
…allocation. This is useful for boards with DMX.
This limits the number of used pixels to total to 512 channels used for transmitting via DMX according to the DMX start and spacing settings.
All DMX functions are handled on its own for ESP32 (remove SparkFunDMX dependency) features: * non-blocking DMX update function * maximum refresh rate setting * some more useful functions For ESP8266 stay with DMXESPSerial for now and wrap that in DMXOutput.
WalkthroughRefactors DMX output into a unified DMXOutput class, removes legacy platform-specific DMX libraries, adds configurable DMX output pin support in config/UI, updates initialization and E1.31 integration, and adjusts pin ownership and GPIO reservation logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Network as E1.31 / Client
participant Core as WLED Core
participant DMX as DMXOutput
participant HW as UART / GPIO
Network->>Core: receive E1.31 packet
Core->>DMX: writeBytes(channelStart, payload)
Core->>DMX: request update()
DMX->>DMX: rate-limit & check busy()
DMX->>HW: emit break (baud/format switch)
DMX->>HW: send channel buffer at DMX speed
DMX-->>Core: update() returns status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
added
enhancement
hardware
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
wled00/dmx_output.h (2)
30-98: Missing<Arduino.h>include and a fewconstcorrectness nits.This header uses
HardwareSerial*,uint8_t,uint16_twithout including anything. It works only because every current consumer transitively pulls inArduino.hviawled.hfirst (clang's standalone parse correctly reports the errors shown in the static-analysis hints). Add an explicit#include <Arduino.h>so the header is self-contained.Also, the query methods should be
constso they can be called on aconst DMXOutput&:🧹 Proposed tweaks
`#ifndef` DMX_OUTPUT_H `#define` DMX_OUTPUT_H + +#include <Arduino.h> ... - uint8_t read(uint16_t channel); + uint8_t read(uint16_t channel) const; ... - unsigned long getLastDmxOut(); + unsigned long getLastDmxOut() const; - bool busy(); + bool busy(); // non-const: delays on ESP8266, touches hw state - void setUpdateRate(uint8_t updateRate); - uint8_t getUpdateRate(); + void setUpdateRate(uint8_t updateRate); + uint8_t getUpdateRate() const; - int timeToNextUpdate(); + int timeToNextUpdate() const;As per coding guidelines: “Mark getter/query methods as
const”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.h` around lines 30 - 98, Add a self-contained Arduino include and make the query/getter methods const: add `#include` <Arduino.h> at the top of the DMXOutput header, and mark the query methods getLastDmxOut(), busy(), getUpdateRate(), timeToNextUpdate() (and any other non-mutating accessors like read()/readBytes() if intended to be non-mutating) as const in their declarations so they can be called on const DMXOutput instances; ensure the signatures in the class (DMXOutput::~DMXOutput, init, end, write, writeBytes, read, readBytes, update, handleDMXOutput, getLastDmxOut, busy, setUpdateRate, getUpdateRate, timeToNextUpdate) remain consistent with any corresponding definitions.
21-27: ParenthesizeDMX_CHANNELSand preferconstexprfor these header constants.
#define DMX_CHANNELS DMX_CHANNEL_TOP + 1is unparenthesized — any future use likeDMX_CHANNELS * Norsizeof(buf)/DMX_CHANNELSwould silently compute the wrong value. Current call sites happen to be safe, but this is a classic foot-gun.Additionally, per the coding guidelines, header-scope compile-time constants should be
constexprrather than#define, which also gives them proper types and namespacing.♻️ Suggested refactor
-#define DMX_CHANNEL_TOP 512 -#define DMX_CHANNELS DMX_CHANNEL_TOP + 1 - -#define DMXSPEED 250000 -#define DMXFORMAT SERIAL_8N2 -#define BREAKSPEED 83000 -#define BREAKFORMAT SERIAL_8N1 // unused, instead, DMXFORMAT is used +constexpr uint16_t DMX_CHANNEL_TOP = 512; +constexpr uint16_t DMX_CHANNELS = DMX_CHANNEL_TOP + 1; +constexpr uint32_t DMXSPEED = 250000; +constexpr uint32_t BREAKSPEED = 83000; +constexpr auto DMXFORMAT = SERIAL_8N2; +// BREAKFORMAT intentionally omitted — unused, DMXFORMAT is used for breaks too.As per coding guidelines: “Prefer
constexprover#definefor compile-time constants in C++”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.h` around lines 21 - 27, Replace the unparenthesized macro and other header `#defines` with typed constexpr variables and ensure the DMX_CHANNELS expression is evaluated correctly; specifically, change the macros DMX_CHANNEL_TOP and DMX_CHANNELS to constexpr (e.g., constexpr int DMX_CHANNEL_TOP = 512; constexpr int DMX_CHANNELS = DMX_CHANNEL_TOP + 1) so DMX_CHANNELS behaves correctly in arithmetic contexts, and similarly convert DMXSPEED, BREAKSPEED (use an appropriate integer type), and named constants like DMXFORMAT/BREAKFORMAT to constexpr or scoped constexpr integers/enum values as appropriate for the platform; keep the original names (DMX_CHANNEL_TOP, DMX_CHANNELS, DMXSPEED, DMXFORMAT, BREAKSPEED, BREAKFORMAT) so call sites remain valid.wled00/dmx_output.cpp (1)
11-50:init()initialization-state check and pin guard miss a couple of edge cases.Two small issues in
init():
- Line 14 uses
if(_uartNo > 0)to mean “already initialized”, but_uartNois only default-initialized to-1whenDMXOutputis a statically constructed global. GivendmxOutputis a global here that's fine, but a secondinit()call after a failed first call (where_uartNowas never assigned) is silently re-entered — consider asserting_dmxSerial == nullptror using>= 0with an explicit “initialized” flag, so the contract matches the doc comment.- Line 37
if(outputPin < 1) return false;is a no-op for the common “disabled” sentinel of-1becauseoutputPinisuint8_t, which makes it255(see thewled.hcomment ondmxOutputPin). Changing the parameter toint8_t(orint) makes this guard actually protect against the documented sentinel.Finally, per the coding guidelines the preprocessor
#errorat line 24–26 could be astatic_assert(SOC_UART_NUM > 1, "...")— but sinceSOC_UART_NUMis only defined via SOC caps,#erroris acceptable; flagging only for awareness.As per coding guidelines: “Use
static_assertover#if ...#error`` for compile-time assertions in C++”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.cpp` around lines 11 - 50, DMXOutput::init currently uses if(_uartNo > 0) to detect prior initialization and takes outputPin as uint8_t which breaks the documented -1 sentinel; change the init precondition to check a proper initialized flag or _uartNo >= 0 and/or assert _dmxSerial == nullptr (referencing _uartNo and _dmxSerial) so a failed prior init doesn't leave init re-entrant, and change the outputPin parameter from uint8_t to a signed type (int8_t or int) and update the guard from if(outputPin < 1) to correctly handle the -1 disabled sentinel before calling PinManager::allocatePin and pinMode/ digitalWrite; leave the SOC_UART_NUM `#error` as-is or replace with static_assert only if SOC_UART_NUM is a compile-time constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/cfg.cpp`:
- Around line 610-612: dmxOutputPin is deserialized as a signed int and then
passed to DMXOutput::init(uint8_t, ...) via CJSON(dmxOutputPin,
if_live_dmx[F("dmxOutputPin")]); validate the integer value before narrowing:
check that dmxOutputPin is within the valid range (>=1 and <= WLED_NUM_PINS) and
handle out-of-range values (e.g., ignore, set to 0/disabled, or log/error)
before casting to uint8_t; update the code path that calls DMXOutput::init (and
the if_live_dmx handling) to perform this range check and only pass a uint8_t
when the value is confirmed valid to avoid integer wrapping.
In `@wled00/data/settings_dmx.htm`:
- Line 57: The inline style on the help paragraph uses an invalid CSS property
`size: 0.8em`; update the <p> element (the help text paragraph referencing "For
pin settings, go to Sync settings.") to use `font-size: 0.8em` instead of `size:
0.8em` (keep the other style rules intact: padding-top, margin-top, font-style:
italic) so the smaller text is applied correctly.
In `@wled00/data/settings_sync.htm`:
- Around line 53-56: The hideNoDMXOutput function currently sets
gId("dmxOutput").style.display = "inline", which can collapse the div layout;
change it to set gId("dmxOutput").style.display = "block" (keep
gId("dmxOutputOff").style.display="none" as is) so the DMX output div
(gId("dmxOutput")) renders as a block containing its <h4> and form row; update
the display assignment in the hideNoDMXOutput function accordingly.
In `@wled00/dmx_output.cpp`:
- Around line 175-182: Validate and clamp DMX configuration before calculating
maxLen: ensure DMXGap != 0 (return/skip or set to 1 if invalid), ensure DMXStart
<= DMX_CHANNELS (or clamp DMXStart to DMX_CHANNELS), and assert DMXChannels >=
1; then compute maxLen using safe subtraction/cast (e.g., treat DMX_CHANNELS -
DMXStart as unsigned guarded value) so division by DMXGap and subtraction cannot
underflow or divide-by-zero; apply these checks around the block that computes
maxLen (references: DMXGap, DMXStart, DMX_CHANNELS, DMXChannels,
strip.getLengthTotal(), maxLen) and clamp len to the sanitized maxLen before
iterating.
- Around line 5-9: The code currently includes "uart.cpp" in dmx_output.cpp for
ESP8266 which will fail compile and can cause ODR/linker issues; instead remove
that include and add an external declaration for the required ESP8266 core
function (declare extern "C" int uart_tx_fifo_available(); or the correct
signature) so the compiler knows the symbol and the linker will resolve it from
the ESP8266 Arduino core; update the conditional branch around the include (the
`#ifdef` ESP8266 section) to declare the function prototype referencing
uart_tx_fifo_available() rather than including uart.cpp and verify the declared
signature matches the core header.
In `@wled00/e131.cpp`:
- Around line 117-121: The proxy currently always forwards from &e131_data[1],
which misaligns Art-Net (art_data starts at index 0) and can read past the
payload; update the proxy logic around e131ProxyUniverse / uni and
dmxOutput.writeBytes to pick the correct data pointer and length based on
protocol: use art_data (offset 0) when the packet is Art‑Net and e131_data+1
(offset 1) for E1.31, and clamp the byte count to the actual payload length
(min(dmxChannels, payloadLength - offset)) before calling dmxOutput.writeBytes
and dmxOutput.update. Ensure you reference the packet type/flags available in
the surrounding code to choose between art_data and e131_data and adjust the
offset accordingly.
In `@wled00/set.cpp`:
- Around line 477-479: When handling request->arg(F("IDMO")).toInt() for
dmxOutputPin (inside the WLED_ENABLE_DMX_OUTPUT block), avoid blindly saving 0
from toInt(): read the raw arg string first, if it's empty leave dmxOutputPin
unchanged, otherwise parse to an int and only assign it when parsed >= 1 and
passes runtime GPIO validation (use your existing runtime-valid pin check or the
same logic used by dmx_output.cpp which rejects outputPin < 1); for any
malformed/invalid value set dmxOutputPin = -1 (disabled) or keep prior -1, but
never store 0 as a valid pin.
In `@wled00/wled.cpp`:
- Around line 571-572: Move the guarded DMX pin reservation so the configured
DMX GPIO is allocated before any LED buses or usermods can claim it: call
dmxOutput.init(dmxOutputPin, 43) immediately after config load (right after
loading settings) and before beginStrip() and usermod setup, and remove the
later dmxOutput.init(...) call currently located after beginStrip(); reference
dmxOutput.init, dmxOutputPin and beginStrip to find and update the code.
In `@wled00/wled.h`:
- Line 43: The DMX feature flag was renamed from WLED_ENABLE_DMX to
WLED_ENABLE_DMX_OUTPUT and legacy uses will be silently ignored; add a migration
guard in wled.h before the DMX include/block that detects if WLED_ENABLE_DMX is
defined and emits a preprocessor warning or error (e.g., `#warning` or `#error`)
instructing users to rename it to WLED_ENABLE_DMX_OUTPUT, and optionally define
WLED_ENABLE_DMX_OUTPUT automatically for backward compatibility when the legacy
flag is present so existing platformio_override.ini/my_config.h/CI configs don’t
drop DMX silently.
- Around line 451-456: The DMX output code silently converts the sentinel
DMX_TXPIN_DEFAULT (-1) to 255 when calling DMXOutput::init, causing spurious
"pin 255" allocation errors; fix by (1) changing DMXOutput::init signature to
accept a signed type (int8_t or int) instead of uint8_t so the sentinel
round-trips and the existing outputPin < 1 guard behaves as intended (update
declaration in dmx_output.h and its implementation), and (2) in the caller
(where dmxOutput.init(dmxOutputPin, 43) is invoked) skip calling dmxOutput.init
entirely when dmxOutputPin equals DMX_TXPIN_DEFAULT (or < 0) to avoid attempting
init with an unset pin; finally, reconcile the default updateRate value between
the DMXOutput::init default (44) and the call site/PR (43) so the documented
default matches the actual used value.
---
Nitpick comments:
In `@wled00/dmx_output.cpp`:
- Around line 11-50: DMXOutput::init currently uses if(_uartNo > 0) to detect
prior initialization and takes outputPin as uint8_t which breaks the documented
-1 sentinel; change the init precondition to check a proper initialized flag or
_uartNo >= 0 and/or assert _dmxSerial == nullptr (referencing _uartNo and
_dmxSerial) so a failed prior init doesn't leave init re-entrant, and change the
outputPin parameter from uint8_t to a signed type (int8_t or int) and update the
guard from if(outputPin < 1) to correctly handle the -1 disabled sentinel before
calling PinManager::allocatePin and pinMode/ digitalWrite; leave the
SOC_UART_NUM `#error` as-is or replace with static_assert only if SOC_UART_NUM is
a compile-time constant.
In `@wled00/dmx_output.h`:
- Around line 30-98: Add a self-contained Arduino include and make the
query/getter methods const: add `#include` <Arduino.h> at the top of the DMXOutput
header, and mark the query methods getLastDmxOut(), busy(), getUpdateRate(),
timeToNextUpdate() (and any other non-mutating accessors like read()/readBytes()
if intended to be non-mutating) as const in their declarations so they can be
called on const DMXOutput instances; ensure the signatures in the class
(DMXOutput::~DMXOutput, init, end, write, writeBytes, read, readBytes, update,
handleDMXOutput, getLastDmxOut, busy, setUpdateRate, getUpdateRate,
timeToNextUpdate) remain consistent with any corresponding definitions.
- Around line 21-27: Replace the unparenthesized macro and other header `#defines`
with typed constexpr variables and ensure the DMX_CHANNELS expression is
evaluated correctly; specifically, change the macros DMX_CHANNEL_TOP and
DMX_CHANNELS to constexpr (e.g., constexpr int DMX_CHANNEL_TOP = 512; constexpr
int DMX_CHANNELS = DMX_CHANNEL_TOP + 1) so DMX_CHANNELS behaves correctly in
arithmetic contexts, and similarly convert DMXSPEED, BREAKSPEED (use an
appropriate integer type), and named constants like DMXFORMAT/BREAKFORMAT to
constexpr or scoped constexpr integers/enum values as appropriate for the
platform; keep the original names (DMX_CHANNEL_TOP, DMX_CHANNELS, DMXSPEED,
DMXFORMAT, BREAKSPEED, BREAKFORMAT) so call sites remain valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 931d3154-cb77-4ca0-b2d7-48b7b035e84f
📒 Files selected for processing (22)
platformio_override.sample.initools/cdata.jswled00/cfg.cppwled00/const.hwled00/data/settings_dmx.htmwled00/data/settings_sync.htmwled00/dmx_output.cppwled00/dmx_output.hwled00/e131.cppwled00/fcn_declare.hwled00/pin_manager.cppwled00/pin_manager.hwled00/set.cppwled00/src/dependencies/dmx/ESPDMX.cppwled00/src/dependencies/dmx/ESPDMX.hwled00/src/dependencies/dmx/LICENSE.mdwled00/src/dependencies/dmx/SparkFunDMX.cppwled00/src/dependencies/dmx/SparkFunDMX.hwled00/wled.cppwled00/wled.hwled00/wled_server.cppwled00/xml.cpp
💤 Files with no reviewable changes (6)
- wled00/src/dependencies/dmx/LICENSE.md
- wled00/fcn_declare.h
- wled00/src/dependencies/dmx/ESPDMX.h
- wled00/src/dependencies/dmx/ESPDMX.cpp
- wled00/src/dependencies/dmx/SparkFunDMX.h
- wled00/src/dependencies/dmx/SparkFunDMX.cpp
| #ifdef WLED_ENABLE_DMX_OUTPUT | ||
| // does not act on out-of-order packets yet | ||
| if (e131ProxyUniverse > 0 && uni == e131ProxyUniverse) { | ||
| for (uint16_t i = 1; i <= dmxChannels; i++) | ||
| dmx.write(i, e131_data[i]); | ||
| dmx.update(); | ||
| dmxOutput.writeBytes(1, &e131_data[1], dmxChannels); | ||
| dmxOutput.update(); |
There was a problem hiding this comment.
Use the protocol-specific DMX data offset before proxying.
Art-Net payloads start at art_data[0], while E1.31 property values reserve index 0 for the start code. The unconditional &e131_data[1] shifts Art-Net output by one channel and reads past the payload for full 512-channel packets.
🐛 Proposed fix
`#ifdef` WLED_ENABLE_DMX_OUTPUT
// does not act on out-of-order packets yet
if (e131ProxyUniverse > 0 && uni == e131ProxyUniverse) {
- dmxOutput.writeBytes(1, &e131_data[1], dmxChannels);
+ const uint16_t dmxDataOffset = (mde == REALTIME_MODE_ARTNET) ? 0 : 1;
+ dmxOutput.writeBytes(1, &e131_data[dmxDataOffset], dmxChannels);
dmxOutput.update();
}
`#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/e131.cpp` around lines 117 - 121, The proxy currently always forwards
from &e131_data[1], which misaligns Art-Net (art_data starts at index 0) and can
read past the payload; update the proxy logic around e131ProxyUniverse / uni and
dmxOutput.writeBytes to pick the correct data pointer and length based on
protocol: use art_data (offset 0) when the packet is Art‑Net and e131_data+1
(offset 1) for E1.31, and clamp the byte count to the actual payload length
(min(dmxChannels, payloadLength - offset)) before calling dmxOutput.writeBytes
and dmxOutput.update. Ensure you reference the packet type/flags available in
the surrounding code to choose between art_data and e131_data and adjust the
offset accordingly.
| #ifdef WLED_ENABLE_DMX_OUTPUT | ||
| dmxOutput.init(dmxOutputPin, 43); |
There was a problem hiding this comment.
Reserve the configured DMX pin before buses and usermods claim it.
dmxOutput.init() is the first actual PinManager allocation for dmxOutputPin, but it runs after beginStrip() and usermod setup. A default LED bus or usermod can claim the configured DMX GPIO first, causing DMX output init to fail.
🔧 Proposed fix
Move the guarded init to immediately after config load and before beginStrip():
DEBUG_PRINTLN(F("Reading config"));
bool needsCfgSave = deserializeConfigFromFS();
DEBUG_PRINTF_P(PSTR("heap %u\n"), getFreeHeapSize());
+
+#ifdef WLED_ENABLE_DMX_OUTPUT
+ if (dmxOutputPin > 0 && dmxOutputPin < (int)WLED_NUM_PINS) dmxOutput.init(static_cast<uint8_t>(dmxOutputPin), 43);
+#endif
`#if` defined(STATUSLED) && STATUSLED>=0Then remove the later init:
-#ifdef WLED_ENABLE_DMX_OUTPUT
- dmxOutput.init(dmxOutputPin, 43);
-#endif
`#ifdef` WLED_ENABLE_DMX_INPUT
dmxInput.init(dmxInputReceivePin, dmxInputTransmitPin, dmxInputEnablePin, dmxInputPort);
`#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/wled.cpp` around lines 571 - 572, Move the guarded DMX pin reservation
so the configured DMX GPIO is allocated before any LED buses or usermods can
claim it: call dmxOutput.init(dmxOutputPin, 43) immediately after config load
(right after loading settings) and before beginStrip() and usermod setup, and
remove the later dmxOutput.init(...) call currently located after beginStrip();
reference dmxOutput.init, dmxOutputPin and beginStrip to find and update the
code.
| uartNo = SOC_UART_NUM - 1; // use last UART as default | ||
| } | ||
| if(uartNo == 0) { | ||
| DEBUG_PRINTF_P(PSTR("DMXOutput: Error: Cannot run on chips with <=1 hardware UART, or with UART0.")); |
There was a problem hiding this comment.
why is this restriction necessary?
There was a problem hiding this comment.
Good question. I'm inclined to say I only updated some module here and these restrictions applied before too, even more restrictive. SparkFunDMX would throw an error if HardwareSerial(2) didn't exist.
This restriction was eased by netmindz in his commit to allow 2-UART SoCs.
The problem IMO is that hardware resources don't have any means to be "reserved/dedicated" to a specific functionality, like PinManager does for pins. I don't have much clue what modules and built-in functionality exists, so what conflicts can or will arise. All I can say is Serial0 would probably be a bad choice, as at least in the beginning we have some debug output there, even if WLED_DEBUG is disabled. If this is enabled, then it gets ofc so much worse.
And even if hw is only used in an ordered, sequential manner, you'd need to pay attention to fully reset all registers, as depending on the HAL, you'd assume reset condition when initializing, thus don't reset bits that were set by other users before.
There was a problem hiding this comment.
Fair point. We really need a resource manager, there is a growing list of features that have hardware resource conflicts. This is something that came up in discussions several times, it is only a question of when and how.
|
I'll try and have a proper look through, but a quick bit of feedback Please don't rename the define for the output feature, it's just an unnecessary breaking change. Now we are on V4 for all builds I'll probably be removing the DMX_INPUT as it's supposed on all esp32s and has minimal size impact |
netmindz
added
the
connectivity
|
Thanks for the PR @Mdbelen , can you clarify which MCUs you have tested with and have you used a DMX tester or logic analyzer to validate the output of your new driver? |
…TPUT" This reverts commit bc80c4a. It does keep the PinManager name change from DMX to DMX_OUTPUT!
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
wled00/e131.cpp (1)
119-121:⚠️ Potential issue | 🔴 CriticalUse the protocol-specific payload offset when proxying DMX.
Art-Net payload starts at
art_data[0], while E1.31 reservesproperty_values[0]for the start code. The unconditional&e131_data[1]shifts every Art-Net channel by one and reads past the payload on full 512-channel packets.🐛 Suggested fix
if (e131ProxyUniverse > 0 && uni == e131ProxyUniverse) { - dmxOutput.writeBytes(1, &e131_data[1], dmxChannels); + const uint16_t dmxDataOffset = (mde == REALTIME_MODE_ARTNET) ? 0 : 1; + dmxOutput.writeBytes(1, &e131_data[dmxDataOffset], dmxChannels); dmxOutput.update(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/e131.cpp` around lines 119 - 121, The proxying currently always uses the E1.31 payload offset (&e131_data[1]) which misaligns Art‑Net packets (art_data starts at art_data[0]) and can read past the buffer on 512‑channel frames; update the proxy logic in the block checking e131ProxyUniverse/uni to choose the correct pointer and offset based on the source protocol (use art_data for Art‑Net starting at index 0, and e131_data starting at index 1 for E1.31) when calling dmxOutput.writeBytes and ensure dmxChannels bounds are respected; locate the branch around e131ProxyUniverse, uni, dmxOutput.writeBytes and adjust to pass the protocol‑appropriate buffer pointer and length.wled00/set.cpp (1)
477-479:⚠️ Potential issue | 🟠 MajorApply and validate the DMX TX pin change instead of only storing
toInt().
toInt()turns empty/malformed input into0, so this can silently lose the-1disabled sentinel. More importantly, the live driver is only initialized once inWLED::setup()(wled00/wled.cpp), so saving a new pin here leaves DMX output bound to the old GPIO until reboot. Please validate the submitted value and either reinitializedmxOutputhere or force a reboot/apply step when the pin changes.🔧 Suggested fix
`#ifdef` WLED_ENABLE_DMX - dmxOutputPin = request->arg(F("IDMO")).toInt(); + if (request->hasArg(F("IDMO"))) { + const int newPin = request->arg(F("IDMO")).toInt(); + if (newPin >= -1 && newPin < WLED_NUM_PINS && newPin != dmxOutputPin) { + dmxOutputPin = newPin; + doReboot = true; // or rebind dmxOutput here + } + } `#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/set.cpp` around lines 477 - 479, The code assigns dmxOutputPin = request->arg(F("IDMO")).toInt() which silently converts empty/malformed input to 0 and loses the -1 sentinel, and it only updates the stored value without reinitializing the DMX driver (initialized in WLED::setup()), so changes don't take effect until reboot; fix by parsing and validating the raw string from request->arg(F("IDMO")) (preserve -1 as a valid "disabled" value and reject non-numeric input), update the stored dmxOutputPin only if the parsed value is valid, and when the pin actually changes either reinitialize the DMX output driver immediately (call the driver init/reset function used in WLED::setup() — reference the same DMX init routine) or mark the config as needing an apply/reboot and return a response instructing the user to reboot/apply.wled00/wled.cpp (1)
571-572:⚠️ Potential issue | 🟠 MajorReserve the configured DMX GPIO before buses and usermods claim it.
dmxOutput.init()is the firstPinManagerallocation for the TX pin, but it runs afterbeginStrip()and usermod setup. If either path grabs the configured GPIO first, DMX output init fails on boot even though the setting is valid.🔧 Suggested fix
DEBUG_PRINTLN(F("Reading config")); bool needsCfgSave = deserializeConfigFromFS(); DEBUG_PRINTF_P(PSTR("heap %u\n"), getFreeHeapSize()); + +#ifdef WLED_ENABLE_DMX + dmxOutput.init(dmxOutputPin, 43); +#endif `#if` defined(STATUSLED) && STATUSLED>=0 if (!PinManager::isPinAllocated(STATUSLED)) {-#ifdef WLED_ENABLE_DMX - dmxOutput.init(dmxOutputPin, 43); -#endif `#ifdef` WLED_ENABLE_DMX_INPUT dmxInput.init(dmxInputReceivePin, dmxInputTransmitPin, dmxInputEnablePin, dmxInputPort); `#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/wled.cpp` around lines 571 - 572, The DMX TX pin is being allocated too late (dmxOutput.init is called after beginStrip() and usermod setup), so other buses/usermods can claim it first; move the reservation earlier by reserving the configured DMX GPIO with the PinManager before any bus or usermod claims it (either call dmxOutput.init(dmxOutputPin, 43) before beginStrip()/usermod setup or explicitly call the PinManager reserve API for the TX pin—e.g., pinManager.reservePin(dmxOutputPin, /*owner*/ DMX_TX) —so the pin is locked for DMX use prior to beginStrip() and usermod initialization).
🧹 Nitpick comments (4)
wled00/dmx_output.cpp (2)
157-167: Consider integer math fortimeToNextUpdate().The float division + conversion + ceil-rounding can be expressed as pure integer math (
(1000 + _updateRate - 1) / _updateRate) which avoids pulling the softfloat path on ESP8266 and is simpler to reason about. This function is called on every loop iteration.♻️ Proposed refactor
- // treat _updateRate as maximum, so round up the refresh delay - float fdmxFrameTime = 1000.0 / _updateRate; - int dmxFrameTime = (uint16_t)fdmxFrameTime; - if(fdmxFrameTime - dmxFrameTime > 0.0) dmxFrameTime += 1; // if fractional part > 0, add one - - return dmxFrameTime - (millis() - _lastDmxOutMillis); + // treat _updateRate as maximum, so round up the refresh delay + const int dmxFrameTime = (1000 + _updateRate - 1) / _updateRate; + return dmxFrameTime - (int)(millis() - _lastDmxOutMillis);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.cpp` around lines 157 - 167, The timeToNextUpdate() function uses float math to compute frame delay which pulls in softfloat on ESP8266 and is inefficient; replace the float/ceil logic with integer arithmetic by computing dmxFrameTime = (1000 + _updateRate - 1) / _updateRate (handle _updateRate==0 earlier as already done), then return dmxFrameTime - (millis() - _lastDmxOutMillis) while keeping the existing early checks for _uartNo < 0 and _updateRate == 0; update only DMXOutput::timeToNextUpdate() and keep references to _updateRate, _lastDmxOutMillis, _uartNo and millis() intact.
5-10:uart.hinclude no longer needed.After inlining the FIFO check with
USS()/USTXC(line 131), onlyesp8266_peri.his actually used from these two headers;uart.hcan be dropped from the ESP8266 branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.cpp` around lines 5 - 10, Remove the now-unneeded uart.h include from the ESP8266 branch in dmx_output.cpp: keep only `#include` "esp8266_peri.h" under the `#ifdef` ESP8266 conditional since the FIFO check was inlined using USS()/USTXC; delete the `#include` "uart.h" line and ensure the conditional block still correctly selects esp8266_peri.h for ESP8266 and hal/uart_ll.h for other platforms.wled00/dmx_output.h (2)
53-92: Mark getter/query methodsconst.
read,readBytes,getLastDmxOut,getUpdateRate,busy, andtimeToNextUpdatedon't mutate instance state and should beconst, matching the project guideline and making them callable from const contexts. (readBytescurrently takes a non-const out-buffer — that's fine, only the method itself needs const qualification.)♻️ Proposed fix
- uint8_t read(uint16_t channel); + uint8_t read(uint16_t channel) const; ... - bool readBytes(uint16_t channelStart, uint8_t values[], uint16_t len); + bool readBytes(uint16_t channelStart, uint8_t values[], uint16_t len) const; ... - unsigned long getLastDmxOut(); + unsigned long getLastDmxOut() const; ... - bool busy(); + bool busy() const; ... - uint8_t getUpdateRate(); + uint8_t getUpdateRate() const; ... - int timeToNextUpdate(); + int timeToNextUpdate() const;(Remember to mirror the
conston the corresponding definitions indmx_output.cpp.)As per coding guidelines: "Mark getter/query methods as
constand usestaticfor methods not accessing instance state".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.h` around lines 53 - 92, Several accessor/query methods in the DMX output class are not marked const; change the declarations of read(uint16_t channel), readBytes(uint16_t channelStart, uint8_t values[], uint16_t len), getLastDmxOut(), getUpdateRate(), busy(), and timeToNextUpdate() to be const methods (e.g., uint8_t read(uint16_t channel) const), and update the matching definitions in dmx_output.cpp to include the same const qualifier so they are callable from const contexts and adhere to the project guideline.
24-30: Preferconstexprover#definefor these compile-time constants.
DMX_CHANNEL_TOP,DMX_CHANNELS,DMXSPEED,BREAKSPEEDare plain integer constants and don't need preprocessor scoping.DMXFORMAT/BREAKFORMATresolve toSERIAL_8N2/SERIAL_8N1which areuint32_tenum-ish values — also representable asconstexpr. This gives them type safety and keeps them out of the global macro namespace (whereDMX_CHANNELSrisks collision with theDMXChannelsglobal variable used inhandleDMXOutput()).♻️ Proposed refactor
-#define DMX_CHANNEL_TOP 512 -#define DMX_CHANNELS (DMX_CHANNEL_TOP + 1) - -#define DMXSPEED 250000 -#define DMXFORMAT SERIAL_8N2 -#define BREAKSPEED 83000 -#define BREAKFORMAT SERIAL_8N1 // unused, instead, DMXFORMAT is used +constexpr uint16_t DMX_CHANNEL_TOP = 512; +constexpr uint16_t DMX_CHANNELS = DMX_CHANNEL_TOP + 1; +constexpr uint32_t DMXSPEED = 250000; +constexpr uint32_t BREAKSPEED = 83000; +constexpr auto DMXFORMAT = SERIAL_8N2; +constexpr auto BREAKFORMAT = SERIAL_8N1; // unused, instead, DMXFORMAT is usedAs per coding guidelines: "Prefer
constexprover#definefor compile-time constants in C++".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.h` around lines 24 - 30, Replace the preprocessor macros with typed constexpr variables: change DMX_CHANNEL_TOP, DMX_CHANNELS, DMXSPEED, BREAKSPEED to constexpr integers (e.g., int or std::size_t where appropriate) and change DMXFORMAT and BREAKFORMAT to constexpr uint32_t (or the exact enum type that SERIAL_8N2/SERIAL_8N1 resolve to); update any includes if needed (e.g., <cstdint>) and ensure references such as DMXChannels in handleDMXOutput() still compile (avoiding macro collisions) by removing the `#define` lines and declaring the new constexpr symbols with the same names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/dmx_output.cpp`:
- Line 131: The declaration of uart_tx_fifo_available (using USS(_uartNo) >>
USTXC) is missing its terminating semicolon which causes compilation failure;
update the statement in dmx_output.cpp where uart_tx_fifo_available is defined
(referencing the symbols uart_tx_fifo_available, USS, USTXC, _uartNo) by adding
the missing semicolon to the end of the line so the local variable declaration
is properly terminated.
- Around line 63-72: The end() method in DMXOutput leaks the TX pin because
init() allocates it via PinManager::allocatePin(_outputPin, true,
PinOwner::DMX_OUTPUT) but end() never releases it; modify DMXOutput::end() to
call PinManager::deallocatePin(_outputPin, PinOwner::DMX_OUTPUT) (or the
appropriate deallocation API) before clearing _outputPin/_uartNo and deleting
_dmxSerial, and also ensure any failure paths in DMXOutput::init() deallocate
the pin if allocation succeeded but initialization failed so the pin is not
permanently owned by DMX_OUTPUT.
In `@wled00/dmx_output.h`:
- Around line 94-101: The member _updateRate of class DMXOutput is left
uninitialized which can cause undefined behavior if accessed before init(); set
a safe in-class initializer for _updateRate (for example 0) in the DMXOutput
declaration so any global/default-constructed DMXOutput has a well-defined
value; update the member declaration for _updateRate and ensure callers such as
timeToNextUpdate() and init() rely on that initialized value.
---
Duplicate comments:
In `@wled00/e131.cpp`:
- Around line 119-121: The proxying currently always uses the E1.31 payload
offset (&e131_data[1]) which misaligns Art‑Net packets (art_data starts at
art_data[0]) and can read past the buffer on 512‑channel frames; update the
proxy logic in the block checking e131ProxyUniverse/uni to choose the correct
pointer and offset based on the source protocol (use art_data for Art‑Net
starting at index 0, and e131_data starting at index 1 for E1.31) when calling
dmxOutput.writeBytes and ensure dmxChannels bounds are respected; locate the
branch around e131ProxyUniverse, uni, dmxOutput.writeBytes and adjust to pass
the protocol‑appropriate buffer pointer and length.
In `@wled00/set.cpp`:
- Around line 477-479: The code assigns dmxOutputPin =
request->arg(F("IDMO")).toInt() which silently converts empty/malformed input to
0 and loses the -1 sentinel, and it only updates the stored value without
reinitializing the DMX driver (initialized in WLED::setup()), so changes don't
take effect until reboot; fix by parsing and validating the raw string from
request->arg(F("IDMO")) (preserve -1 as a valid "disabled" value and reject
non-numeric input), update the stored dmxOutputPin only if the parsed value is
valid, and when the pin actually changes either reinitialize the DMX output
driver immediately (call the driver init/reset function used in WLED::setup() —
reference the same DMX init routine) or mark the config as needing an
apply/reboot and return a response instructing the user to reboot/apply.
In `@wled00/wled.cpp`:
- Around line 571-572: The DMX TX pin is being allocated too late
(dmxOutput.init is called after beginStrip() and usermod setup), so other
buses/usermods can claim it first; move the reservation earlier by reserving the
configured DMX GPIO with the PinManager before any bus or usermod claims it
(either call dmxOutput.init(dmxOutputPin, 43) before beginStrip()/usermod setup
or explicitly call the PinManager reserve API for the TX pin—e.g.,
pinManager.reservePin(dmxOutputPin, /*owner*/ DMX_TX) —so the pin is locked for
DMX use prior to beginStrip() and usermod initialization).
---
Nitpick comments:
In `@wled00/dmx_output.cpp`:
- Around line 157-167: The timeToNextUpdate() function uses float math to
compute frame delay which pulls in softfloat on ESP8266 and is inefficient;
replace the float/ceil logic with integer arithmetic by computing dmxFrameTime =
(1000 + _updateRate - 1) / _updateRate (handle _updateRate==0 earlier as already
done), then return dmxFrameTime - (millis() - _lastDmxOutMillis) while keeping
the existing early checks for _uartNo < 0 and _updateRate == 0; update only
DMXOutput::timeToNextUpdate() and keep references to _updateRate,
_lastDmxOutMillis, _uartNo and millis() intact.
- Around line 5-10: Remove the now-unneeded uart.h include from the ESP8266
branch in dmx_output.cpp: keep only `#include` "esp8266_peri.h" under the `#ifdef`
ESP8266 conditional since the FIFO check was inlined using USS()/USTXC; delete
the `#include` "uart.h" line and ensure the conditional block still correctly
selects esp8266_peri.h for ESP8266 and hal/uart_ll.h for other platforms.
In `@wled00/dmx_output.h`:
- Around line 53-92: Several accessor/query methods in the DMX output class are
not marked const; change the declarations of read(uint16_t channel),
readBytes(uint16_t channelStart, uint8_t values[], uint16_t len),
getLastDmxOut(), getUpdateRate(), busy(), and timeToNextUpdate() to be const
methods (e.g., uint8_t read(uint16_t channel) const), and update the matching
definitions in dmx_output.cpp to include the same const qualifier so they are
callable from const contexts and adhere to the project guideline.
- Around line 24-30: Replace the preprocessor macros with typed constexpr
variables: change DMX_CHANNEL_TOP, DMX_CHANNELS, DMXSPEED, BREAKSPEED to
constexpr integers (e.g., int or std::size_t where appropriate) and change
DMXFORMAT and BREAKFORMAT to constexpr uint32_t (or the exact enum type that
SERIAL_8N2/SERIAL_8N1 resolve to); update any includes if needed (e.g.,
<cstdint>) and ensure references such as DMXChannels in handleDMXOutput() still
compile (avoiding macro collisions) by removing the `#define` lines and declaring
the new constexpr symbols with the same names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 86e36cfb-18e2-4b1e-b3d5-f9e4693dc53f
📒 Files selected for processing (11)
wled00/cfg.cppwled00/data/settings_dmx.htmwled00/data/settings_sync.htmwled00/dmx_input.cppwled00/dmx_output.cppwled00/dmx_output.hwled00/e131.cppwled00/set.cppwled00/wled.cppwled00/wled.hwled00/xml.cpp
✅ Files skipped from review due to trivial changes (2)
- wled00/data/settings_dmx.htm
- wled00/cfg.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/data/settings_sync.htm
| private: | ||
| HardwareSerial* _dmxSerial; | ||
| uint8_t _outputPin; // DMX TX pin | ||
| uint8_t _dmxData[DMX_CHANNELS] = {0}; | ||
| int8_t _uartNo = -1; | ||
| uint8_t _updateRate; | ||
| unsigned long _lastDmxOutMillis = 0; | ||
| }; |
There was a problem hiding this comment.
Initialize _updateRate in the member declaration.
_updateRate has no in-class initializer, so a default-constructed global DMXOutput dmxOutput leaves it indeterminate until init() is called. timeToNextUpdate() guards via _uartNo < 0 today, but any future call path that reads _updateRate before init() would hit UB. Trivial to make watertight:
♻️ Proposed fix
int8_t _uartNo = -1;
- uint8_t _updateRate;
+ uint8_t _updateRate = 43;
unsigned long _lastDmxOutMillis = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wled00/dmx_output.h` around lines 94 - 101, The member _updateRate of class
DMXOutput is left uninitialized which can cause undefined behavior if accessed
before init(); set a safe in-class initializer for _updateRate (for example 0)
in the DMXOutput declaration so any global/default-constructed DMXOutput has a
well-defined value; update the member declaration for _updateRate and ensure
callers such as timeToNextUpdate() and init() rely on that initialized value.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
wled00/dmx_output.cpp (2)
159-169: Optional: integer-only ceiling math intimeToNextUpdate().
fdmxFrameTime/ cast / fractional check can be replaced with a single integer ceiling expression, avoiding the FP divide/subtract on every call (this is invoked from the hot service path viaupdate()).♻️ Proposed refactor
- // treat _updateRate as maximum, so round up the refresh delay - float fdmxFrameTime = 1000.0 / _updateRate; - int dmxFrameTime = (uint16_t)fdmxFrameTime; - if(fdmxFrameTime - dmxFrameTime > 0.0) dmxFrameTime += 1; // if fractional part > 0, add one - - return dmxFrameTime - (millis() - _lastDmxOutMillis); + // treat _updateRate as maximum, so round up the refresh delay + const int dmxFrameTime = (1000 + _updateRate - 1) / _updateRate; // integer ceil(1000 / _updateRate) + return dmxFrameTime - (int)(millis() - _lastDmxOutMillis);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.cpp` around lines 159 - 169, Replace the floating-point ceiling logic in DMXOutput::timeToNextUpdate with integer-only math: compute the frame period as an integer ceiling using (_updateRate) and integer arithmetic (e.g., (1000 + _updateRate - 1) / _updateRate) instead of using fdmxFrameTime, casting and fractional checks; keep the same early returns for _uartNo and _updateRate and return the same expression dmxFrameTime - (millis() - _lastDmxOutMillis) using the new integer dmxFrameTime variable so the hot path avoids FP divide/subtract.
148-157: Mark pure getters asconst.
getLastDmxOut()andgetUpdateRate()don't modify instance state and should beconst; same applies totimeToNextUpdate()(Line 159) — it only reads_uartNo,_updateRate,_lastDmxOutMillis. The header declaration will need to match.-unsigned long DMXOutput::getLastDmxOut() { +unsigned long DMXOutput::getLastDmxOut() const { return _lastDmxOutMillis; } ... -uint8_t DMXOutput::getUpdateRate() { +uint8_t DMXOutput::getUpdateRate() const { return _updateRate; }As per coding guidelines: "Mark getter/query methods as
constand usestaticfor methods not accessing instance state".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wled00/dmx_output.cpp` around lines 148 - 157, The getters getLastDmxOut(), getUpdateRate() and the method timeToNextUpdate() are pure queries and should be marked const; update their definitions in the .cpp (DMXOutput::getLastDmxOut(), DMXOutput::getUpdateRate(), DMXOutput::timeToNextUpdate()) to end with const and make the matching declarations in the header const as well, and if you find any method that does not access instance state at all consider marking it static; ensure signatures in both header and implementation match to fix const-correctness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@wled00/dmx_output.cpp`:
- Around line 159-169: Replace the floating-point ceiling logic in
DMXOutput::timeToNextUpdate with integer-only math: compute the frame period as
an integer ceiling using (_updateRate) and integer arithmetic (e.g., (1000 +
_updateRate - 1) / _updateRate) instead of using fdmxFrameTime, casting and
fractional checks; keep the same early returns for _uartNo and _updateRate and
return the same expression dmxFrameTime - (millis() - _lastDmxOutMillis) using
the new integer dmxFrameTime variable so the hot path avoids FP divide/subtract.
- Around line 148-157: The getters getLastDmxOut(), getUpdateRate() and the
method timeToNextUpdate() are pure queries and should be marked const; update
their definitions in the .cpp (DMXOutput::getLastDmxOut(),
DMXOutput::getUpdateRate(), DMXOutput::timeToNextUpdate()) to end with const and
make the matching declarations in the header const as well, and if you find any
method that does not access instance state at all consider marking it static;
ensure signatures in both header and implementation match to fix
const-correctness.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dcc131bf-6314-4747-87d7-5da36ca2d87b
📒 Files selected for processing (1)
wled00/dmx_output.cpp
|
I've changed back to WLED_ENABLE_DMX without _OUTPUT as requested. Note that I kept the DMX_OUTPUT naming with PinManager. Hope that's okay. |
|
As this moves away from any library (which I am not at all opposed to), it requires thorough testing on all supported platforms. |
|
True in general. As I said in the initial message, the principle is the same between SparkFunDMX, DMXESPOutput (used for ESP8266) and still very similar to this module. The difference in mine is that i set Maybe if I find time, I'll add more UI improvements to allow the TX pin to be changed without rebooting. And add an input field to set the DMXOutput serial port (like with DMXInput) and update rate. |
In my experience each and every ESP can act differently as the hardware implementations are not the same and they do have quirks, so testing is absolutely required. |
3 participants
tl;dr:
Related to #5216 also see previous comments there. This is an alternative to make dmx.update() non-blocking. With less dependencies, more unified between ESP8266&32 and I claim more stable.
Fixes #5133
The core essence of this PR is dmx_output.cpp:57, the rest is cosmetics.
Long:
Independent from https://github.com/Stijn-Jacobs I also encountered the problem that once DMX output gets enabled in v0.15.3, WLED becomes super laggy regarding LED output via ArtNet, and almost unreachable via network. Short digging showed, that the DMX output is implemented in a blocking way, leading to 23ms of CPU time per loop run just for DMX.
This is unacceptable.
Stijn kindly prepared a fix in 5216 and I made mine in parallel locally. While I evaluated Stijns solution, I saw on oscilloscope, that the DMX frame initial MAB pulse, which is supposed to be at least 12us, sometimes is only 8us. I consider this suboptimal and would feel a little unsure using this. The used esp_dmx dependency is comparatively super complicated for what DMX needs, so I decided to post my solution as an alternative here.
The DMX part is quite trivial and the same for DMXESPOutput and SparkFunDMX. Now that DMX Input has been moved to another module altogether, I saw no more reason to keep those two separate dependencies and merged both in this new DMXOutput module. ESP8266 is still mostly blocking (due to limitations in HAL/Arduino), while ESP32 is almost completely non-blocking and takes less than 1ms per run.
The module is prepared to have an update rate element added to the web UI in future. Setting update rate to 0 ensures the max possible rate. Maximum with all 512/513 DMX channels is 43Hz, this is currently the default.
I decided to also change the #defines to better differentiate between the new DMXInput and DMXOutput modules. The new way to enable DMXOutput is to set WLED_ENABLE_DMX_OUTPUT instead of WLED_ENABLE_DMX, which I found confusing. Also, there's the possibility to set the initial TX pin using DMX_TXPIN_DEFAULT=1337. kno.wled.ge would need an update here.
In this branch, I also included some commits from netmindz #5287 adding TX pin setting on web UI, and the update rate limiting from Stijn (sadly couldn't cherry-pick as was mixed with too many other changes).
I did not use the number of DMX-fixtures UI element, because I consider it irrelevant for ESP32 and found it more confusing. This would lower the processor and RAM load marginally. It could be possibly helpful for ESP8266, but I think this is not the focus here.
This is tested on my ESP32 hardware with an oscilloscope. Didn't get the chance to connect my DMX fixtures, yet, but I'm pretty sure that will work. Untested on ESP8266, no hardware available.
Summary by CodeRabbit
New Features
Improvements