Skip to content
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.

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

Include hidden SSID's, scan lists of APs #3464

Closed
wants to merge 2 commits into from

Conversation

dkoneill
Copy link

@dkoneill dkoneill commented Nov 8, 2019

Please consider this refactoring of the WiFiMulti.cpp library to better support lists of APs including those with hidden SSID's. The selection and retry logic has been modified and tested in an environment with multiple AP's hidden and visible.

@tablatronix
Copy link
Contributor

Can you provide more information on this change?

Perhaps more than

			// If the ssid returned from the scan is empty, it is a hidden SSID
			// it appears that the WiFi.begin() function is asynchronous and takes
			// additional time to connect to a hidden SSID. Therefore a delay of 1000ms
			// is added for hidden SSIDs before calling WiFi.status()

I am not sure I see what else was done here

WiFi.disconnect(false,false);
delay(10);

WiFi.persistent(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks hacky, what is its purpose?

Choose a reason for hiding this comment

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

The standard WiFiMulti code will scan for all SSID it can see, and connected you to one that you have requested if it is available... It will show you some information about the hidden SSID, BUT! if the SSID your requesting is hidden, it will NOT connect you to that SSID. The logic in the code is wrong.

The modification we made to WiFiMulti in the pull request above, will connect you to a hidden SSID if it is available....

Copy link
Author

Choose a reason for hiding this comment

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

The lower level implementation details of WiFi.disconnect() are not available for me to examine and determine why the delay is required. Through numerous experiments on a number of secured, unsecured, hidden, and visible SSIDs the delay creates a high degree of success when trying subsequent network connections. The delay put in place when connecting to hidden SSID's was also determined experimentally and results in successful connections almost every time. Removing the delay causes the connections to fail due to the asynchronous nature of the lower level wifi drivers.

Copy link
Contributor

@tablatronix tablatronix Jan 23, 2020

Choose a reason for hiding this comment

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

This sounds like an esp32 equivalent to esp8266/Arduino#4372
I have not encountered this issue on esp32, do you have demo code. I suggest the actual problem be investigated. This comment however was specifically aimed at WiFi.persistent(false);

Choose a reason for hiding this comment

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

My esp32 is not responding

@tablatronix
Copy link
Contributor

Ok so to avoid confusion about the inline comments. This PR is a little confusing. The premise is that there needs to be a delay somewhere ?? to allow connections to hidden aps.

This does not explain all the changes to this code, there are 2 additional params added, there are kludges for disconnect and reconnect ie. toggling persistent and mode back and forth..

If this is indeed an async race condition or timeout against SDK similar to esp8266/Arduino#4372 for esp8266 then an appropriate fix should be implemented.

as far as I can tell this PR describes this 1 and only change.

                         WiFi.begin(..)
			if( scanHidden && ssid_scan.length() == 0) {
			  log_i("[WIFI] Hidden SSID, adding a delay.");
			  delay(1000); 
			}
                        status = WiFi.status();

But there seems to much more going on.

@trlafleur
Copy link

The real issue for this PR is that the search for, and connecting to a hidden SSID was not working in the original code... It has been fixed in this PR. As a side note, it was discovered that a delay was needed due to an async process... dkoneill will comment more on what he discovered and fixed...

@dkoneill
Copy link
Author

Hi Shawn,

This is a refactor of a sub-system that did not work as its original author had intended. The original code was documented to support multiple SSID's and had some logic that tried to select the connection with the best signal strength. Unfortunately the code, through examination and testing, did not a) support multiple SSID's, and b) only worked in the degenerate case of a single SSID, and c) did not support hidden SSID's.

The reason that I looked at this code is that one of the projects I was working on was being used in two different locations each using a unique SSID. I expected that I could add both SSID's and WifiMulti would operate as expected, that is, it would try an SSID, fail, and then try the next. Due to logic errors in WifiMulti, the connection was only successful if the correct SSID was first in the list.

This refactor provides the following:

  1. Multiple SSID's can be added into a list and WifiMulti will iterate through the list until a successful connection is made. This was the intended behavior of the prior version which was not working and has been corrected and the code simplified in this respect.

  2. Backwards compatibility on the interface so that no existing use of the WifiMulti.run() method will break. This is documented in .. libraries/WiFi/examples/WiFiMulti/WiFiMulti.ino

  3. Support for hidden SSID's if the programmer adds the timeout and boolean flag parameters to the WifiMulti.run() method call.

I apologize if I've not clearly documented how this refactor improves the WifiMulti library for espressif users. Let me know if I can assist further.

@CLAassistant
Copy link

CLAassistant commented Apr 28, 2021

CLA assistant check
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
Copy link

stale bot commented Apr 16, 2022

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.

@lucasssvaz
Copy link
Collaborator

Closing in favor of #9202

@lucasssvaz lucasssvaz closed this Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Review needed Issue or PR is awaiting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants