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

[esp32] Fix mgos_wifi_scan never completing if wifi in connecting state #24

Closed
wants to merge 2 commits into from

Conversation

tripflex
Copy link
Contributor

@tripflex tripflex commented Mar 28, 2019

This PR
This PR adds a connecting variable to determine if wifi is in connecting state, and if so, it calls esp_wifi_disconnect before calling scan for wifi networks.

I tested this using my captive portal demo app:
https://github.com/tripflex/captive-portal-wifi-stack-demo

And it fixed the problem perfectly. Because the connect timer is still running from mgos, it will still continue connection attempt after scan is completed, moving on to next sta, etc.


Related to: #18
and: tripflex/captive-portal-wifi-stack#1

If wifi sta is enabled and is attempting to connect to a non-existent SSID, calling mgos_wifi_scan will never complete and mos console shows this error:

W (24763) wifi: Now is connecting, user scan invalid now!

Which if the SSID is not in range or does not exist, it will never end up connecting to anything, and scan is never actually called.

Instead of having some kind of hack in our libraries or apps, I feel as though this should be handled through the wifi lib to stop the connection attempt and actually run the scan, instead of just printing an error to console.

Related to:
espressif/esp-idf#2497

Which is related to the 3.0.x IDF versions

Looks like this is how they fixed it in wifimanager port for esp:
https://github.com/alanswx/ESPAsyncWiFiManager/pull/45/files

IDF Docs specifically reference this issue as well:
https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/wifi.html#scan-when-wi-fi-is-connecting

mos-libs issue: https://github.com/cesanta/mos-libs/issues/26

#23

@tripflex
Copy link
Contributor Author

@cpq @rojer @dimonomid can we get this merged pleaseeeeeee 👍

@bitfab
Copy link

bitfab commented Apr 14, 2019

+1 !

@tripflex
Copy link
Contributor Author

@cpq @rojer @dimonomid

@dimonomid
Copy link
Contributor

@tripflex I don't maintain Cesanta code these days, so would appreciate if you don't mention me here. Thanks.

@tripflex
Copy link
Contributor Author

@tripflex I don't maintain Cesanta code these days, so would appreciate if you don't mention me here. Thanks.

Okay sorry, wasn't aware -- won't tag you anymore

@astout
Copy link

astout commented May 6, 2019

Thanks @tripflex, this is working for me.

esp32/src/esp32_wifi.c Outdated Show resolved Hide resolved
@rojer
Copy link
Contributor

rojer commented May 7, 2019

sorry for the delay, please update the branch and address two comments

@astout
Copy link

astout commented May 7, 2019

FWIW, I spoke too soon about this working for me. I have since upgraded to 2.13.1 libs and even trying @rojer's recommended changes, I still get the "scan invalid now!" message. Would it be bad practice to just call esp_wifi_disconnect before every scan esp_wifi_scan_start in mgos_wifi_dev_start_scan? I have made this change and I am at least getting scan results now. But I am not sure if it could lead to other unknown behaviors.

[edit]
It seems to be the recommended usage now per espressif: espressif/esp-idf#2497

@tripflex
Copy link
Contributor Author

tripflex commented May 7, 2019

@rojer I updated the PR per your recommendations

@rojer
Copy link
Contributor

rojer commented May 7, 2019

@tripflex gh says there are conflicts with the master, please rebase

rojer pushed a commit that referenced this pull request May 10, 2019
h/t @tripflex

Closes #24

#18

CL: wifi: esp32: Call disconnect before scan if in connecting state
@rojer rojer closed this in #27 May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants