Skip to content
This repository has been archived by the owner on Feb 4, 2023. It is now read-only.

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

Merged
merged 1 commit into from
Apr 13, 2021

Conversation

mattbradford83
Copy link
Contributor

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
Copy link
Owner

HI @mattbradford83

Thanks for your impressive bug locating and fixing. 👍 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
Copy link
Contributor Author

mattbradford83 commented Apr 13, 2021

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 khoih-prog merged commit fee3a98 into khoih-prog:master Apr 13, 2021
@khoih-prog
Copy link
Owner

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 added a commit that referenced this pull request Apr 14, 2021
### 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** #49](#49)
2. Add `dnsServer can't start` error message.
khoih-prog added a commit that referenced this pull request Apr 14, 2021
### 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](#49)
2. Add `dnsServer can't start` error message.
@khoih-prog
Copy link
Owner

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants