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

WiFi.persistent(false) does not disable persistent WiFi config. #1393

Closed
everslick opened this issue May 8, 2018 · 10 comments
Closed

WiFi.persistent(false) does not disable persistent WiFi config. #1393

everslick opened this issue May 8, 2018 · 10 comments

Comments

@everslick
Copy link
Contributor

Currently wifi settings are written to the flash regardless of calling WiFi.persistent(false) or not.

In my opinion WiFiGeneric.cpp#L123 should be like:

if (_persistent) {
  esp_wifi_set_storage(WIFI_STORAGE_FLASH);
} else {
  esp_wifi_set_storage(WIFI_STORAGE_RAM);
}

Ref:
#548

Any thoughts on that?

@everslick
Copy link
Contributor Author

everslick commented May 12, 2018

I propose a fix like in #1406.

@everslick
Copy link
Contributor Author

fix meged to master.

@tablatronix
Copy link
Contributor

This is problematic now and can break code and is not a full feature implementation as it has issues with calling persistent during runtime.

  • If calling persistent(false) before wifistart, credentials are not loaded, say in a library constructor
  • If changing persistent before begin(), credentials are not saved, wifista begin() esp_wifi_set_config, then calls esp_wifi_start() <-- oh and no persistent flag here

Not really sure how this should be engineered to deal with these issues. But I feel this is wrong

Also not sure why the decision was made to use passing arguments for persistent states, seems certain to introduce bugs, should be steady state

Perhaps

  • _persistent should be used and not passed around in wifiLowLevelInit and espWiFiStart
  • persistent() != _persistent should change esp_wifi_set_storage() everytime, this might be a problem with the way lowlevel init is tracked not sure. But it is the only way to deal with control over set_config
  • get_config could have wrappers to handle as esp8266 equiv, otherwise get_config will be persistent following and should probably always prefer flash if we ever want autoconnect to work properly.
  • also wifiLowLevelDeinit needs to be working, maybe that will help switching wifiLowLevelInit back and forth.. shrug

@tablatronix
Copy link
Contributor

Ok so using any wifi functions in a constructor is nogo, wifi is not init, in particular nvs is not available yet.
So that is another issue by itself.

@everslick
Copy link
Contributor Author

everslick commented Jun 23, 2018

What is the use case for enabling/disabling persistent configuration storage during runtime in the first place?

My impression was/is that this is a decision that is to be made for a specific firmware implementation once and will not change depending on runtime parameters. I prefer to have 100% control over when the ESP connects to WiFi so I just want to disable autoconnect and persistence as soon as possible during bootup. Then I read my WiFi configuration from my own setup and connect to whatever SSID is stored there.

I see the use for having the config stored in NVS and the reason behind autoconnect (for shorter connection delays) but this is not relevant for all users. And I certainly would not want to switch back and forth between persistent and non persistent configuration. But maybe I just misunderstood your point.

Having said that, I still have a patch in my fork that solves all my persistent config related problems for my specific use case.

--- a/libraries/WiFi/src/WiFiGeneric.cpp
+++ b/libraries/WiFi/src/WiFiGeneric.cpp
@@ -113,6 +113,9 @@ static bool wifiLowLevelInit(bool persistent){
     if(!lowLevelInitDone){
         tcpipInit();
         wifi_init_config_t cfg = WIFI_INIT_CONFIG_DEFAULT();
+        if(!persistent){
+          cfg.nvs_enable = 0;
+        }
         esp_err_t err = esp_wifi_init(&cfg);
         if(err){
             log_e("esp_wifi_init %d", err);
@@ -121,7 +124,6 @@ static bool wifiLowLevelInit(bool persistent){
         if(!persistent){
           esp_wifi_set_storage(WIFI_STORAGE_RAM);
         }
-        esp_wifi_set_mode(WIFI_MODE_NULL);
         lowLevelInitDone = true;
     }
     return true;
@@ -404,9 +406,11 @@ wifi_mode_t WiFiGenericClass::getMode()
     if(!wifiLowLevelInit(_persistent)){
         return WIFI_MODE_MAX;
     }
-    uint8_t mode;
-    esp_wifi_get_mode((wifi_mode_t*)&mode);
-    return (wifi_mode_t)mode;
+    wifi_mode_t mode = WIFI_MODE_NULL;
+    if(_esp_wifi_started){
+      esp_wifi_get_mode(&mode);
+    }
+    return mode;
 }
 
 /**

This disables storing of the current active MODE in NVS and allows WiFi to start in the default (coded) mode. (e.g. my users can activate AP by pressing a button. without this patch AP is still active after a reboot what is not what I expect nor want). The important line is cfg.nvs_enable = 0;

Maybe we can collect some use cases, that the API should support and come up with an implementation for it?

@tablatronix
Copy link
Contributor

tablatronix commented Jun 23, 2018

For running various wifi functions without affecting stored credentials, or modes. For example starting a soft ap for configuration but not breaking or overwriting stored modes and credentials.

ala wifimanager etc.

exactly what you mention in your patch, in esp8266 this is accomplished by toglging in and out of persistent flags. ( of couse in esp8266 storage is not setup in init, it is checked in runtime )

@Aix-la-Chapelle
Copy link

Aix-la-Chapelle commented Jul 2, 2018

Hi, i am having problem with WiFi.persistent(true); , i want it to work the way it is in esp8266, so esp32 remembers which AP it was connected before deepsleep and go for that again when it wakes up, i tried different implementation of WiFiGeneric.cpp found here but still connection time is 3-4 second (just by using 1322 my connection time in DHCP mode became around 1.5 second but in static ip mode it is still 3-4 sec, i need to reduce this connection time under 1 sec , in esp8266 i reached 400-600 ms. but i need esp32 features.
any guideline and help would be appreciated both in IDF or arduino , thanks.,

@arihantdaga
Copy link

I am still facing this error, even though I init my esp without any network, its automatically connecting to the last known network.
I have kept wifi.persistent(false) in all the places but no luck.

        WiFi.persistent(false);
        WiFi.begin();
        WiFi.mode(WIFI_OFF);
        delay(200);   
        WiFi.persistent(false);
        WiFi.enableSTA(true);
        WiFi.persistent(false);

Even though I am not giving any ssid and not calling wifi.begin(ssid,pass), its still connecting to the old network.
I also tried setting up CONFIG_ESP32_WIFI_NVS_ENABLED=0 in build flags, still getting the same result.

Is it a bug or is there any workaround ? Am I doing something wrong ?

@lbernstone
Copy link
Contributor

WiFi.persistent is broken b/c the underlying function in esp-idf is broken, but you are mistaken in thinking that setting WiFi mode or persistent off would erase the credentials (from ram). You must disconnect.
WiFi.disconnect(false, true); will delete the saved credentials and leave the device in station mode.

@tablatronix
Copy link
Contributor

persistent just switches in and out of flash config, if its already saved you have to erase them either via disconnect or eraseconfig as lberstone said

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

No branches or pull requests

5 participants