WiFi settings are not saved if ESP32 is restarted in SaveConfigCallback

[rusty-labs]

Basic Infos

Hardware

WiFimanager Branch/Release: Master

Esp8266/Esp32:

Hardware: Heltec wifi-lora-32-v3

Core Version: 2.4.0, staging

Description

Steps to reproduce

• Connect to AP
• Set incorrect wifi password and save
• Add setSaveConfigCallback to code
• Restart ESP32
• Connect to AP
• Provide correct password and save
• Web shows "Saved" and ESP32 is restarted
• ESP32 is trying to connect to previously provided, incorrect password

Settings in IDE

Module: Heltec wifi-lora-32-v3

Sketch

void saveConfigCallback()
    {
        delay(2000);
        ESP.restart();
    }

        WiFi.mode(WIFI_STA);

        wm.setSaveConfigCallback([&]()
                                 { saveConfigCallback(); });

        wm.setBreakAfterConfig(true);

        wm.addParameter(&paramMqttServer);
        wm.addParameter(&paramMqttPort);
        wm.addParameter(&paramMqttUser);
        wm.addParameter(&paramMqttPassword);

        std::vector<const char *> wm_menu = {"wifi", "exit"};
        wm.setShowInfoUpdate(false);
        wm.setShowInfoErase(false);
        wm.setMenu(wm_menu);

        bool res = wm.autoConnect(_SSID);

[tablatronix]

I suspect breakafterconfig i will test

[rusty-labs]

Yeah, if I don't use setBreakAfterConfig my callback is not even called

[rusty-labs]

Ok there're multiple issues

Settings are not saved if no successful connection occurred,

  WiFi.persistent(true);
  ret = WiFi.begin(ssid.c_str(), pass.c_str(), 0, NULL, connect);
  WiFi.persistent(false);

So I toggled off wm.setSaveConnect(false); I assume if user saves settings, settings should be saved no matter what, and connection procedure should be out of this block

WiFi.persistent(true);
ret = WiFi.begin(ssid.c_str(), pass.c_str(), 0, NULL, false);
WiFi.persistent(false);
// Try to connect here

Other issue is within the UI Opening configuration portal second time shows my previously selected SSID in gray color and I assumed I don't need to select it again, so I changed only password. However on the next bootup SSID is empty while the updated password is there

[rusty-labs]

I uploaded the fix addressing UI issue and saving empty ssid

[tablatronix]

Yeah there is a other issue about that thanks Its already saves if its placeholder. I will think of a better way to represent this like disable until clicked or something

[rusty-labs]

For the sake of consistency it'd be better to use value instead of a placeholder, since custom parameters are using values I also checked behavior on my router in client mode, and it also keeps SSID as a value even if it's wrong

[tablatronix]

Custom params can also use placeholders. Connect fails never save but it ahould not overwrite if ssid was blank. I thought I just fixed this but maybe it was bad or not pushed. Thanks

[tablatronix]

I used placeholder because I do not want wifi args resubmitting if only changing params. This way they do not get sent in plain text everytime you save, if not needed. Also we cannot populate password.

hmm I thought this fixed this

0e2016879b66dc3b4f200b1d91fde5ae9a501f37 9df7acdeeb6a7f614cdcba4a3f8c117ab7c23718

Also why are you restarting in the callback function anyway? saveConfigCallback() and not using return or status? Just Curious

I wonder if wifi_ssid is not populated becuase esp32 wifi was not init or connected yet

[rusty-labs]

Sending SSID in a plain text is totally fine. #

saveConfigCallback() and not using return or status? Just Curious Yeah, I can restart after connection complete too, I just prefer callbacks, in case if I want to use non-blocking mode etc.

I believe the root cause of these problems is that WiFi_SSID and _ssid are not synced, should be setter/getter instead

[rusty-labs]

Password has the same problem

• Provide correct ssid and pass, save
• Load configuration portal again
• Change custom parameters
• Save
• Can't connect to WiFi because password is gone

[rusty-labs]

Ok I fixed all the issues above in my fork

https://github.com/rusty-labs/WiFiManager/blob/981a540793fd1c1b501be7f6071da42fe5aaa3de/WiFiManager.cpp#L2110C3-L2116C4

https://github.com/rusty-labs/WiFiManager/blob/981a540793fd1c1b501be7f6071da42fe5aaa3de/wm_strings_en.h#L66

https://github.com/rusty-labs/WiFiManager/blob/981a540793fd1c1b501be7f6071da42fe5aaa3de/WiFiManager.cpp#L1293C3-L1303C4

I did some other fixes but I think these 3 should be sufficient to solve the problem

[tablatronix]

What browser? Ssid and pass empty should do nothing

[rusty-labs]

It has nothing to do with a browser

1. _ssid = server->arg(F("s")); returns empty ssid, because it's a placeholder not a value
2. handleWifiSave() makes _ssid empty
3. connect=true
4. processConfigPortal saves empty _ssid to flash

[tablatronix]

I still dont understand this issue, or see it.. Where is your example failing?

     if(_ssid == "" && _pass != ""){
      _ssid = WiFi_SSID(true);
     ....
      // skip wifi if no ssid
      if(_ssid == ""){
        #ifdef WM_DEBUG_LEVEL
        DEBUG_WM(WM_DEBUG_VERBOSE,F("No ssid, skipping wifi save"));
        #endif
      }

Empty ssid should not be saved, unless you change pw but it should use WIFI_SSID, what is the test case that this returns "" ? What wifi mode are you in when saving by chance?

[rusty-labs]

Ok I got your latest changes Your fix doesn't work if I provide incorrect SSID and correct Password first, and then after restart provide correct SSID But for the steps I described above it works

Btw this code shouldn't be called if _connectonsave = false https://github.com/tzapu/WiFiManager/blob/d8f8d5478a646a9ec14d0418aa0c4b96c1899b58/WiFiManager.cpp#L1038C3-L1043C4 Also I don't understand why if(_aggresiveReconn && _connectRetries<4) _connectRetries=4; If I set _connectRetries = 2, why all of a sudden it becomes 4 ? Why do you even need _aggresiveReconn ?

Overall the logic is becoming too tangled, the code needs a cleanup, too many states contradicting each other

[tablatronix]

It says it in the source

    bool          _aggresiveReconn        = true; // use an agrressive reconnect strategy, WILL delay conxs
                                                   // on some conn failure modes will add delays and many retries to work around esp and ap bugs, ie, anti de-auth protections
                                                   // https://github.com/tzapu/WiFiManager/issues/1067

It will probably be removed from prod, or optional, but it resolved almost 100% of connection failures reported n issues

1067

[tablatronix]

I will remove it and see if the bug is fixed, and see if I can move this into a workaround check for specific versions and or failure codes. Without it this was causing a absolute failure of esp connection sucess on restarts

[rusty-labs]

_aggresiveReconn is always true and there is no public setter for this, instead you can just _connectRetries = max(connectRetries, 4); at least the logic would be explicitly expressed instead of some hidden bool state But I'd get rid of it too, those who have troubles with a connection can increase number of retries in a client code The library code shouldn't fix specific problems, it should provide an interface to do so Also delay(1000); should be after wifiConnectNew, otherwise execution is just delayed for nothing

[tablatronix]

The delay is necessary for this failure mode

[rusty-labs]

Delay yes, but _aggresiveReconn no, if it's always true

[rusty-labs]

Btw delay not needed, you already have _connectTimeout which is not used, set _connectTimeout = 1000 and it will work as a delay

[tablatronix]

They function differently. Timeout is not the same as delay for reconnect. I added both those options after that test was implemented as a result of its feedback.

i just need to decide how to reimplement it better so its not always required.

E (5966) wifi:Association refused temporarily, comeback time 1048 mSec

[real-bombinho]

I have version 2.0.17 and it seems this is not fixed in this version by the looks of?

I am just porting some working stuff from ESP8266 to the ESP32 and it keeps forgetting the WiFi credentials on each restart on the ESP32.

[tablatronix]

@real-bombinho please create a new issue with some information, unrelated to this issue

[tablatronix]

Your fix doesn't work if I provide incorrect SSID and correct Password first, and then after restart provide correct SSID But for the steps I described above it works

Is this really a issue? who does this?

[tablatronix]

Closing unless you can provide how to reproduce and provide logs. I tested this as much as I can and have no issue