WifiMulti resilience updates #2775

Daemach · 2019-05-13T18:41:50Z

This update to WiFiMulti is designed to do whatever it takes to get and keep the device online so it can download a configuration file that includes the correct AP credentials. It adds the following features:

Failure tracking, to check every entry on the list before resetting and trying them all again.
AllowOpenAP automatically adds open access points to APList and attempts to connect.
StrictMode toggle to disable the current behavior which disconnects from an AP if that AP is not on the APList.
TestConnection connects to a web page on a remote web server and looks for a test phrase to ensure that there is full internet access. (This is most useful with AllowOpenAP which could add APs that have captive portals, such as hotels, airports, coffee shops, or cable networks like xfinity in the US)
Adds an example page for these configuration options.

- Added toggle for strict mode. - Added allowOpenAP mode to automatically connect to open access points - Added testConnection mode to test for captive portals - Added fail checks so WiFiMulti will try every entry in the APList

atanisoft · 2019-05-14T19:13:05Z

libraries/WiFi/src/WiFiMulti.cpp

+        } else if (!_bStrict) {
+            return status;
+        } else {
+            for(uint32_t x = 0; x < APlist.size(); x++) {


since you don't need the index value this can be written as:

for(auto ap : APlist) { if(WiFi.SSID() == ap.ssid) { return status; } }

Much more elegant. I need to play with vectors more...

atanisoft · 2019-05-14T19:14:26Z

libraries/WiFi/src/WiFiMulti.h

@@ -59,13 +59,14 @@ class WiFiMulti
    bool _bStrict = true;
    bool _bAllowOpenAP = false;
    bool _bTestConnection = false;
-    String _testPhrase = "This is a very simple HTML file.";
+    String _testPhrase = "simple HTML file.";
    String _testURL = "http://www.brainjar.com/java/host/test.html";


It may be better to default this to www.google.com or similar that returns a very small page. Checking the actual content is not super critical to test that the connection was successful to the remote endpoint, just that a connection to the remote was successful.

I was/am concerned that a captive portal would return 200 and fool me. This test is almost purely to skip captive portals. I test it with the xfinitywifi AP at the office, but it could be the Starbucks next door. Thoughts? I wonder if looking for a 302 header would be better...

perhaps checking for a 302 would work as well. The ultimate goal would be to avoid downloading the full response when it is not needed in most cases. Only the first few lines of the response will be needed to detect a redirect or if it is a general 200. Having client.find() will result in a lot of heap usage for the duration of the find and it may not even find the string.

will client.find() abort as soon as it finds the target string? 302 Found in this case

Make that 200 OK... I updated the h file.

atanisoft · 2019-05-14T19:15:48Z

libraries/WiFi/src/WiFiMulti.cpp

        return false;
    } else {
-        log_i("connection to test host successful");
+        log_i("Connected to test host");


at this point it should be possible to close the connection without reading the actual data since the point of this is to check that the connection to the internet is successful.

See above. I would rather skip the text check to make it simpler. Just need to make sure we don't get trapped. If this device gets onto xfinitywifi here it doesn't want to get off.

yeah, ideally this would exit once it receives the initial http header details (all lines until a blank line IIRC). Once the header has been received the connection can be closed. I'd recommend read by line and check the line for known keywords (Location: XXX, HTTP XXX 200|302 etc...) That way you can abort earlier from the line by line checking.

I agree with @atanisoft here :) there are some codes that show all is good.

actually what is the target? which codes would mean that something went bad?

All I am trying to do is ensure that the device can actually reach the internet - captive portals are the problem. If you're connected to Wifi but behind a captive portal you're not going anywhere. This functionality is most useful when you have bAllowOpenAP, which automatically connects to any and all open APs in addition to the ones in your list just to get online. In my case, once it's online it connects to my server, grabs a config file with the correct SSID information then turns bAllowOpenAP off and bStrictMode on. It's good functionality to put 1000 brand new devices online just by plugging them in and setting your AP to open mode for 15 minutes.

I'm looking for anything but a 304 by default, but if that isn't good enough, you can set your own URL and look for any test phrase that you want, including text in a webpage. By default I don't want to look for 200's either in case a captive portal happens to return that. Looking for a permanent redirect seems like the best solution because a captive portal wouldn't do that and a site like google or amazon should be available everywhere.

atanisoft · 2019-05-14T19:17:09Z

libraries/WiFi/src/WiFiMulti.cpp

@@ -261,12 +269,10 @@ bool WiFiMulti::testConnection(int32_t i){
    WiFiClient client;
    const int httpPort = 80;


port needs to be dynamically determined from the test url, it is not guaranteed that all servers will expose port 80 (most are moving to https as a default)

Hmm. Can I make an https request directly from wifiClient? I'm trying to keep it simple by not including more libraries than necessary. I'm assuming anyone using this function would set the test page manually. Still, if you feel this is important I'll modify it.

true, but if a user sets a url with https it would likely fail here as well

Yeah. In my example I specify that it must be http://. Need to think it through a bit more.

- Changed to a vector iterator - Changed default test phrase to 200 OK. This function is intended to detect captive portals which use 302 Found.

me-no-dev · 2019-06-05T11:34:11Z

What is the state here @Daemach ? Is this ready for review and merge?

Added delays for stability and changed to amazon.com for connection test.

Daemach · 2019-06-06T14:48:07Z

Yes. Let me know if I need to modify anything.

Copy/paste error :/

atanisoft · 2019-06-21T12:10:09Z

@Daemach can you drop the .vscode directories? I don't think we want those polluting the source tree (especially dynamically generated files)

Daemach · 2019-06-21T19:31:52Z

Missed those. Thanks for the heads-up!

atanisoft · 2019-06-21T19:47:50Z

Missed those. Thanks for the heads-up!

Looks like a few are still there...

Daemach · 2019-06-21T20:57:18Z

Not seeing them - where?

atanisoft · 2019-06-21T21:02:40Z

file diff shows these directories still:
tools/sdk/include/coap/.vscode/
tools/sdk/include/esp_http_server/.vscode/
tools/sdk/include/log/.vscode/
tools/sdk/include/wpa_supplicant/wpa/.vscode/
variants/airnode/.vscode/

stickbreaker · 2019-06-21T21:09:30Z

@Daemach at top of this page click Files Changed Currently it shows 14 Files Changed. You should be able to use the triple dots to selected deleted for each of the vscode file files.

Daemach · 2019-06-21T21:09:30Z

Sheesh! Is there any way to keep vscode from creating those folders?

Daemach · 2019-06-21T21:11:17Z

Thanks for the guidance! All new to me.

stickbreaker · 2019-06-21T21:12:34Z

@Daemach you might want to review git ignore file list 😀 That way when you do a git add it won't include them.

me-no-dev · 2019-07-09T16:49:46Z

@Daemach please rebase this :)

Daemach · 2019-07-11T18:55:15Z

Git is new to me. I think its up to date now. I set the espressif site as upstream then did git fetch upstream, git merge upstream/master and git push origin. If I need to do something else please tell me so I can learn.

me-no-dev · 2019-07-17T07:26:27Z

libraries/WiFi/src/WiFiMulti.h

+    bool _bAllowOpenAP = false;
+    bool _bTestConnection = false;
+    String _testPhrase = "301 Moved";
+    String _testURL = "http://www.amazon.com";


this will not work in China :)

