A new approach for erasing WiFi Settings #8828

mhightower83 · 2023-01-24T15:08:24Z

Add hardware reset function - simulates EXT_RST via accelerated HWDT.

Add reset selection to ESP.eraseConfig() for calling hardware reset after erasing the WiFi Settings.

Update ArduinoOTA to use ESP.eraseConfig(true)

Externalized a function to Stop WiFi, Erase WiFi Settings, and Reset, ArduinoOTA.eraseConfigAndReset().

Add OTA examples to illustrate using changes.

Add support for hardware reset function call - simulates EXT_RST via HWDT. Add reset selection to `ESP.eraseConfig()` for calling hardware reset after erasing the WiFi Settings. Update ArduinoOTA to use `ESP.eraseConfig(true)` Externalized `ArduinoOTA.eraseConfigAndReset()` Add OTA examples to illustrate using erase config changes.

fixed confused example

…o into pr-eraseconfig-reset

mhightower83 · 2023-02-08T22:29:30Z

libraries/ArduinoOTA/ArduinoOTA.cpp

+        eraseConfigAndReset();  // returns on failure
+        //C What is the best action to take on failure?
+        //C  1) On failure, we could invalidate eboot_command buffer -
+        //C     aborting the flash update.
+        //C  2) Just ignore it and restart.
+        //C  3) Retry forever
+        if (_error_callback) {
+          _error_callback(OTA_ERASE_SETTINGS_ERROR);
+        }


At failure, there are no good choices.
I picked 3 to retry forever.
If they power cycle, the download is aborted.

Update:
I think an EXT_RST would result in the flash update completing. RTC should be preserved.

Why a separate condition & callback though? We can retrieve error id

I don't understand your reference. I am using the existing error callback with a new error number to identify the situation.

(review UI sent a draft :/)

Why even mention it? Since we want this to be done, probably unroll is a better option than leave things in an indeterminate state?
Another part - it is the only state that continues execution. We still have eboot part like you said, even after ::end()ing the instance (suppose that from user pov it is an option to recover).

Arduino/libraries/ArduinoOTA/ArduinoOTA.cpp

Lines 328 to 346 in 0e1059d

delay(100);

if (OTA_ERASE_CFG_NO != _eraseConfig) {

eraseConfigAndReset(); // returns on failure

if (_error_callback) {

_error_callback(OTA_ERASE_SETTINGS_ERROR);

}

if (OTA_ERASE_CFG_ABORT_ON_ERROR == _eraseConfig) {

eboot_command_clear();

return;

}

#ifdef OTA_DEBUG

else if (OTA_ERASE_CFG_IGNORE_ERROR == _eraseConfig) {

// Fallthrough and restart

} else {

panic();

}

#endif

}

ESP.restart();

When enabling erase WiFi settings, the user does so by selecting the fail option ignore or abort. Does that satisfy your concerns?

mcspr

Can you expand why we want sys reset happen that way vs. __builtin_trap() or similar? (which iirc works in this case. or, why it is not appropriate to trap vs. using this handler)
Do we want trick SDK into thinking we pressed RST?

mcspr · 2023-02-08T23:35:43Z

libraries/ArduinoOTA/ArduinoOTA.cpp

+        eraseConfigAndReset();  // returns on failure
+        //C What is the best action to take on failure?
+        //C  1) On failure, we could invalidate eboot_command buffer -
+        //C     aborting the flash update.
+        //C  2) Just ignore it and restart.
+        //C  3) Retry forever
+        if (_error_callback) {
+          _error_callback(OTA_ERASE_SETTINGS_ERROR);
+        }


Why a separate condition & callback though? We can retrieve error id

mcspr · 2023-02-08T23:39:05Z

And... why not in Updater? Don't we want this feature across all updater users?

mhightower83 · 2023-02-09T02:24:21Z

And... why not in Updater? Don't we want this feature across all updater users?

If I followed this right, the updater action has to be driven from the library side. Where in the updater are you thinking this can be placed?

Can you expand why we want sys reset happen that way vs. __builtin_trap() or similar? (which iirc works in this case. or, why it is not appropriate to trap vs. using this handler) Do we want trick SDK into thinking we pressed RST?

Yes. I didn't want to give the SDK the opportunity to do the wrong thing. I am trying to create a situation where the SDK will have no reason and minimum opportunity to write to the freshly erase data area. (Which it rightfully thinks it owns.) We are possibly going to load a different SDK. I didn't want to go down the path of a graceful shutdown and reboot with the SDK in full control. I know it is very heavy-handed to force a reboot this way. I felt it was the safer path.

Until I found a method to force a quick hardware reset through software, I rejected this approach. Without it, we would need to force a regular HWDT.

__builtin_trap() compiles to a break 1, 15. The NON-OS SDK's default breakpoint handler is to loop on a waiti 2 until the HWDT kinks in after 8 seconds. I guess this is an option (or a simple while(true);) if the 8-second delay is not an issue; it would save a little code space.

Let me know if you prefer the 8-second HWDT method.

Removed continuous retry of "eraseConfig" and allow the script to assign an error handling option for "eraseConfig" failure. Update example to use error handling option.

mcspr · 2023-02-09T18:50:00Z

I was thinking of something like existing setAsync or installSignature, just another Updater method to set a boolean flag

Update.setEraseConfigBeforeReboot(true);`

Explicit code is probably better then, lets keep the hardware_reboot as-is

mhightower83 · 2023-02-09T21:16:22Z

There is an odd behavior with eboot copy flash and load when an HWDT is involved in any way.
eboot plays with disable/enable WDT in the copy flash function. This leaves the WDT running when it was originally off. After an HWDT the WDT does not appear to be configured; however, a soft restart has it configured.
What I see is that there is an extra HWDT event after the flash copy has finished. After the extra HWDT event, the WD is off again and the flash can load without obstruction.

~~I need to verify more. I think the WDT is always off at the start of eboot~~

Update:

After Power Enable Pin, EXT_RST, or HWDT events, at "main()" in eboot, WDT is disabled. Key WDT hardware registers are zero.

After ESP.restart(), at main() in eboot, WDT is enabled.

In eboot, ets_wdt_enable was defined as void ets_wdt_enable(void). It should have three arguments. These are used to set up the WDT.

After the OTA update to flash, when the reboot was started by: Power Enable Pin, EXT_RST, or HWDT, the bad argument list - caused a fast HWDT. With the flash update complete on the reboot, WDT is disabled, and the image loads.

mhightower83 · 2023-02-10T16:59:24Z

I was thinking of something like existing setAsync or installSignature, just another Updater method to set a boolean flag
Update.setEraseConfigBeforeReboot(true);`

I started to do something like that. I had trouble putting stuff in UpdaterClass that needed to call library methods.
To complete an Update.setEraseConfigBeforeReboot(true), I would need to call ~~two~~ library functions: WiFi.mode(WIFI_OFF) ~~and ESP.eraseConfig(true)~~ from the UpdaterClass.

Correction: ESP.eraseConfig(true) is in core/esp8266 directory so it should be accessible from UpdaterClass.

mcspr · 2023-02-12T18:26:13Z

Out of scope here, but I see #7979 as a possible solution to that
(it was neglected for some time, though, but overall idea of moving sleep modes into ESP should be merged at some point)

3.1.2 is slightly stretched, so holding for a bit.

mcspr

Pending FPM change, going to rewrite into Updater part some time later
lgtm otherwise

libraries/ArduinoOTA/ArduinoOTA.h

cores/esp8266/Esp.h

cores/esp8266/hardware_reset.h

Avoid using "[[noreturn]]" - not accepted for a .c file function Updated to use __attribute__((noreturn)) to handle both .cpp and .c file functions.

mhightower83 added 2 commits January 24, 2023 07:03

style

1a03a41

fixed confused example

mcspr mentioned this pull request Jan 25, 2023

Update from 3.1.0 to 3.1.1 initially behaved as failed then after some minutes started normaly #8808

Closed

5 tasks

mhightower83 added 2 commits January 25, 2023 10:01

improve wording

d061a3c

Add new state to retry eraseConfigAndReset

8375000

d-a-v added the alpha included in alpha release label Feb 8, 2023

mhightower83 added 2 commits February 8, 2023 13:45

Merge branch 'master' into pr-eraseconfig-reset

b1e772c

Merge branch 'pr-eraseconfig-reset' of github.com:mhightower83/Arduin…

dd78bc0

…o into pr-eraseconfig-reset

mhightower83 marked this pull request as ready for review February 8, 2023 22:24

mhightower83 commented Feb 8, 2023

View reviewed changes

mcspr reviewed Feb 8, 2023

View reviewed changes

Removed unreachable error test from examples.

9ccd11c

Removed continuous retry of "eraseConfig" and allow the script to assign an error handling option for "eraseConfig" failure. Update example to use error handling option.

In eboot for function ets_wdt_enable() added missing arguments

feb8a99

Update comments and example

eb7aa17

