Skip to content

Add independent Ethernet/WiFi IP config and primary network interface selection#5650

Draft
kelvtech-co-uk wants to merge 2 commits into
wled:mainfrom
kelvtech-co-uk:feature/eth-dual-interface
Draft

Add independent Ethernet/WiFi IP config and primary network interface selection#5650
kelvtech-co-uk wants to merge 2 commits into
wled:mainfrom
kelvtech-co-uk:feature/eth-dual-interface

Conversation

@kelvtech-co-uk
Copy link
Copy Markdown

@kelvtech-co-uk kelvtech-co-uk commented May 24, 2026

Caution

The majority of this code was generated using claude.ai, I don't have the skills or experience in langages to do this myself unfortunately. However I'm keen to learn so feedback welcomed on how I can improve all of this.

Motivation

I needed a low latency network connection between my Gledopto GL-C-618WL and my media player running Hyperion. However I was unable to provide a wired LAN connection from my network switch due to both devices location. I hacked a solution by modifying my media players CoreELEC network configuration and directly cabling the devices. In this configuration WLED was only useable whilst my media player was turned on as it became the router between my main LAN and the stub network I hacked into CoreELEC. I wanted a better solution and modifying WLED's firmware was easier than modifying my house to run a cable.

Core Changes

  • Add separate static IP/gateway/subnet configuration for Ethernet interface independent of WiFi static IP configuration
  • Add primary network interface selection to control which interface lwIP uses for outbound connections (MQTT, NTP, internet access)
  • Implement setPrimaryNetworkInterface() using netif_set_default() to set lwIP default netif based on user selection
  • Keep both interfaces active simultaneously when Ethernet connects
  • Show both interface IPs in UI when both are active
  • Update WiFi & Network settings UI with Ethernet IP config section and Primary Network Interface selector
  • Ethernet IP config section hidden when Ethernet Type is None

Resolves

Issue #5247 - ethernet static IP not applying independently of WiFi config

Related

Issue #4703 - ESP-NOW + Ethernet GPIO17 PLL instability (referenced in known limitations)

File Changes

  • wled00/wled.h — new global variables
  • wled00/cfg.cpp — serialisation/deserialisation of new ethernet IP config
  • wled00/set.cpp — form POST handler for new UI fields
  • wled00/xml.cpp — s.js population of new UI fields, dual IP display
  • wled00/network.cpp — ETH.config() with new variables, setPrimaryNetworkInterface(), WiFi kept active on ETH connect
  • wled00/wled.cpp — WiFi config() update, setPrimaryNetworkInterface() call in handleConnection()
  • wled00/fcn_declare.h — declaration of setPrimaryNetworkInterface()
  • wled00/data/settings_wifi.htm — Ethernet IP Configuration section, Primary Network Interface selector

Known limitations

  • mDNS resolves to primary interface IP only; per-interface mDNS hostname registration is a candidate for follow-on work
  • Dual interface operation has been validated on boards using ETH_CLOCK_GPIO0_IN (external clock source). Boards using ETH_CLOCK_GPIO17_OUT may experience instability when WiFi and Ethernet operate simultaneously due to ESP32 PLL instability described in ESP-NOW must be disabled on ESP32 Ethernet boards that use ESP32 as PHY clock source; WLED-AP is affected too! #4703. A future enhancement could detect the board clock mode and conditionally disable WiFi on affected boards, preserving existing single-interface behaviour for those users.

Testing

Tested on:

  • WT32-ETH01 (ETH_CLOCK_GPIO0_IN) — dev board, dual interface operation validated, both IPs accessible simultaneously from clients on both subnets
  • Gledopto GL-C-618WL (ETH_CLOCK_GPIO0_IN) — production hardware, OTA updated, ambilight use case validated

Test scenarios covered:

  • Static IP persistence across reboot
  • Both interfaces accessible from clients on respective subnets
  • Primary network interface selection verified via serial debug output confirming correct netif selection (setPNI: log lines) and practical network reachability tests from clients on both of my subnets
  • WiFi-only builds unaffected (all new code inside WLED_USE_ETHERNET guard)
  • Existing WiFi static IP configuration unaffected

Summary by CodeRabbit

  • New Features

    • Independent Ethernet static-IP (address, gateway, subnet) with UI controls shown when Ethernet is enabled.
    • Option to select WiFi or Ethernet as the primary network interface.
    • Network status displays both active interfaces; DNS/mDNS label renamed to “Active IP(s)”.
    • WiFi static-IP accepts and persists a 0.0.0.0 gateway where applicable.
  • Bug Fixes / Reliability

    • More reliable primary-interface selection and improved handling as interfaces connect, change, or disconnect.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds Ethernet static IP/GW/SN state and persistence, UI to configure Ethernet IP and choose a primary interface, init/event wiring to apply static Ethernet addressing, and lwIP-based default-netif selection for ESP32 dual-interface setups.

Changes

Ethernet Static IP & Primary Interface Configuration

Layer / File(s) Summary
State definitions and function declarations
wled00/wled.h, wled00/fcn_declare.h
New globals: ethStaticIP, ethStaticGW, ethStaticSN, ethPrimaryInterface; declaration of setPrimaryNetworkInterface() for ESP32+Ethernet.
Configuration persistence
wled00/cfg.cpp
Deserialize eth.eip/eth.egw/eth.esn into stored octets and eth.epi into ethPrimaryInterface; serialize the same fields back to cfg.json.
Settings UI and form handling
wled00/data/settings_wifi.htm, wled00/set.cpp
Separate WiFi/Ethernet static IP UI; toggleEthIP() shows Ethernet IP block based on board selection; form POST parsing populates ethStaticIP/ethStaticGW/ethStaticSN and ethPrimaryInterface.
Network primary interface management
wled00/network.cpp
Adds lwIP headers; initEthernet() supports static Ethernet addressing with DHCP fallback; adds setPrimaryNetworkInterface() to set lwIP default netif by name prefix when up/has IPv4; maps ARDUINO_EVENT_ETH_GOT_IP and re-selects on network events.
Integration into WiFi/Ethernet initialization
wled00/wled.cpp
Apply WiFi static config when staticIP is set; call setPrimaryNetworkInterface() during initInterfaces() and on newly connected handleConnection() path to ensure selection after interfaces initialize.
Settings display and dual-interface status
wled00/xml.cpp
Populate Ethernet octet fields and primary radio in settings JS; update connected-IP display to show both interface IPs when present or label Ethernet appropriately; increased buffer for formatting.

Sequence Diagram

