Make driver compile on both ESP-IDF 4.4 and 5.5#2
Merged
Conversation
The library targeted ESP-IDF 4.4.7 only; Meshtastic develop has moved to Arduino 3.3.8 / ESP-IDF 5.5.4 (pioarduino). Verified against the upstream IDF 4.4 and 5.5 esp_eth headers, the code did not compile on IDF 5.x. PHY driver: - IDF 5.x renamed the PHY struct member `negotiate` to `autonego_ctrl` (with its own `eth_phy_autoneg_cmd_t`). The unconditional `negotiate` assignment, and `autonego_ctrl` being given a function with an `int cmd` signature, both broke the C++ build on IDF 5.x. Route all callers through a shared `ch390_autonego_apply()` worker and assign the version-correct callback (`negotiate` < 5.0, `autonego_ctrl` >= 5.0). - Drop `custom_ioctl`: the stub only returned ESP_ERR_NOT_SUPPORTED, which is exactly what esp_eth_ioctl produces for a NULL pointer. MAC driver: - Drop `custom_ioctl` for the same reason. - Include esp_mac.h for esp_read_mac()/ESP_MAC_ETH (a separate header on IDF 5.0+). - New IDF 5.5 members (transmit_vargs, add_mac_filter, rm_mac_filter, set_all_multicast) are intentionally left NULL; esp_eth_ioctl NULL-checks them and the CH390 already accepts all multicast (RCR_ALL). ESP32_CH390: - esp_eth_ioctl is a fixed 3-arg call in both IDF versions; remove the bogus 4-arg variadic wrapper. readPHY()/writePHY() now use esp_eth_phy_reg_rw_data_t on IDF 5.x and the inline-value struct on 4.4. - Compare DNS addresses as uint32_t to avoid the ambiguous operator!= with the Arduino-ESP32 3.x IPAddress class. Build/CI: - Add pioarduino (IDF 5.5) build envs to the examples and extend the CI matrix so both toolchains are compiled. All three examples build clean on IDF4 (espressif32) and IDF5 (pioarduino). Fixes #1
The pioarduino platform-espressif32 (ESP-IDF 5.5) refuses to install on
Python 3.9 ("Python version must be 3.10, 3.11, 3.12, 3.13, 3.14"), so the
new IDF5 build matrix failed at platform install. Bump python-version to
3.12 across all jobs; the classic espressif32 (IDF4) jobs build fine on it
too.
There was a problem hiding this comment.
Pull request overview
This PR updates the ESP32-CH390 Arduino Ethernet driver to compile cleanly against both ESP-IDF 4.4 (Arduino-ESP32 2.x / PlatformIO espressif32) and ESP-IDF 5.5 (Arduino-ESP32 3.3.x via pioarduino), primarily by adapting to ESP-IDF’s Ethernet (esp_eth) ABI changes across major versions.
Changes:
- Updates PHY/MAC driver structs and callbacks to match the IDF 4.x vs 5.x esp_eth ABI (e.g., auto-negotiation callback differences) and removes no-op
custom_ioctlstubs. - Fixes C++-level ABI mismatches around
esp_eth_ioctland PHY register IO data structures; adjusts DNS comparisons to avoid Arduino-ESP32 3.xIPAddressoperator ambiguity. - Adds IDF5 (
pioarduino) build environments to examples and expands the GitHub Actions CI matrix to build all examples across both toolchains.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/ESP32_CH390.h | Removes misleading ioctl wrapper helpers; documents fixed esp_eth_ioctl signature across IDF versions. |
| src/ESP32_CH390.cpp | Adjusts DNS comparisons and uses IDF-version-correct PHY register IO structs for esp_eth_ioctl. |
| src/esp_eth_phy_ch390_arduino.cpp | Introduces version-bridging auto-negotiation worker and assigns the correct PHY callback member for IDF 4.x vs 5.x. |
| src/esp_eth_mac_ch390_arduino.cpp | Includes esp_mac.h when available and removes custom_ioctl; documents intentional NULLs for new IDF 5.5 MAC members. |
| README.md | Updates compatibility claims/documentation to explicitly support ESP-IDF 4.4 and 5.5 / Arduino-ESP32 2.x and 3.3.x. |
| examples/StaticIP/platformio.ini | Adds split IDF4/IDF5 PlatformIO environments (espressif32 vs pioarduino). |
| examples/Basic/platformio.ini | Same as above for the Basic example. |
| examples/Advanced/platformio.ini | Same as above for the Advanced example. |
| .github/workflows/platformio.yml | Expands CI to build all examples across IDF4 + IDF5 environments; updates Python version. |
Comments suppressed due to low confidence (1)
src/ESP32_CH390.cpp:140
- In ESP32_CH390::config(), DNS parameters default to 0 (see header/README), but the guard checks against INADDR_NONE (0xFFFFFFFF). This means calling config(ip, gw, mask) with default dns1/dns2 will still set the netif DNS servers to 0.0.0.0, which can break hostname resolution. Consider either changing the default dns1/dns2 to INADDR_NONE (and documenting that sentinel), or adjusting the condition to treat 0.0.0.0 as "unset" so DNS info is only applied when a real server address is provided.
// Compare as uint32_t: the Arduino-ESP32 3.x IPAddress class makes a direct
// `IPAddress != <integer>` comparison ambiguous.
if (static_cast<uint32_t>(dns1) != INADDR_NONE) {
esp_netif_dns_info_t dns_info;
dns_info.ip.u_addr.ip4.addr = static_cast<uint32_t>(dns1);
dns_info.ip.type = ESP_IPADDR_TYPE_V4;
esp_netif_set_dns_info(eth_netif, ESP_NETIF_DNS_MAIN, &dns_info);
}
if (static_cast<uint32_t>(dns2) != INADDR_NONE) {
esp_netif_dns_info_t dns_info;
dns_info.ip.u_addr.ip4.addr = static_cast<uint32_t>(dns2);
dns_info.ip.type = ESP_IPADDR_TYPE_V4;
esp_netif_set_dns_info(eth_netif, ESP_NETIF_DNS_BACKUP, &dns_info);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dns1/dns2 default to 0.0.0.0 (the documented "unset" value), but the guard only skipped INADDR_NONE (255.255.255.255). So config(ip, gw, mask) with the default DNS arguments wrote 0.0.0.0 into the netif DNS slots, breaking hostname resolution. Skip DNS application when the address is 0.0.0.0 as well as INADDR_NONE.
Bump version to 1.1.0 across library.json, library.properties and package.json for the ESP-IDF 4.4 + 5.5 dual-compatibility release. Replace the deprecated `platformio lib build` / `platformio lib examples` script commands in package.json with valid ones (`pio pkg pack` and a `pio run` over the examples).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #1 — the library targeted ESP-IDF 4.4.7 only, but Meshtastic
develophas moved to Arduino 3.3.8 / ESP-IDF 5.5.4 (pioarduino). Verified against the
upstream IDF 4.4 and 5.5
esp_ethheaders, the driver did not compile onIDF 5.x. These
.cppfiles compile as C++, where a mismatched function-pointertype is a hard error — so the existing
#if ESP_IDF_VERSIONguards were notenough.
Changes
PHY driver
esp_eth_phy_smembernegotiate→autonego_ctrl(taking IDF's own
eth_phy_autoneg_cmd_t). The unconditionalnegotiateassignment, and assigning an
int cmdfunction toautonego_ctrl, bothbroke the IDF 5.x build. All callers now route through a shared
ch390_autonego_apply()worker; the version-correct callback is assigned(
negotiatefor IDF < 5.0,autonego_ctrlfor >= 5.0).custom_ioctl— the stub only returnedESP_ERR_NOT_SUPPORTED,identical to what
esp_eth_ioctlreturns for a NULL pointer.MAC driver
custom_ioctl(same reasoning).esp_mac.hforesp_read_mac()/ESP_MAC_ETH— a separateheader since IDF 5.0.
transmit_vargs,add_mac_filter,rm_mac_filter,set_all_multicast) are intentionally left NULL:esp_eth_ioctlNULL-checks them and the CH390 already accepts all multicast (
RCR_ALL).ESP32_CH390
esp_eth_ioctlis a fixed 3-arg call in both IDF versions; removed thebogus 4-arg variadic wrapper.
readPHY()/writePHY()now useesp_eth_phy_reg_rw_data_ton IDF 5.x and the inline-value struct on 4.4.uint32_tto avoid the ambiguousoperator!=against the Arduino-ESP32 3.x
IPAddressclass.Build / CI
the CI matrix so both toolchains are compiled.
Testing
All three examples build clean locally on both toolchains:
espressif32)pioarduino)