From 54d491079538dcc4dd126cc57068649e3bd1a394 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 14 Mar 2026 16:17:57 -1000 Subject: [PATCH 1/2] [ethernet] Address review feedback for RP2040 W5500 - Clean up eth_ on begin() failure to prevent leak and stale pointer - Add LwIPLock around dns_getserver() in get_dns_address() - Add LwIPLock around dns_setserver() in start_connect_() - Add LwIPLock around dns_getserver() in dump_connect_params_() - Reject clock_speed and polling_interval config options on RP2040 --- esphome/components/ethernet/__init__.py | 15 +++++--- .../ethernet/ethernet_component_rp2040.cpp | 36 ++++++++++++------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/esphome/components/ethernet/__init__.py b/esphome/components/ethernet/__init__.py index 0504ab7a1f..eccd0a25a5 100644 --- a/esphome/components/ethernet/__init__.py +++ b/esphome/components/ethernet/__init__.py @@ -247,11 +247,16 @@ def _validate(config): f"{config[CONF_TYPE]} PHY requires RMII interface and is only supported " f"on ESP32 classic and ESP32-P4, not {variant}" ) - elif CORE.is_rp2040 and config[CONF_TYPE] not in RP2040_SPI_ETHERNET_TYPES: - raise cv.Invalid( - f"Only {', '.join(RP2040_SPI_ETHERNET_TYPES)} are supported on RP2040, " - f"not {config[CONF_TYPE]}" - ) + elif CORE.is_rp2040: + if config[CONF_TYPE] not in RP2040_SPI_ETHERNET_TYPES: + raise cv.Invalid( + f"Only {', '.join(RP2040_SPI_ETHERNET_TYPES)} are supported on RP2040, " + f"not {config[CONF_TYPE]}" + ) + if CONF_CLOCK_SPEED in config: + raise cv.Invalid(f"'{CONF_CLOCK_SPEED}' is not supported on RP2040") + if CONF_POLLING_INTERVAL in config: + raise cv.Invalid(f"'{CONF_POLLING_INTERVAL}' is not supported on RP2040") return config diff --git a/esphome/components/ethernet/ethernet_component_rp2040.cpp b/esphome/components/ethernet/ethernet_component_rp2040.cpp index 8fe930bdbf..22a748ef48 100644 --- a/esphome/components/ethernet/ethernet_component_rp2040.cpp +++ b/esphome/components/ethernet/ethernet_component_rp2040.cpp @@ -62,6 +62,8 @@ void EthernetComponent::setup() { if (!success) { ESP_LOGE(TAG, "Failed to initialize W5500 Ethernet"); + delete this->eth_; // NOLINT(cppcoreguidelines-owning-memory) + this->eth_ = nullptr; this->mark_failed(); return; } @@ -178,6 +180,7 @@ network::IPAddresses EthernetComponent::get_ip_addresses() { } network::IPAddress EthernetComponent::get_dns_address(uint8_t num) { + LwIPLock lock; const ip_addr_t *dns_ip = dns_getserver(num); return dns_ip; } @@ -234,6 +237,7 @@ void EthernetComponent::start_connect_() { if (this->manual_ip_.has_value()) { // Static IP was already configured before begin() in setup() // Set DNS servers + LwIPLock lock; if (this->manual_ip_->dns1.is_set()) { ip_addr_t d; d = this->manual_ip_->dns1; @@ -269,19 +273,25 @@ void EthernetComponent::dump_connect_params_() { char mac_buf[MAC_ADDRESS_PRETTY_BUFFER_SIZE]; auto *netif = this->eth_->getNetIf(); - ESP_LOGCONFIG( - TAG, - " IP Address: %s\n" - " Hostname: '%s'\n" - " Subnet: %s\n" - " Gateway: %s\n" - " DNS1: %s\n" - " DNS2: %s\n" - " MAC Address: %s", - network::IPAddress(&netif->ip_addr).str_to(ip_buf), App.get_name().c_str(), - network::IPAddress(&netif->netmask).str_to(subnet_buf), network::IPAddress(&netif->gw).str_to(gateway_buf), - network::IPAddress(dns_getserver(0)).str_to(dns1_buf), network::IPAddress(dns_getserver(1)).str_to(dns2_buf), - this->get_eth_mac_address_pretty_into_buffer(mac_buf)); + const ip_addr_t *dns_ip1; + const ip_addr_t *dns_ip2; + { + LwIPLock lock; + dns_ip1 = dns_getserver(0); + dns_ip2 = dns_getserver(1); + } + ESP_LOGCONFIG(TAG, + " IP Address: %s\n" + " Hostname: '%s'\n" + " Subnet: %s\n" + " Gateway: %s\n" + " DNS1: %s\n" + " DNS2: %s\n" + " MAC Address: %s", + network::IPAddress(&netif->ip_addr).str_to(ip_buf), App.get_name().c_str(), + network::IPAddress(&netif->netmask).str_to(subnet_buf), + network::IPAddress(&netif->gw).str_to(gateway_buf), network::IPAddress(dns_ip1).str_to(dns1_buf), + network::IPAddress(dns_ip2).str_to(dns2_buf), this->get_eth_mac_address_pretty_into_buffer(mac_buf)); } void EthernetComponent::set_clk_pin(uint8_t clk_pin) { this->clk_pin_ = clk_pin; } From ad0eb6aad5b064cddeb6a8e8a6126114caf2c7f4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 14 Mar 2026 16:21:00 -1000 Subject: [PATCH 2/2] [ethernet] Make clock_speed and polling_interval ESP32-only These options are handled by arduino-pico internally on RP2040. Use cv.only_on([Platform.ESP32]) in the schema validators instead of rejecting them in _validate(). --- esphome/components/ethernet/__init__.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/esphome/components/ethernet/__init__.py b/esphome/components/ethernet/__init__.py index eccd0a25a5..dcec324bab 100644 --- a/esphome/components/ethernet/__init__.py +++ b/esphome/components/ethernet/__init__.py @@ -247,16 +247,11 @@ def _validate(config): f"{config[CONF_TYPE]} PHY requires RMII interface and is only supported " f"on ESP32 classic and ESP32-P4, not {variant}" ) - elif CORE.is_rp2040: - if config[CONF_TYPE] not in RP2040_SPI_ETHERNET_TYPES: - raise cv.Invalid( - f"Only {', '.join(RP2040_SPI_ETHERNET_TYPES)} are supported on RP2040, " - f"not {config[CONF_TYPE]}" - ) - if CONF_CLOCK_SPEED in config: - raise cv.Invalid(f"'{CONF_CLOCK_SPEED}' is not supported on RP2040") - if CONF_POLLING_INTERVAL in config: - raise cv.Invalid(f"'{CONF_POLLING_INTERVAL}' is not supported on RP2040") + elif CORE.is_rp2040 and config[CONF_TYPE] not in RP2040_SPI_ETHERNET_TYPES: + raise cv.Invalid( + f"Only {', '.join(RP2040_SPI_ETHERNET_TYPES)} are supported on RP2040, " + f"not {config[CONF_TYPE]}" + ) return config @@ -315,10 +310,13 @@ SPI_SCHEMA = cv.All( cv.Optional(CONF_INTERRUPT_PIN): pins.internal_gpio_input_pin_number, cv.Optional(CONF_RESET_PIN): pins.internal_gpio_output_pin_number, cv.Optional(CONF_CLOCK_SPEED, default="26.67MHz"): cv.All( - cv.frequency, cv.int_range(int(8e6), int(80e6)) + cv.only_on([Platform.ESP32]), + cv.frequency, + cv.int_range(int(8e6), int(80e6)), ), # Set default value (SPI_ETHERNET_DEFAULT_POLLING_INTERVAL) at _validate() cv.Optional(CONF_POLLING_INTERVAL): cv.All( + cv.only_on([Platform.ESP32]), cv.positive_time_period_milliseconds, cv.Range(min=TimePeriodMilliseconds(milliseconds=1)), ),