Eliminate gigantic blocking time inside ESP8266WiFiMulti::run #4888
Conversation
|
For what it's worth, I'm still seeing a single 880+/- 5 ms latency that's occurring between loop() calls that I haven't identified the source of yet. It happens between the wifi.begin() and the completion of the connection. Between the 0->2 and 2->3 state transitions: state: 0 -> 2 (b0) |
|
The failing sub-build: core/test_md5builder.cpp:65: FAILED: Am I going blind or aren't those two strings the same, just like the require wants them to be? |
|
This CI bug is yet unsolved and happens from time to time. I retriggered. |
|
Like this! |
| BestNetwork() : Network({NULL, NULL}), NetworkDb(INT_MIN) {} | ||
| }; | ||
|
|
||
| void ESP8266WiFiMulti::selectBest(BestNetwork& best, int8_t scanResult) { |
There was a problem hiding this comment.
The only external dependency of this function is on WiFi. It doesn't depend on any WiFiMulti method or member. Therefore, there is no point of having the function in the class as a method.
Please make it a static free-standing function in the .cpp.
| @@ -58,6 +60,12 @@ class ESP8266WiFiMulti { | |||
|
|
|||
| private: | |||
| WifiAPlist APlist; | |||
| unsigned long connect_timeout_ms; | |||
|
|
|||
| void selectBest(BestNetwork& best, int8_t scanResult); | |||
There was a problem hiding this comment.
This function should be removed from the class, see comment above.
| @@ -47,6 +47,8 @@ struct WifiAPEntry { | |||
|
|
|||
| typedef std::vector<WifiAPEntry> WifiAPlist; | |||
|
|
|||
| struct BestNetwork; | |||
There was a problem hiding this comment.
If selectBest is removed from the class and made a static free standing function, there is no need to declare this here, because the definition in the .cpp is enough. Please remove.
| unsigned long connect_timeout_ms; | ||
|
|
||
| void selectBest(BestNetwork& best, int8_t scanResult); | ||
| void handleConnectComplete(wl_status_t status); |
There was a problem hiding this comment.
This is a debug method, and there is also no need for it to be part of the class, or exported in the .h. Please remove and make the function a static free standing function in the .cpp.
| @@ -38,8 +38,104 @@ bool ESP8266WiFiMulti::addAP(const char* ssid, const char *passphrase) { | |||
| return APlistAdd(ssid, passphrase); | |||
| } | |||
|
|
|||
| void ESP8266WiFiMulti::handleConnectComplete(wl_status_t status) { | |||
There was a problem hiding this comment.
This function has no dependencies on other class members or methods, and is not public. Please make it a static free standing function.
| (millis() > connect_timeout_ms)) | ||
| { | ||
| handleConnectComplete(status); | ||
| connect_timeout_ms = 0; |
There was a problem hiding this comment.
This is a constructor-inited member, yet it is cleared here. Will it never be used again? Is there a need to init it again?
| wl_status_t ESP8266WiFiMulti::run(void) { | ||
|
|
||
| if (connect_timeout_ms != 0) { |
There was a problem hiding this comment.
The timeout value is cleared above and never set again. Is that correct behavior?
earlephilhower
added
merge-conflict
|
Thanks for your PR, but the core and libraries have changed enough that this PR now has a merge conflict. Could you merge it manually with the latest core, so we can consider it for future releases? |
|
Closing in favor of #7619 , which should be equivalent. |
I'm using an esp8266 + Arduino in a project and need to keep the loop latencies low (low 10's of ms). Using ESP8266WiFiMulti connections result in multi-second pauses due largely to the connection loop inside run().
This change (developed and tested against 2.4.1 but looks to apply to head of master cleanly):
This takes the latency down to only once hitting a wifimulti.run() latency > 10ms, and that at only 14ms.