sequenceDiagram
  participant User as User / Web UI
  participant Form as Settings Form
  participant SetHandler as handleSettingsSet()
  participant Config as cfg.json
  participant InitEth as initEthernet()
  participant Event as Network Events
  participant SetPrimary as setPrimaryNetworkInterface()
  participant lwIP as lwIP Stack
  User->>Form: Enter Ethernet static IP & select primary
  User->>Form: Submit settings
  Form->>SetHandler: POST EIP*, EGW*, ESN*, EPI
  SetHandler->>SetHandler: Parse octets into ethStaticIP/GW/SN
  SetHandler->>Config: Serialize to cfg.json
  InitEth->>InitEth: Read ethStaticIP, call ETH.config()
  Event->>SetPrimary: ETH_CONNECTED / ETH_GOT_IP / WIFI_STA_GOT_IP
  SetPrimary->>SetPrimary: Check ethPrimaryInterface, find en/st netif
  SetPrimary->>lwIP: netif_set_default(selected)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • wled/WLED#5436: Overlaps on Ethernet event handling and hostname setting changes.
  • wled/WLED#4609: Related cfg.json serialization changes touching similar config code paths.
  • wled/WLED#5262: Related adjustments to Ethernet static-IP application timing and ETH.config placement.

Suggested labels

connectivity

Suggested reviewers

  • netmindz
  • softhack007
  • DedeHai
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main feature: independent Ethernet/WiFi IP configuration and primary network interface selection, which accurately reflects the core changes across all modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with 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.

Inline comments:
In `@wled00/network.cpp`:
- Around line 314-353: The function setPrimaryNetworkInterface walks netif_list
and calls netif_set_default without holding the lwIP TCP/IP core lock, causing
thread-safety assertions under CONFIG_LWIP_CHECK_THREAD_SAFETY; wrap the
traversal and the netif_set_default call with the lwIP core lock (include
"lwip/tcpip.h" and call LOCK_TCPIP_CORE(); ... UNLOCK_TCPIP_CORE()) to protect
access to netif_list and netif_set_default, and ensure any early returns still
release the lock; additionally make the interface-name matching in
setPrimaryNetworkInterface more robust by falling back to checking netif_is_up()
and a non-zero IPv4 address if the 'en'/'st' prefix check on netif->name[0..1]
fails.

In `@wled00/set.cpp`:
- Around line 165-169: The code reads the wrong POST key ("EGI") when setting
ethPrimaryInterface, so ethPrimaryInterface always stays false; update the
request->arg call used to set ethPrimaryInterface to use the correct form key
"EPI" (i.e., change the argument passed to request->arg in the
ethPrimaryInterface assignment), keep the same toInt() == 1 check and leave
initEthernet() as-is so the corrected value persists and Ethernet initialization
runs with the intended primary-interface selection.

In `@wled00/wled.cpp`:
- Around line 889-901: There are duplicate assignments to interfacesInited and
wasConnected; remove the first pair (the assignments immediately before the `#if`
block) and keep the single assignment after the ARDUINO_ARCH_ESP32 &&
WLED_USE_ETHERNET conditional so interfacesInited and wasConnected are set once
(leave setPrimaryNetworkInterface() and the final interfacesInited = true;
wasConnected = true; intact).

In `@wled00/xml.cpp`:
- Around line 322-334: The current dual-IP check uses the config flag
ethStaticIP (symbol: ethStaticIP) which hides DHCP-assigned Ethernet addresses;
change the condition to use live interface IPs instead — e.g., test
ETH.localIP() and WiFi.localIP() (or their uint32_t equivalents) rather than
ethStaticIP/ethernetType so both active interfaces are detected; update the
branch that constructs the IP string (functions/variables: ETH.localIP(),
WiFi.localIP(), Network.localIP(), Network.isEthernet()) to rely on those
runtime checks and remove dependence on ethStaticIP.
🪄 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: 25568589-10b6-4906-b56c-039e39bb4f83

📥 Commits

Reviewing files that changed from the base of the PR and between 0f34973 and f9320d8.

📒 Files selected for processing (8)
  • wled00/cfg.cpp
  • wled00/data/settings_wifi.htm
  • wled00/fcn_declare.h
  • wled00/network.cpp
  • wled00/set.cpp
  • wled00/wled.cpp
  • wled00/wled.h
  • wled00/xml.cpp

Comment thread wled00/network.cpp
Comment thread wled00/set.cpp Outdated
Comment thread wled00/wled.cpp Outdated
Comment thread wled00/xml.cpp Outdated
@softhack007 softhack007 added the AI Partly generated by an AI. Make sure that the contributor fully understands the code! label May 24, 2026
@kelvtech-co-uk kelvtech-co-uk force-pushed the feature/eth-dual-interface branch from f9320d8 to f021c81 Compare May 24, 2026 14:09
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
wled00/set.cpp (1)

165-169: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix primary-interface POST key mismatch (EGI vs EPI).

Line 168 reads EGI, but the form submits EPI, so the primary-interface selection does not persist correctly.

Suggested fix
-    ethPrimaryInterface = (request->arg(F("EGI")).toInt() == 1);
+    ethPrimaryInterface = (request->arg(F("EPI")).toInt() == 1);
🤖 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/set.cpp` around lines 165 - 169, The code reads the wrong POST key
("EGI") so the primary-interface selection doesn't persist; update the logic
that sets ethPrimaryInterface (currently using request->arg(F("EGI")).toInt() ==
1) to use the correct form key "EPI" and also guard with
request->hasArg(F("EPI")) (or equivalent) to handle radio-button submission
semantics before calling initEthernet(); adjust only the key and add the hasArg
check in the ethPrimaryInterface assignment site.
🧹 Nitpick comments (1)
wled00/fcn_declare.h (1)

303-306: ⚡ Quick win

Remove the duplicated initEthernet() declaration.

Line 303 already declares initEthernet(), so the duplicate at Line 305 is unnecessary and adds maintenance drift risk.

Suggested fix
 bool initEthernet(); // result is informational
 `#if` defined(ARDUINO_ARCH_ESP32) && defined(WLED_USE_ETHERNET)
-bool initEthernet();
 // byClaude: sets lwIP primary network interface based on ethPrimaryInterface
 // by enabling wled to be dual-homed we need to be deterministic about which interface
 // wled uses for outgoing connections to minimise asyncronous routing issues.
 void setPrimaryNetworkInterface();
 `#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/fcn_declare.h` around lines 303 - 306, Remove the duplicate
declaration of initEthernet(): keep a single prototype for the function (the one
already declared at the top) and delete the second repeated line that appears
with the comment "byClaude: sets lwIP primary network interface based on
ethPrimaryInterface", ensuring only one bool initEthernet(); remains in
fcn_declare.h to avoid redundancy and maintenance drift.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@wled00/wled.h`:
- Around line 393-402: Wrap the AI-generated network config declarations (the
block that defines ethStaticIP, ethStaticGW, ethStaticSN and
ethPrimaryInterface) with the required C++ comment delimiters, e.g. add a
starting comment like "// AI: below section was generated by an AI" immediately
before the WLED_GLOBAL ethStaticIP declaration and an ending comment like "//
AI: end" immediately after the WLED_GLOBAL ethPrimaryInterface line so the
entire block (ethStaticIP, ethStaticGW, ethStaticSN, ethPrimaryInterface) is
enclosed by the AI markers.

---

Duplicate comments:
In `@wled00/set.cpp`:
- Around line 165-169: The code reads the wrong POST key ("EGI") so the
primary-interface selection doesn't persist; update the logic that sets
ethPrimaryInterface (currently using request->arg(F("EGI")).toInt() == 1) to use
the correct form key "EPI" and also guard with request->hasArg(F("EPI")) (or
equivalent) to handle radio-button submission semantics before calling
initEthernet(); adjust only the key and add the hasArg check in the
ethPrimaryInterface assignment site.

---

Nitpick comments:
In `@wled00/fcn_declare.h`:
- Around line 303-306: Remove the duplicate declaration of initEthernet(): keep
a single prototype for the function (the one already declared at the top) and
delete the second repeated line that appears with the comment "byClaude: sets
lwIP primary network interface based on ethPrimaryInterface", ensuring only one
bool initEthernet(); remains in fcn_declare.h to avoid redundancy and
maintenance drift.
🪄 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: e368d689-67b4-4512-8b0c-8d79c30c8f59

📥 Commits

Reviewing files that changed from the base of the PR and between f9320d8 and f021c81.

📒 Files selected for processing (8)
  • wled00/cfg.cpp
  • wled00/data/settings_wifi.htm
  • wled00/fcn_declare.h
  • wled00/network.cpp
  • wled00/set.cpp
  • wled00/wled.cpp
  • wled00/wled.h
  • wled00/xml.cpp

Comment thread wled00/wled.h Outdated
@kelvtech-co-uk kelvtech-co-uk force-pushed the feature/eth-dual-interface branch from f021c81 to 9513309 Compare May 24, 2026 14:45
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
wled00/xml.cpp (1)

324-331: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use live Ethernet state for the dual-IP branch.

Line 324 keys this off ethStaticIP, so DHCP-assigned Ethernet sessions still fall back to the single-IP path even when ETH.localIP() is valid.

Suggested fix
-      if (ethernetType != WLED_ETH_NONE && (uint32_t)ethStaticIP != 0 &&
-          WiFi.localIP() != (uint32_t)0) {
-        // both interfaces active
-        IPAddress ethIP = ETH.localIP();
-        IPAddress wifiIP = WiFi.localIP();
+      IPAddress ethIP = ETH.localIP();
+      IPAddress wifiIP = WiFi.localIP();
+      if (ethernetType != WLED_ETH_NONE &&
+          ethIP != (uint32_t)0 &&
+          wifiIP != (uint32_t)0) {
+        // both interfaces active
         snprintf_P(s, sizeof(s), PSTR("%d.%d.%d.%d (ETH) / %d.%d.%d.%d (WiFi)"),
           ethIP[0], ethIP[1], ethIP[2], ethIP[3],
           wifiIP[0], wifiIP[1], wifiIP[2], wifiIP[3]);
🤖 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/xml.cpp` around lines 324 - 331, The dual-IP branch currently gates on
ethStaticIP which misses DHCP Ethernet addresses; change the condition to check
the actual live Ethernet IP (use ETH.localIP() or compare ETH.localIP() !=
(uint32_t)0) instead of ethStaticIP so DHCP-assigned ETH sessions take the
dual-IP path; update the if that references ethernetType, ethStaticIP and
WiFi.localIP() to use ETH.localIP() availability and keep the same snprintf_P
block that uses ETH.localIP() and WiFi.localIP().
🧹 Nitpick comments (1)
wled00/wled.cpp (1)

718-735: ⚡ Quick win

Wrap these AI-generated C++ blocks with the standard begin/end markers.

The inline // AI: notes don't satisfy the repo convention for larger generated sections.

As per coding guidelines: Embed larger blocks of AI-generated code within // AI: below section was generated by an AI ... // AI: end comments.

Also applies to: 890-896, 1001-1007

🤖 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.cpp` around lines 718 - 735, Wrap the AI-generated C++ sections
in wled.cpp with the repository's standard AI markers by adding a start comment
(e.g., "// AI: below section was generated by an AI - begin") immediately before
the block beginning at the if (multiWiFi[selectedWiFi].staticIP != 0U) that
contains the WiFi.config(...) and DEBUG_PRINTF_P(...) lines, and an end comment
(e.g., "// AI: end") immediately after that block; do the same for the other two
AI blocks referenced around lines 890-896 and 1001-1007 so that each generated
region that touches multiWiFi, WiFi.config, or DEBUG_PRINTF_P is clearly
delimited with the standard begin/end AI comments.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@wled00/network.cpp`:
- Around line 324-361: setPrimaryNetworkInterface() currently only picks the
preferred prefix (eth "en" or wifi "st") and leaves the lwIP default unchanged
if that interface is down or has no IP; modify it so after acquiring the TCPIP
core lock (LOCK_TCPIP_CORE()) you first search netif_list for a ready interface
matching the preferred prefix (using netif_is_up() and checking
netif_iter->ip_addr.u_addr.ip4.addr != 0), and if none found do a second-pass
over netif_list to select any other interface that is up and has an IP as a
fallback, then call netif_set_default(target) when you have a valid target; keep
the existing DEBUG_PRINTF_P/DEBUG_PRINTLN messages and use ip_addr.u_addr.ip4
with ip4_addr1..4 to log the chosen address.

---

Duplicate comments:
In `@wled00/xml.cpp`:
- Around line 324-331: The dual-IP branch currently gates on ethStaticIP which
misses DHCP Ethernet addresses; change the condition to check the actual live
Ethernet IP (use ETH.localIP() or compare ETH.localIP() != (uint32_t)0) instead
of ethStaticIP so DHCP-assigned ETH sessions take the dual-IP path; update the
if that references ethernetType, ethStaticIP and WiFi.localIP() to use
ETH.localIP() availability and keep the same snprintf_P block that uses
ETH.localIP() and WiFi.localIP().

---

