Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions usermods/mpu6050_imu/mpu6050_imu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@
#undef DEBUG_PRINTLN
#undef DEBUG_PRINTF
#ifdef WLED_DEBUG
#define DEBUG_PRINT(x) DEBUGOUT.print(x)
#define DEBUG_PRINTLN(x) DEBUGOUT.println(x)
#define DEBUG_PRINTF(x...) DEBUGOUT.printf(x)
#define DEBUG_PRINT(x) DEBUGOUT(x)
#define DEBUG_PRINTLN(x) DEBUGOUTLN(x)
#define DEBUG_PRINTF(x...) DEBUGOUTF(x)
#else
#define DEBUG_PRINT(x)
#define DEBUG_PRINTLN(x)
Expand Down
15 changes: 4 additions & 11 deletions wled00/FX.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,10 @@
#include "wled.h"
#include "colors.h"
#ifdef WLED_DEBUG
// enable additional debug output
#if defined(WLED_DEBUG_HOST)
#include "net_debug.h"
#define DEBUGOUT NetDebug
#else
#define DEBUGOUT Serial
#endif
#define DEBUGFX_PRINT(x) DEBUGOUT.print(x)
#define DEBUGFX_PRINTLN(x) DEBUGOUT.println(x)
#define DEBUGFX_PRINTF(x...) DEBUGOUT.printf(x)
#define DEBUGFX_PRINTF_P(x...) DEBUGOUT.printf_P(x)
#define DEBUGFX_PRINT(x) DEBUGOUT(x)
#define DEBUGFX_PRINTLN(x) DEBUGOUTLN(x)
#define DEBUGFX_PRINTF(x...) DEBUGOUTF(x)
#define DEBUGFX_PRINTF_P(x...) DEBUGOUTF(x)
#else
#define DEBUGFX_PRINT(x)
#define DEBUGFX_PRINTLN(x)
Expand Down
16 changes: 4 additions & 12 deletions wled00/bus_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,14 @@ make_unique(Args&&... args)
}
#endif

// enable additional debug output
#if defined(WLED_DEBUG_HOST)
#include "net_debug.h"
#define DEBUGOUT NetDebug
#else
#define DEBUGOUT Serial
#endif

#ifdef WLED_DEBUG_BUS
#ifndef ESP8266
#include <rom/rtc.h>
#endif
#define DEBUGBUS_PRINT(x) DEBUGOUT.print(x)
#define DEBUGBUS_PRINTLN(x) DEBUGOUT.println(x)
#define DEBUGBUS_PRINTF(x...) DEBUGOUT.printf(x)
#define DEBUGBUS_PRINTF_P(x...) DEBUGOUT.printf_P(x)
#define DEBUGBUS_PRINT(x) DEBUGOUT(x)
#define DEBUGBUS_PRINTLN(x) DEBUGOUTLN(x)
#define DEBUGBUS_PRINTF(x...) DEBUGOUTF(x)
#define DEBUGBUS_PRINTF_P(x...) DEBUGOUTF(x)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find definition of DEBUGOUTF and DEBUGOUT macros
rg -n --type=cpp --type=h -C3 'define\s+DEBUGOUTF\b'
rg -n --type=cpp --type=h -C3 'define\s+DEBUGOUT\b'

Repository: wled/WLED

Length of output: 1720


🏁 Script executed:

# Check line 482 in bus_manager.h for the actual usage
sed -n '475,490p' wled00/bus_manager.h

# Search for printf_P usage to see if it's used elsewhere
rg -n 'printf_P' wled00/ --type=cpp --type=h | head -20

# Search for historical _P debugging patterns
rg -n 'DEBUGOUT.*_P\b' wled00/ --type=cpp --type=h | head -20

Repository: wled/WLED

Length of output: 2811


DEBUGBUS_PRINTF_P mapping violates PROGMEM string handling.

Line 43 maps DEBUGBUS_PRINTF_P to DEBUGOUTF, which from wled.h (lines 993, 998) calls Serial.printf(x) or NetDebug.printf(x) — not printf_P().

The actual callsite at line 482 uses:

DEBUGBUS_PRINTF_P(PSTR("Bus: Config (%d-%d, type:%d, CO:%d, rev:%d, skip:%d, AW:%d kHz:%d, mA:%d/%d, driver:%s)\n"), ...)

On ESP8266, PSTR() strings reside in PROGMEM and must be read via printf_P(). Calling plain printf() on a PROGMEM pointer produces garbled output or faults.

The project guideline states: "Use PSTR() with DEBUG_PRINTF_P() for format strings." This change violates that by silently dropping the _P distinction.

Either:

  1. Define a separate DEBUGOUTF_P macro that calls printf_P(), or
  2. Make DEBUGOUTF itself PROGMEM-aware (route _P calls through printf_P() on ESP8266).

Then wire DEBUGBUS_PRINTF_P to the correct macro.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wled00/bus_manager.h` at line 43, The DEBUGBUS_PRINTF_P macro currently maps
to DEBUGOUTF which calls plain printf and therefore breaks PROGMEM PSTR() format
strings on ESP8266; fix by adding a PROGMEM-aware path: either implement a new
DEBUGOUTF_P that calls printf_P() (or NetDebug.printf_P()) on ESP8266 and
fallback to printf on other platforms, or modify DEBUGOUTF to detect _P usage
and route to printf_P; then change DEBUGBUS_PRINTF_P to map to DEBUGOUTF_P (or
the new PROGMEM-aware DEBUGOUTF) so calls like DEBUGBUS_PRINTF_P(PSTR(...), ...)
print correctly.

#else
#define DEBUGBUS_PRINT(x)
#define DEBUGBUS_PRINTLN(x)
Expand Down
17 changes: 17 additions & 0 deletions wled00/cfg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,14 @@ bool deserializeConfig(JsonObject doc, bool fromFS) {
CJSON(hueIP[i], if_hue_ip[i]);
#endif

#ifdef WLED_ENABLE_NET_DEBUG
JsonObject if_ndb = interfaces["ndb"];
JsonArray if_ndb_ip = if_ndb["ip"];
for (byte i = 0; i < 4; i++)
CJSON(netDebugPrintIP[i], if_ndb_ip[i]);
CJSON(netDebugPrintPort, if_ndb["port"]);
#endif
Comment on lines +662 to +668
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate netDebugPrintPort range on deserialize.

netDebugPrintPort is accepted directly from JSON here. A negative or out-of-range value can propagate into UDP calls after integer conversion.

💡 Suggested patch
 `#ifdef` WLED_ENABLE_NET_DEBUG
   JsonObject if_ndb = interfaces["ndb"];
   JsonArray if_ndb_ip = if_ndb["ip"];
   for (byte i = 0; i < 4; i++)
     CJSON(netDebugPrintIP[i], if_ndb_ip[i]);
   CJSON(netDebugPrintPort, if_ndb["port"]);
+  if (netDebugPrintPort < 0 || netDebugPrintPort > 65535) netDebugPrintPort = 0; // fallback handled during interface init
 `#endif`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/cfg.cpp` around lines 660 - 666, When deserializing the "ndb"
interface, validate the parsed netDebugPrintPort from interfaces["ndb"] (the
if_ndb JsonObject) before accepting it: ensure the value is within 0..65535 and
is non-negative; if it's out of range or not an integer, either clamp to a safe
default (e.g. 0 or previously configured netDebugPrintPort) and emit a warning,
or reject it and leave netDebugPrintPort unchanged. Update the assignment that
currently takes if_ndb["port"] directly to perform this range check and explicit
conversion to an unsigned 16-bit type so downstream UDP calls use a safe port
value.


JsonObject if_ntp = interfaces[F("ntp")];
CJSON(ntpEnabled, if_ntp["en"]);
getStringFromJson(ntpServerName, if_ntp[F("host")], 33); // "1.wled.pool.ntp.org"
Expand Down Expand Up @@ -1191,6 +1199,15 @@ void serializeConfig(JsonObject root) {
}
#endif

#ifdef WLED_ENABLE_NET_DEBUG
JsonObject if_ndb = interfaces.createNestedObject("ndb");
JsonArray if_ndb_ip = if_ndb.createNestedArray("ip");
for (byte i = 0; i < 4; i++) {
if_ndb_ip.add(netDebugPrintIP[i]);
}
if_ndb["port"] = netDebugPrintPort;
#endif

JsonObject if_ntp = interfaces.createNestedObject("ntp");
if_ntp["en"] = ntpEnabled;
if_ntp[F("host")] = ntpServerName;
Expand Down
2 changes: 1 addition & 1 deletion wled00/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ function populateInfo(i)
cn += `v${i.ver} <i>"${vcn}"</i>${i.release ? '<br>('+i.release+')' : ''}<br><br><table>
${urows}
${urows===""?'':'<tr><td colspan=2><hr style="height:1px;border-width:0;color:gray;background-color:gray"></td></tr>'}
${i.opt&0x100?inforow("Debug","<button class=\"btn btn-xs\" onclick=\"requestJson({'debug':"+(i.opt&0x0080?"false":"true")+"});\"><i class=\"icons "+(i.opt&0x0080?"on":"off")+"\">&#xe08f;</i></button>"):''}
${i.opt&0x100?inforow("Net Debug","<button class=\"btn btn-xs\" onclick=\"requestJson({'debug':"+(i.opt&0x0080?"false":"true")+"});\"><i class=\"icons "+(i.opt&0x0080?"on":"off")+"\">&#xe08f;</i></button>"):''}
${inforow("Build",i.vid)}
${inforow("Signal strength",i.wifi.signal +"% ("+ i.wifi.rssi, " dBm)")}
${inforow("Uptime",getRuntimeStr(i.uptime))}
Expand Down
13 changes: 13 additions & 0 deletions wled00/data/settings_sync.htm
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,19 @@ <h3>Serial</h3>
</select><br>
<i>Keep at 115200 to use Improv. Some boards may not support high rates.</i>
</div>
<div class="sec">
<h3>Net Debug ☾</h3>
<div id="NoNetDebug" class="hide">
<em style="color:#fa0;">This firmware build does not include Net Debug support.<br></em>
</div>
<div id="NetDebug">
Netcat host IP:<br>
<input name="N0" type="number" class="s" min="0" max="255" > .
<input name="N1" type="number" class="s" min="0" max="255" > .
<input name="N2" type="number" class="s" min="0" max="255" > .
<input name="N3" type="number" class="s" min="0" max="255" ><br>
Netcat host Port:<br>
<input name="NP" type="number" min="0" max="99999" ><br>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Constrain Net Debug port to valid UDP range in the form.

Line 299 allows 99999, but valid port values are 1..65535.

🐛 Suggested fix
-<input name="NP" type="number" min="0" max="99999" ><br>
+<input name="NP" type="number" min="1" max="65535" ><br>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<input name="NP" type="number" min="0" max="99999" ><br>
<input name="NP" type="number" min="1" max="65535" ><br>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/data/settings_sync.htm` at line 299, The Net Debug port input (input
name="NP") allows values up to 99999 which exceeds the valid UDP port range;
update the input element for NP to constrain values to the valid port range by
setting min="1" and max="65535" (and keep type="number") so the browser enforces
only 1..65535 can be submitted.

</div>
<hr>
<button type="button" onclick="B()">Back</button><button type="submit">Save</button>
Expand Down
4 changes: 2 additions & 2 deletions wled00/json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ bool deserializeState(JsonObject root, byte callMode, byte presetId)
{
bool stateResponse = root[F("v")] | false;

#if defined(WLED_DEBUG) && defined(WLED_DEBUG_HOST)
#if defined(WLED_DEBUG) && defined(WLED_ENABLE_NET_DEBUG)
netDebugEnabled = root[F("debug")] | netDebugEnabled;
#endif
Comment on lines +371 to 373
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

/json/state can't toggle net debug unless WLED_DEBUG is also on.

The runtime flag is still guarded by WLED_DEBUG && WLED_ENABLE_NET_DEBUG, so a WLED_ENABLE_NET_DEBUG build silently ignores {"debug":...} even though the PR exposes net debug as a runtime feature.

Suggested fix
-  `#if` defined(WLED_DEBUG) && defined(WLED_ENABLE_NET_DEBUG)
+  `#ifdef` WLED_ENABLE_NET_DEBUG
   netDebugEnabled = root[F("debug")] | netDebugEnabled;
   `#endif`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wled00/json.cpp` around lines 371 - 373, The runtime handling currently masks
the net-debug toggle behind WLED_DEBUG; change the conditional so
netDebugEnabled is updated when WLED_ENABLE_NET_DEBUG is defined (regardless of
WLED_DEBUG). Specifically, replace the guarded line that sets netDebugEnabled
from root[F("debug")] so it is compiled when WLED_ENABLE_NET_DEBUG is defined
(use `#if` defined(WLED_ENABLE_NET_DEBUG) ... `#endif`) and keep the assignment
netDebugEnabled = root[F("debug")] | netDebugEnabled; so the /json/state "debug"
field actually toggles net debug at runtime; keep existing symbols
netDebugEnabled and root[F("debug")] to locate and modify the code.


Expand Down Expand Up @@ -866,7 +866,7 @@ void serializeInfo(JsonObject root)
uint16_t os = 0;
#ifdef WLED_DEBUG
os = 0x80;
#ifdef WLED_DEBUG_HOST
#ifdef WLED_ENABLE_NET_DEBUG
os |= 0x0100;
if (!netDebugEnabled) os &= ~0x0080;
#endif
Comment on lines 867 to 872
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Net-debug status bits are still hidden behind the outer WLED_DEBUG guard.

Because this block only runs inside #ifdef WLED_DEBUG, /json/info won't advertise net-debug support/status for WLED_ENABLE_NET_DEBUG-only builds. That makes the runtime feature look unavailable even when it is compiled in.

Suggested fix
   uint16_t os = 0;
   `#ifdef` WLED_DEBUG
-  os  = 0x80;
-    `#ifdef` WLED_ENABLE_NET_DEBUG
-    os |= 0x0100;
-    if (!netDebugEnabled) os &= ~0x0080;
-    `#endif`
+  os |= 0x0080;
   `#endif`
+  `#ifdef` WLED_ENABLE_NET_DEBUG
+  os |= 0x0100;
+  if (netDebugEnabled) os |= 0x0080;
+  `#endif`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wled00/json.cpp` around lines 867 - 872, The net-debug bit handling is
incorrectly inside the WLED_DEBUG preprocessor guard so builds with only
WLED_ENABLE_NET_DEBUG won't advertise support or status; move or duplicate the
WLED_ENABLE_NET_DEBUG-related logic out of the `#ifdef` WLED_DEBUG block so that
the code that sets the os status bits for net-debug runs regardless of
WLED_DEBUG. Specifically, ensure the code that inspects netDebugEnabled and
sets/clears the 0x0100/0x0080 bits (the os variable and netDebugEnabled checks)
is executed for builds with WLED_ENABLE_NET_DEBUG so /json/info reports
net-debug capability and runtime status while still keeping any verbose
debug-only behavior inside WLED_DEBUG.

Expand Down
30 changes: 3 additions & 27 deletions wled00/net_debug.cpp
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai please analyze the net debug core code for possible stability issues, buffer overruns and potential for heap fragmentation. Suggest improvements / re-design options that would improve stability.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch net_debug.h full content
fd -e h -g 'net_debug*' --exec cat {}

echo "===== wled.h DEBUGOUT section ====="
# Extract DEBUGOUT/NetDebug/debugUdp relevant sections from wled.h
rg -n "DEBUGOUT|NetDebug|debugUdp|netDebugPrint|netDebugEnabled|USER_PRINT|USER_FLUSH" wled00/wled.h | head -80

echo "===== debugUdp declaration ====="
rg -n "debugUdp" wled00/ --type cpp --type h -C2

echo "===== WiFiUDP buffer sizes ====="
rg -rn "WiFiUDP|UDP_TX_PACKET_MAX|UDP_RX_PACKET_MAX" wled00/ --type cpp --type h | head -30

echo "===== NetworkDebugPrinter class definition ====="
ast-grep --pattern 'class NetworkDebugPrinter { $$$ }'

echo "===== wled.h net debug block ====="
grep -n -A 5 -B 2 "WLED_ENABLE_NET_DEBUG\|DEBUGOUT\b\|DEBUGOUTLN\|DEBUGOUTF" wled00/wled.h | head -100

Length of output: 6723


@softhack007, here is a detailed stability analysis of the net debug core, with redesign suggestions:


Net Debug Stability Analysis

🔴 Issue 1 — Per-character UDP packets (Critical heap fragmentation)

This is the most severe problem. write(uint8_t c) fires a complete UDP transaction for every single byte:

debugUdp.beginPacket(netDebugPrintIP, netDebugPrintPort); // heap alloc
debugUdp.write(c);                                         // 1 byte payload
debugUdp.endPacket();                                      // heap free + syscall

NetworkDebugPrinter inherits from Print, and Print::print(const char*) iterates character-by-character, calling write(uint8_t) for each. So DEBUG_PRINT("hello world\n") generates 12 heap alloc/free cycles and 12 UDP packets. A typical boot sequence with dozens of DEBUG_PRINTLN calls produces thousands of these micro-transactions. On ESP32, WiFiUDP::beginPacket() calls into lwIP and allocates a pbuf chain from the heap each time; this is the primary driver of heap fragmentation.

Fix — add a line-oriented ring buffer:

// net_debug.h — add to private section:
  static constexpr size_t TX_BUF_SIZE = 512;
  uint8_t  _txBuf[TX_BUF_SIZE];
  size_t   _txLen = 0;

// net_debug.cpp — replace both write() overrides:
size_t NetworkDebugPrinter::write(uint8_t c) {
  if (!WLED_CONNECTED || !netDebugEnabled) return 0;
  if (_txLen < TX_BUF_SIZE) _txBuf[_txLen++] = c;
  if (c == '\n' || _txLen == TX_BUF_SIZE) flush();
  return 1;
}

size_t NetworkDebugPrinter::write(const uint8_t *buf, size_t size) {
  if (!WLED_CONNECTED || buf == nullptr || size == 0 || !netDebugEnabled) return 0;
  size_t written = 0;
  while (written < size) {
    size_t chunk = min(size - written, TX_BUF_SIZE - _txLen);
    memcpy(_txBuf + _txLen, buf + written, chunk);
    _txLen   += chunk;
    written  += chunk;
    if (_txLen == TX_BUF_SIZE || _txBuf[_txLen - 1] == '\n') flush();
  }
  return written;
}

