khoih-prog / ESPAsync_WiFiManager

This is an ESP32 (including ESP32-S2 and ESP32-C3) / ESP8266 WiFi Connection Manager, using ESPAsyncWebServer, with fallback web configuration portal. Use this library for configuring ESP32, ESP8266 modules' WiFi, etc. Credentials at runtime. You can also specify static DNS servers, personalized HostName, fixed or random AP WiFi channel. With examples supporting ArduinoJson 6.0.0+ as well as 5.13.5- . Using AsyncDNSServer instead of DNSServer now.
MIT License
291 stars 73 forks source link

Allow captive portal to run more than once by closing dnsServer cleanly. #49

Closed mattbradford83 closed 3 years ago

mattbradford83 commented 3 years ago

Hello,

I noticed that if the Config Portal is started more than once by calling startConfigPortal(...) without rebooting the device, then only the first instance functions as a captive portal; at least within 10 seconds of each other.

This is due to the fact that the WiFiUdp object in DNSServer.h is never told to stop(), and so the OS holds on to port 53. The result of dnsServer->start(DNS_PORT, "*", WiFi.softAPIP()); in ESPAsync_WiFiManager::setupConfigPortal() is never checked; this will return true if it can secure the port, false otherwise. If you check the result of dnsServer->start(...) for the second call to setupConfigPortal(), you'll see that it returns false.

The easiest way I found to fix this was to replace *dnsServer = DNSServer(); in startConfigPortal(...) with dnsServer->stop();. This will correctly finalise the WiFiUdp object and free up the port for next time.

I'm running this library on a ESP8266 D1 Mini, through PlatformIO.

khoih-prog commented 3 years ago

HI @mattbradford83

Thanks for your impressive bug locating and fixing. :+1: I'll definitely have a look, test and merge.

If possible, to help save me some time to test and verify, could you please post a short code to demonstrate the issue and the fix. If not, I'll still spend time to duplicate and verify this interesting bug fix.

Best Regards,

mattbradford83 commented 3 years ago

Hi @khoih-prog,

No problems at all. If you compile this onto an ESP8266 and run it, it'll start a captive portal. I join it with my iPhone, add some values, then click save. I then close the captive portal on my phone and select "use different network" just to make sure the phone closes the page. I then wait 7 seconds for the config portal to spin up again, and join it. My phone joins it fine, but a captive portal never appears.

I did have an extra snippet in there at one point to confirm my thoughts:

    dnsServer->setErrorReplyCode(DNSReplyCode::NoError);
   if (dnsServer->start(53, "*", WiFi.softAPIP())) {
    Serial.println("DNS Server started on port 53.");
   } else {
    Serial.println("Could not start DNS Server on port 53!");
   }

I put this in the first section of wifi_init() in my code around line 74, the thought being that it didn't matter if your library failed to run this code; if my code had run it before the library it would still work, and it does. BUT, on the second call to wifi_init(); the DNS server fails to start.

I noticed that this snippet is also in some of the libraries you've developed from so I've reached out to them also to make sure the bug is resolved in their versions too.

I hope this helps :)

main.zip

khoih-prog commented 3 years ago

Thanks @mattbradford83

The PR has just been merged. I'll add more dnsServer check + error message as you suggested, then publish a new release .

Best,

khoih-prog commented 3 years ago

ESPAsync_WiFiManager Release v1.6.3 has just been published.

Your contribution has been noted in Contributions and Thanks


Releases v1.6.3

  1. Fix dnsServer not closed to free up DNS port 53. Check Allow captive portal to run more than once by closing dnsServer cleanly. #49
  2. Add dnsServer can't start error message.