Skip to content

WifiMulti resilience updates #2775

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

Closed
wants to merge 13 commits into from
Closed

WifiMulti resilience updates #2775

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f8eaa42
Improvements to WiFiMulti resilience
Daemach May 13, 2019
d5b229c
Added example file for new options
Daemach May 13, 2019
0fab2f8
Modified to include connection testing at startup
Daemach May 14, 2019
fa7d1be
Merge remote-tracking branch 'upstream/master' into wifiMultiExpand
Daemach May 14, 2019
a406a3f
Changed strictmode function, default testPhrase
Daemach May 15, 2019
f2b2ff9
Stability fixes
Daemach Jun 6, 2019
3c5ecab
Merge remote-tracking branch 'upstream/master' into wifiMultiExpand
Daemach Jun 6, 2019
15af6ef
Added upstream
Daemach Jun 6, 2019
592079e
testConnection timeout fix
Daemach Jun 7, 2019
d0d4d17
Removed vscode folders
Daemach Jun 21, 2019
838b075
More vscode cleanup
Daemach Jun 21, 2019
c587b44
Merge remote-tracking branch 'upstream/master' into wifiMultiExpand
Daemach Jul 9, 2019
19ad37e
Rebase
Daemach Jul 10, 2019
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
47 changes: 47 additions & 0 deletions libraries/WiFi/examples/WiFIMultiAdvanced/WiFiMultiAdvanced.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* This sketch tries to connect to the best AP available, and tests for captive portals on open networks
*
*/

#include <WiFi.h>
#include <WiFiMulti.h>

WiFiMulti wifiMulti;

bool bConnected = false;

void setup()
{
Serial.begin(115200);
delay(10);

wifiMulti.addAP("ssid_from_AP_1", "your_password_for_AP_1");
wifiMulti.addAP("ssid_from_AP_2", "your_password_for_AP_2");
wifiMulti.addAP("ssid_from_AP_3", "your_password_for_AP_3");

// These options can help when you need ANY kind of wifi connection to get a config file, report errors, etc.
wifiMulti.setStrictMode(false); // Default is true. Library will disconnect and forget currently connected AP if it's not in the AP list.
wifiMulti.setAllowOpenAP(true); // Default is false. True adds open APs to the list.
wifiMulti.setTestConnection(true); // Default is false. Attempts to connect to a webserver in case of captive portals. Most useful with AllowOpenAP = true.

// Optional - defaults to brainjar, but you should set this to a simple test page on your own server to ensure you can reach it
// wifiMulti.setTestURL("http://www.brainjar.com/java/host/test.html"); // Must include http://
// wifiMulti.setTestPhrase("This is a very simple HTML file."); // Unique but short is best


Serial.println("Connecting Wifi...");
if(wifiMulti.run() == WL_CONNECTED) {
Serial.println("");
Serial.println("WiFi connected");
Serial.println("IP address: ");
Serial.println(WiFi.localIP());
}
}