void NetworkDebugPrinter::flush() {
  if (_txLen == 0 || !WLED_CONNECTED || !netDebugEnabled) return;
  if (debugUdp.beginPacket(netDebugPrintIP, netDebugPrintPort)) {
    debugUdp.write(_txBuf, _txLen);
    debugUdp.endPacket();
  }
  _txLen = 0;
}

This reduces packet count from N-per-string to 1-per-line while keeping _txBuf as a stack-like member (no additional heap allocation).


🔴 Issue 2 — No IP / Port validation before beginPacket()

netDebugPrintPort is int, initialized to 0. netDebugPrintIP is initialized to 0.0.0.0. Neither is validated before calling beginPacket(). On some lwIP builds, sending to 0.0.0.0:0 can assert or silently corrupt socket state.

Fix:

// Add guard at top of both write() / flush():
if (netDebugPrintPort == 0 || (uint32_t)netDebugPrintIP == 0) return 0;

Also, netDebugPrintPort should be uint16_t, not int, to make the type semantics explicit and avoid accidental negative values from deserialization:

// wled.h
WLED_GLOBAL uint16_t netDebugPrintPort _INIT(0);

🟠 Issue 3 — No mutex / thread-safety for the global NetDebug singleton

WLED runs multiple FreeRTOS tasks on ESP32 (network task, async webserver, main loop, etc.). DEBUG_PRINT* macros can be called from several of these. debugUdp is a stateful object — concurrent beginPacket/write/endPacket calls from two tasks will corrupt its internal send-buffer pointer. The _txLen counter in the proposed ring buffer is equally vulnerable.

Fix — add a lightweight portMUX or SemaphoreHandle:

// net_debug.h — private section:
  portMUX_TYPE _mux = portMUX_INITIALIZER_UNLOCKED;

// net_debug.cpp — in flush():
  taskENTER_CRITICAL(&_mux);
  // ... send ...
  taskEXIT_CRITICAL(&_mux);

