W5500 Support for ALL boards #320
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds W5500-based Ethernet board types and UI options; extends EthernetSettings with SPI and RMII pin fields and PHY/clock macros; conditionally uses ETHClass2 and exposes a global Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Configuration
participant Init as Initialization (wled.cpp)
participant PinMgr as Pin Manager
participant ETH as ETH (ETHClass2 / ETH)
participant Log as Logger
rect rgba(100,150,200,0.5)
Note over Init,ETH: W5500 (SPI) Path
Init->>Config: Check eth_type == W5500 / CONFIG_ETH_SPI_ETHERNET_W5500
Config-->>Init: W5500 config
Init->>PinMgr: Allocate SPI pins (MISO,MOSI,CS,RST,INT,SCLK)
PinMgr-->>Init: allocated / failure
Init->>ETH: ETH.begin(SPI3_HOST, pins...)
ETH-->>Init: success / failure
alt Success
Init->>Log: Log W5500 success
Init->>Init: Mark Ethernet configured
else Failure
Init->>PinMgr: Deallocate pins
Init->>Log: Log W5500 failure
end
end
rect rgba(200,100,100,0.5)
Note over Init,ETH: RMII Path (CONFIG_ETH_PHY_INTERFACE_RMII)
Init->>Config: Check RMII config & clock mode
Config-->>Init: RMII pin mapping
Init->>PinMgr: Allocate RMII pins (non-configurable + MDC/MDIO/POWER/CLK)
PinMgr-->>Init: allocated / failure
Init->>ETH: ETH.begin(eth_type, address, MDC, MDIO, ...)
ETH-->>Init: success / failure (link/speed/duplex/MAC)
alt Success
Init->>Log: Log RMII success (link/speed/duplex/MAC)
Init->>Init: Mark Ethernet configured
else Failure
Init->>PinMgr: Deallocate pins
Init->>Log: Log RMII failure
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Checks will all fail because the exiting build environment is not prepped. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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)
13-24: Fix array type mismatch foresp32_nonconfigurable_ethernet_pins.
The non‑RMII definition uses an empty initializer with unspecified size, which conflicts with the fixed‑size extern declaration and can break compilation. Make the definition match the declared size (or guard the declaration in the header).🛠️ Suggested fix
`#ifdef` CONFIG_ETH_PHY_INTERFACE_RMII const managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT] = { { 21, true }, // RMII EMAC TX EN == When high, clocks the data on TXD0 and TXD1 to transmitter { 19, true }, // RMII EMAC TXD0 == First bit of transmitted data { 22, true }, // RMII EMAC TXD1 == Second bit of transmitted data { 25, false }, // RMII EMAC RXD0 == First bit of received data { 26, false }, // RMII EMAC RXD1 == Second bit of received data { 27, true }, // RMII EMAC CRS_DV == Carrier Sense and RX Data Valid }; `#else` -const managed_pin_type esp32_nonconfigurable_ethernet_pins[] = {}; +const managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT] = {}; `#endif`
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 28-30: Remove the unused "inliner" dependency declaration from
package.json and also remove its entry from package-lock.json; verify that
tools/cdata.js imports only "web-resource-inliner" (not "inliner") and then run
npm install (or npm ci) to regenerate the lockfile so package-lock.json no
longer contains "inliner". Ensure package.json still lists
"web-resource-inliner" and that no other files import or require "inliner"
before committing.
🧹 Nitpick comments (3)
wled00/src/dependencies/network/Network.h (1)
5-7: Include placement outside header guard may cause issues.The
ETHClass2.hinclude andextern ETHClass2 ETHdeclaration are placed before the#ifndef Network_hinclude guard (lines 10-11). This means every translation unit that includesNetwork.hwill re-process these lines. WhileETHClass2.hlikely has its own guards, theexterndeclaration placement is unconventional and could cause confusion.Consider moving these inside the include guard for consistency:
Suggested restructure
+#ifndef Network_h +#define Network_h + `#ifdef` ESP8266 `#include` <ESP8266WiFi.h> `#else` // ESP32 `#include` <WiFi.h> - // `#include` <ETH.h> `#include` "ETHClass2.h" extern ETHClass2 ETH; `#endif` -#ifndef Network_h -#define Network_h - class NetworkClasswled00/data/settings_wifi.htm (1)
214-216: Indentation uses spaces instead of tabs.The new options use spaces for indentation, but per coding guidelines, web UI files under
wled00/data/should use tabs for indentation. The existing options in this file use tabs.As per coding guidelines, after editing web UI files, run
npm run buildto regenerate embedded headers.Fix indentation to use tabs
- <option value="15">TTGO T-ETH-Lite S3 (W5500) ☾</option> <!-- WLEDMM --> - <option value="16">WaveShare ESP32-S3-ETH (W5500) ☾</option> <!-- WLEDMM --> - <option value="17">W5500 Generic ☾</option> <!-- WLEDMM --> + <option value="15">TTGO T-ETH-Lite S3 (W5500) ☾</option> <!-- WLEDMM --> + <option value="16">WaveShare ESP32-S3-ETH (W5500) ☾</option> <!-- WLEDMM --> + <option value="17">W5500 Generic ☾</option> <!-- WLEDMM -->wled00/wled.h (1)
356-356: Global ETH instance declaration follows WLED pattern.The
WLED_GLOBAL ETHClass2 ETH;declaration follows the established pattern for WLED global variables. This works correctly with theextern ETHClass2 ETH;declaration inNetwork.h.Consider grouping this with other network-related globals (near line 645 with
udpConnectedetc.) for better organization, though this is a minor nitpick.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
package.jsonwled00/const.hwled00/data/settings_wifi.htmwled00/network.cppwled00/src/dependencies/network/Network.hwled00/wled.cppwled00/wled.hwled00/wled_ethernet.h
🧰 Additional context used
📓 Path-based instructions (3)
wled00/**/*.{cpp,h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ firmware source and headers in wled00
Files:
wled00/const.hwled00/wled.cppwled00/src/dependencies/network/Network.hwled00/network.cppwled00/wled.hwled00/wled_ethernet.h
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**/*.{htm,html,css,js}: Use tabs for indentation in web UI files (.htm/.html/.css/.js) under wled00/data
After editing web UI files, run npm run build to regenerate embedded headers
Manually validate web UI changes via local HTTP server and browser
Files:
wled00/data/settings_wifi.htm
wled00/data/**/*.{htm,html,js,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Check the browser console for JavaScript errors after web UI changes
Files:
wled00/data/settings_wifi.htm
🧠 Learnings (9)
📚 Learning: 2025-10-22T21:50:25.839Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR `#270`, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.
Applied to files:
wled00/const.h
📚 Learning: 2025-10-12T21:55:59.766Z
Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-12T21:55:59.766Z
Learning: Applies to wled00/html_*.h : Do not edit generated embedded web header files: wled00/html_*.h
Applied to files:
wled00/const.hwled00/data/settings_wifi.htmwled00/wled.hwled00/wled_ethernet.h
📚 Learning: 2025-12-21T22:26:03.261Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 293
File: wled00/udp.cpp:668-669
Timestamp: 2025-12-21T22:26:03.261Z
Learning: In MoonModules/WLED-MM on 32-bit ESP32 platforms, changing pixel index variables to `int` type to align with the `setPixelColor(int, ...)` API signature is intentional and safe. Maximum pixel addresses are bounded by UINT16_MAX, and `setPixelColor()` performs internal overflow checks.
Applied to files:
wled00/const.hwled00/wled.h
📚 Learning: 2025-12-28T09:48:59.612Z
Learnt from: netmindz
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-12-28T09:48:59.612Z
Learning: In QEMU mode with WLED_QEMU flag, Network.cpp must check tcpip_adapter directly instead of using the ETH object, because ETH.begin() is skipped to avoid hardware register crashes. Without this, Network.isConnected() always returns false, preventing initInterfaces() and server.begin() from being called.
Applied to files:
wled00/wled.cppwled00/src/dependencies/network/Network.hwled00/wled.h
📚 Learning: 2025-11-02T22:07:15.689Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-11-02T22:07:15.689Z
Learning: In MoonModules/WLED-MM, WLEDMM_FASTPATH will become the default code path. Code inside "#ifndef WLEDMM_FASTPATH" blocks represents deprecated paths that are being phased out and should be ignored during reviews.
Applied to files:
wled00/wled.cppwled00/wled.h
📚 Learning: 2025-10-12T21:55:59.766Z
Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-12T21:55:59.766Z
Learning: Applies to wled00/data/**/*.{htm,html,css,js} : After editing web UI files, run npm run build to regenerate embedded headers
Applied to files:
package.json
📚 Learning: 2025-10-12T21:55:59.766Z
Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-12T21:55:59.766Z
Learning: Applies to wled00/**/*.{cpp,h} : Use spaces (2 per level) for C++ firmware source and headers in wled00
Applied to files:
wled00/wled.h
📚 Learning: 2025-07-02T23:22:57.175Z
Learnt from: netmindz
Repo: MoonModules/WLED-MM PR: 248
File: platformio.ini:1613-1613
Timestamp: 2025-07-02T23:22:57.175Z
Learning: In WLED platformio.ini, the particle system disable flags (WLED_DISABLE_PARTICLESYSTEM1D and WLED_DISABLE_PARTICLESYSTEM2D) are intentionally varied across different build environments based on platform memory constraints. More memory-limited platforms (like ESP8266, ESP32-C3, ESP32-S2) disable both 1D and 2D particle systems, while platforms with more available memory (like esp32_4MB_V4_S with 4MB flash) may only disable the 1D version to preserve flash space while keeping 2D functionality available.
Applied to files:
wled00/wled.h
📚 Learning: 2026-01-07T23:30:25.779Z
Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 308
File: platformio.ini:560-560
Timestamp: 2026-01-07T23:30:25.779Z
Learning: In platformio.ini, ESP32-S3 environments intentionally have WLED_USE_SHARED_RMT commented out (keeping RMTHI driver enabled) for testing purposes, while ESP32-S2 has it enabled (using shared RMT driver) to avoid boot loops. This inconsistency is temporary while investigating why RMTHI causes interrupt watchdog resets on S3. Once the S3 issue is understood, both S2 and S3 will use the same driver flags.
Applied to files:
wled00/wled.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: wled_build / Builds (esp32S3_WROOM-2_M)
- GitHub Check: wled_build / Builds (esp32c3dev_4MB_M)
- GitHub Check: wled_build / Builds (abc_wled_controller_v43_V4_S)
- GitHub Check: wled_build / Builds (esp32S3_8MB_S)
- GitHub Check: wled_build / Builds (esp8266pro_16MB_M)
- GitHub Check: wled_build / Builds (esp32S3_16MB_PSRAM_M_HUB75)
- GitHub Check: wled_build / Builds (esp32s2_PSRAM_M)
- GitHub Check: wled_build / Builds (esp32S3_4MB_PSRAM_S)
- GitHub Check: wled_build / Builds (esp32s2_PSRAM_S)
- GitHub Check: wled_build / Builds (esp32S3_8MB_PSRAM_M)
- GitHub Check: wled_build / Builds (esp32_4MB_PSRAM_S)
- GitHub Check: wled_build / Builds (esp32dev_compat)
- GitHub Check: wled_build / Builds (esp32_4MB_M_eth)
- GitHub Check: wled_build / Builds (esp32_16MB_V4_M_debug)
- GitHub Check: wled_build / Builds (esp32_4MB_V4_M)
🔇 Additional comments (8)
wled00/src/dependencies/network/Network.h (1)
5-7: ETHClass2 integration looks correct for ESP32 path.The replacement of
ETH.hwithETHClass2.hand the extern declaration aligns with the correspondingWLED_GLOBAL ETHClass2 ETHdefinition inwled.h. This follows WLED's established pattern for global variables.Based on learnings, verify that QEMU mode (with
WLED_QEMUflag) still works correctly, as the learning notes that ETH.begin() is skipped in QEMU to avoid hardware register crashes.wled00/const.h (2)
312-312: LGTM! Ethernet type count updated correctly.The count is correctly updated from 15 to 18 to accommodate the three new W5500-based Ethernet types. The comment accurately reflects the additions.
329-331: New Ethernet type constants are well-defined.The three new constants (15, 16, 17) are sequential and properly follow the existing
WLED_ETH_OLIMEX_GTW(14). The naming convention is consistent with existing entries. These values align with the UI options added insettings_wifi.htm.wled00/wled.h (1)
111-112: ETHClass2 integration is consistent across headers.The replacement of
ETH.hwithETHClass2.hmatches the change inNetwork.h. Keeping the commented-out original include is helpful for future reference. As per coding guidelines, 2-space indentation is used correctly.wled00/wled_ethernet.h (1)
21-58: Struct extension for SPI pins looks solid.
The added pin fields and type adjustments align with the new W5500 path and keep RMII compatibility intact.wled00/network.cpp (2)
26-306: Board pin table updates look consistent.
The added SPI pin fields and new W5500 board entries integrate cleanly with the expanded settings structure.
372-392: Ethernet link diagnostics are a nice improvement.
The added link/speed/duplex logging plus the default event log should help with field debugging.wled00/wled.cpp (1)
1051-1135: Dual‑path Ethernet init is clean and well‑guarded.
Pin allocation, failure handling, and success logging look consistent across W5500 and RMII branches.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Ok, I can fix that. It's just that https://github.com/troyhacks/ETHClass2 is missing in lib_deps ? |
Yes, that and raising the platform package version. |
|
Even works on the ESP32-C3... which also surprised me at 64x64 (4096 pixels) over Art-Net at over 45 FPS with Particle Effects even. (This did require me to use the standard libs and not Tasmora as is currently the package for the C3. And some swearing and many failed attempts to figure out how to get this C3 to boot at all - even before the W5500 code. 😁) |
|
Ooo, CodeRabbit called me a pro. 🏆 |
… just to print the new IP to the console.
There was a problem hiding this comment.
Actionable comments posted: 0
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)
13-24: Fix array size mismatch in non-RMII fallback declaration.The empty array
esp32_nonconfigurable_ethernet_pins[]in the non-RMII case will cause out-of-bounds access. Code inxml.cpp:244andcfg.cpp:749unconditionally loop overWLED_ETH_RSVD_PINS_COUNT(6 iterations) when ethernet is enabled, regardless of the RMII configuration. IfWLED_USE_ETHERNETis enabled butCONFIG_ETH_PHY_INTERFACE_RMIIis not, these loops will access indices 0–5 of a zero-sized array.Change the non-RMII fallback to match the declared size:
Fix
`#else` -const managed_pin_type esp32_nonconfigurable_ethernet_pins[] = {}; +const managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT] = {}; `#endif`
|
We can't merge until the builds are green |
So how do we change this? Using a never version overall of the V4 IDF or some other configuration? |
netmindz
left a comment
netmindz
left a comment
There was a problem hiding this comment.
Failing builds need to be addressed prior to merge
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Around line 462-463: The platform_packages upgrade for the esp32_idf_V4
profile (platform_packages = platformio/framework-arduinoespressif32 @
3.20017.241212+sha.dcc1105b) is newer than the V4 baseline and must be validated
by building a representative environment; run a full build of at least one
common environment such as esp32_4MB_V4_M (and optionally one S3/S2/C3 variant)
using the updated platform/platform_packages settings to confirm there are no
compile or linker regressions, report any failures, and only merge once the
build(s) succeed or adjust the platform_packages back to a known-good version.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/wled.h`:
- Around line 359-363: Network.h currently includes "ETHClass2.h" and declares
extern ETHClass2 ETH without the same IDF-version guard used in wled.h, causing
use of ETHClass2 when it's unavailable; wrap both the `#include` "ETHClass2.h" and
the declaration extern ETHClass2 ETH inside the identical conditional `#if`
ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 5) (and matching `#endif`) so
Network.h only references ETHClass2 for IDF >= 4.4.5 exactly as wled.h does.
| #ifndef ESP8266 | ||
| #if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 5) | ||
| WLED_GLOBAL ETHClass2 ETH; | ||
| #endif | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ETHClass2, global ETH instances, and extern ETH declarations
rg -n "ETHClass2|extern\s+.*\bETH\b|^\s*ETH\s+ETH" wled00/ --max-count=20Repository: MoonModules/WLED-MM
Length of output: 293
🏁 Script executed:
#!/bin/bash
# Also search for WLED_GLOBAL with ETH context
rg -n "WLED_GLOBAL.*ETH" wled00/ --max-count=20Repository: MoonModules/WLED-MM
Length of output: 363
🏁 Script executed:
#!/bin/bash
# Find ETHClass2 definition/class
rg -n "class\s+ETHClass2|typedef.*ETHClass2" --max-count=20Repository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check wled00/wled.h around lines 359-363 for guards
sed -n '350,370p' wled00/wled.hRepository: MoonModules/WLED-MM
Length of output: 921
🏁 Script executed:
#!/bin/bash
# Check Network.h around lines 6-7 for guards
sed -n '1,20p' wled00/src/dependencies/network/Network.hRepository: MoonModules/WLED-MM
Length of output: 411
🏁 Script executed:
#!/bin/bash
# Find and examine ETHClass2.h
find wled00 -name "ETHClass2.h" -type fRepository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Find ETHClass2.h location
find wled00 -name "ETHClass2.h" -type fRepository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check ETHClass2.h for any global ETH definitions
cat wled00/libraries/network/ETHClass2.h | head -100Repository: MoonModules/WLED-MM
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Search for ETHClass2.h file
find wled00 -name "ETHClass2.h" 2>/dev/nullRepository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Broader search for ETHClass2
find . -name "*ETHClass2*" 2>/dev/null | head -20Repository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check git ls-files for any ETHClass2 files
git ls-files | grep -i ethclassRepository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Search for where ETHClass2 might be defined (could be in Arduino libraries)
rg "class\s+ETHClass2|typedef.*ETHClass2" --type cpp --type h | head -20Repository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if ETHClass2 is from Arduino SDK or included from elsewhere
rg "#include.*ETHClass2" -B 5 -A 5Repository: MoonModules/WLED-MM
Length of output: 1223
🏁 Script executed:
#!/bin/bash
# Check what guards are around the extern declaration in Network.h
sed -n '1,15p' wled00/src/dependencies/network/Network.hRepository: MoonModules/WLED-MM
Length of output: 284
🏁 Script executed:
#!/bin/bash
# Check if ETHClass2.h is in the repo or expected from Arduino SDK
find . -type f -name "*.h" | xargs grep -l "class ETHClass2" 2>/dev/nullRepository: MoonModules/WLED-MM
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check for uses of ETH in Network.cpp to understand the dependency
rg "ETH\." wled00/src/dependencies/network/Network.cpp -n | head -20Repository: MoonModules/WLED-MM
Length of output: 486
🏁 Script executed:
#!/bin/bash
# Get full context of IDF version guards in wled.h
sed -n '108,120p' wled00/wled.hRepository: MoonModules/WLED-MM
Length of output: 488
🏁 Script executed:
#!/bin/bash
# Check if there's an alternate ETH definition or include for IDF < 4.4.5
sed -n '108,125p' wled00/wled.hRepository: MoonModules/WLED-MM
Length of output: 601
🏁 Script executed:
#!/bin/bash
# Verify Network.h has no IDF version guard around extern
sed -n '1,12p' wled00/src/dependencies/network/Network.h | cat -nRepository: MoonModules/WLED-MM
Length of output: 339
🏁 Script executed:
#!/bin/bash
# Check full Network.h header to ensure no other guards missed
head -20 wled00/src/dependencies/network/Network.h | cat -nRepository: MoonModules/WLED-MM
Length of output: 551
🏁 Script executed:
#!/bin/bash
# Verify there's no conditional compilation elsewhere
rg "ifdef|if.*IDF.*VERSION" wled00/src/dependencies/network/Network.hRepository: MoonModules/WLED-MM
Length of output: 77
🏁 Script executed:
#!/bin/bash
# Search for where ETHClass2 comes from - check if it's defined locally or expected from SDK
rg "ETHClass2" -t cpp -t h | grep -v "extern\|#include\|WLED_GLOBAL" | head -10Repository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Verify that for IDF < 4.4.5, there's a fallback to standard ETH.h in wled.h
rg "#include <ETH\.h>" wled00/Repository: MoonModules/WLED-MM
Length of output: 161
Guard the extern declaration in Network.h to match wled.h's IDF version checks.
The extern ETHClass2 ETH; declaration in wled00/src/dependencies/network/Network.h (line 7) is not guarded by IDF version, but wled00/wled.h conditionally includes ETHClass2.h only for IDF ≥ 4.4.5 and falls back to standard <ETH.h> for earlier versions. This mismatch will cause Network.h to reference the non-existent ETHClass2 type when IDF < 4.4.5. Wrap the #include "ETHClass2.h" and extern ETHClass2 ETH; in Network.h with the same #if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 5) guard to ensure consistency.
🤖 Prompt for AI Agents
In `@wled00/wled.h` around lines 359 - 363, Network.h currently includes
"ETHClass2.h" and declares extern ETHClass2 ETH without the same IDF-version
guard used in wled.h, causing use of ETHClass2 when it's unavailable; wrap both
the `#include` "ETHClass2.h" and the declaration extern ETHClass2 ETH inside the
identical conditional `#if` ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 5) (and
matching `#endif`) so Network.h only references ETHClass2 for IDF >= 4.4.5 exactly
as wled.h does.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/wled.cpp`:
- Around line 1136-1139: In initEthernet the variable ethernet_settings es is
being redeclared in the `#else` fallback branch; remove the duplicate declaration
and reuse the previously declared es (the one referenced earlier in
initEthernet) so that the code assigns es = ethernetBoards[ethernetType]; then
proceed to allocate managed_pin_type pinsToAllocate[10] and the rest of the
fallback logic; ensure you only declare managed_pin_type pinsToAllocate and do
not redeclare ethernet_settings es inside that branch.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Line 265: Replace the unpinned ETHClass2 git dependency URL with a
commit-pinned URL so builds are reproducible: update the ETHClass2 entry (the
line containing "https://github.com/troyhacks/ETHClass2") to reference a
specific commit hash (append #<commit-hash> or use the platformio git syntax)
instead of the bare repo URL, matching how other dependencies are pinned in
platformio.ini; ensure the chosen commit is from the intended branch (main or
dev) and commit hash is added exactly after the repo reference.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Around line 425-426: The Tasmota-based environments fail W5500 because the
included framework-arduinoespressif32 is stuck at 3.20009.0; update the Tasmota
platform package declaration so it explicitly pulls framework-arduinoespressif32
>= 3.20010.0. Locate the platform_packagesTasmota (and/or platformTasmota URL)
and modify it to set platform_packagesTasmota to include
"framework-arduinoespressif32 @ 3.20010.0" (or a later 3.2001x+ build), or point
platformTasmota to a newer Tasmota release that bundles
framework-arduinoespressif32 >= 3.20010.0 so all `${esp32.platformTasmota}`
environments get the required framework.
♻️ Duplicate comments (2)
platformio.ini (2)
463-464: Framework version upgrade requires build validation.This framework version update was previously flagged for testing and marked as addressed in commit 4917c13. The version 3.20017.241212 (December 2024) is newer than the V4 baseline (3.20009.0) and meets the PR requirement of > 3.20009.0 for W5500 support.
265-265: ETHClass2 dependency remains unpinned.This issue was previously flagged and marked as addressed in commit 4917c13, but the dependency is still unpinned in the current code. Pin ETHClass2 to a specific commit hash to ensure build reproducibility:
- https://github.com/troyhacks/ETHClass2 + https://github.com/troyhacks/ETHClass2#<commit-hash>
| platformV4 = espressif32@6.12.0 | ||
| platformV4_packages = platformio/framework-arduinoespressif32 @ 3.20017.241212+sha.dcc1105b ;; select arduino-esp32 v2.0.9 (arduino-esp32 2.0.10 thru 2.0.14 are buggy so avoid them) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "platformio.ini" -type f 2>/dev/null | head -5Repository: MoonModules/WLED-MM
Length of output: 45
🏁 Script executed:
head -500 platformio.ini | tail -100Repository: MoonModules/WLED-MM
Length of output: 5453
🏁 Script executed:
sed -n '410,430p' platformio.iniRepository: MoonModules/WLED-MM
Length of output: 1219
🏁 Script executed:
sed -n '1860,1870p' platformio.iniRepository: MoonModules/WLED-MM
Length of output: 582
🏁 Script executed:
sed -n '2550,2565p' platformio.iniRepository: MoonModules/WLED-MM
Length of output: 917
🌐 Web query:
Tasmota platform-espressif32 2023.06.02 framework-arduinoespressif32 version
💡 Result:
platform-espressif32 (release around 2023-06-02 / v6.3.2) used framework-arduinoespressif32 version 3.20009.0 (Arduino core 2.0.9). [1][2]
Sources:
- PlatformIO discussion mapping platform 6.3.2 → framework-arduinoespressif32 @ 3.20009.0 (2.0.9). [1]
- platform-espressif32 releases / repo (context on platform versions and framework packages). [2]
🏁 Script executed:
# Check what other environments reference platformTasmota
grep -n "platformTasmota\|platform_packagesTasmota" platformio.ini | head -20Repository: MoonModules/WLED-MM
Length of output: 1078
🏁 Script executed:
# Check if there are comments explaining why Tasmota is used
grep -B2 -A2 "platform_packagesTasmota" platformio.ini | head -40Repository: MoonModules/WLED-MM
Length of output: 1895
Update Tasmota platform framework version to support W5500.
Multiple environments using ${esp32.platformTasmota} (lines 1865–1866, 2557–2558, 2672–2673, 2730–2731, 2836–2837, 3091–3092) will fail W5500 support because Tasmota platform 2023.06.02 includes framework-arduinoespressif32 @ 3.20009.0, which does not satisfy the requirement of > 3.20009.0.
Update platform_packagesTasmota (line 417) to explicitly specify a framework version ≥ 3.20010.0, or update the platformTasmota URL to a newer Tasmota release that includes the required framework version.
🤖 Prompt for AI Agents
In `@platformio.ini` around lines 425 - 426, The Tasmota-based environments fail
W5500 because the included framework-arduinoespressif32 is stuck at 3.20009.0;
update the Tasmota platform package declaration so it explicitly pulls
framework-arduinoespressif32 >= 3.20010.0. Locate the platform_packagesTasmota
(and/or platformTasmota URL) and modify it to set platform_packagesTasmota to
include "framework-arduinoespressif32 @ 3.20010.0" (or a later 3.2001x+ build),
or point platformTasmota to a newer Tasmota release that bundles
framework-arduinoespressif32 >= 3.20010.0 so all `${esp32.platformTasmota}`
environments get the required framework.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/wled_ethernet.h`:
- Around line 25-27: The build fails because ETH_PHY_W5500 is referenced (e.g.,
in network.cpp and wled.cpp under CONFIG_ETH_SPI_ETHERNET_W5500) but currently
commented out in wled_ethernet.h; restore a fallback definition by uncommenting
or re-adding the guarded macro (ETH_PHY_W5500) in wled_ethernet.h so it maps to
ETH_PHY_MAX when not provided by the external ESP32 ETH library, ensuring the
define is available whenever CONFIG_ETH_SPI_ETHERNET_W5500 is enabled.
♻️ Duplicate comments (2)
platformio.ini (2)
425-426: Platform/framework bumps: please validate with a representative build.Toolchain changes this large can surface subtle regressions. A quick build of one classic ESP32 V4 env and one S2/S3/C3 env would help.
Also applies to: 429-430, 463-464
1865-1866: Tasmota-based envs may still be on framework 3.20009.0 (< W5500 requirement).These envs now rely on
platformTasmota+platform_packagesTasmota, butplatform_packagesTasmotais empty and the Tasmota release appears to be 2023.06.02 (framework 3.20009.0). That conflicts with the stated requirement of> 3.20009.0for W5500.🐛 Suggested fix (outside this hunk)
-platform_packagesTasmota = +platform_packagesTasmota = + platformio/framework-arduinoespressif32 @ 3.20017.241212+sha.dcc1105bAlso applies to: 2557-2558, 2672-2673, 2730-2731, 2836-2837, 3091-3092
🧹 Nitpick comments (1)
wled00/wled_ethernet.h (1)
35-40: Default new SPI pin fields toGPIO_NUM_NCto avoid accidental GPIO0 reservations.Aggregate initializers that don’t set the new fields will default them to
0, which is a real pin on ESP32. Consider adding default member initializers to make “not connected” explicit.♻️ Proposed fix (defaults in both structs)
- int eth_miso_pin; - int eth_mosi_pin; - int eth_cs_pin; - int eth_rst_pin; - int eth_int_pin; - int eth_sclk_pin; + int eth_miso_pin = GPIO_NUM_NC; + int eth_mosi_pin = GPIO_NUM_NC; + int eth_cs_pin = GPIO_NUM_NC; + int eth_rst_pin = GPIO_NUM_NC; + int eth_int_pin = GPIO_NUM_NC; + int eth_sclk_pin = GPIO_NUM_NC;- int eth_miso_pin; - int eth_mosi_pin; - int eth_cs_pin; - int eth_rst_pin; - int eth_int_pin; - int eth_sclk_pin; + int eth_miso_pin = GPIO_NUM_NC; + int eth_mosi_pin = GPIO_NUM_NC; + int eth_cs_pin = GPIO_NUM_NC; + int eth_rst_pin = GPIO_NUM_NC; + int eth_int_pin = GPIO_NUM_NC; + int eth_sclk_pin = GPIO_NUM_NC;Also applies to: 52-65
wled00/wled_ethernet.h
Outdated
| // #ifndef ETH_PHY_W5500 | ||
| // #define ETH_PHY_W5500 ETH_PHY_MAX | ||
| // #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all references to ETH_PHY_W5500 in wled00/
rg -n "ETH_PHY_W5500" wled00Repository: MoonModules/WLED-MM
Length of output: 611
🏁 Script executed:
#!/bin/bash
# Also check for ETH_PHY_W5500 outside wled00/ to get full picture
rg -n "ETH_PHY_W5500" .Repository: MoonModules/WLED-MM
Length of output: 625
🏁 Script executed:
#!/bin/bash
# Check the current state of wled00/wled_ethernet.h to see context around lines 25-27
head -40 wled00/wled_ethernet.h | cat -nRepository: MoonModules/WLED-MM
Length of output: 1979
🏁 Script executed:
#!/bin/bash
# Search for ETH_PHY definitions to understand the pattern
rg -n "ETH_PHY_" wled00/wled_ethernet.hRepository: MoonModules/WLED-MM
Length of output: 321
🏁 Script executed:
#!/bin/bash
# Check if ETH_PHY_W5500 is defined in ETH library headers
rg -n "define ETH_PHY_W5500" .Repository: MoonModules/WLED-MM
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Look for ETH library headers that might define this
find . -name "*.h" -type f | xargs rg -l "ETH_PHY" | head -20Repository: MoonModules/WLED-MM
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Check context around the references in network.cpp to understand build paths
sed -n '268,276p' wled00/network.cpp | cat -n
sed -n '284,292p' wled00/network.cpp | cat -n
sed -n '300,308p' wled00/network.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1180
🏁 Script executed:
#!/bin/bash
# Check context around references in wled.cpp
sed -n '1050,1070p' wled00/wled.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1294
ETH_PHY_W5500 is actively referenced but undefined—this will cause build failures for W5500 configurations.
Code references ETH_PHY_W5500 in network.cpp (lines 272, 288, 304 in default board configs) and wled.cpp (lines 1055, 1068 within CONFIG_ETH_SPI_ETHERNET_W5500 guard), but the define is commented out. When CONFIG_ETH_SPI_ETHERNET_W5500 is enabled and the external ETH library doesn't define this constant, the build will fail. Either uncomment the guarded define or ensure the ESP32 ETH library provides it.
🤖 Prompt for AI Agents
In `@wled00/wled_ethernet.h` around lines 25 - 27, The build fails because
ETH_PHY_W5500 is referenced (e.g., in network.cpp and wled.cpp under
CONFIG_ETH_SPI_ETHERNET_W5500) but currently commented out in wled_ethernet.h;
restore a fallback definition by uncommenting or re-adding the guarded macro
(ETH_PHY_W5500) in wled_ethernet.h so it maps to ETH_PHY_MAX when not provided
by the external ESP32 ETH library, ensuring the define is available whenever
CONFIG_ETH_SPI_ETHERNET_W5500 is enabled.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Around line 1488-1489: The esp32_4MB_M_eth environment enables ethernet via
the -D WLED_USE_ETHERNET define but then excludes the ETHClass2 library in its
lib_ignore, causing build failures; update the esp32_4MB_M_eth environment by
removing ETHClass2 from its lib_ignore so the ETHClass2 library is available
when -D WLED_USE_ETHERNET is set (look for the esp32_4MB_M_eth env block, the -D
WLED_USE_ETHERNET define, and the lib_ignore entry containing ETHClass2).
♻️ Duplicate comments (1)
platformio.ini (1)
1865-1867: Tasmota environments still don't meet W5500 framework requirement.Multiple environments using
${esp32.platformTasmota}(lines 1865-1867, 2557-2559, 2672-2674, 2730-2732, 2836-2838, 3091-3093) will not support W5500 because the Tasmota platform 2023.06.02 (line 417) bundles framework-arduinoespressif32 @ 3.20009.0, which does not satisfy the >3.20009.0 requirement stated in the PR objectives.Update line 418 to explicitly override the framework version:
platformTasmota = https://github.com/tasmota/platform-espressif32/releases/download/2023.06.02/platform-espressif32.zip -platform_packagesTasmota = +platform_packagesTasmota = platformio/framework-arduinoespressif32 @ 3.20017.241212+sha.dcc1105bOr point
platformTasmotato a newer Tasmota release that includes a framework version ≥ 3.20010.0.Based on learnings: This issue was previously flagged but has not been resolved.
Also applies to: 2557-2559, 2672-2674, 2730-2732, 2836-2838, 3091-3093
🧹 Nitpick comments (1)
wled00/xml.cpp (1)
254-266: Fix indentation:switchshould be indented inside theifblock.The
switchstatement on line 255 has the same indentation as theifon line 254. Per coding guidelines (2 spaces per level), theswitchshould be indented 2 additional spaces since it's inside theifblock.Compare with the equivalent code in
cfg.cpp(lines 759-760) which has correct indentation.🔧 Suggested fix
if (ethernetBoards[ethernetType].eth_type != ETH_PHY_W5500) { - switch (ethernetBoards[ethernetType].eth_clk_mode) { - case ETH_CLOCK_GPIO0_IN: - case ETH_CLOCK_GPIO0_OUT: - oappend(SET_F(",0")); - break; - case ETH_CLOCK_GPIO16_OUT: - oappend(SET_F(",16")); - break; - case ETH_CLOCK_GPIO17_OUT: - oappend(SET_F(",17")); - break; - } + switch (ethernetBoards[ethernetType].eth_clk_mode) { + case ETH_CLOCK_GPIO0_IN: + case ETH_CLOCK_GPIO0_OUT: + oappend(SET_F(",0")); + break; + case ETH_CLOCK_GPIO16_OUT: + oappend(SET_F(",16")); + break; + case ETH_CLOCK_GPIO17_OUT: + oappend(SET_F(",17")); + break; + } + } } - }
| lib_ignore = ${esp32_4MB_M_base.lib_ignore} | ||
| ETHClass2 |
There was a problem hiding this comment.
Inconsistent configuration: ethernet enabled but ETHClass2 library ignored.
The esp32_4MB_M_eth environment defines -D WLED_USE_ETHERNET (line 1476) but then ignores the ETHClass2 library (line 1489). This will cause build failures because the ethernet code requires the ETHClass2 library.
Remove ETHClass2 from lib_ignore for this environment:
lib_ignore = ${esp32_4MB_M_base.lib_ignore}
- ETHClass2🤖 Prompt for AI Agents
In `@platformio.ini` around lines 1488 - 1489, The esp32_4MB_M_eth environment
enables ethernet via the -D WLED_USE_ETHERNET define but then excludes the
ETHClass2 library in its lib_ignore, causing build failures; update the
esp32_4MB_M_eth environment by removing ETHClass2 from its lib_ignore so the
ETHClass2 library is available when -D WLED_USE_ETHERNET is set (look for the
esp32_4MB_M_eth env block, the -D WLED_USE_ETHERNET define, and the lib_ignore
entry containing ETHClass2).
There was a problem hiding this comment.
Actionable comments posted: 0
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)
13-24: Fix invalid empty-array definition in non‑RMII builds.
const managed_pin_type esp32_nonconfigurable_ethernet_pins[];is ill‑formed in C++ (incomplete array definition) and will fail to compile whenCONFIG_ETH_PHY_INTERFACE_RMIIis not set (e.g., W5500-only builds). Provide a real definition (or make itexternwith a definition elsewhere) and ensure it’s safe when the reserved‑pin count is zero.🐛 Proposed fix
`#ifdef` CONFIG_ETH_PHY_INTERFACE_RMII const managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT] = { { 21, true }, { 19, true }, { 22, true }, { 25, false }, { 26, false }, { 27, true }, }; -#else -const managed_pin_type esp32_nonconfigurable_ethernet_pins[]; +#else +// Empty placeholder for non-RMII builds; loop bounds should keep this unused. +const managed_pin_type esp32_nonconfigurable_ethernet_pins[1] = {}; `#endif`
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/network.cpp`:
- Around line 22-24: The fallback `#else` defines
esp32_nonconfigurable_ethernet_pins as a zeroed managed_pin_type array which
causes GPIO0 to be treated as reserved; change this to a const array initialized
with entries set to GPIO_NUM_NC (and passive/not reserved) so consumers like
cfg.cpp and xml.cpp see non-configurable pins as "not connected" in non-RMII
builds; specifically update the esp32_nonconfigurable_ethernet_pins declaration
(managed_pin_type esp32_nonconfigurable_ethernet_pins[]) to be const and
initialize all elements to GPIO_NUM_NC values.
wled00/network.cpp
Outdated
| #else | ||
| managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT]; | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the current state of wled00/network.cpp around lines 22-24
head -30 wled00/network.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1479
🏁 Script executed:
# Search for all usages of esp32_nonconfigurable_ethernet_pins
rg -n 'esp32_nonconfigurable_ethernet_pins' wled00/Repository: MoonModules/WLED-MM
Length of output: 1503
🏁 Script executed:
# Search for CONFIG_ETH_PHY_INTERFACE_RMII guards and their context
rg -n -B5 -A5 'CONFIG_ETH_PHY_INTERFACE_RMII' wled00/network.cppRepository: MoonModules/WLED-MM
Length of output: 791
🏁 Script executed:
# Check the context around wled.cpp usage at lines 1082-1087
sed -n '1070,1095p' wled00/wled.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1412
🏁 Script executed:
# Check the context around wled.cpp usage at lines 1140-1145
sed -n '1128,1155p' wled00/wled.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1415
🏁 Script executed:
# Check the context around cfg.cpp usage at line 754
sed -n '745,765p' wled00/cfg.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1760
🏁 Script executed:
# Check the context around xml.cpp usage at line 249
sed -n '240,260p' wled00/xml.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1993
🏁 Script executed:
# Check what the `#else` at line ~1140 is protecting
sed -n '1000,1150p' wled00/wled.cpp | grep -n -B20 "Ethernet initialization should only succeed once"Repository: MoonModules/WLED-MM
Length of output: 1509
🏁 Script executed:
# Search for where ethernetType is assigned and under what guards
rg -n 'ethernetType\s*=' wled00/wled.cpp | head -20Repository: MoonModules/WLED-MM
Length of output: 106
🏁 Script executed:
# Check the top-level guards around ethernet initialization in wled.cpp
sed -n '1000,1050p' wled00/wled.cpp | cat -n | head -60Repository: MoonModules/WLED-MM
Length of output: 2132
🏁 Script executed:
# Get broader context to confirm what guards cfg.cpp and xml.cpp usages
sed -n '750,760p' wled00/cfg.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1139
🏁 Script executed:
# Check xml.cpp context around line 249
sed -n '245,255p' wled00/xml.cpp | cat -nRepository: MoonModules/WLED-MM
Length of output: 1280
🏁 Script executed:
# Verify if WLED_USE_ETHERNET can be defined without CONFIG_ETH_PHY_INTERFACE_RMII
rg -n 'WLED_USE_ETHERNET|CONFIG_ETH_PHY_INTERFACE_RMII' wled00/wled_ethernet.h | head -20Repository: MoonModules/WLED-MM
Length of output: 129
Initialize esp32_nonconfigurable_ethernet_pins to GPIO_NUM_NC in non-RMII fallback.
The zero-initialized array in the #else branch (lines 22–24) is accessed without CONFIG_ETH_PHY_INTERFACE_RMII guards in cfg.cpp (line 754) and xml.cpp (line 249). In non-RMII builds, this results in all entries being {0, false}, incorrectly reserving GPIO0.
Initialize all entries to GPIO_NUM_NC and mark the array const:
`#else`
-managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT];
+const managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT] = {
+ { GPIO_NUM_NC, false },
+ { GPIO_NUM_NC, false },
+ { GPIO_NUM_NC, false },
+ { GPIO_NUM_NC, false },
+ { GPIO_NUM_NC, false },
+ { GPIO_NUM_NC, false },
+};
`#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.
| #else | |
| managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT]; | |
| #endif | |
| `#else` | |
| const managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT] = { | |
| { GPIO_NUM_NC, false }, | |
| { GPIO_NUM_NC, false }, | |
| { GPIO_NUM_NC, false }, | |
| { GPIO_NUM_NC, false }, | |
| { GPIO_NUM_NC, false }, | |
| { GPIO_NUM_NC, false }, | |
| }; | |
| `#endif` |
🤖 Prompt for AI Agents
In `@wled00/network.cpp` around lines 22 - 24, The fallback `#else` defines
esp32_nonconfigurable_ethernet_pins as a zeroed managed_pin_type array which
causes GPIO0 to be treated as reserved; change this to a const array initialized
with entries set to GPIO_NUM_NC (and passive/not reserved) so consumers like
cfg.cpp and xml.cpp see non-configurable pins as "not connected" in non-RMII
builds; specifically update the esp32_nonconfigurable_ethernet_pins declaration
(managed_pin_type esp32_nonconfigurable_ethernet_pins[]) to be const and
initialize all elements to GPIO_NUM_NC values.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/wled_ethernet.h`:
- Around line 25-27: The WLED_ETH_RSVD_PINS_COUNT macro is being redefined to 0
unconditionally after the RMII branch, which overrides the RMII-specific value
6; update the header so WLED_ETH_RSVD_PINS_COUNT remains conditional: remove or
guard the later redefinition and ensure the RMII branch's "#define
WLED_ETH_RSVD_PINS_COUNT 6" is the active definition when
CONFIG_ETH_PHY_INTERFACE_RMII is set (so the reserved-pin array in
EthernetSettings is non-zero for RMII builds), while keeping the non-RMII
default define only when CONFIG_ETH_PHY_INTERFACE_RMII is not defined.
| #ifdef CONFIG_ETH_PHY_INTERFACE_RMII | ||
| #define WLED_ETH_RSVD_PINS_COUNT 6 | ||
| typedef struct EthernetSettings { |
There was a problem hiding this comment.
Reserved‑pin count is overridden to 0 for RMII builds.
Line 67 redefines WLED_ETH_RSVD_PINS_COUNT after the RMII branch, so the RMII value of 6 gets reset to 0. That makes the reserved‑pin list empty (and may create a zero‑length array), which can allow RMII pins to be re‑allocated and cause hardware conflicts. Please keep the define conditional.
✅ Suggested fix
-#ifdef CONFIG_ETH_PHY_INTERFACE_RMII
-#define WLED_ETH_RSVD_PINS_COUNT 6
+#ifdef CONFIG_ETH_PHY_INTERFACE_RMII
+#define WLED_ETH_RSVD_PINS_COUNT 6
typedef struct EthernetSettings {
uint8_t eth_address;
int eth_power;
int eth_mdc;
int eth_mdio;
int eth_miso_pin;
int eth_mosi_pin;
int eth_cs_pin;
int eth_rst_pin;
int eth_int_pin;
int eth_sclk_pin;
eth_phy_type_t eth_type;
eth_clock_mode_t eth_clk_mode;
} ethernet_settings;
-#else
+#else
+#define WLED_ETH_RSVD_PINS_COUNT 0
`#define` ETH_PHY_LAN8720 0
`#define` ETH_CLOCK_GPIO0_IN 0
`#define` ETH_CLOCK_GPIO0_OUT 1
`#define` ETH_CLOCK_GPIO17_OUT 3
`#define` ETH_CLOCK_GPIO16_OUT 2
`#define` ETH_PHY_IP101 1
@@
} ethernet_settings;
`#endif`
@@
-#define WLED_ETH_RSVD_PINS_COUNT 0
extern managed_pin_type esp32_nonconfigurable_ethernet_pins[WLED_ETH_RSVD_PINS_COUNT];Also applies to: 67-68
🤖 Prompt for AI Agents
In `@wled00/wled_ethernet.h` around lines 25 - 27, The WLED_ETH_RSVD_PINS_COUNT
macro is being redefined to 0 unconditionally after the RMII branch, which
overrides the RMII-specific value 6; update the header so
WLED_ETH_RSVD_PINS_COUNT remains conditional: remove or guard the later
redefinition and ensure the RMII branch's "#define WLED_ETH_RSVD_PINS_COUNT 6"
is the active definition when CONFIG_ETH_PHY_INTERFACE_RMII is set (so the
reserved-pin array in EthernetSettings is non-zero for RMII builds), while
keeping the non-RMII default define only when CONFIG_ETH_PHY_INTERFACE_RMII is
not defined.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/network.cpp`:
- Around line 26-28: UI allows selecting W5500 boards even when
CONFIG_ETH_SPI_ETHERNET_W5500 is disabled and compile-time aliasing
(ETH_PHY_W5500 → ETH_PHY_MAX) hides errors; update both the UI and runtime
validation: remove or conditionally hide W5500 options in settings_wifi.htm when
CONFIG_ETH_SPI_ETHERNET_W5500 is not defined, and add an explicit runtime check
in the Ethernet initialization code in wled.cpp (where W5500 init is currently
`#ifdef`'d around the W5500 init block) to detect if a W5500 board is selected
while support is disabled (check selected board IDs for the TTGO T-ETH-Lite S3 /
WaveShare / W5500 Generic entries, ETH_PHY_W5500, and GPIO_NUM_NC for RMII pins)
and emit a clear user-facing error message and abort or prevent falling through
to RMII initialization.
♻️ Duplicate comments (1)
wled00/network.cpp (1)
13-24: Fix non‑RMII fallback array definition to avoid invalid pin reservations / compile errors.The
#elsebranch currently declaresesp32_nonconfigurable_ethernet_pinswithout a size or initialization, so non‑RMII builds that index this symbol will fail or treat GPIO0 as reserved. Please keep the array shape consistent and initialize toGPIO_NUM_NC.🐛 Proposed fix
-#else -managed_pin_type esp32_nonconfigurable_ethernet_pins;; -#endif +#else +const managed_pin_type esp32_nonconfigurable_ethernet_pins[6] = { + { GPIO_NUM_NC, false }, + { GPIO_NUM_NC, false }, + { GPIO_NUM_NC, false }, + { GPIO_NUM_NC, false }, + { GPIO_NUM_NC, false }, + { GPIO_NUM_NC, false }, +}; +#endif
| #ifndef CONFIG_ETH_SPI_ETHERNET_W5500 | ||
| #define ETH_PHY_W5500 ETH_PHY_MAX | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for W5500 gating and ETH_PHY_MAX handling
rg -n 'CONFIG_ETH_SPI_ETHERNET_W5500|ETH_PHY_W5500|ETH_PHY_MAX' wled00Repository: MoonModules/WLED-MM
Length of output: 1228
🏁 Script executed:
# Check xml.cpp context around the ETH_PHY_W5500 filter
sed -n '244,270p' wled00/xml.cpp
# Check cfg.cpp context around the ETH_PHY_W5500 filter
sed -n '746,780p' wled00/cfg.cpp
# Check wled.cpp runtime checks
sed -n '1051,1070p' wled00/wled.cppRepository: MoonModules/WLED-MM
Length of output: 5013
🏁 Script executed:
# Search for where ethernet board options are generated/displayed
rg -n 'ethernetBoards|WLED_NUM_ETH|WLED_ETH' wled00/*.{cpp,h} | head -40
# Look for UI/JSON generation that includes board list
rg -B5 -A5 'eth.*board|board.*select|board.*option' wled00/xml.cpp | head -50Repository: MoonModules/WLED-MM
Length of output: 3780
🏁 Script executed:
# Search for where UI options for ethernet boards are generated
rg -n 'addOption|option.*eth|WLED_ETH_W5500' wled00/xml.cpp | head -30
# Look for board name strings
rg -n 'W5500|Waveshare|TTGO|Olimex' wled00Repository: MoonModules/WLED-MM
Length of output: 2865
🏁 Script executed:
# Check if settings_wifi.htm has any conditional logic for W5500 boards
cat -n wled00/data/settings_wifi.htm | grep -A5 -B5 'W5500\|TTGO T-ETH'
# Check if there's any JavaScript gating logic
grep -n 'CONFIG_ETH_SPI_ETHERNET_W5500\|W5500' wled00/data/*.js 2>/dev/null | head -20
# Check the full board definition array in network.cpp
sed -n '260,315p' wled00/network.cppRepository: MoonModules/WLED-MM
Length of output: 2530
🏁 Script executed:
# Check if the HTML is conditionally generated or static
grep -B20 'W5500 Generic' wled00/data/settings_wifi.htm | head -30
# Look for any server-side gating logic that might be generating HTML conditionally
rg -n 'settings_wifi|board.*option|ethernetType' wled00/xml.cpp | head -20
# Check what happens if W5500 is selected but not compiled
sed -n '1040,1100p' wled00/wled.cppRepository: MoonModules/WLED-MM
Length of output: 5396
W5500 boards can be selected and saved even when CONFIG_ETH_SPI_ETHERNET_W5500 is disabled, causing silent failures at runtime.
The compile-time mapping (ETH_PHY_W5500 → ETH_PHY_MAX) prevents build errors, but W5500 gating is incomplete:
- UI does not hide W5500 boards: Options for TTGO T-ETH-Lite S3, WaveShare, and W5500 Generic are statically listed in settings_wifi.htm with no conditional display.
- Runtime check is gated: The W5500 initialization code at wled.cpp:1055 is wrapped in
#ifdef CONFIG_ETH_SPI_ETHERNET_W5500, so when disabled, the runtime check is skipped entirely. - Silent failures: If a user selects a W5500 board when support is disabled, code falls through to RMII initialization, which fails during pin allocation (W5500 boards have
GPIO_NUM_NCfor mandatory RMII pins likeeth_mdc/eth_mdio) without a clear W5500-specific error message.
Either gate W5500 board options in the UI when the config is disabled, or add explicit runtime validation with a user-facing error message.
🤖 Prompt for AI Agents
In `@wled00/network.cpp` around lines 26 - 28, UI allows selecting W5500 boards
even when CONFIG_ETH_SPI_ETHERNET_W5500 is disabled and compile-time aliasing
(ETH_PHY_W5500 → ETH_PHY_MAX) hides errors; update both the UI and runtime
validation: remove or conditionally hide W5500 options in settings_wifi.htm when
CONFIG_ETH_SPI_ETHERNET_W5500 is not defined, and add an explicit runtime check
in the Ethernet initialization code in wled.cpp (where W5500 init is currently
`#ifdef`'d around the W5500 init block) to detect if a W5500 board is selected
while support is disabled (check selected board IDs for the TTGO T-ETH-Lite S3 /
WaveShare / W5500 Generic entries, ETH_PHY_W5500, and GPIO_NUM_NC for RMII pins)
and emit a clear user-facing error message and abort or prevent falling through
to RMII initialization.
3 participants
This should allow anyone to add Ethernet to any board.
Some notes:
Apologies for the json files. Just ignore those, it's cruft from jumping between branches.
W5500 is SPI and that means it's slower than RMII - which is only on the original ESP32 and the ESP32-P4... but it's still highly functional. There's also things that can we tweaked in the IDF (when building Ardino-ESP32) as well as at WLED compile-time to potentially get more performance out of these.
The "W5500 Lite" (the smaller DIP-style module on the left) will absolutely require additional 3.3v power. It seems I'm not alone in saying that nobody has ever run one off the 3.3v rail of an ESP32. It doesn't need a lot, it's just very picky. The older "blue W5500" boards (right) seem to be much more tolerant and can run off an ESP32 rail, but you might want to add a big cap to absorb the surges and avoid brownouts.
Research says also just having a dedicated regulator will help as well - the Lite version just doesn't do itself any favors. I have used a breadboard power supply like this to give enough juice for the W5500 Lite to work:
platformio/framework-arduinoespressif32 has to be any version ABOVE 3.20009.0 (I tested the next higher and the latest version and both worked). This may not strictly be needed if someone wanted to figure out the one missing function and "backport" it somehow, but the easy method is to just use a slightly newer version.
ETH.h is replaced with a backport LilyGo did from IDFv5 which I split out from their meta-library to: https://github.com/troyhacks/ETHClass2
I expanded the ethernet board struct to handle all 3 types of Ethernet - two RMII types and the W5500 - and it will likely handle ones we haven't used yet that Espressif supports. I think it's clever, @softhack007 may disagree. 😊
I think our move to IDFv4 means I don't need to use my modified AsyncUDP library anymore! Huzzah! It's sending tons of AsyncUDP data just fine. If we move to IDFv5 at some point, this PR will be mostly moot as support for all of these devices are in there natively.
There's some "saving the pins in the config file" logic I didn't touch - nothing uses them, as far as I know. If I'm wrong, I can dig into that. I assume most people pick the one that works from the drop-down and move on with their life.
I also checked to make sure it still works on the original ESP32 RMII Ethernet devices, and it does:
...and on the existing S3 boards I have from LilyGo and WaveShare, which are now user-selectable instead of compile-time flags!
...and here it is on an original ESP32 doing W5500 Ethernet. This is based on some random board I had lying around, so pins could be changed. We do have global SPI defines we can do, but I didn't implement those - but the W5500 needs 3 more pins (interrupt, CS/SS, and reset). My TTGO "Koala" board seems to be marked as having "hardware" SPI pins at 12,13,14 for MISO, MOSI, and SCLK.
And proof of life of W5500 on a reggo ESP32:
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.