void loop()
{
if( wifiMulti.run() != WL_CONNECTED) {
Serial.println("WiFi not connected!");
delay(1000);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

190 changes: 168 additions & 22 deletions libraries/WiFi/src/WiFiMulti.cpp
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,19 @@ uint8_t WiFiMulti::run(uint32_t connectTimeout)
int8_t scanResult;
uint8_t status = WiFi.status();
if(status == WL_CONNECTED) {
for(uint32_t x = 0; x < APlist.size(); x++) {
if(WiFi.SSID()==APlist[x].ssid){
if (_bTestConnection && !_bWFMInit){
if (testConnection()) {
_bWFMInit = true;
return status;
}
} else if (!_bStrict) {
return status;
} else {
for(uint32_t x = 0; x < APlist.size(); x++) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if(WiFi.SSID()==APlist[x].ssid){
return status;
}
}
}
WiFi.disconnect(false,false);
delay(10);
Expand All @@ -62,10 +71,12 @@ uint8_t WiFiMulti::run(uint32_t connectTimeout)
if(scanResult == WIFI_SCAN_RUNNING) {
// scan is running
return WL_NO_SSID_AVAIL;
} else if(scanResult >= 0) {
} else if (scanResult >= 0) {
// scan done analyze
WifiAPlist_t bestNetwork { NULL, NULL };
int32_t bestIndex = 0;
WifiAPlist_t bestNetwork { NULL, NULL, NULL };
int bestNetworkDb = INT_MIN;
int bestNetworkSec;
uint8_t bestBSSID[6];
int32_t bestChannel = 0;

Expand All @@ -75,6 +86,9 @@ uint8_t WiFiMulti::run(uint32_t connectTimeout)
log_e("[WIFI] no networks found");
} else {
log_i("[WIFI] %d networks found", scanResult);

int8_t failCount = 0;
int8_t foundCount = 0;
for(int8_t i = 0; i < scanResult; ++i) {

String ssid_scan;
Expand All @@ -85,21 +99,47 @@ uint8_t WiFiMulti::run(uint32_t connectTimeout)

WiFi.getNetworkInfo(i, ssid_scan, sec_scan, rssi_scan, BSSID_scan, chan_scan);

if (_bAllowOpenAP && sec_scan == WIFI_AUTH_OPEN){
bool found = false;
for(uint32_t o = 0; o < APlist.size(); o++) {
WifiAPlist_t check = APlist[o];
if (ssid_scan == check.ssid){
found = true;
}
}
if (!found){
log_i("[WIFI] added %s to APList", ssid_scan);
APlistAdd(ssid_scan.c_str());
// failCount++;
// foundCount++;
}
}

bool known = false;
for(uint32_t x = 0; x < APlist.size(); x++) {
WifiAPlist_t entry = APlist[x];

if(ssid_scan == entry.ssid) { // SSID match
known = true;
if(rssi_scan > bestNetworkDb) { // best network
if(sec_scan == WIFI_AUTH_OPEN || entry.passphrase) { // check for passphrase if not open wlan
bestNetworkDb = rssi_scan;
bestChannel = chan_scan;
memcpy((void*) &bestNetwork, (void*) &entry, sizeof(bestNetwork));
memcpy((void*) &bestBSSID, (void*) BSSID_scan, sizeof(bestBSSID));

if(ssid_scan == entry.ssid) { // It's on the list
log_i("known ssid: %s, failed: %i", entry.ssid, entry.fail);
foundCount++;
if (!entry.fail){
known = true;
log_i("rssi_scan: %d, bestNetworkDb: %d", rssi_scan, bestNetworkDb);
if(rssi_scan > bestNetworkDb) { // best network
if(_bAllowOpenAP || (sec_scan == WIFI_AUTH_OPEN || entry.passphrase)) { // check for passphrase if not open wlan
log_i("best network is now: %s", ssid_scan);
bestIndex = x;
bestNetworkSec = sec_scan;
bestNetworkDb = rssi_scan;
bestChannel = chan_scan;
memcpy((void*) &bestNetwork, (void*) &entry, sizeof(bestNetwork));
memcpy((void*) &bestBSSID, (void*) BSSID_scan, sizeof(bestBSSID));
}
}
break;
} else {
failCount++;
}
break;
}
}

Expand All @@ -109,40 +149,66 @@ uint8_t WiFiMulti::run(uint32_t connectTimeout)
log_d(" %d: [%d][%02X:%02X:%02X:%02X:%02X:%02X] %s (%d) %c", i, chan_scan, BSSID_scan[0], BSSID_scan[1], BSSID_scan[2], BSSID_scan[3], BSSID_scan[4], BSSID_scan[5], ssid_scan.c_str(), rssi_scan, (sec_scan == WIFI_AUTH_OPEN) ? ' ' : '*');
}
}
log_i("foundCount = %d, failCount = %d", foundCount, failCount);

if (foundCount == failCount){
failCount = 0;
resetFails();
}
foundCount = 0;
}

// clean up ram
WiFi.scanDelete();

if(bestNetwork.ssid) {
log_i("[WIFI] Connecting BSSID: %02X:%02X:%02X:%02X:%02X:%02X SSID: %s Channal: %d (%d)", bestBSSID[0], bestBSSID[1], bestBSSID[2], bestBSSID[3], bestBSSID[4], bestBSSID[5], bestNetwork.ssid, bestChannel, bestNetworkDb);
log_i("[WIFI] Connecting BSSID: %02X:%02X:%02X:%02X:%02X:%02X SSID: %s Channel: %d (%d)", bestBSSID[0], bestBSSID[1], bestBSSID[2], bestBSSID[3], bestBSSID[4], bestBSSID[5], bestNetwork.ssid, bestChannel, bestNetworkDb);

WiFi.begin(bestNetwork.ssid, bestNetwork.passphrase, bestChannel, bestBSSID);
WiFi.begin(bestNetwork.ssid, (_bAllowOpenAP && bestNetworkSec == WIFI_AUTH_OPEN) ? NULL:bestNetwork.passphrase, bestChannel, bestBSSID);
status = WiFi.status();
_bWFMInit = true;

auto startTime = millis();
// wait for connection, fail, or timeout
while(status != WL_CONNECTED && status != WL_NO_SSID_AVAIL && status != WL_CONNECT_FAILED && (millis() - startTime) <= connectTimeout) {
delay(10);
delay(100);
status = WiFi.status();
}

switch(status) {
case 3:
log_i("[WIFI] Connecting done.");
log_d("[WIFI] SSID: %s", WiFi.SSID().c_str());
log_d("[WIFI] IP: %s", WiFi.localIP().toString().c_str());
log_d("[WIFI] MAC: %s", WiFi.BSSIDstr().c_str());
log_d("[WIFI] Channel: %d", WiFi.channel());

if (_bTestConnection){
// We connected to an AP but if it's a captive portal we're not going anywhere. Test it.
if (testConnection()){
resetFails();
} else {
markAsFailed(bestIndex);
WiFi.disconnect(false,false);
delay(100);
status = WiFi.status();
}
} else {
resetFails();
}

break;
case 1:
log_e("[WIFI] Connecting Failed AP not found.");
markAsFailed(bestIndex);
break;
case 4:
log_e("[WIFI] Connecting Failed.");
markAsFailed(bestIndex);
break;
default:
log_e("[WIFI] Connecting Failed (%d).", status);
markAsFailed(bestIndex);
break;
}
} else {
Expand All @@ -161,6 +227,82 @@ uint8_t WiFiMulti::run(uint32_t connectTimeout)
return status;
}

void WiFiMulti::markAsFailed(int32_t i) {
APlist[i].fail = true;
log_i("marked %s as failed",APlist[i].ssid);
}


void WiFiMulti::resetFails(){
log_i("resetting failure flags");
for(uint32_t i = 0; i < APlist.size(); i++) {
APlist[i].fail = false;
}
}

void WiFiMulti::setStrictMode(bool bStrict) {
_bStrict = bStrict;
}

void WiFiMulti::setAllowOpenAP(bool bAllowOpenAP) {
_bAllowOpenAP = bAllowOpenAP;
}

void WiFiMulti::setTestConnection(bool bTestConnection){
_bTestConnection = bTestConnection;
}
void WiFiMulti::setTestPhrase(const char* testPhrase){
_testPhrase = testPhrase;
}
void WiFiMulti::setTestURL(String testURL){
_testURL = testURL;
}

bool WiFiMulti::testConnection(){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

//parse url
int8_t split = _testURL.indexOf('/',7);
String host = _testURL.substring(7, split);
String url = _testURL.substring(split,_testURL.length());

log_i("Testing connection to %s. Test phrase is \"%s\"",_testURL.c_str(), _testPhrase.c_str());
// Use WiFiClient class to create TCP connections
WiFiClient client;
const int httpPort = 80;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (!client.connect(host.c_str(), httpPort)) {
log_e("Connection failed");
return false;
} else {
log_i("Connected to test host");
Copy link
Collaborator

@atanisoft atanisoft May 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

// This will send the request to the server
client.print(String("GET ") + url + " HTTP/1.1\r\n" +
"Host: " + host + "\r\n" +
"Connection: close\r\n\r\n");
unsigned long timeout = millis();
while (client.available() == 0) {
if (millis() - timeout > 5000) {
log_e(">>>Client timeout!");
client.stop();
return false;
}
}
bool bSuccess = false;
// Read all the lines of the reply from server and print them to Serial
while(client.available()) {
// String line = client.readStringUntil('\r');
// Serial.print(line);
bSuccess = client.find(_testPhrase.c_str());
if (bSuccess){
log_i("Success. Found test phrase");
} else {
log_e("Failed. Can't find test phrase");
}
return bSuccess;
}

}

// ##################################################################################

bool WiFiMulti::APlistAdd(const char* ssid, const char *passphrase)
Expand All @@ -170,13 +312,13 @@ bool WiFiMulti::APlistAdd(const char* ssid, const char *passphrase)

if(!ssid || *ssid == 0x00 || strlen(ssid) > 31) {
// fail SSID to long or missing!
log_e("[WIFI][APlistAdd] no ssid or ssid to long");
log_e("[WIFI][APlistAdd] no ssid or ssid too long");
return false;
}

if(passphrase && strlen(passphrase) > 63) {
// fail passphrase to long!
log_e("[WIFI][APlistAdd] passphrase to long");
log_e("[WIFI][APlistAdd] passphrase too long");
return false;
}

Expand All @@ -197,9 +339,11 @@ bool WiFiMulti::APlistAdd(const char* ssid, const char *passphrase)
} else {
newAP.passphrase = NULL;
}

newAP.fail = false;

APlist.push_back(newAP);
log_i("[WIFI][APlistAdd] add SSID: %s", newAP.ssid);
log_i("[WIFI][APlistAdd] added SSID: %s", newAP.ssid);
return true;
}

Expand All @@ -213,6 +357,8 @@ void WiFiMulti::APlistClean(void)
if(entry.passphrase) {
free(entry.passphrase);
}
//why doesn't this work???
//free(entry.fail);
}
APlist.clear();
}
Expand Down
23 changes: 22 additions & 1 deletion libraries/WiFi/src/WiFiMulti.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
typedef struct {
char * ssid;
char * passphrase;
bool fail;
} WifiAPlist_t;

class WiFiMulti
Expand All @@ -42,12 +43,32 @@ class WiFiMulti

bool addAP(const char* ssid, const char *passphrase = NULL);

void setStrictMode(bool bStrict);
void setAllowOpenAP(bool bAllowOpenAP);
void setTestConnection(bool bTestConnection);
void setTestPhrase(const char* testPhrase);
void setTestURL(String url);

uint8_t run(uint32_t connectTimeout=5000);

void APlistClean(void);

private:
std::vector<WifiAPlist_t> APlist;

bool _bStrict = true;
bool _bAllowOpenAP = false;
bool _bTestConnection = false;
String _testPhrase = "simple HTML file.";
String _testURL = "http://www.brainjar.com/java/host/test.html";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

bool _bWFMInit = false;

void markAsFailed(int32_t i);
void resetFails();

bool testConnection();
bool APlistAdd(const char* ssid, const char *passphrase = NULL);
void APlistClean(void);


};

Expand Down