Skip to content

Conversation

@Aogu181
Copy link

@Aogu181 Aogu181 commented Dec 4, 2025

This submission is to support configurations for the GLEDOPTO Ethernet series, currently supporting models GL-C-616WL and GL-C-618WL. Relevant product information is as follows:

GL-C-616WL: https://gledopto.com/h-pd-86.html
GL-C-618WL: https://gledopto.com/h-pd-64.html

Summary by CodeRabbit

  • New Features
    • Added support for Gledopto Series Ethernet boards as a selectable network configuration option.
    • Network settings UI updated to include the new Ethernet option and reflect the expanded list of supported Ethernet board types (14 total).

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Walkthrough

Adds one new Ethernet board type "Gledopto Series With Ethernet": a new constant, a new WiFi settings option, and a new ethernetBoards entry with specific GPIO mappings.

Changes

Cohort / File(s) Summary
Gledopto Ethernet Board Support
wled00/const.h, wled00/data/settings_wifi.htm, wled00/network.cpp
Added WLED_ETH_GLEDOPTO = 13 to const.h; added <option value="13">Gledopto Series With Ethernet</option> to settings_wifi.htm; appended a new ethernetBoards entry in network.cpp with eth_address=1, eth_power=5, eth_mdc=23, eth_mdio=33, eth_type=LAN8720, eth_clk_mode=ETH_CLOCK_GPIO0_IN.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review attention: verify GPIO pin assignments, eth_type and eth_clk_mode correctness, and that the UI option value matches the constant/enum.

Suggested reviewers

  • netmindz

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding support for Gledopto Series With Ethernet devices. It is concise, clear, and directly reflects the core purpose of the changeset across all three modified files.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14a7280 and f2a3502.

📒 Files selected for processing (1)
  • wled00/data/settings_wifi.htm (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • wled00/data/settings_wifi.htm

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
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

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

149-149: Minor: Align comment formatting with other board comments.

For consistency with other board section comments in this file (e.g., lines 24, 38, 48), consider removing the leading space:

-  // Gledopto Series With Ethernet
+ // Gledopto Series With Ethernet
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc5b504 and a2b64ad.

📒 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/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, run npm run build to 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
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
🧠 Learnings (12)
📚 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/data/settings_wifi.htm
  • wled00/network.cpp
  • 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/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)

Applied to files:

  • wled00/data/settings_wifi.htm
📚 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/data/settings*.htm : Name settings pages as settings*.htm within the web UI

Applied to files:

  • wled00/data/settings_wifi.htm
📚 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-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
📚 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.h
📚 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
🔇 Additional comments (2)
wled00/const.h (1)

369-370: LGTM! Ethernet type count and constant properly defined.

The changes correctly:

  • Increment the board type count from 13 to 14
  • Define WLED_ETH_GLEDOPTO with value 13 (matching the 14th array entry at index 13)
  • Maintain consistency with the HTML option value and network.cpp array index

The bounds check in initEthernet() (Line 171 in network.cpp: ethernetType >= WLED_NUM_ETH_TYPES) will properly validate the new board type.

Also applies to: 385-385

wled00/network.cpp (1)

149-157: Verify pin configuration against Gledopto hardware datasheets.

The web search results provide general Gledopto device information but do not contain manufacturer-specific Ethernet PHY pinout documentation for GL-C-616WL and GL-C-618WL models.

The configuration shown uses eth_mdio = 33, which differs from typical ESP32 RMII boards that map MDIO to GPIO18. The eth_mdc = 23 aligns with common mappings, and ETH_CLOCK_GPIO0_IN is a valid alternative clock configuration.

Without access to official Gledopto datasheets or hardware documentation, this configuration cannot be confirmed. If this code is based on known-working hardware or community configurations for these models, that basis should be documented in the code comment for future reference.

@TakissX
Copy link

TakissX commented Dec 7, 2025

Will this eventually be integrated?

Add Gledopto Series With Ethernet
Add Gledopto Series With Ethernet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants