Conversation
WalkthroughAdds Gledopto Ethernet board support: a new Ethernet type constant, a UI dropdown option, and a new board entry in the ethernetBoards[] array with its pin/configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
wled00/const.h (1)
368-385: Enum and count are coherent; consider adding a compile‑time guard
WLED_NUM_ETH_TYPES= 14 correctly matches the new highest IDWLED_ETH_GLEDOPTO(13) and the 14 entries inethernetBoards[], so the range check ininitEthernet()remains valid.To prevent future mismatches, you could add a small compile‑time check near this block, for example:
static_assert(WLED_NUM_ETH_TYPES == WLED_ETH_GLEDOPTO + 1, "WLED_NUM_ETH_TYPES must match highest ethernet type + 1");This is optional but keeps the enum and count in sync as more boards are added. Based on learnings, aligning counts with symbolic constants improves maintainability.
wled00/network.cpp (1)
138-155: New Gledopto entry matches enum/indexing; add a label and optional size checkThe added
ethernetBoardsentry matchesWLED_ETH_GLEDOPTO= 13 and keeps the array length aligned withWLED_NUM_ETH_TYPES(14), soethernetBoards[ethernetType]remains safe.Two small optional improvements for clarity/safety:
- Label the new board explicitly:
- // LILYGO T-POE Pro - // https://github.com/Xinyuan-LilyGO/LilyGO-T-ETH-Series/blob/master/schematic/T-POE-PRO.pdf + // LILYGO T-POE Pro + // https://github.com/Xinyuan-LilyGO/LilyGO-T-ETH-Series/blob/master/schematic/T-POE-PRO.pdf { 0, // eth_address, @@ ETH_PHY_LAN8720, // eth_type, ETH_CLOCK_GPIO0_OUT // eth_clk_mode }, + + // Gledopto Series With Ethernet { 1, // eth_address, 5, // eth_power, 23, // eth_mdc, 33, // eth_mdio, ETH_PHY_LAN8720, // eth_type, ETH_CLOCK_GPIO0_IN // eth_clk_mode },
- Optionally assert array size vs. type count right after the array:
static_assert(sizeof(ethernetBoards) / sizeof(ethernetBoards[0]) == WLED_NUM_ETH_TYPES, "ethernetBoards size must match WLED_NUM_ETH_TYPES");Both are non‑breaking and make future additions less error‑prone.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
wled00/const.h(2 hunks)wled00/data/settings_wifi.htm(1 hunks)wled00/network.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/const.h
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings_wifi.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings_wifi.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings_wifi.htm
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/network.cpp
🧠 Learnings (10)
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/const.h
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/const.h
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
wled00/const.h
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
wled00/const.hwled00/network.cpp
📚 Learning: 2025-09-01T10:26:17.959Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.
Applied to files:
wled00/const.hwled00/network.cpp
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Applied to files:
wled00/const.h
📚 Learning: 2025-11-30T15:29:00.711Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4456
File: usermods/deep_sleep/deep_sleep.cpp:224-230
Timestamp: 2025-11-30T15:29:00.711Z
Learning: In WLED, `touchThreshold` is a global variable defined in wled00/wled.h and is accessible to all usermods. It stores the touch sensitivity threshold for touch-capable GPIO pins on ESP32 variants and should not be flagged as undefined when used in usermods.
Applied to files:
wled00/const.h
📚 Learning: 2025-09-13T13:13:36.092Z
Learnt from: netmindz
Repo: wled/WLED PR: 3777
File: wled00/bus_manager.cpp:982-989
Timestamp: 2025-09-13T13:13:36.092Z
Learning: In the WLED HUB75 implementation, when HUB75 matrix initialization fails after pin allocation, pins are intentionally not deallocated because they're physically connected to the HUB75 panel and cannot be reused for other purposes. This design choice prioritizes simplicity over strict software resource management consistency.
Applied to files:
wled00/const.h
📚 Learning: 2025-04-26T12:51:04.993Z
Learnt from: KrX3D
Repo: wled/WLED PR: 4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Applied to files:
wled00/const.h
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
wled00/const.h
🔇 Additional comments (1)
wled00/data/settings_wifi.htm (1)
257-274: Ethernet type value wiring looks consistent; remember UI rebuild stepThe new option
value="13"for “Gledopto Series With Ethernet” is consistent with the newWLED_ETH_GLEDOPTOconstant and the addedethernetBoards[13]entry. Please just make sure to runnpm run buildso the embeddedhtml_*.hheaders are regenerated before building firmware, per project guidelines.
|
This configuration is suitable for both GL-C-618WL and GL-C-616WL |
|
Why didn't this get merged into the nightly? Or the main release? |
Still under review, see for details: #5156 |
2 participants
Add Gledopto Series With Ethernet
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.