You can set the testURL to whatever you want. Do you have a better suggestion for a URL that will work everywhere and also redirects with something other than 304? I'm trying to detect captive portals which normally return 304 or 200.

me-no-dev · 2019-07-17T07:27:27Z

libraries/WiFi/src/WiFiMulti.cpp

+    _testURL = testURL;
+}
+
+bool WiFiMulti::testConnection(){


why not add the URL as an argument (defaulting to NULL) and then parse it to get the port and so on?

I figured most people would just want to turn it on and forget it so I defaulted the test URL and phrase to amazon which I thought would be universal.

Daemach · 2019-09-20T18:31:06Z

I would still like to see this added. I’ll rebase on 1.0.3

CLAassistant · 2021-04-28T17:02:28Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

stale · 2022-04-16T17:13:36Z

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

VojtechBartoska · 2023-12-20T13:55:58Z

Hello, @Daemach please sign CLA, thanks.

lucasssvaz · 2023-12-22T22:50:24Z

libraries/WiFi/examples/WiFIMultiAdvanced/WiFiMultiAdvanced.ino

+        Serial.println("WiFi not connected!");
+        delay(5000);
+    }
+}


Suggested change

}

}

lucasssvaz · 2023-12-22T22:51:03Z

libraries/WiFi/src/WiFiMulti.cpp

