WiFiManager connection attempt to unconfigured ("blank") SSID after restart on SAMD

[bizprof]

Describe the bug

I have encountered a situation where WiFiManager tries to connect to unconfigured SSIDs (stored with the default value "blank" in EEPROM) after power cycling the board. This is unwanted behavior, as in some use cases WiFi parameters will have to be configured much later, or WiFi might be unavailable for longer periods of operation. This version of WiFiManager seems to be blocking the CPU while connection attempts are made, which impacts normal operation of the board (in my case, running a Nixie clock).

Steps to Reproduce

Using the supplied example script SAMD_WiFiNINA.ino (without Dynamic parameters, and RESET_IF_CONFIG_TIMEOUT set to "false" in defines.h), After loading the script, I let the CP time out without connecting to it at all. I disconnect the board from power and then reconnect. The script now tries to connect to SSID "blank" multiple times. Only then it goes into the CP. After the CP times out, the script tries to connect to the "blank" SSIDs again. And so on. See log below.

Expected behavior

I expect the script to behave the same each time the board restarts. I.e., if I do not configure any SSIDs, it should stay in CP mode forever, as it does the first time the script is loaded (indicated by the log entry "[WN] bg:Stay forever in CP:No ConfigDat").

Actual behavior

The "blank" entries seem to be mistaken for actual config data, and hence there is an attempt to connect to them, with all the performance impact that entails. Maybe don't store blank entries at all, but leave them as unconfigured. Similar scenarios might occur when only one of the SSIDs is configured, and the other one is left "blank".

Debug log

FIRST run of the script (going straight into CP, as expected):

Starting SAMD_WiFiNINA on SAMD NANO_33_IOT
WiFiManager_NINA_Lite v1.1.2
[WN] Hostname=SAMD-Master-Controller
Flag read = 0xffffffff
No doubleResetDetected
SetFlag write = 0xd0d01234
[WN] CCSum=0xe11e,RCSum=0xffffffff
[WN] InitCfgFile,sz=236
[WN] SaveEEPROM,Sz=1024,DataSz=0,WCSum=0xc7d
[WN] bg: isForcedConfigPortal = false
[WN] bg:Stay forever in CP:No ConfigDat
[WN] SSID=WIFININA_51F485,PW=MyWIFININA_51F485
[WN] IP=192.168.4.1,CH=5
[WN] s:configTimeout = 0
Stop doubleResetDetecting
ClearFlag write = 0xd0d04321
FF

SUBSEQUENT runs of the script after reconnecting board:

Starting SAMD_WiFiNINA on SAMD NANO_33_IOT
WiFiManager_NINA_Lite v1.1.2
[WN] Hostname=SAMD-Master-Controller
Flag read = 0xd0d04321
No doubleResetDetected
SetFlag write = 0xd0d01234
[WN] CCSum=0xc7d,RCSum=0xc7d
[WN] ======= Start Stored Config Data =======   ---> Maybe blank entries shouldn't have been stored in the first place?
[WN] Hdr=WIFININA,SSID=blank,PW=blank
[WN] SSID1=blank,PW1=blank
[WN] BName=blank
[WN] ConMultiWifi
[WN] con2WF:SSID=blank,PW=blank
[WN] Remaining retry_time=3
[WN] Remaining retry_time=2
[WN] Remaining retry_time=1
[WN] Failed using index=1, retry_time=0
[WN] Remaining retry_time=3
[WN] Remaining retry_time=2
[WN] Remaining retry_time=1
[WN] Failed using index=0, retry_time=0
[WN] con2WF:failed
[WN] b:NoW
[WN] SSID=WIFININA_51F485,PW=MyWIFININA_51F485
[WN] IP=192.168.4.1,CH=5
[WN] s:millis() = 19026, configTimeout = 139026
Stop doubleResetDetecting
ClearFlag write = 0xd0d04321
FFFFFF[WN] r:WLost.ReconW
[WN] ConMultiWifi
[WN] Using index=1, lastConnectedIndex=0
[WN] con2WF:SSID=blank,PW=blank
[WN] Remaining retry_time=2
[WN] Remaining retry_time=1
[WN] Failed using index=0, retry_time=0
[WN] Remaining retry_time=3
[WN] Remaining retry_time=2
[WN] Remaining retry_time=1
[WN] Failed using index=1, retry_time=0
[WN] con2WF:failed
F[WN] r:WLost.ReconW
.....
[WN] ConMultiWifi
[WN] Using index=0, lastConnectedIndex=1
[WN] con2WF:SSID=blank,PW=blank
[WN] Remaining retry_time=2
......

Information

• Arduino IDE version: 1.8.13 on Windows 10
• Board: Arduino NANO 33 IoT with latest firmware 1.4.3

Really appreciate you taking the time looking into these issues.
WiFiManager_NINA_Light does a wonderful job, hence I'm using it in different scenarios, which might push its boundaries.
Keep up the great work. Thank you!

[khoih-prog]

Hi @bizprof

Again thanks for your informative and accurate bug report 👍 .
I already nailed down the culprit and will publish a new release today. Happy bug catching and hope you will catch some more to be included in today's release ;-))

A snap shot

Starting SAMD_WiFiNINA on SAMD NANO_33_IOT
WiFiManager_NINA_Lite v1.1.3
[WN] Hostname=SAMD-Master-Controller
Flag read = 0xd0d04321
No doubleResetDetected
SetFlag write = 0xd0d01234
[WN] Invalid Stored Config Data    <==== whenever "blank" or ""
[WN] bg: isForcedConfigPortal = false
[WN] bg:Stay forever in CP:No ConfigDat
[WN] SSID=WIFININA_51F485,PW=MyWIFININA_51F485
[WN] IP=192.168.4.1,CH=6
[WN] s:configTimeout = 0
F
Your stored Credentials :
Blynk Server1 = account.duckdns.org
Token1 = token1
Blynk Server2 = account.ddns.net
Token2 = token2
Port = 8080
MQTT Server = mqtt.duckdns.org
Stop doubleResetDetecting
ClearFlag write = 0xd0d04321
FFFFFFFFF FFFFF

[bizprof]

Great, thank you for the quick reaction! If I may add another (hopefully small) request, it's about the comments in defines.h, in this section of the code:

// Force some params in Blynk, only valid for library version 1.0.1 and later
#define TIMEOUT_RECONNECT_WIFI 10000L
#define RESET_IF_CONFIG_TIMEOUT true
#define CONFIG_TIMEOUT_RETRYTIMES_BEFORE_RESET 5

It took me a while to understand these parameters, especially since they are quite a way down from the top of the file, and the reference to "Blynk" threw me off in the beginning, as I am not using Blynk (yet).

Maybe you can change the comment to make it a bit more intuitive?

Thank you as always!

Michael

[khoih-prog]

Sure will do. If you can suggest or add some comments somewhere, please don't hesitate.
Sophisticated users normally have better vision than my tunnel-vision.

[bizprof]

On this particular code section, maybe something along the lines of:

// Configure behavior in case we can't connect to previously configured SSIDs
#define TIMEOUT_RECONNECT_WIFI                    10000L                   // How long to try connecting to a given SSID, in ms
#define RESET_IF_CONFIG_TIMEOUT                   false                         // Reset board if we can't connect to any SSID?
#define CONFIG_TIMEOUT_RETRYTIMES_BEFORE_RESET    5              // If yes, how many times to try before reset

Hope I captured the correct meanings/intent of those parameters?

[khoih-prog]

Will change as follows:

// Permit running CONFIG_TIMEOUT_RETRYTIMES_BEFORE_RESET times before reset hardware
// to permit user another chance to config. Only if Config Data is valid.
// If Config Data is invalid, this has no effect as Config Portal will persist
#define RESET_IF_CONFIG_TIMEOUT                   true

// Permitted range of user-defined RETRY_TIMES_RECONNECT_WIFI between 2-5 times
#define RETRY_TIMES_RECONNECT_WIFI                3

// Permitted range of user-defined CONFIG_TIMEOUT_RETRYTIMES_BEFORE_RESET between 2-100
#define CONFIG_TIMEOUT_RETRYTIMES_BEFORE_RESET    5

// Config Timeout 120s (default 60s). Applicable only if Config Data is Valid
#define CONFIG_TIMEOUT                      120000L

#define USE_DYNAMIC_PARAMETERS              true

[bizprof]

👍

[khoih-prog]

The new WiFiManager_NINA_Lite release v1.1.3 has been published.

Please test.

---

Release v1.1.3

1. Fix invalid "blank" or NULL Config Data treated as Valid. Check WiFiManager connection attempt to unconfigured ("blank") SSID after restart on SAMD #8
2. Fix compiler warnings.

[bizprof]

Thanks for the new release. Did a quick test, it now works for my scenario, when no SSID is configured.
Testing a combination of 1 valid SSID (regardless of whether it is entered in first or 2nd place in the CP) and the other SSID left blank does not seem to work properly. It always sees the config data as invalid, unless I define 2 SSIDs. Here's the log:

Entering config with just the first SSID/Pwd set:

[WN] h:repl id
[WN] h:items updated =1
[WN] h:key =id, value =xxxxx
[WN] h:repl pw
[WN] h:items updated =2
[WN] h:key =pw, value =xxxxx
[WN] h:repl id1
[WN] h:items updated =3
[WN] h:key =id1, value =
[WN] h:repl pw1
[WN] h:items updated =4
[WN] h:key =pw1, value =
[WN] h:repl nm
[WN] h:items updated =5
[WN] h:key =nm, value =blank
[WN] h:UpdEEPROM
[WN] SaveEEPROM,Sz=1024,DataSz=0,WCSum=0xb8f
[WN] h:Rst

After reset it should connect to first SSID, but doesn't:

Starting SAMD_WiFiNINA on SAMD NANO_33_IOT
WiFiManager_NINA_Lite v1.1.3
[WN] Hostname=SAMD-WIFININA-51F485
Flag read = 0xd0d04321
No doubleResetDetected
SetFlag write = 0xd0d01234
[WN] Invalid Stored WiFi Config Data
[WN] bg: isForcedConfigPortal = false
[WN] bg:Stay forever in CP:No ConfigDat
[WN] SSID=NixieClock,PW=0123456789
[WN] IP=192.168.4.1,CH=10
[WN] s:configTimeout = 0

[khoih-prog]

This behaviour is intended.

Both SSID/PWD must be valid, you have to input the same SSID/PWD even you need only one set. Never leave anything "blank" or NULL.

The trade-off was made to simplify/shorten the code.

[bizprof]

Ok, that's fine then. Suggest to highlight the first 4 input fields in the CP as mandatory (e.g., by adding an asterisk or a short comment to the input field labels), to make it clearer to the end user. Thanks!

[khoih-prog]

OK. Will do for next release.

[khoih-prog]

Hi Michael,

Testing a combination of 1 valid SSID (regardless of whether it is entered in first or 2nd place in the CP) and the other SSID left blank does not seem to work properly.

The enhancement has been made and new WiFiManager_NINA_Lite v1.2.0 published.

Please check To use and input only one set of WiFi SSID and PWD and verify.

Looking forward to receiving more bug reports and enhancement requests from you.

Thanks and Regards,

KH

---

Release v1.2.0

1. Permit optionally inputting one set of WiFi SSID/PWD by using REQUIRE_ONE_SET_SSID_PW == true
2. Enforce WiFi Password minimum length of 8 chars
3. Enhance MultiWiFi connection logic

[bizprof]

Hi Khoi, this is great, appreciate the quick turnaround! Have done some tests, and it works well for my scenario. More along the lines of an enhancement request (I don't want to publish it as such, though, before I hear your feedback): From looking at your code, I think you are halfway there already to make the number of SSID/Pwd inputs more generic. You got NUM_WIFI_CREDENTIALS, which is currently set to 2, but could be 1 (which would take care of the single SSID scenario), or potentially even bigger numbers. And you have already programmed the dynamic parameters, so combining these approaches, it might make a huge difference in usability of the CP for new users, when only as many paramters are shown as need to be configured. Looking forward to your feedback. Thanks! Michael

[khoih-prog]

I think you are halfway there already to make the number of SSID/Pwd inputs more generic. You got NUM_WIFI_CREDENTIALS, which is currently set to 2, but could be 1

Glad that you see what I intended (NUM_WIFI_CREDENTIALS) for long future. I still think 2 is the best number (to provide backup and reliability). More than 2 is unrealistic, even not complicated to handle (much simpler than dynamic params).

Not many users are as sophisticated as you are They treat the library as fast food and are not willing to contribute and give back. I always got complaint about too many configurable parameters, and the code requires user to spend some time to read README and understand how to configure.

Just a few:

but this thing has gotten so complicated with the number of configuration variables, etc that it's hard to see the forest for the trees.

• Can I use this? #44
• Documentation lacking minimal example #25

But I'll leave it for future expansion, if reality requires. Thanks.

[bizprof]

Hi Khoi,

Thanks for the feedback. I agree, it's hard to balance broad functionality with ease of use. Pretty much every company that makes standard software (MS, Oracle, SAP, and even many Open Source projects) have run into this problem. The usual approach is to make it look simple enough at first glance, by hiding the more advanced settings. For specific requirements, there is a ton of parameters to tweak (just think of how many entries there are in the Windows registry!).

I like your fast food metaphor, that sums it up very well. I'm not good at coding, but I'm trying to contribute in a different way. Can't expect something for nothing all the time. You must have been working way past midnight your time yesterday to get the new release out. People take it for granted, which is a very short-sighted position!

Please don't despair. I have chosen this WiFiManager for a reason, and I hope many others will, too. You are tackling the problem of cross-platform software compatibility/portability, which was the steepest learning curve for me as an Arduino newbie. Took me a while to understand why the programs I wrote for one board could not be made to work on a different board. Your more generic development approach is certainly the right way forward!

All the best, Michael

[khoih-prog]

Thanks for your understanding. It's the passion, the knowledge that my works can somehow helps someone (as I had been helped) and the user's appreciation keep me going, thru many long days and nights.

Hopefully, my retired days are still long enough to permit me to keep going.

Best Regards,

[bizprof]

Hi Khoi,

Thought I'd suggest some code changes which don't mess with the other functionality, but in the end (and, under the hood, so to say) get to a flexible number of WiFi credentials, determined by NUM_WIFI_CREDENTIALS. My first suggestion (probably the easiest one), starts on code line 818 in WiFiManager_NINA_Lite_SAMD.h, where the display of WiFi credentials could be made dynamic:

    void displayConfigData(WiFiNINA_Configuration configData)
    {
      WN_LOGERROR1(F("Hdr="), configData.header);
	  
	  for (uint16_t i = 0; i < NUM_WIFI_CREDENTIALS; i++)
	  { 	
		WN_LOGERROR5(F("Credentials Set #"), i, F(": SSID="), configData.WiFi_Creds[i].wifi_ssid, F(", PW="),  configData.WiFi_Creds[i].wifi_pw);
	  }
      WN_LOGERROR1(F("BName="), configData.board_name);

The log would now looks like this:

Starting SAMD_WiFiNINA on SAMD NANO_33_IOT
WiFiManager_NINA_Lite v1.2.0
[WN] Hostname=SAMD-WIFININA-51F485
Flag read = 0xd0d04321
No doubleResetDetected
SetFlag write = 0xd0d01234
[WN] CCSum=0xf9f,RCSum=0xf9f
[WN] ======= Start Stored Config Data =======
[WN] Hdr=WIFININA
[WN] Credentials Set #0: SSID=xxxx, PW=xxxx
[WN] Credentials Set #1: SSID=blank, PW=blank
[WN] BName=blank
[WN] ConMultiWifi
[WN] First connection, Using index=0

Let me know what you think, just trying to help...

Regards, Michael

[bizprof]

And around line 1064:

//////////////////////////////////////////////

void NULLTerminateConfig()
{
  //#define HEADER_MAX_LEN      16
  //#define SERVER_MAX_LEN      32
  //#define TOKEN_MAX_LEN       36
  
  // NULL Terminating to be sure
  WIFININA_config.header[HEADER_MAX_LEN - 1] = 0;
  for (uint16_t i = 0; i < NUM_WIFI_CREDENTIALS; i++)
  { 	
	WIFININA_config.WiFi_Creds[i].wifi_ssid[SSID_MAX_LEN - 1] = 0;
	WIFININA_config.WiFi_Creds[i].wifi_pw  [PASS_MAX_LEN - 1] = 0;
  }
  WIFININA_config.board_name[BOARD_NAME_MAX_LEN - 1]  = 0;
}
        
//////////////////////////////////////////////

[bizprof]

Around line 1258:

#endif
          for (uint16_t i = 0; i < NUM_WIFI_CREDENTIALS; i++)
		  { 		
			strcpy(WIFININA_config.WiFi_Creds[i].wifi_ssid,   WM_NO_CONFIG);
			strcpy(WIFININA_config.WiFi_Creds[i].wifi_pw,     WM_NO_CONFIG);
		  }    
          strcpy(WIFININA_config.board_name, WM_NO_CONFIG);
          
#if USE_DYNAMIC_PARAMETERS

[khoih-prog]

Hi @bizprof

Again thanks much for your willingness to help here and I'm impressed with your suggestion. You'll see this knowledge gaining by yourself, via direct experience, by reading and understanding the code, will follow you the whole life. This is contrary to what I meant by fast food metaphor.

Glad that you see what I intended (NUM_WIFI_CREDENTIALS) for long future. I still think 2 is the best number (to provide backup and reliability). More than 2 is unrealistic, even not complicated to handle (much simpler than dynamic params).

As I said, it's easy for me to make this NUM_WIFI_CREDENTIALS dynamic, especially back to 1 as the old day. Or even several more. The other unknown dynamic params are much harder to implement.

I certainly will have a look later to add optional NUM_WIFI_CREDENTIALS = 1, if you insist this is helpful for certain use-case, even I still prefer 2 as a way to push and make users familiar with more reliable usage. Sometimes good habits must be forced onto you to change bad ones.

But just continue with your experiment of code changing. It'll definitely be helpful to you as well as me. Hopefully some day I pass to you to co-maintain this library.

[bizprof]

Thanks, Khoi! Trying my best not to mess up your code. One little change at a time. There are 4 more code sections in WiFiManager_NINA_Lite_SAMD.h (I assume the files for the other platforms are similar?) that would need to be adapted. As they are quite dependend on each other, will tackle this tomorrow, perhaps. The good thing is, you might get rid of the just introduced REQUIRE_ONE_SET_SSID_PW parameter - it would be covered by the NUM_WIFI_CREDENTIALS = 1 setting :-)
Totally agree on your position re "make NUM_WIFI_CREDENTIALS dynamic", this probably requires round trips back to the CP server to display/hide additional sets of WiFi credentials.

[bizprof]

Hi Khoi, quick question in regards to the code in handleRequest. I see that you used the standard strcpy and strncpy functions, which in turn forced you to handle low-level string operations, such as terminating parameters with "0", if they are shorter than the allocated variable. Would you be open to using the strlcpy function instead, which does that by default? Please let me know before I spend time modifying it. Thanks!

[khoih-prog]

Hi Michael,

Thanks a lot to identify many possible improvements which I'm sure too many to count 👍 .

The standard C functions that can be used to avoid buffer overflow, strncpy and strncat, have serious design flaws that make them difficult and unnecessarily slow to use correctly. strlcpy and strlcat are designed so that correct usage is as simple as possible.

Using strcpy and strncpy is my bad old-and-die-hard habit 👎 to do it the hard way. Using strlcpy is much better and safer.
Please check if the code size and library support is OK across platforms. Then retest to be sure. Appreciate your help.

I'll certainly retest and include in the code after you finalize via PR (Pull-Request).

[khoih-prog]

Hi Michael,

As we're having many discussions which can be personal, lengthy and contains too many details and sometimes erroneous. I think they are suitable to put in a private repo. I'm opening General_Discussion and invite you (and several other contributing people) in where we can freely discuss many improvements to many other libraries, including this one.

[bizprof]

That's a great idea! I'm not too familiar with GitHub yet, just wanted to avoid opening up a new Issue all the time. I'll try the PR route, too, please bear with me, as I have never done that before :-)

[khoih-prog]

That's where I'll show you steps-be-steps to familiarize with GitHub, PR, etc.

[khoih-prog]

Already sent you the invitation to the private repo. Pls accept, then you will have access.