Skip to content

Update ESP8266WiFiSTAClass::waitForConnectResult() for the same behavior with ESP32. #4985

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.

Sign up for GitHub

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Update ESP8266WiFiSTAClass::waitForConnectResult() for the same behavior with ESP32. #4985

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0be5be6
Update ESP8266WiFiSTAClass::waitForConnectResult() for the same beha…
ShinDaiIchi Jul 30, 2018
d2c4cc7
Add td_split() for spliting String to String Array by multi-delimi…
ShinDaiIchi Jul 30, 2018
b39fc18
Add td_split() for spliting String to String Array
ShinDaiIchi Jul 30, 2018
579a9a6
Update WString.h
ShinDaiIchi Jul 30, 2018
5c4ff27
Update WString.cpp
ShinDaiIchi Jul 30, 2018
8abc913
Merge branch 'master' into master
devyte Aug 1, 2018
8544d30
Update ESP8266WiFiSTAClass::waitForConnectResult with connection's …
ShinDaiIchi Aug 2, 2018
956510e
Update ESP8266WiFiSTAClass::waitForConnectResult with connection's …
ShinDaiIchi Aug 2, 2018
52fcfa3
Merge branch 'master' into master
devyte Aug 9, 2018
c5fec54
Update (wait_secs==0)? etc...
ShinDaiIchi Aug 10, 2018
cba45ea
Merge branch 'master' into master
d-a-v Sep 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions cores/esp8266/WString.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,7 @@ class String {
void toLowerCase(void);
void toUpperCase(void);
void trim(void);

// parsing/conversion

long toInt(void) const;
float toFloat(void) const;

Expand Down
3 changes: 2 additions & 1 deletion libraries/ESP8266WiFi/src/ESP8266WiFiSTA.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,8 @@ uint8_t ESP8266WiFiSTAClass::waitForConnectResult() {
if((wifi_get_opmode() & 1) == 0) {
return WL_DISCONNECTED;
}
while(status() == WL_DISCONNECTED) {
int i = 0;
while((!status() || status() >= WL_DISCONNECTED) && i++ < 100) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a max of 10s time. Some connections could take longer, especially under poor signal conditions. Some users could not want to spend so much time trying to connect and want to give up sooner. I suggest taking an argument that defaults to this when not set.

Copy link
Author

Choose a reason for hiding this comment

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

If default time trying to connect when not set is 15 secs. What do you think ?

Copy link
Collaborator

@d-a-v d-a-v Jul 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

What would happen with a default non infinite value when user's network connectivity / the router is for any reason powered down or disconnected for a while (a longer while than this default value), or takes long to power on, or whatever ?

My point is that usually sketches patiently wait for connectivity. With this change, all sketches would need to be updated to loop on the connectivity loop.

I don't question this PR, only the historical default behavior / backward compatibility.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops @d-a-v is correct: either we maintain backwards compatibility and merge now, or we keep this breaking change and wait until release 3.0.0 to merge. I prefer the former.
So:

  1. the function should take a timeout argument, in ms, which defaults to some value that disables the timeout (std::numeric_limits?)
  2. If the user wants a timeout, he can specify a value for the argument.
  3. In a future release we'll change the default to some value like 10 or 15s.

@ShinDaiIchi Could you please implement the changes and update the PR?

Copy link
Author

Choose a reason for hiding this comment

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

Already update with WiFi connecting's wait-timeout with backwards compatibility.

delay(100);
}
return status();
Expand Down