Skip to content
New issue

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

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

Already on GitHub? # to your account

IDF v4 Ethernet/Network Fixes #4526

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
23 changes: 18 additions & 5 deletions wled00/network.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ int getSignalQuality(int rssi)
return quality;
}

#if ESP_IDF_VERSION_MAJOR >= 4
Copy link
Collaborator

Choose a reason for hiding this comment

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

I went the other way around:

#if defined(ESP8266)
  #define ARDUINO_EVENT_WIFI_AP_STADISCONNECTED WIFI_EVENT_SOFTAPMODE_STADISCONNECTED
  #define ARDUINO_EVENT_WIFI_AP_STACONNECTED    WIFI_EVENT_SOFTAPMODE_STACONNECTED
  #define ARDUINO_EVENT_WIFI_STA_GOT_IP         WIFI_EVENT_STAMODE_GOT_IP
  #define ARDUINO_EVENT_WIFI_STA_CONNECTED      WIFI_EVENT_STAMODE_CONNECTED
  #define ARDUINO_EVENT_WIFI_STA_DISCONNECTED   WIFI_EVENT_STAMODE_DISCONNECTED
#elif defined(ARDUINO_ARCH_ESP32) && !defined(ESP_ARDUINO_VERSION_MAJOR) //ESP_IDF_VERSION_MAJOR==3
  // not strictly IDF v3 but Arduino core related
  #define ARDUINO_EVENT_WIFI_AP_STADISCONNECTED SYSTEM_EVENT_AP_STADISCONNECTED
  #define ARDUINO_EVENT_WIFI_AP_STACONNECTED    SYSTEM_EVENT_AP_STACONNECTED
  #define ARDUINO_EVENT_WIFI_STA_LOST_IP        SYSTEM_EVENT_STA_LOST_IP
  #define ARDUINO_EVENT_WIFI_STA_GOT_IP         SYSTEM_EVENT_STA_GOT_IP
  #define ARDUINO_EVENT_WIFI_STA_CONNECTED      SYSTEM_EVENT_STA_CONNECTED
  #define ARDUINO_EVENT_WIFI_STA_DISCONNECTED   SYSTEM_EVENT_STA_DISCONNECTED
  #define ARDUINO_EVENT_WIFI_AP_START           SYSTEM_EVENT_AP_START
  #define ARDUINO_EVENT_WIFI_AP_STOP            SYSTEM_EVENT_AP_STOP
  #define ARDUINO_EVENT_WIFI_SCAN_DONE          SYSTEM_EVENT_SCAN_DONE
  #define ARDUINO_EVENT_ETH_START               SYSTEM_EVENT_ETH_START
  #define ARDUINO_EVENT_ETH_CONNECTED           SYSTEM_EVENT_ETH_CONNECTED
  #define ARDUINO_EVENT_ETH_DISCONNECTED        SYSTEM_EVENT_ETH_DISCONNECTED
#endif

#define SYSTEM_EVENT_ETH_CONNECTED ARDUINO_EVENT_ETH_CONNECTED
#define SYSTEM_EVENT_ETH_DISCONNECTED ARDUINO_EVENT_ETH_DISCONNECTED
#define SYSTEM_EVENT_ETH_START ARDUINO_EVENT_ETH_START
#define SYSTEM_EVENT_ETH_GOT_IP ARDUINO_EVENT_ETH_GOT_IP
#endif

//handle Ethernet connection event
void WiFiEvent(WiFiEvent_t event)
Expand All @@ -178,12 +184,21 @@ void WiFiEvent(WiFiEvent_t event)
case SYSTEM_EVENT_ETH_START:
DEBUG_PRINTLN(F("ETH Started"));
break;
case SYSTEM_EVENT_ETH_GOT_IP:
if (Network.isEthernet()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need to check if this is Ethernet? After all, this is Ethernet event.

On a side note there is a bug in network.cpp dependency.

bool NetworkClass::isConnected()
{
#if defined(ARDUINO_ARCH_ESP32) && defined(WLED_USE_ETHERNET)
  return (WiFi.localIP()[0] != 0 && WiFi.status() == WL_CONNECTED) || (ETH.localIP()[0] != 0 && ETH.linkUp());
#else
  return (WiFi.localIP()[0] != 0 && WiFi.status() == WL_CONNECTED);
#endif
}

bool NetworkClass::isEthernet()
{
#if defined(ARDUINO_ARCH_ESP32) && defined(WLED_USE_ETHERNET)
  return (ETH.localIP()[0] != 0) && WiFi.getMode() == WIFI_MODE_NULL;
#endif
  return false;
}

This is the correct implementation if you care to add it to this PR.
@softhack007 has asked about this bug.

if (!apActive) {
DEBUG_PRINTLN(F("WiFi Connected *and* ETH Connected. Disabling WIFi"));
WiFi.disconnect(true);
} else {
DEBUG_PRINTLN(F("WiFi Connected *and* ETH Connected. Leaving AP WiFi active"));
}
} else {
DEBUG_PRINTLN(F("WiFi Connected. No ETH"));
}
break;
case SYSTEM_EVENT_ETH_CONNECTED:
{
DEBUG_PRINTLN(F("ETH Connected"));
if (!apActive) {
WiFi.disconnect(true);
}
if (multiWiFi[0].staticIP != (uint32_t)0x00000000 && multiWiFi[0].staticGW != (uint32_t)0x00000000) {
ETH.config(multiWiFi[0].staticIP, multiWiFi[0].staticGW, multiWiFi[0].staticSN, dnsAddress);
} else {
Expand All @@ -207,8 +222,6 @@ void WiFiEvent(WiFiEvent_t event)
break;
#endif
default:
DEBUG_PRINTF_P(PSTR("Network event: %d\n"), (int)event);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still valuable debugging info. Can you please revert?

break;
}
}

13 changes: 11 additions & 2 deletions wled00/wled.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1010,8 +1010,17 @@ void WLED::handleConnection()
}
} else if (!interfacesInited) { //newly connected
DEBUG_PRINTLN();
DEBUG_PRINT(F("Connected! IP address: "));
DEBUG_PRINTLN(Network.localIP());
DEBUG_PRINTLN("");
DEBUG_PRINT(F("Connected! IP address: http://"));
DEBUG_PRINT(Network.localIP());
if (Network.isEthernet()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless network.cpp is updated this will return true if static IP is set.
I will disconnect WiFi in such case even if link is not up.

#if ESP32
DEBUG_PRINTLN(" via Ethernet (disabling WiFi)");
WiFi.disconnect(true);
#endif
} else {
DEBUG_PRINTLN(" via WiFi");
}
if (improvActive) {
if (improvError == 3) sendImprovStateResponse(0x00, true);
sendImprovStateResponse(0x04);
Expand Down