Ethernet priority - prioritize Ethernet over WiFi with proper IP wait #185
Ethernet priority - prioritize Ethernet over WiFi with proper IP wait #185craigmillard86 wants to merge 14 commits intoCarlosDerSeher:developfrom
Conversation
- Add network_has_ip() function to check if interface has valid IP - Wait for Ethernet to get IP before selecting it (not just link up) - If WiFi connects first, wait up to 5 seconds for Ethernet - Disable WiFi when Ethernet is active to save power - Don't overwrite netif from mDNS results This ensures Ethernet is reliably selected when both interfaces are configured, even if WiFi's DHCP completes faster.
…-snapclient into Ethernet-priority
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Please use develop branch |
Sorry, updated! |
|
What happens if it is connected to wifi and cable is plugged in much later? I've always wanted to implement a handover of connections, of course only if there is no playback. But I guess this is a PR on it's own and out of scope here. |
Currently I dont believe it switches, will take a look later, but as you say probably a seperate PR :) Will investigate |
…ected and viceversa.
- Add 2-second delay after disconnect before reconnecting to give server time to clean up old socket state (fixes server sending audio to stale WiFi connection instead of new Ethernet connection) - Use esp_netif_set_default_netif() instead of stopping WiFi for cleaner network switching - Add eth_on_playback_stopped() callback so player can trigger pending Ethernet takeover after playback ends (avoids interrupting audio) - Add app_request_reconnect() for external modules to trigger reconnection - Guard against divide-by-zero in player when chkInFrames is 0 during reconnection Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Actually the code was half working, I have now updated further to allow for full handover between wifi and ethernet. Would be great for some further testing, but seems pretty reliable on my setup. |
Wrap ethernet-specific code with CONFIG guards so the project builds correctly when ethernet is disabled in menuconfig: - network_interface.h: Guard eth_on_playback_stopped() declaration - player.c: Guard extern declaration and function call - CMakeLists.txt: Only compile eth_interface.c when ethernet enabled
| want_eth_takeover = false; | ||
| xSemaphoreGive(connIpSemaphoreHandle); | ||
| /* Request reconnect so main re-evaluates network and uses WiFi */ | ||
| extern void app_request_reconnect(void); |
There was a problem hiding this comment.
Could you put this somewhere at the top of this file? I don't like it inside the function
| /* Request main to reconnect so the snapclient binds to the preferred | ||
| * interface (ETH) for the next connection cycle. | ||
| */ | ||
| extern void app_request_reconnect(void); |
| /* Request main to reconnect so the snapclient binds to the preferred | ||
| * interface (ETH) for the next connection cycle. | ||
| */ | ||
| extern void app_request_reconnect(void); |
| /* Called by other modules to request the snapclient reconnect to the | ||
| * server (useful when the preferred network interface changes). | ||
| */ | ||
| void app_request_reconnect(void) { |
There was a problem hiding this comment.
Maybe we should use some different form of signaling here. A queue or the event system would be much cleaner don't you think
| vTaskDelay(pdMS_TO_TICKS(1000)); | ||
| } | ||
|
|
||
| network_selected: |
| } | ||
|
|
||
| // allow external modules to request a reconnect (set by app_request_reconnect()) | ||
| extern volatile bool reconnect_requested; |
There was a problem hiding this comment.
As said earlier a queue or event system should be the preferred method
|
|
||
| typedMsgCurrentPos = 0; | ||
|
|
||
| // ESP_LOGI(TAG, "BM type %d ts %ld.%ld, refers to %u", |
There was a problem hiding this comment.
This is here for debugging purposes. Maybe leave it in but set it to DEBUG and uncomment
…omponent communication - Replace volatile bool reconnect_requested with thread-safe EventGroups - Remove extern declarations (playerstarted, app_request_reconnect) for loose coupling - Move event group functions to network_interface.c (always compiled, not just with Ethernet) - Add network_playback_started/stopped() API for player to signal state changes - Add atomic test-and-clear for reconnect requests using xEventGroupWaitBits - Add playback monitor task with graceful shutdown via EVENT_MONITOR_SHUTDOWN_BIT - Add esp_err_t return values for proper error propagation - Add network_get_event_group() for eth_interface.c to access shared event group This addresses PR CarlosDerSeher#185 feedback about tight coupling and race conditions.
|
Based on the comments above and a few other reasons i have refactored this and merged with the static-IP changes as they interact a fair bit. |
The Problem
When both Ethernet and WiFi are configured on the ESP32, the original code had a couple of issues:
Race Condition at Boot
WiFi typically connects faster, certainly on my network, than Ethernet (WiFi authentication completing in 1-2 seconds, while Ethernet DHCP can take 3-5 seconds). The original code would select whichever interface came up first, meaning WiFi was almost always chosen even when Ethernet was available and preferred.
Link vs IP Detection
The original code only checked if the network interface link was up, not whether it had obtained a valid IP address. An interface can be "up" but still waiting for DHCP, making it unusable for communication.
mDNS Interface Confusion
The mDNS query for snapserver discovery would set its result's netif pointer, potentially overwriting the already-selected network interface and causing routing issues.
WiFi Power Waste
When Ethernet was connected and working, WiFi remained active, wasting power and potentially causing interference or confusion with dual network paths.
The Solution
WiFi Recovery
At the start of each selection attempt, esp_wifi_start() is called to ensure WiFi can reconnect if it was previously stopped. This handles the case where Ethernet was selected but later disconnected.
IP-Based Detection
The system checks if interfaces have a valid IP address using network_has_ip(). This function verifies both that the interface is up AND has obtained an IP (IPv4 or IPv6). An IP of 0.0.0.0 is not considered valid.
Ethernet Wait Period
If Ethernet doesn't have an IP but WiFi does, the system waits up to 5 seconds for Ethernet to obtain an IP. This handles the common scenario where WiFi connects faster than Ethernet during boot. The wait uses 500ms intervals with status logging.
Final Selection
After the wait period (or immediately if Ethernet already has IP):
If Ethernet has IP: Select Ethernet, stop WiFi to save power
If only WiFi has IP: Select WiFi
If neither has IP: Continue waiting in the main loop
WiFi Power Management
When Ethernet is selected, esp_wifi_stop() is called to disable the WiFi radio and save power. Error handling ensures the stop command succeeded.
mDNS Fix
The mDNS result's netif is no longer used to set the selected interface. The network selection is made independently, and mDNS just provides the snapserver address.
Key Functions
network_has_ip(esp_netif_t *netif) - Returns true if the interface has a valid IP address (not 0.0.0.0 for IPv4, or has link-local IPv6)
esp_wifi_start() / esp_wifi_stop() - Control WiFi radio state
esp_netif_get_ip_info() - Get IPv4 address information
esp_netif_get_ip6_linklocal() - Get IPv6 link-local address (when IPv6 enabled)
Add network_has_ip() function to check if interface has valid IP
Wait for Ethernet to get IP before selecting it (not just link up)
If WiFi connects first, wait up to 5 seconds for Ethernet
Disable WiFi when Ethernet is active to save power
Don't overwrite netif from mDNS results
This ensures Ethernet is reliably selected as the more stable interface for audio streaming when both interfaces are configured, even if WiFi's DHCP completes faster.