Skip to content

Update IP addresses in ESP_NETIF to stop static addresses being overwritten #3133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions targets/ESP32/_Network/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/esp32_ethernet_options.h.in
${CMAKE_BINARY_DIR}/targets/${RTOS}/${TARGET_BOARD}/esp32_ethernet_options.h @ONLY)

# append networking files, if enabled
list(APPEND TARGET_ESP32_IDF_NETWORK_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/NF_ESP32_Network.cpp)
list(APPEND TARGET_ESP32_IDF_NETWORK_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/NF_ESP32_Ethernet.cpp)
list(APPEND TARGET_ESP32_IDF_NETWORK_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/NF_ESP32_Wireless.cpp)
list(APPEND TARGET_ESP32_IDF_NETWORK_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/NF_ESP32_SmartConfig.cpp)
Expand Down
3 changes: 3 additions & 0 deletions targets/ESP32/_Network/NF_ESP32_Ethernet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ esp_err_t NF_ESP32_InitialiseEthernet(uint8_t *pMacAdr)
// attach Ethernet driver to TCP/IP stack
ESP_ERROR_CHECK(esp_netif_attach(eth_netif, esp_eth_new_netif_glue(eth_handle)));

// COnfigure static address if required in config
ESP_ERROR_CHECK(NF_ESP32_ConfigureNetworkByIndex(IDF_ETH_DEF, eth_netif));

// start Ethernet driver state machine
ESP_ERROR_CHECK(esp_eth_start(eth_handle));

Expand Down
118 changes: 118 additions & 0 deletions targets/ESP32/_Network/NF_ESP32_Network.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
//
// Copyright (c) .NET Foundation and Contributors
// See LICENSE file in the project root for full license information.
//

// This file includes common method for networking code

#include "NF_ESP32_Network.h"
#include "esp_netif_net_stack.h"

// Wait for the network interface to become available
int NF_ESP32_Wait_NetNumber(int num)
{
int number = 0;

esp_netif_t *espNetif;

while (true)
{
switch (num)
{
case IDF_WIFI_STA_DEF:
espNetif = esp_netif_get_handle_from_ifkey("WIFI_STA_DEF");
break;

case IDF_WIFI_AP_DEF:
espNetif = esp_netif_get_handle_from_ifkey("WIFI_AP_DEF");
break;

case IDF_ETH_DEF:
espNetif = esp_netif_get_handle_from_ifkey("ETH_DEF");
break;

case IDF_OT_DEF:
espNetif = esp_netif_get_handle_from_ifkey("OT_DEF");
break;

default:
// can't reach here
HAL_AssertEx();
break;
}

if (espNetif != NULL)
{
break;
}

vTaskDelay(20 / portTICK_PERIOD_MS);
}

return espNetif->lwip_netif->num;
}

HAL_Configuration_NetworkInterface * NF_ESP32_GetNetworkConfigBlock(int index)
{
HAL_Configuration_NetworkInterface *networkConfig =
(HAL_Configuration_NetworkInterface *)platform_malloc(sizeof(HAL_Configuration_NetworkInterface));

if (networkConfig != NULL)
{
if (ConfigurationManager_GetConfigurationBlock(networkConfig, DeviceConfigurationOption_Network, index))
{
return networkConfig;
}
}

return NULL;
}

//
// Configure network settings for a espressif network interface
//
esp_err_t NF_ESP32_ConfigureNetwork(esp_netif_t *netIf, HAL_Configuration_NetworkInterface *config)
{
esp_err_t ec;
esp_netif_ip_info_t ip_info;

ec = esp_netif_get_ip_info(netIf, &ip_info);
if (ec != ESP_OK)
{
return ec;
}

bool enableDHCP = (config->StartupAddressMode == AddressMode_DHCP);

// Set static addresses
if (config->IPv4Address != 0)
{
ip_info.ip.addr = config->IPv4Address;
ip_info.netmask.addr = config->IPv4NetMask;
ip_info.gw.addr = config->IPv4GatewayAddress;

ec = esp_netif_set_ip_info(netIf, &ip_info);

// Make sure DHCP client is disabled
netIf->flags = (esp_netif_flags_t)(netIf->flags & ~ESP_NETIF_DHCP_CLIENT);
Copy link

@coderabbitai coderabbitai bot Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid direct flag manipulation to stop DHCP client

Directly modifying netIf->flags may be fragile if the ESP-IDF API changes. Use the official ESP-IDF API function esp_netif_dhcpc_stop() to disable the DHCP client.

- netIf->flags = (esp_netif_flags_t)(netIf->flags & ~ESP_NETIF_DHCP_CLIENT);
+ esp_err_t dhcp_stop_result = esp_netif_dhcpc_stop(netIf);
+ // It's okay if DHCP was already stopped
+ if (dhcp_stop_result != ESP_OK && dhcp_stop_result != ESP_ERR_ESP_NETIF_DHCP_ALREADY_STOPPED)
+ {
+     return dhcp_stop_result;
+ }
📝 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.

Suggested change
netIf->flags = (esp_netif_flags_t)(netIf->flags & ~ESP_NETIF_DHCP_CLIENT);
esp_err_t dhcp_stop_result = esp_netif_dhcpc_stop(netIf);
// It's okay if DHCP was already stopped
if (dhcp_stop_result != ESP_OK && dhcp_stop_result != ESP_ERR_ESP_NETIF_DHCP_ALREADY_STOPPED)
{
return dhcp_stop_result;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AdrianSoundy this looks a valid observation. Do you agree?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

}

return ec;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing DHCP client enablement when configured

The function calculates whether DHCP should be enabled but only handles disabling it for static IP configurations. When enableDHCP is true, the code should enable the DHCP client.

esp_err_t NF_ESP32_ConfigureNetwork(esp_netif_t *netIf, HAL_Configuration_NetworkInterface *config)
{
    esp_err_t ec;
    esp_netif_ip_info_t ip_info;

    ec = esp_netif_get_ip_info(netIf, &ip_info);
    if (ec != ESP_OK) 
    {
          return ec;
    }

    bool enableDHCP = (config->StartupAddressMode == AddressMode_DHCP);

+   // Handle DHCP configuration
+   if (enableDHCP)
+   {
+       // Enable DHCP client
+       ec = esp_netif_dhcpc_start(netIf);
+       return ec;
+   }
+   // Static IP configuration
    // Set static addresses
    if (config->IPv4Address != 0)
    {
        ip_info.ip.addr = config->IPv4Address;
        ip_info.netmask.addr = config->IPv4NetMask;
        ip_info.gw.addr = config->IPv4GatewayAddress;

        ec = esp_netif_set_ip_info(netIf, &ip_info);

        // Make sure DHCP client is disabled
        netIf->flags = (esp_netif_flags_t)(netIf->flags & ~ESP_NETIF_DHCP_CLIENT);
+       ec = esp_netif_dhcpc_stop(netIf);
+       if (ec != ESP_OK && ec != ESP_ERR_ESP_NETIF_DHCP_ALREADY_STOPPED)
+       {
+           return ec;
+       }
    }

    return ec;
}
📝 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.

Suggested change
//
// Configure network settings for a espressif network interface
//
esp_err_t NF_ESP32_ConfigureNetwork(esp_netif_t *netIf, HAL_Configuration_NetworkInterface *config)
{
esp_err_t ec;
esp_netif_ip_info_t ip_info;
ec = esp_netif_get_ip_info(netIf, &ip_info);
if (ec != ESP_OK)
{
return ec;
}
bool enableDHCP = (config->StartupAddressMode == AddressMode_DHCP);
// Set static addresses
if (config->IPv4Address != 0)
{
ip_info.ip.addr = config->IPv4Address;
ip_info.netmask.addr = config->IPv4NetMask;
ip_info.gw.addr = config->IPv4GatewayAddress;
ec = esp_netif_set_ip_info(netIf, &ip_info);
// Make sure DHCP client is disabled
netIf->flags = (esp_netif_flags_t)(netIf->flags & ~ESP_NETIF_DHCP_CLIENT);
}
return ec;
}
//
// Configure network settings for a espressif network interface
//
esp_err_t NF_ESP32_ConfigureNetwork(esp_netif_t *netIf, HAL_Configuration_NetworkInterface *config)
{
esp_err_t ec;
esp_netif_ip_info_t ip_info;
ec = esp_netif_get_ip_info(netIf, &ip_info);
if (ec != ESP_OK)
{
return ec;
}
bool enableDHCP = (config->StartupAddressMode == AddressMode_DHCP);
// Handle DHCP configuration
if (enableDHCP)
{
// Enable DHCP client
ec = esp_netif_dhcpc_start(netIf);
return ec;
}
// Static IP configuration
// Set static addresses
if (config->IPv4Address != 0)
{
ip_info.ip.addr = config->IPv4Address;
ip_info.netmask.addr = config->IPv4NetMask;
ip_info.gw.addr = config->IPv4GatewayAddress;
ec = esp_netif_set_ip_info(netIf, &ip_info);
// Make sure DHCP client is disabled
netIf->flags = (esp_netif_flags_t)(netIf->flags & ~ESP_NETIF_DHCP_CLIENT);
ec = esp_netif_dhcpc_stop(netIf);
if (ec != ESP_OK && ec != ESP_ERR_ESP_NETIF_DHCP_ALREADY_STOPPED)
{
return ec;
}
}
return ec;
}


esp_err_t NF_ESP32_ConfigureNetworkByIndex(int index, esp_netif_t *netIf)
{
esp_err_t ec;

HAL_Configuration_NetworkInterface *networkConfig = NF_ESP32_GetNetworkConfigBlock(index);
if (networkConfig == NULL)
{
return ESP_FAIL;
}

ec = NF_ESP32_ConfigureNetwork(netIf, networkConfig);

platform_free(networkConfig);

return ec;
}
57 changes: 13 additions & 44 deletions targets/ESP32/_Network/NF_ESP32_Wireless.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ wifi_mode_t NF_ESP32_GetCurrentWifiMode()
return current_wifi_mode;
}

esp_err_t NF_ESP32_ConfigureNetworkStation(esp_netif_t *netIf)
{
return NF_ESP32_ConfigureNetworkByIndex(IDF_WIFI_STA_DEF, netIf);
}

void NF_ESP32_DeinitWifi()
{
// clear flags
Expand Down Expand Up @@ -155,6 +160,14 @@ esp_err_t NF_ESP32_InitaliseWifi()
// create Wi-Fi STA (ignoring return)
wifiStaNetif = esp_netif_create_default_wifi_sta();

// Set static address if configured
// ignore any errors
ec = NF_ESP32_ConfigureNetworkStation(wifiStaNetif);
if (ec != ESP_OK)
{
ESP_LOGE(TAG, "Unable to configure Wifi station - result %d", ec);
}

// We need to start the WIFI stack before the station can Connect
// Also we can only get the NetIf number used by ESP IDF after it has been started.
// Starting will also start the Soft- AP (if we have enabled it).
Expand Down Expand Up @@ -514,47 +527,3 @@ bool NF_ESP32_WirelessAP_Close()
}

#endif

// Wait for the network interface to become available
int NF_ESP32_Wait_NetNumber(int num)
{
int number = 0;

esp_netif_t *espNetif;

while (true)
{
switch (num)
{
case IDF_WIFI_STA_DEF:
espNetif = esp_netif_get_handle_from_ifkey("WIFI_STA_DEF");
break;

case IDF_WIFI_AP_DEF:
espNetif = esp_netif_get_handle_from_ifkey("WIFI_AP_DEF");
break;

case IDF_ETH_DEF:
espNetif = esp_netif_get_handle_from_ifkey("ETH_DEF");
break;

case IDF_OT_DEF:
espNetif = esp_netif_get_handle_from_ifkey("OT_DEF");
break;

default:
// can't reach here
HAL_AssertEx();
break;
}

if (espNetif != NULL)
{
break;
}

vTaskDelay(20 / portTICK_PERIOD_MS);
}

return espNetif->lwip_netif->num;
}
3 changes: 3 additions & 0 deletions targets/ESP32/_include/NF_ESP32_Network.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,8 @@ void NF_ESP32_Start_wifi_smart_config(void);

// Helpers
int NF_ESP32_Wait_NetNumber(int num);
HAL_Configuration_NetworkInterface * NF_ESP32_GetNetworkConfigBlock(int index);
esp_err_t NF_ESP32_ConfigureNetwork(esp_netif_t *netIf, HAL_Configuration_NetworkInterface *config);
esp_err_t NF_ESP32_ConfigureNetworkByIndex(int index, esp_netif_t *netIf);

#endif // NF_ESP32_NETWORK_H
Loading