Closed jbaudoux closed 2 weeks ago
fredlcore
commented
3 months ago I appreciate you working on our code, but I would very much appreciate it if you let me know first what you are planning to do and discuss this with me first. That saves at least me and most likely both of us a lot of time if I don't have to go through several pull requests and changed files and have to do a lot of guesswork...
jbaudoux
commented
3 months ago I appreciate you working on our code, but I would very much appreciate it if you let me know first what you are planning to do and discuss this with me first. That saves at least me and most likely both of us a lot of time if I don't have to go through several pull requests and changed files and have to do a lot of guesswork...
This was in draft and I wasn't expecting you to react so quickly. I still have to provide you the complete description so that you can easily understand the difference (before / after).
I have TCP socket issue that I try to understand. As I was looking at the setup debug message, I improved a bit readability. Besides this, I don't want to change anything here
fredlcore
commented
3 months ago The event messages are still new and will most likely only be enabled when developer debug mode is active. This may lead to improper formatting etc., but since this will be only used in rare circumstances, I don't think it is an issue. In any case, it should not lead to a situation where output for "normal" mode is removed (as it seems to be the lines 7723++ you mentioned above.
fredlcore
commented
3 months ago Also, you removed this line:
if (!Ethernet.begin(mac)) createTemporaryAP(); // DHCP
Why? Do you understand what it does?
jbaudoux
commented
3 months ago Also, you removed this line:
if (!Ethernet.begin(mac)) createTemporaryAP(); // DHCPWhy? Do you understand what it does?
I just moved down this part to not repeat all the conditions in static or dhcp
jbaudoux
commented
3 months ago The event messages are still new and will most likely only be enabled when developer debug mode is active. This may lead to improper formatting etc., but since this will be only used in rare circumstances, I don't think it is an issue. In any case, it should not lead to a situation where output for "normal" mode is removed (as it seems to be the lines 7723++ you mentioned above.
I wasn't in debug mode, but to be safe, I put it back but with labels.
Ethernet connected.
Ethernet got IP: 172.19.3.5 netmask: 255.255.255.0 gateway: 172.19.3.1
IP: 172.19.3.5 netmask: 255.255.255.0 gateway: 172.19.3.1
Waiting 3 seconds to give Ethernet shield time to get ready...Also, you removed this line:
if (!Ethernet.begin(mac)) createTemporaryAP(); // DHCPWhy? Do you understand what it does?I just moved down this part to not repeat all the conditions in static or dhcp
Ok, I didn't see that. One of the reasons why I prefer opening a bug report or feature request before submitting PRs where the intention can be discussed first as this makes code review or PRs a lot easier. In this case: Have you checked that a failed Ethernet.begin() is based on the same conditions as a failed Ethernet.localIP()? Will for example a failed DHCP request lead to a "false" localIP()? Or is 0.0.0.0 considered "true"?
jbaudoux
commented
3 months ago You're right. I was initially using Ethernet.connected() but this is not in stable. I put it back and added in Eth class.
Result without lan cable:
Ethernet Started.
Waiting for DHCP address........................................................................................................................................................................................................
Setting up AP 'BSB-LAN'
IP address of BSB-LAN: 192.168.4.1
Connect to access point 'BSB-LAN' with password 'BSB-LPB-PPS-LAN' and open the IP address.
IP: 0.0.0.0 netmask: 0.0.0.0 gateway: 0.0.0.0With cable:
Ethernet Started.
Ethernet connected.
Ethernet got IP: 172.19.3.5 netmask: 255.255.255.0 gateway: 172.19.3.1
IP: 172.19.3.5 netmask: 255.255.255.0 gateway: 172.19.3.1Under what condition is Ethernet.connected() returning true? Hardware link? IP obtained? What happens when LAN is selected and cable is connected and fixed IP address is used but there is a mis-configuration in the IP settings that prevent the network from working properly? What happens if you're on an ESP32 system that has WiFi only but is set to LAN in the configuration?
fredlcore
commented
3 months ago Under what condition is
Ethernet.connected()returning true? Can you elaborate on this, please, @jbaudoux? I'd like to merge this if this function fails on the same (or more) conditions asEthernet.begin(). But especially the last two mentioned cases need to be covered here.
jbaudoux
commented
3 months ago Under what condition is
Ethernet.connected()returning true? Can you elaborate on this, please, @jbaudoux? I'd like to merge this if this function fails on the same (or more) conditions asEthernet.begin(). But especially the last two mentioned cases need to be covered here.
I'll run more test soon and let you know
fredlcore
commented
3 months ago I checked the sources and this specific bit is set/cleared upon the event ARDUINO_EVENT_ETH_CONNECTED or ARDUINO_EVENT_ETH_DISCONNECTED respectively. So this is what the event handler already covers. However, I think that I observed that if there is no link in the first place, there is no "disconnected" event, and the same goes IIRC for cases where there is no Ethernet adapter in the first place (i.e. on a NodeMCU). This means that if a user has set the network type to LAN on a NodeMCU, this condition won't start the AP.
jbaudoux
commented
3 months ago fixed IP address is used but there is a mis-configuration in the IP settings that prevent the network from working properly
Not sure I understand properly what you mean by mis-configuration. Whatever local IP I set, it get's the IP I configured and continue.
What happens if you're on an ESP32 system that has WiFi only but is set to LAN in the configuration?
LAN DHCP or static, there is no change, it's resetting on begin as the brownout detector triggers
Starting network connection via Ethernet/LAN
...
E (1241) esp.emac: emac_esp32_init(348): reset timeout
E (1242) esp_eth: esp_eth_driver_install(214): init mac failed
[ 1248][E][ETH.cpp:324] begin(): esp_eth_driver_install failed
Setting up AP 'BSB-LAN'
Brownout detector was triggered
ets Jun 8 2016 00:22:57
rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
Huh? Where is that brownout coming from? That's usually a hardware issue. The default configuration is for network type to be set to LAN. If you flash that onto a ESP32-NodeMCU, you would be locked out from the system, but the temporary AP allows you to access and change the configuration so that you don't have to re-flash. If your approach leads to either brown-outs or does not detect this scenario properly, I cannot accept it.
fredlcore
commented
3 months ago This is what I get on an ESP32-NodeMCU when the default network_type value is not changed:
21:31:32.657 > Starting network connection via Ethernet/LAN
21:31:32.660 > ...
21:31:33.662 > E (1260) esp.emac: emac_esp32_init(348): reset timeout
21:31:33.665 > E (1261) esp_eth: esp_eth_driver_install(214): init mac failed
21:31:33.670 > [ 1278][E][ETH.cpp:324] begin(): esp_eth_driver_install failed
21:31:33.676 > Setting up AP 'BSB-LAN'Up to this point, no network event fires, but at least ETH_CONNECTED_BIT is initially zero, so setting up the temporary AP works in this case. What I can't test here is if the same also works if a) an invalid fixed IP is configured which results to an inaccessible network configuration and b) if DHCP is configured, but no IP address could be obtained.
jbaudoux
commented
3 months ago Huh? Where is that brownout coming from? That's usually a hardware issue.
Indeed. I tried with a different esp32 NodeMCU board and this is what I get:
Starting network connection via Ethernet/LAN...
E (1235) esp.emac: emac_esp32_init(348): reset timeout
E (1236) esp_eth: esp_eth_driver_install(214): init mac failed
[ 1260][E][ETH.cpp:324] begin(): esp_eth_driver_install failed
Setting up AP 'BSB-LAN'
IP address of BSB-LAN: 192.168.4.1
Connect to access point 'BSB-LAN' with password 'BSB-LPB-PPS-LAN' and open the IP address.
IP: 192.168.178.88 netmask: 255.255.255.0 gateway: 192.168.178.1
Waiting 3 seconds to give Ethernet shield time to get ready...
Trying to get NTP time...
Acquisition failed, trying again in one minute...We see it's not waiting to acquire an IP
Starting network connection via Ethernet/LAN...
E (1235) esp.emac: emac_esp32_init(348): reset timeout
E (1236) esp_eth: esp_eth_driver_install(214): init mac failed
[ 1260][E][ETH.cpp:324] begin(): esp_eth_driver_install failed
Setting up AP 'BSB-LAN'
IP address of BSB-LAN: 192.168.4.1
Connect to access point 'BSB-LAN' with password 'BSB-LPB-PPS-LAN' and open the IP address.
IP: 0.0.0.0 netmask: 0.0.0.0 gateway: 0.0.0.0
Waiting 3 seconds to give Ethernet shield time to get ready...
Trying to get NTP time...
Acquisition failed, trying again in one minute......
E (1235) esp.emac: emac_esp32_init(348): reset timeout
E (1236) esp_eth: esp_eth_driver_install(214): init mac failed
[ 1260][E][ETH.cpp:324] begin(): esp_eth_driver_install failed
Setting up AP 'BSB-LAN'
IP address of BSB-LAN: 192.168.4.1
Connect to access point 'BSB-LAN' with password 'BSB-LPB-PPS-LAN' and open the IP address.
Waiting for DHCP address........................................................................................................................................................................................................
0.0.0.0
0.0.0.0
0.0.0.0
Waiting 3 seconds to give Ethernet shield time to get ready...
Trying to get NTP time...
Acquisition failed, trying again in one minute...Have your tests been done with a plugged-in network cable or not? My questions before were based on the assumption that a network cable is conencted and the conditions a) and b) are met.
fredlcore
commented
2 weeks ago I have implemented most of this manually after this PR was no longer compatible with framework 3.0.1. Thanks!
You can do whatever you want with this PR. Besides improving logged message, nothing is changed
Before
Issue
After