espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.3k stars 7.35k forks source link

WiFi.waitForConnectResult Fails if called in a loop #7095

Open dougparkarosa opened 2 years ago

dougparkarosa commented 2 years ago

Board

LILYGO® TTGO T-Beam V1.1 ESP32

Device Description

LILYGO® TTGO T-Beam V1.1 ESP32

Hardware Configuration

The device has nothing attached other than the standard parts soldered onto the board.

Version

v2.0.4

IDE Name

PlatformIO core 6.1.3

Operating System

Ubuntu 22.04

Flash frequency

40 MHz

PSRAM enabled

no

Upload speed

460800

Description

I have a loop to try to connect to different possible WiFi networks. The first one in the list is one that isn't available at the location I'm currently testing at. So it fails to connect. The second one is available and shows up if I call WiFi.scanNetworks(). I expect the first call to waitForConnectResult to fail. Which it does. However, because status is stored in a global varialbe "static wl_status_t _sta_status = WL_NO_SHIELD;"

In the code in waitForConnectResult "while((!status() || status() >= WL_DISCONNECTED) && (millis() - start) < timeoutLength) ..." status() returns WL_NO_SHIELD the first time this is called, and subsequently returns WL_NO_SSID_AVAIL. Which means the next call to waitForConnectResult doesn't call delay and simply returns the stale WL_NO_SSID_AVAIL flag (1). This behavior is unexpected. Also, the code I have was based on code I wrote a few years ago, that in my recollection worked.

Inserting a call to "WiFi._setStatus(WL_NO_SHIELD);" before calling WiFi.begin(...) on the next SSID allows waitForConnectResult to work. The global variable _sta_status could be reset to a sensible value in WiFi.begin. I can provide code if my description is not sufficient.

Sketch

// This is the code from my class InetConnector.
// InetConnector has the appropriately initialized members mSSID and mLocalIP
// It also provides several other methods but aren't pertinent to this defect.

void InetConnector::connectToWiFi() {
  // Wifi's to look for.
  std::cout << "Connecting to WiFi" << std::endl;
#define USE_MULTIPLE_SSIDS 1
#if USE_MULTIPLE_SSIDS
  const int kNumSSIDs = 5;
  const char *ssids[kNumSSIDs] = {"NotAvailable", "available1",
                                  "available2", "available3",
                                  "available4"};
  const char *passwords[kNumSSIDs]{"password", "", "", "", ""};
#endif

  WiFi.mode(WIFI_STA); // Client

  WiFi.setMinSecurity(WIFI_AUTH_OPEN);

  int n = WiFi.scanNetworks();
  std::cout << "Found " << n << " networks" << std::endl;

  for (int i = 0; i < n; ++i) {
    std::cout << i << ": " << WiFi.SSID(i).c_str() << " (" << WiFi.RSSI(i)
              << ")" << ((WiFi.encryptionType(i) == WIFI_AUTH_OPEN) ? " " : "*")
              << std::endl;
    delay(10);
  }

#if USE_MULTIPLE_SSIDS
  int mWiFiIndex = -1;
  const unsigned long kTimeout = 1000000 * 60; // 60 seconds
  for (int i = 0; i < kNumSSIDs; ++i) {
    std::cout << "Trying: " << ssids[i] << std::endl;

    // Controls a global static variable.
    // Must set this back or logic in waitForConnectResult will never
    // find a network after first failure.
    WiFi._setStatus(WL_NO_SHIELD);

    auto beginStatus = WiFi.begin(ssids[i], passwords[i]);
    auto status = WiFi.waitForConnectResult(kTimeout);
    std::cout << "beginStatus: " << beginStatus << " status: " << (int)status
              << std::endl;
    if (status == WL_CONNECTED) {
      mWiFiIndex = i;
      break;
    }
  }

  if (mWiFiIndex >= 0) {
    mSSID = ssids[mWiFiIndex];
    std::cout << "WIFI SSID: " << mSSID.c_str() << std::endl;
  }
#else
  WiFi.begin("available1", "");
  std::cout << "Connecting to WiFi ..";
  while (WiFi.status() != WL_CONNECTED) {
    std::cout << '.';
    delay(1000);
  }
  std::cout << "Connected: " << WiFi.localIP().toString().c_str() << std::endl;
#endif // USE_MULTIPLE_SSIDS
  mLocalIP = WiFi.localIP();
}