Could you please merge the master branch and resolve the conflicts ?

lucasssvaz · 2023-12-22T22:52:11Z

libraries/WiFi/src/WiFiMulti.h

+    bool _bAllowOpenAP = false;
+    bool _bTestConnection = false;
+    String _testPhrase = "301 Moved";
+    String _testURL = "http://www.amazon.com";


Idk if you need some specific kind of page but google's URL would work for most places, including China.

me-no-dev · 2024-01-23T11:39:24Z

Continued in: #9139

Daemach added 4 commits May 13, 2019 11:05

Improvements to WiFiMulti resilience

f8eaa42

- Added toggle for strict mode. - Added allowOpenAP mode to automatically connect to open access points - Added testConnection mode to test for captive portals - Added fail checks so WiFiMulti will try every entry in the APList

Added example file for new options

d5b229c

Modified to include connection testing at startup

0fab2f8

Merge remote-tracking branch 'upstream/master' into wifiMultiExpand

fa7d1be

atanisoft reviewed May 14, 2019

View reviewed changes

Changed strictmode function, default testPhrase

a406a3f

- Changed to a vector iterator - Changed default test phrase to 200 OK. This function is intended to detect captive portals which use 302 Found.

Daemach added 2 commits June 6, 2019 07:33

Stability fixes

f2b2ff9

Added delays for stability and changed to amazon.com for connection test.

Merge remote-tracking branch 'upstream/master' into wifiMultiExpand

3c5ecab

Daemach added 2 commits June 6, 2019 07:54

Added upstream

15af6ef

testConnection timeout fix

592079e

Copy/paste error :/

Removed vscode folders

d0d4d17

More vscode cleanup

838b075

Daemach added 2 commits July 9, 2019 12:07

Merge remote-tracking branch 'upstream/master' into wifiMultiExpand

c587b44

Rebase

19ad37e

me-no-dev reviewed Jul 17, 2019

View reviewed changes

stale bot added the wontfix label Apr 16, 2022

VojtechBartoska added Area: BT&Wifi BT & Wifi related issues and removed wontfix labels Apr 20, 2022

VojtechBartoska added this to the 3.0.0-RC1 milestone Nov 28, 2023

VojtechBartoska requested a review from lucasssvaz December 20, 2023 13:56

VojtechBartoska added the Status: Review needed Issue or PR is awaiting review label Dec 20, 2023

VojtechBartoska requested a review from P-R-O-C-H-Y December 20, 2023 13:56

lucasssvaz reviewed Dec 22, 2023

View reviewed changes

SuGlider mentioned this pull request Jan 19, 2024

Improves WiFiMulti #9139

Merged

me-no-dev closed this Jan 23, 2024

		@@ -261,12 +269,10 @@ bool WiFiMulti::testConnection(int32_t i){
		WiFiClient client;
		const int httpPort = 80;

WifiMulti resilience updates #2775

WifiMulti resilience updates #2775

Conversation

Daemach commented May 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atanisoft May 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

me-no-dev commented Jun 5, 2019

Daemach commented Jun 6, 2019

atanisoft commented Jun 21, 2019

Daemach commented Jun 21, 2019

atanisoft commented Jun 21, 2019

Daemach commented Jun 21, 2019

atanisoft commented Jun 21, 2019

stickbreaker commented Jun 21, 2019

Daemach commented Jun 21, 2019

Daemach commented Jun 21, 2019

stickbreaker commented Jun 21, 2019 • edited Loading

me-no-dev commented Jul 9, 2019

Daemach commented Jul 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Daemach commented Sep 20, 2019

CLAassistant commented Apr 28, 2021

stale bot commented Apr 16, 2022

VojtechBartoska commented Dec 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

me-no-dev commented Jan 23, 2024

atanisoft May 14, 2019 •

edited

Loading

stickbreaker commented Jun 21, 2019 •

edited

Loading