nf-interpreter icon indicating copy to clipboard operation
nf-interpreter copied to clipboard

Update IP addresses in ESP_NETIF to stop static addresses being overwritten

Open AdrianSoundy opened this issue 9 months ago • 7 comments

Description

This has been an issue since the IDF changed from TCPIP adapters to the ESP_NETIF interface. The problem is the ESP_NETIF starts the DHCP when the WIFI connects overriding the static IP settings.

This change updates IP addresses in ESP_NETIF by call esp_netif_set_ip_info() for Ethernet & Wireless networks. Also tidies up code by moving common code to new file NF_ESP32_Network.cpp

Motivation and Context

  • Fixes nanoFramework/Home#1087

How Has This Been Tested?

Some testing was done at the time these changes where done.
Further testing should be done to confirm chnage is working correctly.

Types of changes

  • [ ] Improvement (non-breaking change that improves a feature, code or algorithm)
  • [x] Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • [ ] New feature (non-breaking change which adds functionality to code)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Config and build (change in the configuration and build system, has no impact on code or features)
  • [ ] Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • [ ] Dependencies/declarations (update dependencies or assembly declarations and changes associated, has no impact on code or features)
  • [ ] Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • [x] My code follows the code style of this project (only if there are changes in source code).
  • [ ] My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • [ ] I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • [x] I have read the CONTRIBUTING document.
  • [x] I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

Summary by CodeRabbit

  • New Features

    • Enhanced ESP32 networking capabilities with improved static IP support for Ethernet and refined Wi-Fi station setup, offering more consistent connectivity.
    • Added functions for managing network configurations, including retrieving network configuration blocks and configuring network settings based on provided configurations.
  • Refactor

    • Streamlined the network initialization process by removing redundant waiting mechanisms, ensuring a more efficient and reliable configuration experience without altering existing functionality.
  • Documentation

    • Updated header files to include new function declarations related to network configuration management.

AdrianSoundy avatar Mar 14 '25 06:03 AdrianSoundy

Walkthrough

This update integrates new ESP32 network configuration functionality. A new source file is added to the ESP32 networking build list and implements functions to retrieve network configuration blocks, configure network settings, and apply settings by index. The Ethernet event handler now applies static IP configuration upon connection. The wireless initialization applies static IP configuration for the Wi-Fi station interface. The Ethernet code replaces error aborts with non-aborting checks. The LWIP sockets driver is extended with an ESP32-specific network configuration update method. Corresponding header declarations are added.

Changes

File(s) Change Summary
targets/ESP32/.../CMakeLists.txt Added NF_ESP32_Network.cpp to the ESP32 networking source files list.
targets/ESP32/_Network/NF_ESP32_Ethernet.cpp Replaced ESP_ERROR_CHECK with ESP_ERROR_CHECK_WITHOUT_ABORT in Ethernet initialization to avoid aborting on errors.
targets/ESP32/_Network/NF_ESP32_Network.cpp
targets/ESP32/_include/NF_ESP32_Network.h
Added new networking utility functions: NF_ESP32_GetEspNetif, NF_ESP32_Wait_NetNumber, NF_ESP32_GetNetworkConfigBlock, and NF_ESP32_ConfigureNetworkByConfigIndex with declarations.
targets/ESP32/_Network/NF_ESP32_Wireless.cpp Removed NF_ESP32_Wait_NetNumber function; added call to NF_ESP32_ConfigureNetworkByConfigIndex for Wi-Fi station interface in initialization with error logging.
targets/ESP32/_common/targetHAL_Network.cpp Added call to NF_ESP32_ConfigureNetworkByConfigIndex in Ethernet event handler on ETHERNET_EVENT_CONNECTED before posting availability.
src/PAL/Lwip/lwIP_Sockets.cpp Added ESP32-specific implementation of LWIP_SOCKETS_Driver::UpdateAdapterConfiguration using ESP-IDF APIs for DHCP, DNS, MAC, and IP updates; original implementation unchanged for other platforms.

Assessment against linked issues

Objective Addressed Explanation
Wifi Static IP assignment does not work (Issue #1087) Static IP configuration applied via new network config functions and invoked during Wi-Fi and Ethernet initialization.

Suggested labels

Platform: ESP32

Suggested reviewers

  • josesimoes

📜 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 183cc5cde4caebc70d2e647612dd84c78fa1f124 and f1da0d458747a3fc0def30a1a4f3e5ea80b1d057.

📒 Files selected for processing (1)
  • src/PAL/Lwip/lwIP_Sockets.cpp (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/PAL/Lwip/lwIP_Sockets.cpp
✨ Finishing Touches
  • [ ] 📝 Generate Docstrings

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

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Mar 14 '25 06:03 coderabbitai[bot]

@alberk8 Are you able to test this is working OK. I did this change a while ago and got distracted with something else so PR was never raised. This has now been rebased with current version and should still be OK. I am currently away so can't complete it for a couple of weeks.

AdrianSoundy avatar Mar 14 '25 06:03 AdrianSoundy

There are some useful suggestions coming out of coderabbitai for existing code but I don't have time at the moment to follow them up. I can do that once I am back from holiday.

AdrianSoundy avatar Mar 14 '25 06:03 AdrianSoundy

@AdrianSoundy , I have tested on ESP32 with WIFI and it still using Router DHCP IP. The output shows the static IP that is set but checking with the router interface it shows that the actual IP is 192.168.1.97, it does not seems to respond to ping.

 bool isConnected = false;

 IPConfiguration ipconfig = new IPConfiguration("192.168.1.153", "255.255.255.0", "192.168.1.1", new string[] { "192.168.1.1" });

 isConnected = WifiNetworkHelper.ConnectFixAddress(_ssid, _password, ipconfig, WifiReconnectionKind.Automatic, requiresDateTime: true, token: cancelToken.Token);

 Console.WriteLine("Connected to Wifi: " + isConnected + " Date: " + DateTime.UtcNow.ToString());
 if (isConnected)
 {
     var networkInterface = NetworkInterface.GetAllNetworkInterfaces()[0];
     var ipAddress = networkInterface.IPv4Address.ToString();
     Debug.WriteLine("IP Address: " + ipAddress);
 }

Output Connected to Wifi: True Date: 01/01/1970 00:17:32 IP Address: 192.168.1.153

alberk8 avatar Mar 16 '25 10:03 alberk8

@AdrianSoundy , I have tested on ESP32 with WIFI and it still using Router DHCP IP. The output shows the static IP that is set but checking with the router interface it shows that the actual IP is 192.168.1.97, it does not seems to respond to ping.

 bool isConnected = false;

 IPConfiguration ipconfig = new IPConfiguration("192.168.1.153", "255.255.255.0", "192.168.1.1", new string[] { "192.168.1.1" });

 isConnected = WifiNetworkHelper.ConnectFixAddress(_ssid, _password, ipconfig, WifiReconnectionKind.Automatic, requiresDateTime: true, token: cancelToken.Token);

 Console.WriteLine("Connected to Wifi: " + isConnected + " Date: " + DateTime.UtcNow.ToString());
 if (isConnected)
 {
     var networkInterface = NetworkInterface.GetAllNetworkInterfaces()[0];
     var ipAddress = networkInterface.IPv4Address.ToString();
     Debug.WriteLine("IP Address: " + ipAddress);
 }

Output Connected to Wifi: True Date: 01/01/1970 00:17:32 IP Address: 192.168.1.153

It is possible that the router is caching the IP for the default 24 hour period, and it is not forcing it?!

networkfusion avatar Mar 16 '25 19:03 networkfusion

No, I reboot everything and the ESP32 in question was not not connected to the router for more than 3 days.

alberk8 avatar Mar 17 '25 00:03 alberk8

No, I reboot everything and the ESP32 in question was not connected to the router for more than 3 days.

Thanks for testing. I can't look at this until I am back in NZ around 30th March. Currently in Australia. I know I did do some testing and it seemed to be working.

AdrianSoundy avatar Mar 17 '25 08:03 AdrianSoundy

@alberk8 @josesimoes This should be complete now can you have a look

AdrianSoundy avatar Jul 22 '25 04:07 AdrianSoundy