Alternatively, for a non-IRQ context, a binary SemaphoreHandle_t is less invasive (won't disable interrupts). On ESP8266, the single-core architecture makes this less critical but still worth guarding against re-entrancy via yield() inside lwIP.


🟠 Issue 4 — DEBUGOUT macro ternary without statement wrapping

`#define` DEBUGOUT(x) netDebugEnabled?NetDebug.print(x):Serial.print(x)

This bare ternary can cause surprising parse behavior when used as a statement in certain contexts (e.g., immediately after an if without braces, or as part of a comma expression). The return value is silently discarded. Prefer a do { } while(0) pattern or an if/else form:

`#define` DEBUGOUT(x)   do { if (netDebugEnabled) NetDebug.print(x);   else Serial.print(x);   } while(0)
`#define` DEBUGOUTLN(x) do { if (netDebugEnabled) NetDebug.println(x); else Serial.println(x); } while(0)
`#define` DEBUGOUTF(x...) do { if (netDebugEnabled) NetDebug.printf(x); else Serial.printf(x); } while(0)

🟡 Issue 5 — USER_FLUSH() is a no-op when net debug is active

`#ifdef` WLED_ENABLE_NET_DEBUG
  `#define` USER_FLUSH() {}   // ← silent no-op

With the proposed ring buffer, a flush() call becomes meaningful (drains buffered content). The no-op means that any partial line left in the buffer (e.g., a message without a trailing \n) will never be sent. USER_FLUSH() should call NetDebug.flush() once flush() is properly implemented.


🟡 Issue 6 — No size cap check in write(const uint8_t*, size_t)

Currently:

size = debugUdp.write(buf, size);

The UDP MTU on the local network is ~1472 bytes (Ethernet) and the ESP32 lwIP send-buffer for UDP is bounded by CONFIG_LWIP_UDP_RECVMBOX_SIZE. A caller passing a very large size (unlikely for debug strings but possible in principle) will silently truncate without returning a correct written count. The ring buffer approach above naturally handles this by chunking.


Summary Table

Issue Severity Impact
Per-character UDP packets 🔴 Critical Heavy heap fragmentation, network spam
No IP/port validation 🔴 Critical Potential lwIP assert / socket corruption
No thread-safety (mutex) 🟠 Moderate Data corruption under concurrent tasks
Bare ternary DEBUGOUT macros 🟠 Moderate Parse hazard in edge call sites
USER_FLUSH() no-op 🟡 Low Partial lines silently dropped
No UDP size cap 🟡 Low Silent truncation for large buffers
int instead of uint16_t for port 🟡 Low Semantic / type safety

The single highest-impact change is Issue 1 (ring buffer with newline-triggered flush) — it addresses heap fragmentation, packet count, and indirectly the size-cap issue in one pass.


🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4893
File: wled00/set.cpp:95-95
Timestamp: 2026-03-14T20:56:46.543Z
Learning: Guideline: Ensure WiFi hostname is set after WiFi.mode() but before WiFi.begin() to avoid default esp-XXXXXX hostname being used in DHCP. This ordering only takes effect after the STA interface exists (so avoid placing hostname setting before initConnection). In WLED, place the hostname configuration inside initConnection() (after WiFi.disconnect(true) and before WiFi.begin()) rather than in earlier boot code like deserializeConfig(). This rule should be applied in code reviews for WLED’s network initialization paths in wled00/*.cpp, and note that on ESP8266 the ordering is less strict but still desirable for consistency.

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:45.980Z
Learning: When calling raw lwIP APIs (e.g., around `ntpUdp.begin()` or any `lwIP`/`tcpip`-layer call) in this codebase on ESP32 Arduino-ESP32 platforms where core locking/checking is enabled, wrap the lwIP call with `LOCK_TCPIP_CORE()` / `UNLOCK_TCPIP_CORE()` from `lwip/tcpip.h`. This prevents thread-safety/core-violation crashes without requiring `sdkconfig` changes.

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/cfg.cpp:673-673
Timestamp: 2026-03-27T21:17:45.980Z
Learning: When using lwIP “raw” APIs in WLED on ESP32 (Arduino-ESP32 / IDF 5.5+), don’t rely on LOCK_TCPIP_CORE()/UNLOCK_TCPIP_CORE() unless CONFIG_LWIP_TCPIP_CORE_LOCKING=y is guaranteed. CONFIG_LWIP_CHECK_THREAD_SAFETY=y can trigger the assertion “Required to lock TCPIP core functionality!” when raw lwIP calls occur off the TCPIP thread. If the locking mode isn’t enabled (or can’t be changed via sdkconfig), schedule lwIP work (e.g., ntpUdp.begin() and similar raw lwIP calls) via tcpip_callback() so it runs on the TCPIP thread; this should work regardless of the locking-mode setting. Review any similar raw lwIP usage for correct thread/context handling.

Learnt from: softhack007
Repo: wled/WLED PR: 5048
File: wled00/wled.cpp:698-700
Timestamp: 2026-03-28T01:37:15.541Z
Learning: In this WLED codebase, when using `DEBUG_PRINTLN(F("..."))`, an explicit trailing `\n` inside the `F("...")` string (e.g., `DEBUG_PRINTLN(F("Warning!\n"))`) may be intentional to create a blank line in debug output as a visual separator. During code review, do not automatically flag these as “double newline” or recommend removing the `\n`—first verify with the author/context that the extra newline is deliberate.

Learnt from: willmmiles
Repo: wled/WLED PR: 5462
File: wled00/json.cpp:1189-1198
Timestamp: 2026-03-30T15:32:02.808Z
Learning: When working with WLED strings stored in PROGMEM on ESP8266/ESP32, assume you can’t use `strchr_P` for character scanning. If you need to scan a PROGMEM string for a character (e.g., searching for '@' within effect data), review changes to ensure they perform an explicit RAM copy first (e.g., via `strncpy_P` into a buffer) and then scan in RAM. Avoid proposing incremental/streaming PROGMEM character-by-character scans without a RAM buffer on these targets, since it’s impractical with the available APIs.

Learnt from: DedeHai
Repo: wled/WLED PR: 5464
File: wled00/FX_fcn.cpp:1699-1701
Timestamp: 2026-04-09T07:26:09.816Z
Learning: In WLED’s WLED code, if a pixel/buffer allocation uses `BFRALLOC_NOBYTEACCESS` (and especially on classic ESP32 where byte-level access to IRAM-resident buffers is unsafe), avoid using byte-wise operations like `memset`/`memcpy` on that buffer. Specifically, do not combine `BFRALLOC_CLEAR | BFRALLOC_NOBYTEACCESS` and do not perform `memcpy`/`memset` over `Segment::pixels` (e.g., in `setGeometry()`/`finalizeInit()` or copy ctor/assignment). Instead, use element-wise 32-bit access (loop over `uint32_t*` and assign/copy per element) to ensure 32-bit access only.

Learnt from: DedeHai
Repo: wled/WLED PR: 5464
File: wled00/FX_fcn.cpp:1699-1701
Timestamp: 2026-04-09T09:14:29.218Z
Learning: In WLED, when reviewing ESP32 code that uses pixel buffers allocated with `BFRALLOC_NOBYTEACCESS` (e.g., `WS2812FX::_pixels` and `Segment::pixels`), treat `memset`/`memcpy` on those buffers as an intentionally low-risk, accepted pattern. These buffers are designed to be IRAM-safe for 32-bit-only access, and the Xtensa newlib assembly implementations perform 32-bit word stores for word-aligned, word-multiple-sized buffers (so the IRAM constraint is satisfied in practice). Do not flag `memset`/`memcpy` on `BFRALLOC_NOBYTEACCESS` pixel buffers as a critical bug; if any concern arises, downgrade severity to low and focus on potential future regressions rather than an immediate correctness violation.

Learnt from: softhack007
Repo: wled/WLED PR: 5508
File: wled00/FX.cpp:4137-4142
Timestamp: 2026-04-16T09:49:58.587Z
Learning: In WLED effect/overlay implementations (C++ under wled00), ensure effects operate through the SEGMENT.* read/write APIs for their own pixels/overlay state. Reserve global frame reads/writes like strip.getPixelColor* for copy_* style post-processing only. Do not manually composite/blend across lower layers inside the effect; segment-to-segment blending is handled later by WS2812FX::blendSegment(), so effects should produce their per-segment contributions without trying to composite with underlying layers themselves.

Original file line number Diff line number Diff line change
@@ -1,23 +1,11 @@
#include "wled.h"

#ifdef WLED_DEBUG_HOST
#ifdef WLED_ENABLE_NET_DEBUG

size_t NetworkDebugPrinter::write(uint8_t c) {
if (!WLED_CONNECTED || !netDebugEnabled) return 0;

if (!debugPrintHostIP && !debugPrintHostIP.fromString(netDebugPrintHost)) {
#ifdef ESP8266
WiFi.hostByName(netDebugPrintHost, debugPrintHostIP, 750);
#else
#ifdef WLED_USE_ETHERNET
ETH.hostByName(netDebugPrintHost, debugPrintHostIP);
#else
WiFi.hostByName(netDebugPrintHost, debugPrintHostIP);
#endif
#endif
}

debugUdp.beginPacket(debugPrintHostIP, netDebugPrintPort);
debugUdp.beginPacket(netDebugPrintIP, netDebugPrintPort);
Copy link
Copy Markdown
Member

@softhack007 softhack007 May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's also a good time to scrutinise the net debug UDP core code - I found it quite unreliable in the past, even when only dumping packets via netcat. Maybe we should directly move to a standard logging protocol? Afaik the main difference is in establishing the connection, after that its basicially a printf with some meta data added.

Netdebug was an overnight hack by a contributor, it was never tested under "real load".

debugUdp.write(c);
debugUdp.endPacket();
return 1;
Expand All @@ -26,19 +14,7 @@ size_t NetworkDebugPrinter::write(uint8_t c) {
size_t NetworkDebugPrinter::write(const uint8_t *buf, size_t size) {
if (!WLED_CONNECTED || buf == nullptr || !netDebugEnabled) return 0;

if (!debugPrintHostIP && !debugPrintHostIP.fromString(netDebugPrintHost)) {
#ifdef ESP8266
WiFi.hostByName(netDebugPrintHost, debugPrintHostIP, 750);
#else
#ifdef WLED_USE_ETHERNET
ETH.hostByName(netDebugPrintHost, debugPrintHostIP);
#else
WiFi.hostByName(netDebugPrintHost, debugPrintHostIP);
#endif
#endif
}

debugUdp.beginPacket(debugPrintHostIP, netDebugPrintPort);
debugUdp.beginPacket(netDebugPrintIP, netDebugPrintPort);
size = debugUdp.write(buf, size);
debugUdp.endPacket();
return size;
Expand Down
2 changes: 1 addition & 1 deletion wled00/net_debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
class NetworkDebugPrinter : public Print {
private:
WiFiUDP debugUdp; // needs to be here otherwise UDP messages get truncated upon destruction
IPAddress debugPrintHostIP;
// IPAddress debugPrintHostIP;
public:
virtual size_t write(uint8_t c);
virtual size_t write(const uint8_t *buf, size_t s);
Expand Down
8 changes: 8 additions & 0 deletions wled00/set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,14 @@ void handleSettingsSet(AsyncWebServerRequest *request, byte subPage)
reconnectHue();
#endif

#ifdef WLED_ENABLE_NET_DEBUG
for (int i=0;i<4;i++){
String a = "N"+String(i);
netDebugPrintIP[i] = request->arg(a).toInt();
}
netDebugPrintPort = request->arg("NP").toInt();
#endif
Comment on lines +526 to +532
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard and validate Net Debug fields before applying them.

Line 529 and Line 531 currently overwrite values even when args are absent/invalid, which can silently reset settings to 0 on partial submissions.

🐛 Suggested hardening
 `#ifdef` WLED_ENABLE_NET_DEBUG
-      for (int i=0;i<4;i++){
-        String a = "N"+String(i);
-        netDebugPrintIP[i] = request->arg(a).toInt();
-      }
-      netDebugPrintPort = request->arg("NP").toInt();
+      for (int i = 0; i < 4; i++) {
+        String a = "N" + String(i);
+        if (request->hasArg(a)) {
+          netDebugPrintIP[i] = (uint8_t)constrain(request->arg(a).toInt(), 0, 255);
+        }
+      }
+      if (request->hasArg(F("NP"))) {
+        int np = request->arg(F("NP")).toInt();
+        if (np > 0 && np <= 65535) netDebugPrintPort = np;
+      }
 `#endif`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/set.cpp` around lines 526 - 532, The net debug block currently
unconditionally overwrites netDebugPrintIP[] and netDebugPrintPort which can
reset them to 0 when args are missing or invalid; change the logic to first
check for the presence and validity of each argument from request (e.g. for each
"N"+String(i) call request->hasArg(...) or test that request->arg(...) is
non-empty and numeric) before assigning netDebugPrintIP[i], and likewise verify
request->hasArg("NP") and that its value is a valid port number before assigning
netDebugPrintPort so existing settings are preserved when args are
absent/invalid.


t = request->arg(F("BD")).toInt();
if (t >= 96 && t <= 15000) serialBaud = t;
updateBaudRate(serialBaud *100);
Expand Down
31 changes: 29 additions & 2 deletions wled00/wled.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ void WLED::setup()
#if defined(WLED_DEBUG) && defined(ARDUINO_ARCH_ESP32) && (defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32C3) || ARDUINO_USB_CDC_ON_BOOT)
delay(2500); // allow CDC USB serial to initialise
#endif
#if !defined(WLED_DEBUG) && defined(ARDUINO_ARCH_ESP32) && !defined(WLED_DEBUG_HOST) && ARDUINO_USB_CDC_ON_BOOT
#if !defined(WLED_DEBUG) && defined(ARDUINO_ARCH_ESP32) && !defined(WLED_ENABLE_NET_DEBUG) && ARDUINO_USB_CDC_ON_BOOT
Serial.setDebugOutput(false); // switch off kernel messages when using USBCDC
#endif
DEBUG_PRINTLN();
Expand Down Expand Up @@ -442,7 +442,7 @@ void WLED::setup()
usePWMFixedNMI(); // link the NMI fix
#endif

#if defined(WLED_DEBUG) && !defined(WLED_DEBUG_HOST)
#if defined(WLED_DEBUG) && !defined(WLED_ENABLE_NET_DEBUG)
PinManager::allocatePin(hardwareTX, true, PinOwner::DebugOut); // TX (GPIO1 on ESP32) reserved for debug output
#endif
#ifdef WLED_ENABLE_DMX //reserve GPIO2 as hardcoded DMX pin
Expand Down Expand Up @@ -827,6 +827,33 @@ void WLED::initInterfaces()
}
#endif

#ifdef WLED_ENABLE_NET_DEBUG
if (netDebugPrintIP[0] == 0) {
if (!netDebugPrintIP && !netDebugPrintIP.fromString(WLED_DEBUG_HOST)) {
#ifdef ESP8266
WiFi.hostByName(WLED_DEBUG_HOST, netDebugPrintIP, 750);
#else
#ifdef WLED_USE_ETHERNET
ETH.hostByName(WLED_DEBUG_HOST, netDebugPrintIP);
#else
WiFi.hostByName(WLED_DEBUG_HOST, netDebugPrintIP);
#endif
#endif
} else {
IPAddress ipAddress = Network.localIP();
netDebugPrintIP[0] = ipAddress[0];
netDebugPrintIP[1] = ipAddress[1];
netDebugPrintIP[2] = ipAddress[2];
}
}
if (netDebugPrintPort == 0)
#ifdef WLED_DEBUG_PORT
netDebugPrintPort = WLED_DEBUG_PORT;
#else
netDebugPrintPort = 7868; //Default value
#endif
#endif
Comment on lines +830 to +855
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== WLED_DEBUG_HOST define presence =="
rg -n --type=cpp --type=h '^\s*#\s*define\s+WLED_DEBUG_HOST\b|^\s*#\s*ifndef\s+WLED_DEBUG_HOST\b'

echo
echo "== netDebug init block =="
rg -n -C4 'netDebugPrintIP|WLED_DEBUG_HOST|netDebugPrintPort' wled00/wled.cpp

echo
echo "== netDebugPrintIP octet assignments in wled.cpp =="
rg -n 'netDebugPrintIP\[[0-3]\]\s*=' wled00/wled.cpp

Repository: wled/WLED

Length of output: 91


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Lines 830-855 in wled00/wled.cpp ==="
sed -n '830,855p' wled00/wled.cpp

echo
echo "=== Context around netDebugPrintIP declaration ==="
rg -n 'netDebugPrintIP' wled00/wled.cpp | head -20

echo
echo "=== IPAddress class definition ==="
fd -t f '\.h$' -x grep -l 'class IPAddress' {} \;

echo
echo "=== Check if WLED_DEBUG_HOST is defined anywhere in project ==="
rg -r '\.{1,2}' --type=cpp --type=h 'define.*WLED_DEBUG_HOST|WLED_DEBUG_HOST\s*=' . 2>/dev/null || true

Repository: wled/WLED

Length of output: 1432


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Search for WLED_DEBUG or DEBUG_HOST in config files ==="
find . -type f \( -name "*.h" -o -name "*.cpp" -o -name "*.json" -o -name "*.md" \) -exec grep -l "DEBUG_HOST\|DEBUG_PORT" {} \; 2>/dev/null || true

echo
echo "=== Check wled.h for netDebugPrintIP declaration ==="
grep -n 'netDebugPrintIP\|IPAddress' wled00/wled.h | head -20

echo
echo "=== Look for IPAddress usage with bracket notation ==="
rg -n 'IPAddress.*\[[0-3]\]' wled00/ | head -10

echo
echo "=== Check default values or config setup ==="
rg -n 'WLED_DEBUG_HOST|netDebugPrintIP' wled00/set.cpp | head -20

Repository: wled/WLED

Length of output: 825


Net-debug init has compile-time dependency, inverted logic, and incomplete IP assignment bugs.

Line 832 references WLED_DEBUG_HOST without #ifdef WLED_DEBUG_HOST guard despite it being undefined in the codebase. The condition if (!netDebugPrintIP && !netDebugPrintIP.fromString(WLED_DEBUG_HOST)) inverts the fallback logic: when fromString() succeeds, the else branch overwrites it with the local IP instead of keeping the parsed value. Additionally, the local IP fallback (lines 844–846) assigns only octets 0–2, leaving octet 3 unset when IPAddress requires all 4 octets.

Suggested fix
 `#ifdef` WLED_ENABLE_NET_DEBUG
-  if (netDebugPrintIP[0] == 0) {
-    if (!netDebugPrintIP && !netDebugPrintIP.fromString(WLED_DEBUG_HOST)) {
+  bool netDebugIpUnset = (netDebugPrintIP[0] == 0 && netDebugPrintIP[1] == 0 && netDebugPrintIP[2] == 0 && netDebugPrintIP[3] == 0);
+  if (netDebugIpUnset) {
+    `#ifdef` WLED_DEBUG_HOST
+    if (!netDebugPrintIP.fromString(WLED_DEBUG_HOST)) {
       `#ifdef` ESP8266
         WiFi.hostByName(WLED_DEBUG_HOST, netDebugPrintIP, 750);
       `#else`
         `#ifdef` WLED_USE_ETHERNET
           ETH.hostByName(WLED_DEBUG_HOST, netDebugPrintIP);
         `#else`
           WiFi.hostByName(WLED_DEBUG_HOST, netDebugPrintIP);
         `#endif`
       `#endif`
-    } else {
-      IPAddress ipAddress = Network.localIP();
-      netDebugPrintIP[0] = ipAddress[0];
-      netDebugPrintIP[1] = ipAddress[1];
-      netDebugPrintIP[2] = ipAddress[2];
     }
+    `#else`
+    IPAddress ipAddress = Network.localIP();
+    netDebugPrintIP[0] = ipAddress[0];
+    netDebugPrintIP[1] = ipAddress[1];
+    netDebugPrintIP[2] = ipAddress[2];
+    netDebugPrintIP[3] = ipAddress[3];
+    `#endif`
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#ifdef WLED_ENABLE_NET_DEBUG
if (netDebugPrintIP[0] == 0) {
if (!netDebugPrintIP && !netDebugPrintIP.fromString(WLED_DEBUG_HOST)) {
#ifdef ESP8266
WiFi.hostByName(WLED_DEBUG_HOST, netDebugPrintIP, 750);
#else
#ifdef WLED_USE_ETHERNET
ETH.hostByName(WLED_DEBUG_HOST, netDebugPrintIP);
#else
WiFi.hostByName(WLED_DEBUG_HOST, netDebugPrintIP);
#endif
#endif
} else {
IPAddress ipAddress = Network.localIP();
netDebugPrintIP[0] = ipAddress[0];
netDebugPrintIP[1] = ipAddress[1];
netDebugPrintIP[2] = ipAddress[2];
}
}
if (netDebugPrintPort == 0)
#ifdef WLED_DEBUG_PORT
netDebugPrintPort = WLED_DEBUG_PORT;
#else
netDebugPrintPort = 7868; //Default value
#endif
#endif
`#ifdef` WLED_ENABLE_NET_DEBUG
bool netDebugIpUnset = (netDebugPrintIP[0] == 0 && netDebugPrintIP[1] == 0 && netDebugPrintIP[2] == 0 && netDebugPrintIP[3] == 0);
if (netDebugIpUnset) {
`#ifdef` WLED_DEBUG_HOST
if (!netDebugPrintIP.fromString(WLED_DEBUG_HOST)) {
`#ifdef` ESP8266
WiFi.hostByName(WLED_DEBUG_HOST, netDebugPrintIP, 750);
`#else`
`#ifdef` WLED_USE_ETHERNET
ETH.hostByName(WLED_DEBUG_HOST, netDebugPrintIP);
`#else`
WiFi.hostByName(WLED_DEBUG_HOST, netDebugPrintIP);
`#endif`
`#endif`
}
`#else`
IPAddress ipAddress = Network.localIP();
netDebugPrintIP[0] = ipAddress[0];
netDebugPrintIP[1] = ipAddress[1];
netDebugPrintIP[2] = ipAddress[2];
netDebugPrintIP[3] = ipAddress[3];
`#endif`
}
if (netDebugPrintPort == 0)
`#ifdef` WLED_DEBUG_PORT
netDebugPrintPort = WLED_DEBUG_PORT;
`#else`
netDebugPrintPort = 7868; //Default value
`#endif`
`#endif`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/wled.cpp` around lines 830 - 855, Wrap usage of WLED_DEBUG_HOST in an
`#ifdef` WLED_DEBUG_HOST block and invert the fromString logic so a successful
parse keeps the configured host IP; specifically, inside the netDebugPrintIP
initialization (symbol: netDebugPrintIP and its method fromString) attempt
netDebugPrintIP.fromString(WLED_DEBUG_HOST) only when WLED_DEBUG_HOST is defined
and if that call returns true do not overwrite netDebugPrintIP, otherwise fall
back to Network.localIP(); when assigning the fallback IP from Network.localIP()
(symbol: Network::localIP or Network.localIP) copy all four octets (0..3) into
netDebugPrintIP so the fourth byte is not left uninitialized; leave the
netDebugPrintPort defaulting logic as-is but ensure the
WLED_DEBUG_HOST-dependent code is guarded by `#ifdef/`#endif.


#ifndef WLED_DISABLE_ALEXA
// init Alexa hue emulation
if (alexaEnabled)
Expand Down
64 changes: 47 additions & 17 deletions wled00/wled.h
Original file line number Diff line number Diff line change
Expand Up @@ -974,41 +974,71 @@ WLED_GLOBAL JsonDocument *pDoc _INIT(&gDoc);
#endif
WLED_GLOBAL volatile uint8_t jsonBufferLock _INIT(0);

// enable net debug output over UDP; use -D WLED_ENABLE_NET_DEBUG to activate.
// Legacy: defining WLED_DEBUG_HOST='"ip_or_host"' also enables net debug and
// pre-seeds the target IP at compile time.
#if defined(WLED_DEBUG_HOST) && !defined(WLED_ENABLE_NET_DEBUG)
#define WLED_ENABLE_NET_DEBUG
#endif

// enable additional debug output
#if defined(WLED_DEBUG_HOST)
#if defined(WLED_ENABLE_NET_DEBUG)
#include "net_debug.h"
// On the host side, use netcat to receive the log statements: nc -l 7868 -u
// use -D WLED_DEBUG_HOST='"192.168.xxx.xxx"' or FQDN within quotes
#define DEBUGOUT NetDebug
WLED_GLOBAL bool netDebugEnabled _INIT(true);
WLED_GLOBAL char netDebugPrintHost[33] _INIT(WLED_DEBUG_HOST);
#ifndef WLED_DEBUG_PORT
#define WLED_DEBUG_PORT 7868
#endif
WLED_GLOBAL int netDebugPrintPort _INIT(WLED_DEBUG_PORT);
WLED_GLOBAL bool netDebugEnabled _INIT(false);
WLED_GLOBAL int netDebugPrintPort _INIT(0);
WLED_GLOBAL IPAddress netDebugPrintIP _INIT_N(((0, 0, 0, 0))); // IP address of the bridge
#define DEBUGOUT(x) netDebugEnabled?NetDebug.print(x):Serial.print(x)
#define DEBUGOUTLN(x) netDebugEnabled?NetDebug.println(x):Serial.println(x)
#define DEBUGOUTF(x...) netDebugEnabled?NetDebug.printf(x):Serial.printf(x)
#define DEBUGOUTFlush() netDebugEnabled?NetDebug.flush():Serial.flush()
#else
#define DEBUGOUT Serial
#define DEBUGOUT(x) Serial.print(x)
#define DEBUGOUTLN(x) Serial.println(x)
#define DEBUGOUTF(x...) Serial.printf(x)
#define DEBUGOUTFlush() Serial.flush()
Comment on lines +991 to +999
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Object-style DEBUGOUT member calls (incompatible with function-like DEBUGOUT) =="
rg -nP --type=cpp --type=h '\bDEBUGOUT\s*\.\s*(print|println|printf|flush)\s*\('

echo
echo "== Bare DEBUGOUT identifier usages in affected areas =="
rg -nP '\bDEBUGOUT\b(?!\s*\()' wled00/wled.cpp usermods/audioreactive/audio_reactive.cpp

Repository: wled/WLED

Length of output: 1346


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check the exact content of wled.h around lines 990-998
echo "=== wled.h lines 980-1010 ==="
sed -n '980,1010p' wled00/wled.h | cat -n

echo
echo "=== Check if there are any wrapper functions for stream compatibility ==="
rg -nA5 'debugOutStream|DEBUGOUT_STREAM|Print.*DEBUGOUT' wled00/wled.h

echo
echo "=== Verify the exact definitions of DEBUGOUT-related macros ==="
rg -nP '^\s*#define\s+(DEBUGOUT|DEBUGSR_|PLOT_)' wled00/wled.h usermods/audioreactive/audio_reactive.cpp | head -20

Repository: wled/WLED

Length of output: 1519


DEBUGOUT API change breaks existing stream-style call sites.

Making DEBUGOUT a function-style macro breaks code that uses it as a stream object. usermods/audioreactive/audio_reactive.cpp:55-67 defines DEBUGSR_PRINT, DEBUGSR_PRINTLN, and DEBUGSR_PRINTF using DEBUGOUT.print(), DEBUGOUT.println(), and DEBUGOUT.printf() syntax. Additionally, wled00/wled.cpp:323 calls server.printStatus(DEBUGOUT), passing DEBUGOUT as a stream argument. These become compile-time errors under the new macro definition.

Consider using a wrapper function that returns a reference to the appropriate stream, allowing both function-call and stream-object usage patterns:

`#if` defined(WLED_ENABLE_NET_DEBUG)
  inline Print& debugOutStream() { return netDebugEnabled ? static_cast<Print&>(NetDebug) : static_cast<Print&>(Serial); }
`#else`
  inline Print& debugOutStream() { return static_cast<Print&>(Serial); }
`#endif`
`#define` DEBUGOUT_STREAM() debugOutStream()
`#define` DEBUGOUT(x) DEBUGOUT_STREAM().print(x)
`#define` DEBUGOUTLN(x) DEBUGOUT_STREAM().println(x)
`#define` DEBUGOUTF(x...) DEBUGOUT_STREAM().printf(x)
`#define` DEBUGOUTFlush() DEBUGOUT_STREAM().flush()

Then migrate stream-argument call sites (e.g., printStatus(...)) to use DEBUGOUT_STREAM().

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/wled.h` around lines 990 - 998, The DEBUGOUT macros were changed to
function-style calls which breaks sites expecting a stream object (e.g.,
DEBUGSR_PRINT macros that call DEBUGOUT.print()/println()/printf() and
server.printStatus(DEBUGOUT)); restore stream-style usage by adding a helper
that returns a Print& (e.g., implement inline Print& debugOutStream() that
returns NetDebug or Serial based on netDebugEnabled and build flag), expose a
macro like DEBUGOUT_STREAM() that expands to debugOutStream(), and then redefine
DEBUGOUT/DEBUGOUTLN/DEBUGOUTF/DEBUGOUTFlush to call
DEBUGOUT_STREAM().print/println/printf/flush while leaving DEBUGOUT_STREAM()
available for passing into functions such as printStatus and the
DEBUGSR_PRINT/DEBUGSR_PRINTLN/DEBUGSR_PRINTF sites so both function-call and
stream-object patterns work.

Copy link
Copy Markdown
Member

@softhack007 softhack007 May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEBUGSR_ macros in audioreactive UM should be adjusted. It's not necessary to have a legacy support helper only for this usermod.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== DEBUGSR_ macros in audioreactive ==="
rg -nP '#define\s+DEBUGSR_' usermods/audioreactive/audio_reactive.cpp

echo
echo "=== wled.cpp printStatus call ==="
rg -n 'printStatus' wled00/wled.cpp

Length of output: 363


@softhack007, understood — the DEBUGSR_ macros in the audioreactive usermod just need to be updated to use the new function-style macros directly. Here's the fix:

// usermods/audioreactive/audio_reactive.cpp, lines 55-57
`#define` DEBUGSR_PRINT(x)    DEBUGOUT(x)
`#define` DEBUGSR_PRINTLN(x)  DEBUGOUTLN(x)
`#define` DEBUGSR_PRINTF(x...) DEBUGOUTF(x)

One separate issue remains in wled00/wled.cpp line 323:

server.printStatus(DEBUGOUT);  // DEBUGOUT is no longer a stream object

printStatus() takes a Print& argument, so DEBUGOUT (now a function-like macro) can't be passed here directly. This call site will need to be handled — either by replacing it with a different logging approach, or by introducing a small helper just for this one call site. How would you like to handle that one?


✏️ Learnings added
Learnt from: softhack007
Repo: wled/WLED PR: 5559
File: wled00/wled.h:990-998
Timestamp: 2026-05-03T11:37:43.177Z
Learning: In the WLED audioreactive usermod (usermods/audioreactive/audio_reactive.cpp), DEBUGSR_PRINT/DEBUGSR_PRINTLN/DEBUGSR_PRINTF macros should use the function-style DEBUGOUT(x)/DEBUGOUTLN(x)/DEBUGOUTF(x...) macros, not the old stream-style DEBUGOUT.print()/println()/printf() syntax.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

#endif

#ifdef WLED_DEBUG
#ifndef ESP8266
#include <rom/rtc.h>
#endif
#define DEBUG_PRINT(x) DEBUGOUT.print(x)
#define DEBUG_PRINTLN(x) DEBUGOUT.println(x)
#define DEBUG_PRINTF(x...) DEBUGOUT.printf(x)
#define DEBUG_PRINTF_P(x...) DEBUGOUT.printf_P(x)
#define DEBUG_PRINT(x) DEBUGOUT(x)
#define DEBUG_PRINTLN(x) DEBUGOUTLN(x)
#define DEBUG_PRINTF(x...) DEBUGOUTF(x)
#define DEBUG_PRINTF_P(x...) DEBUGOUTF(x)
#else
#define DEBUG_PRINT(x)
#define DEBUG_PRINTLN(x)
#define DEBUG_PRINTF(x...)
#define DEBUG_PRINTF_P(x...)
#endif

// macros to print "user messages" to Serial
// cannot do this on -S2, due to buggy USBCDC serial driver
Copy link
Copy Markdown
Member

@softhack007 softhack007 May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@netmindz ah, you already have a provision for USER_PRINT 👍 did not see this when writing my first comment.

The all you'd need is to also import caUseSerial from WLED-MM - this allows to always support USER_PRINT even without netdebug.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai please verify if the restriction for -S2 (buggy USBCDC) is still needed. For comparison, there is a workaround in WLED-MM but it has the side-effect that sometimes serial output could get lost

https://github.com/MoonModules/WLED-MM/blob/459a66491d7a3993ef456eb5cbfaad4c9d806f3b/wled00/wled.cpp#L498

https://github.com/MoonModules/WLED-MM/blob/459a66491d7a3993ef456eb5cbfaad4c9d806f3b/wled00/wled.cpp#L525-L556

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

Let me look at the referenced WLED-MM code and check the current state of the S2 USBCDC bug.


🏁 Script executed:

#!/bin/bash
# Find all S2/USBCDC related guards and comments in WLED codebase
rg -n "S2|USBCDC|CDC_ON_BOOT|USB_CDC|canUseSerial" --type=cpp --type=h -C2

Length of output: 86561


🏁 Script executed:

#!/bin/bash
# Find canUseSerial and S2 serial-related guards
rg -n "canUseSerial|CONFIG_IDF_TARGET_ESP32S2|ARDUINO_USB_CDC_ON_BOOT|USBCDC" --type=cpp --type=h -C3

echo "---"
# Also check fcn_declare for canUseSerial declaration
rg -n "canUseSerial" -C3

Length of output: 37006


🏁 Script executed:

#!/bin/bash
# Find canUseSerial and S2 serial-related guards
rg -n "canUseSerial|CONFIG_IDF_TARGET_ESP32S2|ARDUINO_USB_CDC_ON_BOOT|USBCDC" --type=cpp --type=h -C3

echo "=== wled.cpp S2 serial handling ==="
rg -n "S2\|serial\|Serial\|canUseSerial" wled00/wled.cpp -i -C2 | head -80

echo "=== fcn_declare for canUseSerial ==="
rg -n "canUseSerial" -C3

Length of output: 37075


🌐 Web query:

ESP32-S2 USBCDC Arduino Serial driver bug status 2025

💡 Result:

As of 2025, the main “Arduino Serial over native USB CDC on ESP32‑S2” bugs people commonly refer to were already fixed upstream in the Espressif Arduino core:

  • Dropped RX data / setRxBufferSize() not taking effect / RX queue forced to 256 bytes (USB CDC on boot): fixed by PR #6413, merged Mar 28, 2022 (milestone 2.0.3). So any Arduino-ESP32 core ≥ 2.0.3 should include this fix. (github.com)
  • ESP_LOG* / log_e not printing when using USB‑CDC (TinyUSB OTG) on ESP32‑S2/S3: fixed by PR #10123, merged Aug 13, 2024; it’s listed in the Arduino-ESP32 3.1.0-RC1 release notes and later. (github.com)

If what you’re seeing in 2025 is “no serial output / can’t open COM port”, that’s often configuration/host-side, not the old core bug—e.g. needing Tools → “USB CDC On Boot” enabled and the right USB mode/flash settings. (docs.espressif.com)

Citations:


Let me look at the canUseSerial() implementation and fetch the WLED-MM code for comparison.


🏁 Script executed:

#!/bin/bash
echo "=== canUseSerial definition ==="
ast-grep --pattern 'bool canUseSerial() { $$$ }'
rg -n "canUseSerial" --type=cpp --type=h -A5

echo "=== fcn_declare.h for canUseSerial ==="
rg -n "canUseSerial" wled00/fcn_declare.h -C3

echo "=== WLED-MM referenced lines via curl ==="
curl -s "https://raw.githubusercontent.com/MoonModules/WLED-MM/459a66491d7a3993ef456eb5cbfaad4c9d806f3b/wled00/wled.cpp" | sed -n '490,560p'

Length of output: 4309


🏁 Script executed:

#!/bin/bash
echo "=== canUseSerial definition ==="
rg -n "canUseSerial" -A10

echo "=== arduino-esp32 issue 9043 context - S2 setTxTimeoutMs ==="
curl -s "https://raw.githubusercontent.com/MoonModules/WLED-MM/459a66491d7a3993ef456eb5cbfaad4c9d806f3b/wled00/wled.cpp" | sed -n '470,500p'

Length of output: 2178


🏁 Script executed:

#!/bin/bash
echo "=== Search canUseSerial in all files ==="
rg -rn "canUseSerial" .

echo "=== wled.cpp lines around Serial for canUseSerial ==="
grep -n "canUseSerial\|bool.*serial\|Serial.*bool" wled00/wled.cpp wled00/util.cpp wled00/network.cpp 2>/dev/null | head -30

echo "=== Check fcn_declare.h for canUseSerial ==="
cat wled00/fcn_declare.h | grep -n "canUseSerial\|serial"

Length of output: 1237


🏁 Script executed:

#!/bin/bash
echo "=== wled_serial.cpp canUseSerial ==="
cat wled00/wled_serial.cpp | grep -n "canUseSerial" -A10 -B5

echo "=== Check all .cpp and .h files for canUseSerial definition ==="
grep -rn "bool canUseSerial\|canUseSerial()" wled00/ --include="*.cpp" --include="*.h" --include="*.ino"

Length of output: 359


@softhack007 — I checked the codebase and researched the current status of the S2 USBCDC driver. Here are the key findings:


1. The S2 USBCDC restriction is still relevant

The S2 CDC-over-USB crash is still being actively worked around in the current codebase — in wled00/FX_2Dfcn.cpp, there is an explicit delay(1) guard specifically for S2:

`#if` defined(CONFIG_IDF_TARGET_ESP32S2)
delay(1); // on S2 the CDC output can crash without a delay
`#endif`

The WLED-MM workaround applies Serial.setTxTimeoutMs(0) (for arduino-esp32 issue #9043) only to S3/C3/C6/P4 — the ESP32-S2 is intentionally excluded:

// WLED-MM: only for S3/C3/C6/P4, NOT S2:
`#if` ARDUINO_USB_CDC_ON_BOOT && (defined(CONFIG_IDF_TARGET_ESP32S3) || defined(CONFIG_IDF_TARGET_ESP32C3) || ...)
Serial.setTxTimeoutMs(0); // potential side-effect: incomplete debug output

Some specific arduino-esp32 bugs have been fixed — the RX buffer bug was resolved in PR #6413 (merged March 2022, milestone 2.0.3), and the ESP_LOG* not printing via TinyUSB was fixed in PR #10123 (merged August 2024, in 3.1.0-RC1). However, blocking TX/flush behavior for the S2 remains unaddressed by setTxTimeoutMs(0), which is why WLED-MM explicitly excludes the S2 from that fix.


2. Critical issue: canUseSerial() is undefined ⚠️

More importantly, searching the entire PR branch reveals that canUseSerial() is referenced in three macros but never defined anywhere:

// wled.h — only usages, no definition found:
`#define` USER_PRINT(x)      { if (canUseSerial()) DEBUGOUT(x); }
`#define` USER_PRINTLN(x)    { if (canUseSerial()) DEBUGOUTLN(x); }
`#define` USER_PRINTF(x...)  { if (canUseSerial()) DEBUGOUTF(x); }

This #else branch is entered when neither WLED_DEBUG nor WLED_ENABLE_NET_DEBUG is defined — i.e., the standard release build. Any such build will fail to compile/link with an undefined reference.

The intent appears to be a runtime guard similar to WLED-MM's if (Serial) checks. A minimal implementation could be:

// in wled00/fcn_declare.h or inline:
inline bool canUseSerial() {
`#if` defined(ARDUINO_USB_CDC_ON_BOOT) || defined(ARDUINO_USB_MODE)
  return (bool)Serial; // only safe if USB CDC has actually initialized
`#else`
  return true;         // standard UART serial is always ready after Serial.begin()
`#endif`
}

For S2 specifically, given the CDC crash risk under heavy output, a more conservative approach (like WLED-MM's Serial.setTxTimeoutMs(0) equivalent or the delay(1) pattern) may also be needed. The WLED-MM note that setTxTimeoutMs(0) has "the side-effect that sometimes serial output could get lost" is the acknowledged trade-off.

Summary: The S2 restriction comment is still warranted. The more pressing issue is that canUseSerial() must be defined before this PR is mergeable.


🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

#if defined(WLED_DEBUG) || defined(WLED_ENABLE_NET_DEBUG)
// use DEBUG_PRINT
#define USER_PRINT(x) DEBUG_PRINT(x)
#define USER_PRINTLN(x) DEBUG_PRINTLN(x)
#define USER_PRINTF(x...) DEBUG_PRINTF(x)
#ifdef WLED_ENABLE_NET_DEBUG
#define USER_FLUSH() {}
#else
#define USER_FLUSH() {DEBUGOUTFlush();}
#endif
Comment on lines +1019 to +1028
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

USER_PRINT* becomes a no-op in net-debug-only builds.

This branch is selected when WLED_ENABLE_NET_DEBUG is enabled, but it forwards to DEBUG_PRINT*, which expands to nothing unless WLED_DEBUG is also defined. That drops the runtime/user-facing messages this PR is trying to expose over Net Debug. As per coding guidelines, Use DEBUG_PRINTF() / DEBUG_PRINTLN() for debug output in C++ (compiled out unless -D WLED_DEBUG).

Suggested fix
-#if defined(WLED_DEBUG) || defined(WLED_ENABLE_NET_DEBUG)
-  // use DEBUG_PRINT
+#if defined(WLED_DEBUG)
+  // use DEBUG_PRINT
   `#define` USER_PRINT(x) DEBUG_PRINT(x)
   `#define` USER_PRINTLN(x) DEBUG_PRINTLN(x)
   `#define` USER_PRINTF(x...) DEBUG_PRINTF(x)
+  `#ifdef` WLED_ENABLE_NET_DEBUG
+    `#define` USER_FLUSH() {}
+  `#else`
+    `#define` USER_FLUSH() {DEBUGOUTFlush();}
+  `#endif`
+#elif defined(WLED_ENABLE_NET_DEBUG)
+  // use net debug routing directly even when WLED_DEBUG is off
+  `#define` USER_PRINT(x) DEBUGOUT(x)
+  `#define` USER_PRINTLN(x) DEBUGOUTLN(x)
+  `#define` USER_PRINTF(x...) DEBUGOUTF(x)
   `#ifdef` WLED_ENABLE_NET_DEBUG
     `#define` USER_FLUSH() {}
   `#else`
     `#define` USER_FLUSH() {DEBUGOUTFlush();}
   `#endif`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wled00/wled.h` around lines 1019 - 1028, The USER_PRINT*, USER_PRINTLN, and
USER_PRINTF macros currently forward to DEBUG_PRINT* even when only
WLED_ENABLE_NET_DEBUG is defined, which makes them no-ops because DEBUG_PRINT is
compiled out unless WLED_DEBUG is set; change the macro logic so that when
WLED_DEBUG is defined USER_PRINT*/USER_PRINTF map to DEBUG_PRINT*/DEBUG_PRINTF,
but when only WLED_ENABLE_NET_DEBUG is defined they map to the Net-Debug output
functions (do not forward to DEBUG_PRINT); also ensure USER_FLUSH uses the
Net-Debug flush in that branch (keep references to USER_PRINT, USER_PRINTLN,
USER_PRINTF, DEBUG_PRINT, WLED_ENABLE_NET_DEBUG, WLED_DEBUG, USER_FLUSH,
DEBUGOUTFlush to locate and update the macros).

#else
// if serial is available, we use Serial.print directly
#define USER_PRINT(x) { if (canUseSerial()) DEBUGOUT(x); }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai where is the implementation of ``canUseSerial()`?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "canUseSerial" --type=cpp --type=c -C3

Length of output: 501


🏁 Script executed:

#!/bin/bash
rg -n "canUseSerial" -C3

Length of output: 501


🏁 Script executed:

#!/bin/bash
# Search in .ino files (not picked up by default rg type filters)
rg -n "canUseSerial" --glob="*.ino" -C3
echo "---"
# Also check fcn_declare.h for declaration
grep -n "canUseSerial" wled00/fcn_declare.h
echo "---"
# Check all files including hidden
rg -n "canUseSerial" --no-ignore -C3

Length of output: 509


🏁 Script executed:

#!/bin/bash
# Check if canUseSerial exists on main branch
git fetch origin main 2>/dev/null
git grep -n "canUseSerial" origin/main

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look for any serial availability/guard helper functions
rg -n "bool.*[Ss]erial" wled00/ -C2
echo "---"
# Check if serialCanRX / serialCanTX are used as guards
rg -n "serialCan" wled00/ -C2

Length of output: 12284


@softhack007canUseSerial() does not exist anywhere in this repository. It is referenced in the newly added USER_PRINT/USER_PRINTLN/USER_PRINTF macros (Lines 1030–1032 of wled.h) that are active in the default build (when neither WLED_DEBUG nor WLED_ENABLE_NET_DEBUG is defined), but there is no declaration or definition of it in any .h, .cpp, or .ino file on this branch — or on main.

This would cause a compilation error for a standard build. The existing WLED idiom for guarding serial output is the global boolean serialCanTX (declared in wled.h line 529 and set in wled.cpp lines 524–525 based on pin-allocation checks), which is already used consistently throughout wled_serial.cpp and ir.cpp.

The macros should likely read:

`#define` USER_PRINT(x)      { if (serialCanTX) Serial.print(x); }
`#define` USER_PRINTLN(x)    { if (serialCanTX) Serial.println(x); }
`#define` USER_PRINTF(x...)  { if (serialCanTX) Serial.printf(x); }

(or route through DEBUGOUT/DEBUGOUTLN/DEBUGOUTF if that's preferred, but still guarded by serialCanTX).


🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Try to fetch the WLED-MM canUseSerial function from raw GitHub
curl -s "https://raw.githubusercontent.com/MoonModules/WLED-MM/459a66491d7a3993ef456eb5cbfaad4c9d806f3b/wled00/wled_serial.cpp" | sed -n '80,110p'

Length of output: 1794


@softhack007 — confirmed, that's exactly the missing piece. The function retrieved from WLED-MM is richer than a simple serialCanTX check — it guards against several serial-conflict scenarios:

bool canUseSerial(void) {
  // On S3/C3/C6/P4, USB CDC blocks if disconnected
  `#if` ARDUINO_USB_CDC_ON_BOOT && (defined(CONFIG_IDF_TARGET_ESP32S3) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6) || defined(CONFIG_IDF_TARGET_ESP32P4)) && !defined(WLED_ENABLE_NET_DEBUG)
  if (!Serial) return false;
  `#endif`
  // TX pin allocated to LEDs or other non-debug function
  if (pinManager.isPinAllocated(hardwareTX) && (pinManager.getPinOwner(hardwareTX) != PinOwner::DebugOut))
    return false;
  // Serial in use for Adalight / TPM2NET / generic realtime streaming
  if ((realtimeMode == REALTIME_MODE_GENERIC) || (realtimeMode == REALTIME_MODE_ADALIGHT) || (realtimeMode == REALTIME_MODE_TPM2NET))
    return false;
  // Don't interfere with IMPROV
  if (improvActive > 0) return false;
  // Continuous Serial Streaming active
  if (continuousSendLED == true) return false;
  return true;
}

Two things to note for the port:

  1. The !defined(WLED_DEBUG_HOST) guard in the USB-CDC check should be updated to !defined(WLED_ENABLE_NET_DEBUG) to match the new flag used in this PR.
  2. The function needs to be declared in fcn_declare.h (or equivalent) so it is visible when wled.h macros expand it — that declaration is also absent from this PR.

Without both the definition and the declaration, any build that doesn't define WLED_DEBUG or WLED_ENABLE_NET_DEBUG will fail to compile at the USER_PRINT/USER_PRINTLN/USER_PRINTF macros.


✏️ Learnings added
Learnt from: softhack007
Repo: wled/WLED PR: 5559
File: wled00/wled.h:1030-1030
Timestamp: 2026-05-03T11:55:40.143Z
Learning: In WLED (wled00/wled_serial.cpp), the function `canUseSerial()` (ported from WLED-MM) checks whether Serial output is safe to use: it blocks on disconnected USB-CDC (S3/C3/C6/P4 with ARDUINO_USB_CDC_ON_BOOT), TX pin conflicts, realtime modes GENERIC/ADALIGHT/TPM2NET, improvActive > 0, and continuousSendLED. This function must be declared in fcn_declare.h and defined in wled_serial.cpp. The upstream WLED main branch does NOT have this function — it only has the simpler `serialCanTX` boolean. The WLED_ENABLE_NET_DEBUG guard (not WLED_DEBUG_HOST) should be used inside it.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the minimal replacement could be checking if (serialCanTX

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@softhack007 — agreed, serialCanTX is the right minimal replacement here. The three affected macros in the #else branch become:

  // if serial is available, we use Serial.print directly
  `#define` USER_PRINT(x)      { if (serialCanTX) Serial.print(x); }
  `#define` USER_PRINTLN(x)    { if (serialCanTX) Serial.println(x); }
  `#define` USER_PRINTF(x...)  { if (serialCanTX) Serial.printf(x); }
  `#define` USER_FLUSH()       { Serial.flush(); }

This avoids the missing-function compile error and stays consistent with how serialCanTX is already used throughout wled_serial.cpp and ir.cpp. No declaration/definition changes are needed in fcn_declare.h or wled_serial.cpp.


✏️ Learnings added
Learnt from: softhack007
Repo: wled/WLED PR: 5559
File: wled00/wled.h:1030-1030
Timestamp: 2026-05-03T11:57:15.014Z
Learning: In WLED (wled00/wled.h), the USER_PRINT/USER_PRINTLN/USER_PRINTF macros (active when neither WLED_DEBUG nor WLED_ENABLE_NET_DEBUG is defined) should guard serial output with `if (serialCanTX)` — i.e., `#define USER_PRINT(x) { if (serialCanTX) Serial.print(x); }`. The `canUseSerial()` function from WLED-MM is NOT present in upstream WLED and must NOT be referenced here; use the existing `serialCanTX` boolean instead.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

#define USER_PRINTLN(x) { if (canUseSerial()) DEBUGOUTLN(x); }
#define USER_PRINTF(x...) { if (canUseSerial()) DEBUGOUTF(x); }
#define USER_FLUSH() {DEBUGOUTFlush();}
#endif


#ifdef WLED_DEBUG_FS
#define DEBUGFS_PRINT(x) DEBUGOUT.print(x)
#define DEBUGFS_PRINTLN(x) DEBUGOUT.println(x)
#define DEBUGFS_PRINTF(x...) DEBUGOUT.printf(x)
#define DEBUGFS_PRINT(x) DEBUGOUT(x)
#define DEBUGFS_PRINTLN(x) DEBUGOUTLN(x)
#define DEBUGFS_PRINTF(x...) DEBUGOUTF(x)
#else
#define DEBUGFS_PRINT(x)
#define DEBUGFS_PRINTLN(x)
Expand Down
11 changes: 10 additions & 1 deletion wled00/xml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ static void appendGPIOinfo(Print& settingsScript)
settingsScript.print(2); // DMX hardcoded pin
firstPin = false;
#endif
#if defined(WLED_DEBUG) && !defined(WLED_DEBUG_HOST)
#if defined(WLED_DEBUG) && !defined(WLED_ENABLE_NET_DEBUG)
if (!firstPin) settingsScript.print(',');
settingsScript.print(hardwareTX); // debug output (TX) pin
firstPin = false;
Expand Down Expand Up @@ -586,6 +586,15 @@ void getSettingsJS(byte subPage, Print& settingsScript)
#ifndef WLED_ENABLE_ADALIGHT
settingsScript.print(F("toggle('Serial');"));
#endif

#ifdef WLED_ENABLE_NET_DEBUG
sappend('v',SET_F("N0"),netDebugPrintIP[0]);
sappend('v',SET_F("N1"),netDebugPrintIP[1]);
sappend('v',SET_F("N2"),netDebugPrintIP[2]);
sappend('v',SET_F("N3"),netDebugPrintIP[3]);
sappend('v',SET_F("NP"),netDebugPrintPort);
#endif

}

if (subPage == SUBPAGE_TIME)
Expand Down
Loading