fix(net): Don't unregister events if there are interfaces still open (espressif#9706)

* fix(net): Don't unreg events if there are netifs

Unregister IP events only if all other netifs are stopped.

* fix(eth): Delete mac and phy on end

* fix(net): Update pin naming and log levels

Author: me-no-dev

Diff for: libraries/Ethernet/src/ETH.cpp

@@ -42,14 +42,15 @@
 #include "esp_netif_defaults.h"
 #include "esp_eth_phy.h"
 
-static ETHClass *_ethernets[3] = {NULL, NULL, NULL};
+#define NUM_SUPPORTED_ETH_PORTS 3
+static ETHClass *_ethernets[NUM_SUPPORTED_ETH_PORTS] = {NULL, NULL, NULL};
 static esp_event_handler_instance_t _eth_ev_instance = NULL;
 
 static void _eth_event_cb(void *arg, esp_event_base_t event_base, int32_t event_id, void *event_data) {
 
   if (event_base == ETH_EVENT) {
     esp_eth_handle_t eth_handle = *((esp_eth_handle_t *)event_data);
-    for (int i = 0; i < 3; ++i) {
+    for (int i = 0; i < NUM_SUPPORTED_ETH_PORTS; ++i) {
       if (_ethernets[i] != NULL && _ethernets[i]->handle() == eth_handle) {
         _ethernets[i]->_onEthEvent(event_id, event_data);
       }
@@ -60,14 +61,14 @@ static void _eth_event_cb(void *arg, esp_event_base_t event_base, int32_t event_
 // This callback needs to be aware of which interface it should match against
 static void onEthConnected(arduino_event_id_t event, arduino_event_info_t info) {
   if (event == ARDUINO_EVENT_ETH_CONNECTED) {
-    uint8_t index = 3;
-    for (int i = 0; i < 3; ++i) {
+    uint8_t index = NUM_SUPPORTED_ETH_PORTS;
+    for (int i = 0; i < NUM_SUPPORTED_ETH_PORTS; ++i) {
       if (_ethernets[i] != NULL && _ethernets[i]->handle() == info.eth_connected) {
         index = i;
         break;
       }
     }
-    if (index == 3) {
+    if (index == NUM_SUPPORTED_ETH_PORTS) {
       log_e("Could not find ETH interface with that handle!");
       return;
     }
@@ -118,7 +119,7 @@ void ETHClass::_onEthEvent(int32_t event_id, void *event_data) {
 }
 
 ETHClass::ETHClass(uint8_t eth_index)
-  : _eth_handle(NULL), _eth_index(eth_index), _phy_type(ETH_PHY_MAX), _glue_handle(NULL)
+  : _eth_handle(NULL), _eth_index(eth_index), _phy_type(ETH_PHY_MAX), _glue_handle(NULL), _mac(NULL), _phy(NULL)
 #if ETH_SPI_SUPPORTS_CUSTOM
     ,
     _spi(NULL)
@@ -534,7 +535,11 @@ bool ETHClass::beginSPI(
   if (_spi != NULL) {
     pinMode(_pin_cs, OUTPUT);
     digitalWrite(_pin_cs, HIGH);
-    perimanSetPinBusExtraType(_pin_cs, "ETH_CS");
+    char cs_num_str[3];
+    itoa(_eth_index, cs_num_str, 10);
+    strcat(strcpy(_cs_str, "ETH_CS["), cs_num_str);
+    strcat(_cs_str, "]");
+    perimanSetPinBusExtraType(_pin_cs, _cs_str);
   }
 #endif
 
@@ -586,8 +591,6 @@ bool ETHClass::beginSPI(
   spi_devcfg.spics_io_num = _pin_cs;
   spi_devcfg.queue_size = 20;
 
-  esp_eth_mac_t *mac = NULL;
-  esp_eth_phy_t *phy = NULL;
 #if CONFIG_ETH_SPI_ETHERNET_W5500
   if (type == ETH_PHY_W5500) {
     eth_w5500_config_t mac_config = ETH_W5500_DEFAULT_CONFIG(spi_host, &spi_devcfg);
@@ -606,8 +609,8 @@ bool ETHClass::beginSPI(
       mac_config.custom_spi_driver.write = _eth_spi_write;
     }
 #endif
-    mac = esp_eth_mac_new_w5500(&mac_config, &eth_mac_config);
-    phy = esp_eth_phy_new_w5500(&phy_config);
+    _mac = esp_eth_mac_new_w5500(&mac_config, &eth_mac_config);
+    _phy = esp_eth_phy_new_w5500(&phy_config);
   } else
 #endif
 #if CONFIG_ETH_SPI_ETHERNET_DM9051
@@ -623,8 +626,8 @@ bool ETHClass::beginSPI(
       mac_config.custom_spi_driver.write = _eth_spi_write;
     }
 #endif
-    mac = esp_eth_mac_new_dm9051(&mac_config, &eth_mac_config);
-    phy = esp_eth_phy_new_dm9051(&phy_config);
+    _mac = esp_eth_mac_new_dm9051(&mac_config, &eth_mac_config);
+    _phy = esp_eth_phy_new_dm9051(&phy_config);
   } else
 #endif
 #if CONFIG_ETH_SPI_ETHERNET_KSZ8851SNL
@@ -640,8 +643,8 @@ bool ETHClass::beginSPI(
       mac_config.custom_spi_driver.write = _eth_spi_write;
     }
 #endif
-    mac = esp_eth_mac_new_ksz8851snl(&mac_config, &eth_mac_config);
-    phy = esp_eth_phy_new_ksz8851snl(&phy_config);
+    _mac = esp_eth_mac_new_ksz8851snl(&mac_config, &eth_mac_config);
+    _phy = esp_eth_phy_new_ksz8851snl(&phy_config);
   } else
 #endif
   {
@@ -650,7 +653,7 @@ bool ETHClass::beginSPI(
   }
 
   // Init Ethernet driver to default and install it
-  esp_eth_config_t eth_config = ETH_DEFAULT_CONFIG(mac, phy);
+  esp_eth_config_t eth_config = ETH_DEFAULT_CONFIG(_mac, _phy);
   ret = esp_eth_driver_install(&eth_config, &_eth_handle);
   if (ret != ESP_OK) {
     log_e("SPI Ethernet driver install failed: %d", ret);
@@ -743,40 +746,46 @@ bool ETHClass::beginSPI(
 #if ETH_SPI_SUPPORTS_CUSTOM
   if (_spi == NULL) {
 #endif
-    if (!perimanSetPinBus(_pin_cs, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), -1, -1)) {
+    if (!perimanSetPinBus(_pin_cs, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), _eth_index, -1)) {
       goto err;
     }
+    perimanSetPinBusExtraType(_pin_cs, "ETH_SPI_CS");
 #if ETH_SPI_SUPPORTS_CUSTOM
   }
 #endif
 #if ETH_SPI_SUPPORTS_NO_IRQ
   if (_pin_irq != -1) {
 #endif
-    if (!perimanSetPinBus(_pin_irq, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), -1, -1)) {
+    if (!perimanSetPinBus(_pin_irq, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), _eth_index, -1)) {
       goto err;
     }
+    perimanSetPinBusExtraType(_pin_irq, "ETH_IRQ");
 #if ETH_SPI_SUPPORTS_NO_IRQ
   }
 #endif
   if (_pin_sck != -1) {
-    if (!perimanSetPinBus(_pin_sck, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), -1, -1)) {
+    if (!perimanSetPinBus(_pin_sck, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), _eth_index, -1)) {
       goto err;
     }
+    perimanSetPinBusExtraType(_pin_sck, "ETH_SPI_SCK");
   }
   if (_pin_miso != -1) {
-    if (!perimanSetPinBus(_pin_miso, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), -1, -1)) {
+    if (!perimanSetPinBus(_pin_miso, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), _eth_index, -1)) {
       goto err;
     }
+    perimanSetPinBusExtraType(_pin_miso, "ETH_SPI_MISO");
   }
   if (_pin_mosi != -1) {
-    if (!perimanSetPinBus(_pin_mosi, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), -1, -1)) {
+    if (!perimanSetPinBus(_pin_mosi, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), _eth_index, -1)) {
       goto err;
     }
+    perimanSetPinBusExtraType(_pin_mosi, "ETH_SPI_MOSI");
   }
   if (_pin_rst != -1) {
-    if (!perimanSetPinBus(_pin_rst, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), -1, -1)) {
+    if (!perimanSetPinBus(_pin_rst, ESP32_BUS_TYPE_ETHERNET_SPI, (void *)(this), _eth_index, -1)) {
       goto err;
     }
+    perimanSetPinBusExtraType(_pin_rst, "ETH_RST");
   }
 
   Network.onSysEvent(onEthConnected, ARDUINO_EVENT_ETH_CONNECTED);
@@ -840,11 +849,33 @@ void ETHClass::end(void) {
       return;
     }
     _eth_handle = NULL;
+    //delete mac
+    if (_mac != NULL) {
+      _mac->del(_mac);
+      _mac = NULL;
+    }
+    //delete phy
+    if (_phy != NULL) {
+      _phy->del(_phy);
+      _phy = NULL;
+    }
   }
 
   if (_eth_ev_instance != NULL) {
-    if (esp_event_handler_unregister(ETH_EVENT, ESP_EVENT_ANY_ID, &_eth_event_cb) == ESP_OK) {
-      _eth_ev_instance = NULL;
+    bool do_not_unreg_ev_handler = false;
+    for (int i = 0; i < NUM_SUPPORTED_ETH_PORTS; ++i) {
+      if (_ethernets[i] != NULL && _ethernets[i]->netif() != NULL && _ethernets[i]->netif() != _esp_netif) {
+        do_not_unreg_ev_handler = true;
+        break;
+      }
+    }
+    if (!do_not_unreg_ev_handler) {
+      if (esp_event_handler_unregister(ETH_EVENT, ESP_EVENT_ANY_ID, &_eth_event_cb) == ESP_OK) {
+        _eth_ev_instance = NULL;
+        log_v("Unregistered event handler");
+      } else {
+        log_e("Failed to unregister event handler");
+      }
     }
   }
 

Diff for: libraries/Ethernet/src/ETH.h

@@ -184,8 +184,11 @@ class ETHClass : public NetworkInterface {
   uint8_t _eth_index;
   eth_phy_type_t _phy_type;
   esp_eth_netif_glue_handle_t _glue_handle;
+  esp_eth_mac_t *_mac;
+  esp_eth_phy_t *_phy;
 #if ETH_SPI_SUPPORTS_CUSTOM
   SPIClass *_spi;
+  char _cs_str[10];
 #endif
   uint8_t _spi_freq_mhz;
   int8_t _pin_cs;

Diff for: libraries/Network/src/NetworkInterface.cpp

@@ -239,8 +239,21 @@ int NetworkInterface::waitStatusBits(int bits, uint32_t timeout_ms) const {
 
 void NetworkInterface::destroyNetif() {
   if (_ip_ev_instance != NULL) {
-    esp_event_handler_unregister(IP_EVENT, ESP_EVENT_ANY_ID, &_ip_event_cb);
-    _ip_ev_instance = NULL;
+    bool do_not_unreg_ev_handler = false;
+    for (int i = 0; i < ESP_NETIF_ID_MAX; ++i) {
+      if (_interfaces[i] != NULL && _interfaces[i] != this) {
+        do_not_unreg_ev_handler = true;
+        break;
+      }
+    }
+    if (!do_not_unreg_ev_handler) {
+      if (esp_event_handler_unregister(IP_EVENT, ESP_EVENT_ANY_ID, &_ip_event_cb) == ESP_OK) {
+        _ip_ev_instance = NULL;
+        log_v("Unregistered event handler");
+      } else {
+        log_e("Failed to unregister event handler");
+      }
+    }
   }
   if (_esp_netif != NULL) {
     esp_netif_destroy(_esp_netif);
@@ -251,6 +264,12 @@ void NetworkInterface::destroyNetif() {
     _interface_event_group = NULL;
     _initial_bits = 0;
   }
+  for (int i = 0; i < ESP_NETIF_ID_MAX; ++i) {
+    if (_interfaces[i] != NULL && _interfaces[i] == this) {
+      _interfaces[i] = NULL;
+      break;
+    }
+  }
 }
 
 bool NetworkInterface::initNetif(Network_Interface_ID interface_id) {

Diff for: libraries/PPP/src/PPP.cpp

@@ -283,6 +283,7 @@ bool PPPClass::begin(ppp_modem_model_t model, uint8_t uart_num, int baud_rate) {
     } else {
       pinMode(_pin_rst, OUTPUT);
     }
+    perimanSetPinBusExtraType(_pin_rst, "PPP_MODEM_RST");
     digitalWrite(_pin_rst, !_pin_rst_act_low);
     delay(200);
     digitalWrite(_pin_rst, _pin_rst_act_low);

Diff for: libraries/WiFi/src/AP.cpp

@@ -85,7 +85,7 @@ static void _onApArduinoEvent(arduino_event_t *ev) {
   if (_ap_network_if == NULL || ev->event_id < ARDUINO_EVENT_WIFI_AP_START || ev->event_id > ARDUINO_EVENT_WIFI_AP_GOT_IP6) {
     return;
   }
-  log_d("Arduino AP Event: %d - %s", ev->event_id, Network.eventName(ev->event_id));
+  log_v("Arduino AP Event: %d - %s", ev->event_id, Network.eventName(ev->event_id));
   if (ev->event_id == ARDUINO_EVENT_WIFI_AP_START) {
     if (_ap_network_if->getStatusBits() & ESP_NETIF_WANT_IP6_BIT) {
       esp_err_t err = esp_netif_create_ip6_linklocal(_ap_network_if->netif());

Diff for: libraries/WiFi/src/STA.cpp

@@ -107,7 +107,7 @@ static void _onStaArduinoEvent(arduino_event_t *ev) {
     return;
   }
   static bool first_connect = true;
-  log_d("Arduino STA Event: %d - %s", ev->event_id, Network.eventName(ev->event_id));
+  log_v("Arduino STA Event: %d - %s", ev->event_id, Network.eventName(ev->event_id));
 
   if (ev->event_id == ARDUINO_EVENT_WIFI_STA_START) {
     _sta_network_if->_setStatus(WL_DISCONNECTED);
@@ -162,11 +162,11 @@ static void _onStaArduinoEvent(arduino_event_t *ev) {
       _sta_network_if->connect();
     }
   } else if (ev->event_id == ARDUINO_EVENT_WIFI_STA_GOT_IP) {
-#if ARDUHAL_LOG_LEVEL >= ARDUHAL_LOG_LEVEL_DEBUG
+#if ARDUHAL_LOG_LEVEL >= ARDUHAL_LOG_LEVEL_VERBOSE
     uint8_t *ip = (uint8_t *)&(ev->event_info.got_ip.ip_info.ip.addr);
     uint8_t *mask = (uint8_t *)&(ev->event_info.got_ip.ip_info.netmask.addr);
     uint8_t *gw = (uint8_t *)&(ev->event_info.got_ip.ip_info.gw.addr);
-    log_d(
+    log_v(
       "STA IP: %u.%u.%u.%u, MASK: %u.%u.%u.%u, GW: %u.%u.%u.%u", ip[0], ip[1], ip[2], ip[3], mask[0], mask[1], mask[2], mask[3], gw[0], gw[1], gw[2], gw[3]
     );
 #endif