Nitpick comments:
In `@wled00/wled.cpp`:
- Around line 718-735: Wrap the AI-generated C++ sections in wled.cpp with the
repository's standard AI markers by adding a start comment (e.g., "// AI: below
section was generated by an AI - begin") immediately before the block beginning
at the if (multiWiFi[selectedWiFi].staticIP != 0U) that contains the
WiFi.config(...) and DEBUG_PRINTF_P(...) lines, and an end comment (e.g., "//
AI: end") immediately after that block; do the same for the other two AI blocks
referenced around lines 890-896 and 1001-1007 so that each generated region that
touches multiWiFi, WiFi.config, or DEBUG_PRINTF_P is clearly delimited with the
standard begin/end AI comments.
🪄 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: e5f32be6-ef47-4688-87b3-7df31626be8f

📥 Commits

Reviewing files that changed from the base of the PR and between f021c81 and 9513309.

📒 Files selected for processing (8)
  • wled00/cfg.cpp
  • wled00/data/settings_wifi.htm
  • wled00/fcn_declare.h
  • wled00/network.cpp
  • wled00/set.cpp
  • wled00/wled.cpp
  • wled00/wled.h
  • wled00/xml.cpp

Comment thread wled00/network.cpp
… selection

- Add separate static IP/gateway/subnet configuration for Ethernet interface
  independent of WiFi static IP configuration
- Add primary network interface selection to control which interface lwIP
  uses for outbound connections (MQTT, NTP, internet access)
- Implement setPrimaryNetworkInterface() using netif_set_default() to set
  lwIP default netif based on user selection
- Keep both interfaces active simultaneously when Ethernet connects
- Show both interface IPs in UI when both are active
- Update WiFi & Network settings UI with Ethernet IP config section and
  Primary Network Interface selector
- Ethernet IP config section hidden when Ethernet Type is None

Resolves: ethernet static IP not applying independently of WiFi config
Related: wled#5247

Known limitations:
- mDNS resolves to primary interface IP only; per-interface mDNS hostname
  registration is a candidate for follow-on work
- Dual interface operation has been validated on boards using ETH_CLOCK_GPIO0_IN
  (external clock source). Boards using ETH_CLOCK_GPIO17_OUT may experience
  instability when WiFi and Ethernet operate simultaneously due to ESP32 PLL
  instability described in wled#4703. A future enhancement could detect the board
  clock mode and conditionally disable WiFi on affected boards, preserving
  existing single-interface behaviour for those users.
@kelvtech-co-uk kelvtech-co-uk force-pushed the feature/eth-dual-interface branch from 9513309 to 015e2e2 Compare May 24, 2026 15:12
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
wled00/wled.cpp (1)

718-737: ⚡ Quick win

Use the repo’s exact AI fence comments.

These markers add extra text (..., comments), so they no longer match the documented // AI: below section was generated by an AI / // AI: end format.

Suggested normalization
-// AI: below section was generated by an AI ...
+// AI: below section was generated by an AI
...
-// AI: end comments
+// AI: end

As per coding guidelines "Embed larger blocks of AI-generated code within // AI: below section was generated by an AI ... // AI: end comments".

Also applies to: 892-903, 1004-1013

🤖 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.cpp` around lines 718 - 737, The AI fence comments are not using
the repo's exact markers and include extra text; locate the AI-generated blocks
around the WiFi static IP code (identify by WiFi.config(...) usage and
multiWiFi[selectedWiFi] references) and replace any altered/extra comment lines
with the exact markers "// AI: below section was generated by an AI" at the
start and "// AI: end" at the end (no extra ellipses or words); apply the same
normalization to the other AI blocks noted (around the regions containing
WiFi.config/multiWiFi at the other ranges).
🤖 Prompt for all review comments with 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.

Inline comments:
In `@wled00/cfg.cpp`:
- Around line 138-153: Wrap the two AI-generated ethernet config blocks with the
repository's AI delimiters by adding the start marker comment before the block
that deserializes eth_ip/eth_gw/eth_sn and the end marker after the
CJSON(ethPrimaryInterface, ethernet[F("epi")]) line; specifically enclose the
code that references eth_ip, eth_gw, eth_sn, ethStaticIP, ethStaticGW,
ethStaticSN and the CJSON call for ethPrimaryInterface (ethernet[F("epi")])
between the comments "// AI: below section was generated by an AI" and "// AI:
end" so both the larger deserialization block and the primary interface line are
fully wrapped.

In `@wled00/network.cpp`:
- Around line 279-299: The diff contains large AI-generated blocks (e.g., the
ethernet static/DHCP block that calls ETH.config(ethStaticIP, ethStaticGW,
ethStaticSN, dnsAddress) and the corresponding DHCP branch using INADDR_NONE)
that are not wrapped with the required AI delimiters; ensure you surround each
such larger AI-generated section with the comment markers // AI: below section
was generated by an AI  and // AI: end (the function
setPrimaryNetworkInterface() is already wrapped, so apply the same wrapper to
the ethernet static/DHCP block shown and the other large AI-generated block
around the code referenced near the later region that corresponds to the 551-575
change), placing the start marker immediately before the AI block and the end
marker immediately after it so all large AI-inserted sections follow the coding
guideline.

In `@wled00/set.cpp`:
- Around line 150-168: Wrap the new Ethernet POST parsing block (the code that
checks request->hasArg(F("EIP0")) and fills ethStaticIP, ethStaticGW,
ethStaticSN, and sets ethPrimaryInterface) with the required AI section
delimiters by adding a leading comment "// AI: below section was generated by an
AI" immediately before the existing block and a trailing comment "// AI: end"
immediately after it, preserving all existing logic and indentation so the
parsing of EIP#, EGW#, ESN# and the EPI radio-button handling remains unchanged.

---

Nitpick comments:
In `@wled00/wled.cpp`:
- Around line 718-737: The AI fence comments are not using the repo's exact
markers and include extra text; locate the AI-generated blocks around the WiFi
static IP code (identify by WiFi.config(...) usage and multiWiFi[selectedWiFi]
references) and replace any altered/extra comment lines with the exact markers
"// AI: below section was generated by an AI" at the start and "// AI: end" at
the end (no extra ellipses or words); apply the same normalization to the other
AI blocks noted (around the regions containing WiFi.config/multiWiFi at the
other ranges).
🪄 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: 31dad564-0c87-47bb-a407-504dc4f3e459

📥 Commits

Reviewing files that changed from the base of the PR and between 9513309 and 015e2e2.

📒 Files selected for processing (8)
  • wled00/cfg.cpp
  • wled00/data/settings_wifi.htm
  • wled00/fcn_declare.h
  • wled00/network.cpp
  • wled00/set.cpp
  • wled00/wled.cpp
  • wled00/wled.h
  • wled00/xml.cpp

Comment thread wled00/cfg.cpp
Comment thread wled00/network.cpp
Comment thread wled00/set.cpp
Comment on lines +150 to +168
// AI: read ethernet static IP configuration from form POST.
// Each IP address is submitted as four separate octet fields (0-255),
// reassembled here into IPAddress objects matching the same pattern
// used for WiFi static IP fields (IP{n}0-3, GW{n}0-3, SN{n}0-3).
if (request->hasArg(F("EIP0"))) {
for (int i = 0; i < 4; i++) {
char eip[6], egw[6], esn[6];
snprintf_P(eip, sizeof(eip), PSTR("EIP%d"), i);
snprintf_P(egw, sizeof(egw), PSTR("EGW%d"), i);
snprintf_P(esn, sizeof(esn), PSTR("ESN%d"), i);
ethStaticIP[i] = request->arg(eip).toInt();
ethStaticGW[i] = request->arg(egw).toInt();
ethStaticSN[i] = request->arg(esn).toInt();
}
}
// AI: read primary network interface selection.
// PNI field value 0 = WiFi is primary interface, 1 = Ethernet is primary interface.
// Radio buttons only submit when selected so use hasArg with value check.
ethPrimaryInterface = (request->arg(F("EPI")).toInt() == 1);
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

Add required AI section delimiters around this generated C++ block.

The new ethernet POST parsing block is a larger AI-generated section and should be wrapped with the required begin/end AI markers.

Suggested fix
+    // AI: below section was generated by an AI
     // AI: read ethernet static IP configuration from form POST.
     // Each IP address is submitted as four separate octet fields (0-255),
     // reassembled here into IPAddress objects matching the same pattern
     // used for WiFi static IP fields (IP{n}0-3, GW{n}0-3, SN{n}0-3).
     if (request->hasArg(F("EIP0"))) {
       for (int i = 0; i < 4; i++) {
         char eip[6], egw[6], esn[6];
         snprintf_P(eip, sizeof(eip), PSTR("EIP%d"), i);
         snprintf_P(egw, sizeof(egw), PSTR("EGW%d"), i);
         snprintf_P(esn, sizeof(esn), PSTR("ESN%d"), i);
         ethStaticIP[i]  = request->arg(eip).toInt();
         ethStaticGW[i]  = request->arg(egw).toInt();
         ethStaticSN[i]  = request->arg(esn).toInt();
       }
     }
     // AI: read primary network interface selection.
     // PNI field value 0 = WiFi is primary interface, 1 = Ethernet is primary interface.
     // Radio buttons only submit when selected so use hasArg with value check.
     ethPrimaryInterface = (request->arg(F("EPI")).toInt() == 1);
+    // AI: end

As per coding guidelines "Embed larger blocks of AI-generated code within // AI: below section was generated by an AI ... // AI: end comments".

🤖 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/set.cpp` around lines 150 - 168, Wrap the new Ethernet POST parsing
block (the code that checks request->hasArg(F("EIP0")) and fills ethStaticIP,
ethStaticGW, ethStaticSN, and sets ethPrimaryInterface) with the required AI
section delimiters by adding a leading comment "// AI: below section was
generated by an AI" immediately before the existing block and a trailing comment
"// AI: end" immediately after it, preserving all existing logic and indentation
so the parsing of EIP#, EGW#, ESN# and the EPI radio-button handling remains
unchanged.

@softhack007
Copy link
Copy Markdown
Member

@kelvtech-co-uk force-pushed the feature/eth-dual-interface branch from 9513309 to 015e2e2

@kelvtech-co-uk one important rule - even when its non-technical - is to not force-push to an open PR on github. Reviewer comments sometimes vanish, and the review bot often loses track and stays in "nothing to review" forever.

https://github.com/wled/WLED?tab=contributing-ov-file#during-review

I'll restart the bot for this PR to fix it - you may receive some duplicate comments from coderabbit.

@softhack007
Copy link
Copy Markdown
Member

@coderabbitai full review

please create a prioritized list of suggested improvements and bugfixes after your review (major items only).

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

@softhack007 I’ll trigger a full review now. From the diff pass, the major prioritized items I’d focus on are:

  1. P0 — Settings changes may not apply until reboot

    • handleSettingsSet() saves Ethernet IP/primary-interface values and calls initEthernet() at wled00/set.cpp Line 169, but initEthernet() is designed as a one-shot initializer. Once Ethernet is already configured, later saves won’t re-run ETH.config().
    • Suggested fix: either explicitly require/recommend reboot after Ethernet IP changes, or detect changes and apply what can be applied immediately (setPrimaryNetworkInterface() for EPI, and a controlled Ethernet restart/reboot prompt for static IP changes).
  2. P0/P1 — Preprocessor guards are inconsistent

    • New globals are declared under ARDUINO_ARCH_ESP32 && WLED_USE_ETHERNET, but deserialization in wled00/cfg.cpp Lines 141-153 is only under WLED_USE_ETHERNET.
    • If any non-ESP32 build ever defines WLED_USE_ETHERNET, this can become a compile break. Suggested fix: use the same guard consistently for all new Ethernet-static-IP and primary-interface logic.
  3. P1 — setPrimaryNetworkInterface() relies on fragile lwIP netif names

    • wled00/network.cpp Lines 317-326 identifies interfaces by "en" and "st". That may work on tested platforms, but it is tied to Arduino-ESP32/IDF internals.
    • Suggested fix: prefer a stable ESP/Arduino API if available, or isolate this mapping behind a helper with clear fallback/logging and version notes.
  4. P1 — Dual-interface behavior needs an address-advertisement/multicast audit

    • Several existing paths still use Network.localIP() / Network.subnetMask() for advertised URLs, Art-Net/E1.31, UDP sysinfo, Improv, Espalexa/SSDP, and subnet checks.
    • With both interfaces active, these may advertise or bind to only one interface even though the UI shows both. Suggested fix: document current single-primary behavior, or update key discovery/multicast paths to be interface-aware where needed.
  5. P2 — Clean up generated-code comments before merge

    • There are many AI: / “generated by an AI” comments in production files. They add noise and may not match project style.
    • Suggested fix: replace with concise technical comments only where they explain non-obvious behavior.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== repo / branch ==="
git status --short --branch

echo
echo "=== changed files against base ==="
BASE_REF="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main 2>/dev/null || true)"
if [ -n "${BASE_REF}" ]; then
  git diff --name-status "${BASE_REF}"...HEAD
else
  git diff --name-status origin/main...HEAD || true
fi

echo
echo "=== unified diff for reviewed files ==="
if [ -n "${BASE_REF}" ]; then
  git diff --unified=80 "${BASE_REF}"...HEAD -- \
    wled00/cfg.cpp \
    wled00/data/settings_wifi.htm \
    wled00/fcn_declare.h \
    wled00/network.cpp \
    wled00/set.cpp \
    wled00/wled.cpp \
    wled00/wled.h \
    wled00/xml.cpp
else
  git diff --unified=80 origin/main...HEAD -- \
    wled00/cfg.cpp \
    wled00/data/settings_wifi.htm \
    wled00/fcn_declare.h \
    wled00/network.cpp \
    wled00/set.cpp \
    wled00/wled.cpp \
    wled00/wled.h \
    wled00/xml.cpp || true
fi

echo
echo "=== relevant symbol locations ==="
rg -n -C 5 'setPrimaryNetworkInterface|ethStaticIP|ethStaticGW|ethStaticSN|ethPrimaryInterface|ETH\.config|ETH_GOT_IP|WIFI_STA_GOT_IP|WiFi\.config|Network\.localIP|EIP0|EPI|toggleEthIP|ethip' wled00 || true

Length of output: 148514


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== repo / branch ==="
git status --short --branch

echo
echo "=== changed files against base ==="
BASE_REF="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main 2>/dev/null || true)"
if [ -n "${BASE_REF}" ]; then
  git diff --name-status "${BASE_REF}"...HEAD
else
  git diff --name-status origin/main...HEAD || true
fi

echo
echo "=== unified diff for reviewed files ==="
if [ -n "${BASE_REF}" ]; then
  git diff --unified=80 "${BASE_REF}"...HEAD -- \
    wled00/cfg.cpp \
    wled00/data/settings_wifi.htm \
    wled00/fcn_declare.h \
    wled00/network.cpp \
    wled00/set.cpp \
    wled00/wled.cpp \
    wled00/wled.h \
    wled00/xml.cpp
else
  git diff --unified=80 origin/main...HEAD -- \
    wled00/cfg.cpp \
    wled00/data/settings_wifi.htm \
    wled00/fcn_declare.h \
    wled00/network.cpp \
    wled00/set.cpp \
    wled00/wled.cpp \
    wled00/wled.h \
    wled00/xml.cpp || true
fi

echo
echo "=== relevant symbol locations ==="
rg -n -C 5 'setPrimaryNetworkInterface|ethStaticIP|ethStaticGW|ethStaticSN|ethPrimaryInterface|ETH\.config|ETH_GOT_IP|WIFI_STA_GOT_IP|WiFi\.config|Network\.localIP|EIP0|EPI|toggleEthIP|ethip' wled00 || true

Length of output: 148514


✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot added the connectivity Issue regarding protocols, WiFi connection or availability of interfaces label May 25, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
wled00/network.cpp (1)

190-195: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

initEthernet() one-shot guard blocks applying updated Ethernet config.

After first success, this early return prevents reapplying changed Ethernet IP/GW/SN from settings saves, so users can persist values that won’t take effect until reboot.

🤖 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/network.cpp` around lines 190 - 195, The static one-shot guard
successfullyConfiguredEthernet in initEthernet() prevents reapplying updated
Ethernet IP/GW/SN from settings; remove or change this behavior so new settings
take effect: either delete the early return and rely on initEthernet() being
idempotent, or replace the boolean check with a comparison between the saved
settings (IP, gateway, subnet) and the currently applied Ethernet values (e.g.,
Ethernet.localIP(), gatewayIP(), subnetMask()) and only skip re-init when they
match; alternatively, when settings are saved, reset/clear
successfullyConfiguredEthernet so initEthernet() will re-run. Ensure you update
initEthernet() and any settings-save code paths so changed IP/GW/SN are applied
without a reboot.
🧹 Nitpick comments (1)
wled00/xml.cpp (1)

335-340: 💤 Low value

Use snprintf_P for consistency and safety.

Lines 335 and 340 use sprintf() while line 330 uses snprintf_P(). For consistency and to prevent potential buffer overflows in future modifications, use snprintf_P throughout.

♻️ Suggested fix
       } else {
         IPAddress localIP = Network.localIP();
-        sprintf(s, "%d.%d.%d.%d", localIP[0], localIP[1], localIP[2], localIP[3]);
+        snprintf_P(s, sizeof(s), PSTR("%d.%d.%d.%d"), localIP[0], localIP[1], localIP[2], localIP[3]);
         if (Network.isEthernet()) strcat_P(s, PSTR(" (Ethernet)"));
       }
       `#else`
       IPAddress localIP = Network.localIP();
-      sprintf(s, "%d.%d.%d.%d", localIP[0], localIP[1], localIP[2], localIP[3]);
+      snprintf_P(s, sizeof(s), PSTR("%d.%d.%d.%d"), localIP[0], localIP[1], localIP[2], localIP[3]);
       `#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/xml.cpp` around lines 335 - 340, Replace the unsafe/ inconsistent
sprintf calls that format the IP into buffer s with snprintf_P to match the
existing usage (and avoid overflow); specifically change the two occurrences
that use sprintf(s, "%d.%d.%d.%d", localIP[0], ...) in xml.cpp to use
snprintf_P(s, sizeof buffer, PSTR("%d.%d.%d.%d"), localIP[0], ...), and when
appending the Ethernet label use strcat_P only after ensuring snprintf_P left
space (or include the label in the snprintf_P format when Network.isEthernet()
is true) so that formatting is done safely and consistently with the earlier
snprintf_P usage.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@wled00/cfg.cpp`:
- Around line 135-155: The deserialization block is only guarded by
WLED_USE_ETHERNET but accesses globals and functions declared under
ARDUINO_ARCH_ESP32 && WLED_USE_ETHERNET (ethStaticIP, ethStaticGW, ethStaticSN,
ethPrimaryInterface, initEthernet), which can break non-ESP32 builds; wrap the
portions that reference those ESP32-only symbols in a combined compile guard
(e.g., `#if` defined(ARDUINO_ARCH_ESP32) && defined(WLED_USE_ETHERNET)) or split
the logic so generic JSON parsing of ethernet["type"] remains under
WLED_USE_ETHERNET while the code that reads eth_ip/eth_gw/eth_sn, assigns
ethStaticIP/ethStaticGW/ethStaticSN, sets ethPrimaryInterface, and calls
initEthernet is only compiled when ARDUINO_ARCH_ESP32 is defined, ensuring no
ESP32-only globals are referenced on other platforms.

In `@wled00/network.cpp`:
- Around line 322-336: The selection is fragile because it matches
netif_iter->name against hardcoded prefixes via targetName; instead, track and
use esp-netif handles or impl names provided by ESP-NETIF APIs rather than
parsing lwIP netif->name. Replace the prefix check around netif_iter (and any
use of targetName) by either (a) keeping the esp_netif_t returned when creating
the WiFi/ethernet interfaces and comparing netif_iter to
esp_netif_get_netif_impl_handle(...) (i.e., use the stored esp_netif_t handle
for the preferred interface) or (b) call
esp_netif_get_netif_impl_name(esp_netif_handle) to get the canonical impl name
and compare that, and then call netif_set_default() under LOCK_TCPIP_CORE() for
the chosen esp_netif/netif; remove reliance on literal "st"/"en" checks
(symbols: targetName, netif_iter, netif_list, LOCK_TCPIP_CORE,
netif_set_default, esp_netif_get_netif_impl_name, esp_netif_t).

---

Outside diff comments:
In `@wled00/network.cpp`:
- Around line 190-195: The static one-shot guard successfullyConfiguredEthernet
in initEthernet() prevents reapplying updated Ethernet IP/GW/SN from settings;
remove or change this behavior so new settings take effect: either delete the
early return and rely on initEthernet() being idempotent, or replace the boolean
check with a comparison between the saved settings (IP, gateway, subnet) and the
currently applied Ethernet values (e.g., Ethernet.localIP(), gatewayIP(),
subnetMask()) and only skip re-init when they match; alternatively, when
settings are saved, reset/clear successfullyConfiguredEthernet so initEthernet()
will re-run. Ensure you update initEthernet() and any settings-save code paths
so changed IP/GW/SN are applied without a reboot.

---

Nitpick comments:
In `@wled00/xml.cpp`:
- Around line 335-340: Replace the unsafe/ inconsistent sprintf calls that
format the IP into buffer s with snprintf_P to match the existing usage (and
avoid overflow); specifically change the two occurrences that use sprintf(s,
"%d.%d.%d.%d", localIP[0], ...) in xml.cpp to use snprintf_P(s, sizeof buffer,
PSTR("%d.%d.%d.%d"), localIP[0], ...), and when appending the Ethernet label use
strcat_P only after ensuring snprintf_P left space (or include the label in the
snprintf_P format when Network.isEthernet() is true) so that formatting is done
safely and consistently with the earlier snprintf_P usage.
🪄 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: 88d30fab-1ff8-41b9-9d51-2f0a6ffb541f

📥 Commits

Reviewing files that changed from the base of the PR and between 68b61f0 and 302a3b6.

📒 Files selected for processing (8)
  • wled00/cfg.cpp
  • wled00/data/settings_wifi.htm
  • wled00/fcn_declare.h
  • wled00/network.cpp
  • wled00/set.cpp
  • wled00/wled.cpp
  • wled00/wled.h
  • wled00/xml.cpp

Comment thread wled00/cfg.cpp
Comment on lines 135 to 155
#ifdef WLED_USE_ETHERNET
JsonObject ethernet = doc[F("eth")];
CJSON(ethernetType, ethernet["type"]);
// NOTE: Ethernet configuration takes priority over other use of pins
// AI: deserialize ethernet static IP configuration.
// Each address is stored as a 4-element JSON array, one byte per octet,
// matching the same pattern used for WiFi static IP (nw.ins[n].ip/gw/sn).
JsonArray eth_ip = ethernet[F("eip")];
JsonArray eth_gw = ethernet[F("egw")];
JsonArray eth_sn = ethernet[F("esn")];
if (!eth_ip.isNull()) {
for (size_t i = 0; i < 4; i++) {
CJSON(ethStaticIP[i], eth_ip[i]);
CJSON(ethStaticGW[i], eth_gw[i]);
CJSON(ethStaticSN[i], eth_sn[i]);
}
}
// AI: deserialize primary network interface selection.
// false = WiFi is default gateway (default), true = Ethernet is default gateway.
CJSON(ethPrimaryInterface, ethernet[F("epi")]);
initEthernet();
#endif
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

Unify Ethernet compile guards in deserialization path.

This block is guarded only by WLED_USE_ETHERNET but references globals that are declared only under ARDUINO_ARCH_ESP32 && WLED_USE_ETHERNET. That can fail non-ESP32 builds when WLED_USE_ETHERNET is set.

Suggested fix
-#ifdef WLED_USE_ETHERNET
+#if defined(ARDUINO_ARCH_ESP32) && defined(WLED_USE_ETHERNET)
   JsonObject ethernet = doc[F("eth")];
   CJSON(ethernetType, ethernet["type"]);
   ...
   initEthernet();
 `#endif`

As per coding guidelines "Verify feature-flag names and ensure WLED_ENABLE_* and WLED_DISABLE_* flags exactly match the documented list to avoid silent build variations".

📝 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_USE_ETHERNET
JsonObject ethernet = doc[F("eth")];
CJSON(ethernetType, ethernet["type"]);
// NOTE: Ethernet configuration takes priority over other use of pins
// AI: deserialize ethernet static IP configuration.
// Each address is stored as a 4-element JSON array, one byte per octet,
// matching the same pattern used for WiFi static IP (nw.ins[n].ip/gw/sn).
JsonArray eth_ip = ethernet[F("eip")];
JsonArray eth_gw = ethernet[F("egw")];
JsonArray eth_sn = ethernet[F("esn")];
if (!eth_ip.isNull()) {
for (size_t i = 0; i < 4; i++) {
CJSON(ethStaticIP[i], eth_ip[i]);
CJSON(ethStaticGW[i], eth_gw[i]);
CJSON(ethStaticSN[i], eth_sn[i]);
}
}
// AI: deserialize primary network interface selection.
// false = WiFi is default gateway (default), true = Ethernet is default gateway.
CJSON(ethPrimaryInterface, ethernet[F("epi")]);
initEthernet();
#endif
`#if` defined(ARDUINO_ARCH_ESP32) && defined(WLED_USE_ETHERNET)
JsonObject ethernet = doc[F("eth")];
CJSON(ethernetType, ethernet["type"]);
// AI: deserialize ethernet static IP configuration.
// Each address is stored as a 4-element JSON array, one byte per octet,
// matching the same pattern used for WiFi static IP (nw.ins[n].ip/gw/sn).
JsonArray eth_ip = ethernet[F("eip")];
JsonArray eth_gw = ethernet[F("egw")];
JsonArray eth_sn = ethernet[F("esn")];
if (!eth_ip.isNull()) {
for (size_t i = 0; i < 4; i++) {
CJSON(ethStaticIP[i], eth_ip[i]);
CJSON(ethStaticGW[i], eth_gw[i]);
CJSON(ethStaticSN[i], eth_sn[i]);
}
}
// AI: deserialize primary network interface selection.
// false = WiFi is default gateway (default), true = Ethernet is default gateway.
CJSON(ethPrimaryInterface, ethernet[F("epi")]);
initEthernet();
`#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/cfg.cpp` around lines 135 - 155, The deserialization block is only
guarded by WLED_USE_ETHERNET but accesses globals and functions declared under
ARDUINO_ARCH_ESP32 && WLED_USE_ETHERNET (ethStaticIP, ethStaticGW, ethStaticSN,
ethPrimaryInterface, initEthernet), which can break non-ESP32 builds; wrap the
portions that reference those ESP32-only symbols in a combined compile guard
(e.g., `#if` defined(ARDUINO_ARCH_ESP32) && defined(WLED_USE_ETHERNET)) or split
the logic so generic JSON parsing of ethernet["type"] remains under
WLED_USE_ETHERNET while the code that reads eth_ip/eth_gw/eth_sn, assigns
ethStaticIP/ethStaticGW/ethStaticSN, sets ethPrimaryInterface, and calls
initEthernet is only compiled when ARDUINO_ARCH_ESP32 is defined, ensuring no
ESP32-only globals are referenced on other platforms.

Comment thread wled00/network.cpp
Comment on lines +322 to +336
// AI: interface name prefixes in arduino-esp32 IDF V4 (Tasmota platform):
// 'en' = ethernet, 'st' = WiFi STA. Validated on IDF 4.4.8.
const char *targetName = ethPrimaryInterface ?
"en" :
"st";

// AI: acquire lwIP TCP/IP core lock before accessing netif_list
// and calling netif_set_default() to avoid thread-safety assertions
LOCK_TCPIP_CORE();

for (netif_iter = netif_list; netif_iter != NULL; netif_iter = netif_iter->next) {
if (!netif_is_up(netif_iter) || netif_iter->ip_addr.u_addr.ip4.addr == 0) continue;
const bool isPreferred = (netif_iter->name[0] == targetName[0] &&
netif_iter->name[1] == targetName[1]);
if (isPreferred) {
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 | 🏗️ Heavy lift

🧩 Analysis chain

🌐 Web query:

For Arduino-ESP32/ESP-IDF, is there a documented guarantee that lwIP netif names use "st" for WiFi STA and "en" for Ethernet across supported versions? If not, what stable API should be used to identify the default WiFi/Ethernet netif?

💡 Result:

No—there is no documented, stable guarantee in ESP-IDF/Arduino-ESP32 that the underlying lwIP netif name will always use exactly “st” for WiFi STA and “en” for Ethernet across supported versions. In ESP-IDF, the recommended stable abstraction is ESP-NETIF: it creates and manages network interfaces and exposes handles for “default” WiFi (station/AP) and Ethernet interfaces. The ESP-NETIF API explicitly provides helpers for default WiFi STA/Ethernet initialization and returns an esp_netif_t handle, rather than requiring you to parse lwIP netif->name. For example, ESP-NETIF default Wi-Fi initialization is provided by functions that return an esp_netif handle (e.g., esp_netif_create_default_wifi_sta) and similarly Ethernet examples exist; the programming guide describes these as the convenient/default initialization paths. [1][2] If you still need to identify which underlying lwIP netif corresponds to an esp_netif_t, ESP-NETIF provides a stable API to query the “net interface name” used by the underlying TCP/IP stack implementation (lwIP): esp_netif_get_netif_impl_name(esp_netif_t *esp_netif, char *name). This is the API intended to read the implementation-specific name, not to rely on a fixed naming convention like “st”/“en”. [3] Stable approach to identify the default WiFi/Ethernet netif: 1) Use the ESP-NETIF default-interface creation APIs and keep the returned esp_netif_t handles. 2) If needed for compatibility/debugging, call esp_netif_get_netif_impl_name(handle, buf) to get the underlying lwIP name. Evidence: ESP-IDF’s ESP-NETIF documentation states that it “provides some convenient methods of creating the most common ones, such as Wi‑Fi and Ethernet” and that these APIs return an esp_netif handle configured with default settings. [1] Separately, the esp_netif API explicitly defines esp_netif_get_netif_impl_name as “Get net interface name from network stack implementation,” clarifying that the name comes from the underlying TCP/IP stack and is therefore implementation-/stack-dependent. [3] In contrast, the lwIP netif naming scheme (netif->name is only two characters plus index/num in classic lwIP) is an implementation detail of lwIP itself and is not presented by ESP-NETIF docs as a contractual guarantee to remain “st”/“en” forever. (So depending on netif->name for semantic identity is not a documented contract in ESP-IDF/ESP-NETIF.) [4]

Citations:


Fix fragile WiFi/Ethernet selection by removing hardcoded lwIP netif->name prefixes ("st"/"en")

wled00/network.cpp (lines 322-336) matches preferred interfaces by checking netif_iter->name against "st"/"en", but ESP-IDF/ESP-NETIF do not document a stable guarantee that lwIP will use those exact prefixes across versions/implementations—so primary selection can silently degrade to the wrong/fallback interface. Use ESP-NETIF’s default-interface creation APIs and keep the returned esp_netif_t handles (or map to the implementation-specific name via esp_netif_get_netif_impl_name()), instead of parsing netif->name.

🤖 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/network.cpp` around lines 322 - 336, The selection is fragile because
it matches netif_iter->name against hardcoded prefixes via targetName; instead,
track and use esp-netif handles or impl names provided by ESP-NETIF APIs rather
than parsing lwIP netif->name. Replace the prefix check around netif_iter (and
any use of targetName) by either (a) keeping the esp_netif_t returned when
creating the WiFi/ethernet interfaces and comparing netif_iter to
esp_netif_get_netif_impl_handle(...) (i.e., use the stored esp_netif_t handle
for the preferred interface) or (b) call
esp_netif_get_netif_impl_name(esp_netif_handle) to get the canonical impl name
and compare that, and then call netif_set_default() under LOCK_TCPIP_CORE() for
the chosen esp_netif/netif; remove reliance on literal "st"/"en" checks
(symbols: targetName, netif_iter, netif_list, LOCK_TCPIP_CORE,
netif_set_default, esp_netif_get_netif_impl_name, esp_netif_t).

@softhack007
Copy link
Copy Markdown
Member

P2 — Clean up generated-code comments before merge

  • There are many AI: / “generated by an AI” comments in production files. They add noise and may not match project style.
  • Suggested fix: replace with concise technical comments only where they explain non-obvious behavior.

@coderabbitai clarification: // AI: below section was generated by an AI comments are the project-specific convention to highlight AI-generated code blocks, these can be kept inside webUI files as they will be stripped out during the uglify/minify process during firmware builds. I agree that documentation inside these blocks needs to be understandable and relevant, both for maintainability and for a meaningful PR review.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

Tip

For best results, initiate chat on the files or code changes.

@softhack007 — noted and appreciated, thanks for the clarification. I'll revise the P2 item accordingly:

P2 (revised) — Review documentation quality within AI-generated blocks

  • The // AI: below section was generated by an AI markers are an intentional project convention for webUI files and will be stripped during uglify/minify — they should be kept as-is.
  • The valid concern is narrower: comments inside those blocks should be technically accurate, concise, and maintainable. Any comments that are redundant, inaccurate, or simply restate what the code already makes obvious should be cleaned up before merge.

(ᵔ◡ᵔ)∫


✏️ Learnings added
Learnt from: softhack007
URL: https://github.com/wled/WLED/pull/5650

Timestamp: 2026-05-25T20:45:31.250Z
Learning: In the WLED project (`wled/WLED`), `// AI: below section was generated by an AI` comments in webUI files (e.g., `wled00/data/settings_wifi.htm`) are an intentional project convention to highlight AI-generated code blocks. These comments are stripped out during the uglify/minify firmware build process and should not be flagged for removal. Only the *quality and accuracy of documentation within those blocks* is subject to review.

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.

@DedeHai DedeHai marked this pull request as draft May 26, 2026 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Partly generated by an AI. Make sure that the contributor fully understands the code! connectivity Issue regarding protocols, WiFi connection or availability of interfaces enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants