-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fixes ethernet initialisation of static IP settings and modified some debug info #5262
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
base: main
Are you sure you want to change the base?
Fixes ethernet initialisation of static IP settings and modified some debug info #5262
Conversation
…ome debug messages.
WalkthroughReorganizes Ethernet configuration parsing in cfg.cpp by relocating the Ethernet deserialization block within deserializeConfig, and refactors Ethernet initialization logic in network.cpp by moving static IP configuration from the ETH_CONNECTED event handler to the post-ETH.begin phase and updating related logging messages. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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. Comment |
DedeHai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, looks good to me (I can't test as I have no such hardware)
|
Cool - do I need to do anything else or are we good @DedeHai? |
|
all good, just waiting for some more feedback |
|
@DedeHai & @softhack007 - sorry to nag, but can this BR be reviewed and approved? |
softhack007
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also I don't have any ethernet board to test with.
Code changes look ok for me, the PR seems to just move some code block around,
|
@softhack007 we can just "release into the wild" and see if there is any negative feedback. |
|
Correct... just moving code blocks around. It had no right to work before and it didn't. I'd be surprised if the changes break anything else. |
Summary
#5247
The static IP configuration happened too late in the ethernet initialization sequence.
The ETH.config() call from the event handler was moved to just after ETH.begin().
Additionally, initEthernet() was called before config deserialization was complete.
The initEthernet() was moved later in the config deserialization to ensure static IP and DNS configuration is completed prior to ethernet initialisation.
Testing performed
Build with default_envs = esp32_eth (no platformio_override.ini used)
WiFi Setup already exists (AP SSID customised, static IP set with custom DNS server address and AP opens set to 'No Connection after boot')
Tests done with and without DHCP being enabled on the Ethernet network. Expected behaviour seen regardless of DHCP or static IP Address usage.
No Ethernet connected
Ethernet connected
Power cycled board with existing WiFi Setup
Ethernet connection registered
WiFi AP stopped and Access point disabled
UI accessible via Ethernet and the defined static IP
WiFi Setup config retained and loaded
Disconnected ethernet
Ethernet disconnection event registered
WiFi AP started (AP SSID customised name observed)
WiFi AP tested and connected OK (4.3.2.1)
Removed all config files via http://4.3.2.1/edit
Rebooted board
WiFi AP started (AP SSID default name observed)
WiFi AP tested and connected OK (4.3.2.1)
WiFi Setup defaults observed
Static IP address, gateway and DNS set
Ethernet type set to QuinLED-Dig-Octa & T-ETH-POE
WiFi Setup saved
Confirmed that WiFiSetup changes were retained
Connected Ethernet, connection event registered
WiFi AP stopped and Access point disabled
UI accessible via Ethernet and the defined static IP
Rebooted board with ethernet connected
UI accessible via Ethernet and the defined static IP
Erased flash via esptool
Ethernet connected
Uploaded firmware build with fixes
WiFi AP started (AP SSID default name observed)
WiFi AP tested and connected OK (4.3.2.1)
WiFi Setup defaults observed
Ethernet connection down due to Ethernet Type being set to 'None' by default.
Static IP address, gateway, DNS and AP SSID customised name set
Ethernet type set to QuinLED-Dig-Octa & T-ETH-POE
WiFi Setup saved
Ethernet connection registered
WiFi AP stopped and Access point disabled
UI accessible via Ethernet and the defined static IP
WiFi Setup config retained and loaded
Disconnected ethernet
Ethernet disconnection event registered
WiFi AP started (AP SSID customised name observed)
WiFi AP tested and connected OK (4.3.2.1)