Debug Message

None.

Other Steps to Reproduce

My workaround works on my hardware. I haven't tried it anywhere else. It seems like it will fail on pretty much any ESP32.

I couldn't find any reported defect that is clearly the same as this. One or two may or may not be related.

I have checked existing issues, online documentation and the Troubleshooting Guide

SuGlider commented 2 years ago

@dougparkarosa - There is WiFiMulti Class in ESP32 Arduino WiFi that helps in connecting to the first available network in a list of multiple SSID/Pwd.

I guess that this is what you may be looking for. Check the example from the Arduino IDE or from here: https://github.com/espressif/arduino-esp32/blob/master/libraries/WiFi/examples/WiFiMulti/WiFiMulti.ino

dougparkarosa commented 2 years ago

WiFiMulti could work in this case. I never tried, it. Fundamentally waitForConnectResult is broken: Consider the code:

uint8_t WiFiSTAClass::waitForConnectResult(unsigned long timeoutLength) { //1 and 3 have STA enabled if((WiFiGenericClass::getMode() & WIFI_MODE_STA) == 0) { return WL_DISCONNECTED; } unsigned long start = millis(); while((!status() || status() >= WL_DISCONNECTED) && (millis() - start) < timeoutLength) { delay(100); } return status(); }

Note the path through for failure doesn't update status, yet depends on it returning WL_DISCONNECTED or higher to call delay(). Which means that if it times out it will return the previous status, which in my case was WL_NO_SSID_AVAIL (1). So status needs to be 0 (status() == WL_IDLE_STATUS) or >= 6 (WL_DISCONNECTED). I never tracked down the side effects that get status into the WL_NO_SSID_AVAIL state, it's a global variable so it is hard to track. My observations was that the first SSID that I tried had a long delay before it failed. In looking at the code it was clear that status() must be the culprit. So I tracked down what status did, noticed that it was initialized to WL_NO_SHIELD (255), found the _setStatus() method and used it to re initialized the value of status, and it works. No knowing, and not wanting to devote many hours to figuring out the very large number of possible routes for status to change, I am not sure of a "correct" solution. My instinct is that status should be set to something reasonable in the failure case of timeout. WiFiGenericClass::_eventCallback sets WL_NO_SSID_AVAIL on failure, when I first call waitForConnectResult.

WiFiMulti uses the following logic to wait for a connection: status = WiFi.status();

        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);
            status = WiFi.status();
        }

Note the use of WL_NO_SSID_AVAIL and the similarities to waitForConnectionResult. Since the purpose of the logic in both cases appears to be equivalent, the code duplication seems ill advised. Both classes should probably share the same code.

SuGlider commented 2 years ago

@dougparkarosa - You seem to have spent some time on it. Thanks! Would you like to submit a PR to fix waitForConnectResult() ?

dougparkarosa commented 2 years ago

I'm very busy working on my PhD. I have already spent more time on this than I should have. My work around is sufficient for my needs now.

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Rodrigo Garcia @.> Sent: Thursday, August 11, 2022 1:02:26 PM To: espressif/arduino-esp32 @.> Cc: Doug Park @.>; Mention @.> Subject: Re: [espressif/arduino-esp32] WiFi.waitForConnectResult Fails if called in a loop (Issue #7095)

@dougparkarosahttps://github.com/dougparkarosa - You seem to have spent some time on it. Thanks! Would you like to submit a PR to fix waitForConnectResult() ?

— Reply to this email directly, view it on GitHubhttps://github.com/espressif/arduino-esp32/issues/7095#issuecomment-1212432248, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHM35IMT6EJMLC2RPFCYL53VYVL5FANCNFSM553AKM6A. You are receiving this because you were mentioned.Message ID: @.***>

SuGlider commented 2 years ago

OK. Thanks for reporting the issue. Do you think this issue can be closed?

pegleg-dragon commented 1 year ago

This bug is still present. It just cost me 2 hours of mystified debugging.

I have the same situation: an ordered list of WiFi SSIDs I want to try. WiFiMulti isn't appropriate because it connects to the strongest SSID, rather than accepting a specified preference order.

The workaround of calling WiFi._setStatus(WL_NO_SHIELD); just before WiFi.begin() solved the issue for me. Thanks, Doug.