Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 29, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b47272
fix(net): Don't unreg events if there are netifs
me-no-dev May 28, 2024
cdf0acb
fix(eth): Delete mac and phy on end
me-no-dev May 28, 2024
a856b26
fix(net): Update pin naming and log levels
me-no-dev May 28, 2024
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
79 changes: 55 additions & 24 deletions libraries/Ethernet/src/ETH.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
{
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions libraries/Ethernet/src/ETH.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 21 additions & 2 deletions libraries/Network/src/NetworkInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions libraries/PPP/src/PPP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion libraries/WiFi/src/AP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
6 changes: 3 additions & 3 deletions libraries/WiFi/src/STA.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down