mhightower83 added 2 commits March 27, 2023 20:18

Merge branch 'master' into pr-eraseconfig-reset

5f7adcf

Wording

c7a3ed8

d-a-v added the merge-conflict PR has a merge conflict that needs manual correction label Mar 30, 2023

mhightower83 added 2 commits March 30, 2023 11:04

Merge branch 'master' into pr-eraseconfig-reset

cd5a852

Rebuilt eboot.elf with current tools from ./get.py

892e6a2

d-a-v removed the merge-conflict PR has a merge conflict that needs manual correction label Mar 30, 2023

mcspr reviewed Mar 31, 2023

View reviewed changes

libraries/ArduinoOTA/ArduinoOTA.h Outdated Show resolved Hide resolved

cores/esp8266/Esp.h Outdated Show resolved Hide resolved

mhightower83 added 3 commits April 16, 2023 08:10

Merge branch 'master' into pr-eraseconfig-reset

c952ffc

Requested changes.

f8a8251

Merge branch 'master' into pr-eraseconfig-reset

40fba0a

mhightower83 added 3 commits July 21, 2023 13:16

Merge branch 'master' into pr-eraseconfig-reset

f3a492e

cleanup comments

0e1059d

Merge branch 'master' into pr-eraseconfig-reset

721e6d3

mcspr approved these changes Jul 27, 2023

View reviewed changes

cores/esp8266/hardware_reset.h Outdated Show resolved Hide resolved

mhightower83 and others added 2 commits July 27, 2023 18:44

Update hardware_reset

696b96b

Avoid using "[[noreturn]]" - not accepted for a .c file function Updated to use __attribute__((noreturn)) to handle both .cpp and .c file functions.

Merge branch 'master' into pr-eraseconfig-reset

ef0c1e2

d-a-v merged commit a348833 into esp8266:master Aug 29, 2023
56 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A new approach for erasing WiFi Settings #8828

A new approach for erasing WiFi Settings #8828

mhightower83 commented Jan 24, 2023 •

edited

Loading

mhightower83 Feb 8, 2023 •

edited

Loading

mcspr Feb 8, 2023

mhightower83 Feb 9, 2023

mcspr Feb 9, 2023

mhightower83 Jul 25, 2023

mcspr left a comment

mcspr Feb 8, 2023

mcspr commented Feb 8, 2023

mhightower83 commented Feb 9, 2023

mcspr commented Feb 9, 2023

mhightower83 commented Feb 9, 2023 •

edited

Loading

mhightower83 commented Feb 10, 2023 •

edited

Loading

mcspr commented Feb 12, 2023

mcspr left a comment •

edited

Loading

	delay(100);
	if (OTA_ERASE_CFG_NO != _eraseConfig) {
	eraseConfigAndReset(); // returns on failure
	if (_error_callback) {
	_error_callback(OTA_ERASE_SETTINGS_ERROR);
	}
	if (OTA_ERASE_CFG_ABORT_ON_ERROR == _eraseConfig) {
	eboot_command_clear();
	return;
	}
	#ifdef OTA_DEBUG
	else if (OTA_ERASE_CFG_IGNORE_ERROR == _eraseConfig) {
	// Fallthrough and restart
	} else {
	panic();
	}
	#endif
	}
	ESP.restart();

A new approach for erasing WiFi Settings #8828

A new approach for erasing WiFi Settings #8828

Conversation

mhightower83 commented Jan 24, 2023 • edited Loading

mhightower83 Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

mcspr Feb 8, 2023

Choose a reason for hiding this comment

mhightower83 Feb 9, 2023

Choose a reason for hiding this comment

mcspr Feb 9, 2023

Choose a reason for hiding this comment

mhightower83 Jul 25, 2023

Choose a reason for hiding this comment

mcspr left a comment

Choose a reason for hiding this comment

mcspr Feb 8, 2023

Choose a reason for hiding this comment

mcspr commented Feb 8, 2023

mhightower83 commented Feb 9, 2023

mcspr commented Feb 9, 2023

mhightower83 commented Feb 9, 2023 • edited Loading

mhightower83 commented Feb 10, 2023 • edited Loading

mcspr commented Feb 12, 2023

mcspr left a comment • edited Loading

Choose a reason for hiding this comment

mhightower83 commented Jan 24, 2023 •

edited

Loading

mhightower83 Feb 8, 2023 •

edited

Loading

mhightower83 commented Feb 9, 2023 •

edited

Loading

mhightower83 commented Feb 10, 2023 •

edited

Loading

mcspr left a comment •

edited

Loading