espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.77k stars 7.44k forks source link

Wifi API is INDETERMINISTIC, and does not conform to Arduino API specification #1718

Closed stevenj closed 5 years ago

stevenj commented 6 years ago

Hardware:

Board: ESP32 IDE name: Arduino IDE

Description:

The ESP32 version of the WiFi api is broken and does not conform to the Arduino WiFi API specification. Notably, the persistence of the connection leads to broken programs, network errors, faults that are difficult to track and is NOT what the Arduino version of the library does.

Sketch:

#include <WiFi.h>

//SSID of your network
char ssid[] = "yourNetwork";
//password of your WPA Network
char pass[] = "secretPassword";

void setup()
{
 WiFi.begin(ssid, pass);
}

void loop () {}

This Sketch has DETERMINISTIC behaviour on Arduino, and INDETERMINISTIC behaviour on the ESP32 implementation.

On Arduino, this will simply start a client connection to the network. On the ESP32 what it does can not be defined, simply from the code. It might do what the Arduino one does, it might also, start an AP with the name "ESP_xxxxxx" if you did a flash wipe, or it might start an AP with a name previously used by a different program running on the same hardware.

This is a VERY VERY bad API behaviour. The only way to avoid it is explicitly call WiFi.mode() before WiFi.begin(). It can lead to all sorts of problems. let me illustrate:

A School room has two ESP32 boards, and the teacher sets an assignment, make the two boards communicate. The students are split into two groups, one to program an AP, the other to program the Client. They are given two of the labs ESP32's. Everything seems fine, the kids are making progress, day ends. Next day they are given the same two ESP32's but they have swapped boards. No problem? Wrong. Now the client team, who are following online tutorials at using the Arduino WiFi API have a AP running on their board, that was set up by the Server team yesterday. Even though no where in their code do they set up an AP. They start getting connection problems. Why? Because they have TWO AP's with the same name, running on two ESP32s, one on the Server teams ESP32 and one on the client teams ESP32, the client ESP32 AP won't do anything, but any device trying to connect will connect to it 50% of the time, so the the server team think their server is broken, the clients are connected but they aren't getting any messages. WHY? WHY?

And the problem isn't anything they did, it's because the ESP32 implementation of this library is broken and set up an AP when no one requested that it do so.

yoursunny commented 6 years ago

ESP32 and ESP8266 by default persist the WiFi settings. The AP or STA created in a previous session is initialized even before setup() happens. To avoid this situation, it's a good idea to put WiFi.persistent(false); before any other WiFi function calls.

stevenj commented 6 years ago

Thank you for confirming my bug report. In summary:

  1. Does not conform with Arduino API.
  2. Is not, by default, deterministic behaviour
  3. If you do a flash clear the ESP32 creates a spurious AP, so no its not always the result of a "previous session" sometimes its just a bogus AP with no code backing it up.
  4. Creates unnecessary confusion for unsuspecting users.

The default should be NOT persistent, because that would then conform with the Arduino API and would be deterministic behaviour (the ESP32 would only do what it was actually instructed to do). If someone wanted this strange persistent behaviour (Why someone would want an AP if their code had no way to interact with it is a mystery to me, but whatever). then that minority of users should have to put WiFi.persistent(true); in their code, not the other way around.

yoursunny commented 6 years ago

I don’t have an “official” Arduino so I won’t say whether ESP’s API is right or wrong. It does need additional documentation on persistent function.

Why someone would want an AP if their code had no way to interact with it

The persistent feature is not only for AP, but also for STA. An ESP as STA could start connecting to the remembered AP as soon as it boots up or wakes up from deep sleep, even before the user sketch starts running. Therefore, it could complete the task faster and return to deep sleep earlier, and preserve battery power.

stevenj commented 6 years ago

Again, this is no reason for it to be default behaviour. IF you are running in this particular mode, AND you want to use persistent STA connections you should need to explicitly enable it.

Explicit is better than Implicit, AND code should not do "surprising" things. This does both, implicit behaviour which is only useful in a specific circumstance but is the default state, and surprising in that its not what one would normally expect.

It also does not address the issue that the ESP32 creates a spurious AP after a flash reset. Which is a waste of power if the coder does not take corrective action. Defeating the presumed battery saving benefits of a persistent STA.

yoursunny commented 6 years ago

Yes, I agree that the default should have been WiFi.persistent(false). That also reduces wear and tear on the flash memory. However, changing it would break existing applications that expect the WiFi settings to persist.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stevenj commented 5 years ago

I think this issue still exists.

stale[bot] commented 5 years ago

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 5 years ago

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.