Merge pull request #1258 from mcspr/deferred-reset-crash

Deferred reset and crash handler safeguards

code/espurna/config/prototypes.h

@@ -69,6 +69,8 @@ extern "C" {
 #include <EEPROM_Rotate.h>
 EEPROM_Rotate EEPROMr;
 
+void eepromSectorsDebug();
+
 // -----------------------------------------------------------------------------
 // GPIO
 // -----------------------------------------------------------------------------

code/espurna/debug.ino

@@ -209,6 +209,11 @@ void debugSetup() {
  */
 extern "C" void custom_crash_callback(struct rst_info * rst_info, uint32_t stack_start, uint32_t stack_end ) {
 
+    // Do not record crash data when resetting the board
+    if (checkNeedsReset()) {
+        return;
+    }
+
     // This method assumes EEPROM has already been initialized
     // which is the first thing ESPurna does
 
@@ -231,9 +236,13 @@ extern "C" void custom_crash_callback(struct rst_info * rst_info, uint32_t stack
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_START, stack_start);
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_END, stack_end);
 
-    // write stack trace to EEPROM
+    // starting address of Embedis data plus reserve
+    const uint16_t settings_start = SPI_FLASH_SEC_SIZE - settingsSize() - 0x10;
+
+    // write stack trace to EEPROM and avoid overwriting settings
     int16_t current_address = SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_TRACE;
     for (uint32_t i = stack_start; i < stack_end; i++) {
+        if (current_address >= settings_start) break;
         byte* byteValue = (byte*) i;
         EEPROMr.write(current_address++, *byteValue);
     }
@@ -273,16 +282,23 @@ void debugDumpCrashInfo() {
     EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EPC3, epc3);
     EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EXCVADDR, excvaddr);
     EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_DEPC, depc);
+
     DEBUG_MSG_P(PSTR("[DEBUG] epc1=0x%08x epc2=0x%08x epc3=0x%08x\n"), epc1, epc2, epc3);
     DEBUG_MSG_P(PSTR("[DEBUG] excvaddr=0x%08x depc=0x%08x\n"), excvaddr, depc);
 
     uint32_t stack_start, stack_end;
     EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_START, stack_start);
     EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_END, stack_end);
-    DEBUG_MSG_P(PSTR("[DEBUG] >>>stack>>>\n[DEBUG] "));
+
+    DEBUG_MSG_P(PSTR("[DEBUG] sp=0x%08x end=0x%08x\n"), stack_start, stack_end);
+
     int16_t current_address = SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_TRACE;
     int16_t stack_len = stack_end - stack_start;
+
     uint32_t stack_trace;
+
+    DEBUG_MSG_P(PSTR("[DEBUG] >>>stack>>>\n[DEBUG] "));
+
     for (int16_t i = 0; i < stack_len; i += 0x10) {
         DEBUG_MSG_P(PSTR("%08x: "), stack_start + i);
         for (byte j = 0; j < 4; j++) {

code/espurna/eeprom.ino

@@ -8,6 +8,11 @@ EEPROM MODULE
 
 // -----------------------------------------------------------------------------
 
+bool _eeprom_commit = false;
+
+uint32_t _eeprom_commit_count = 0;
+bool _eeprom_last_commit_result = false;
+
 void eepromRotate(bool value) {
     // Enable/disable EEPROM rotation only if we are using more sectors than the
     // reserved by the memory layout
@@ -34,15 +39,44 @@ String eepromSectors() {
     return response;
 }
 
+void eepromSectorsDebug() {
+    DEBUG_MSG_P(PSTR("[MAIN] EEPROM sectors: %s\n"), (char *) eepromSectors().c_str());
+    DEBUG_MSG_P(PSTR("[MAIN] EEPROM current: %lu\n"), eepromCurrent());
+}
+
+bool _eepromCommit() {
+    _eeprom_commit_count++;
+    _eeprom_last_commit_result = EEPROMr.commit();
+    return _eeprom_last_commit_result;
+}
+
+void eepromCommit() {
+    _eeprom_commit = true;
+}
+
 #if TERMINAL_SUPPORT
 
 void _eepromInitCommands() {
 
     settingsRegisterCommand(F("EEPROM"), [](Embedis* e) {
         infoMemory("EEPROM", SPI_FLASH_SEC_SIZE, SPI_FLASH_SEC_SIZE - settingsSize());
+        eepromSectorsDebug();
+        if (_eeprom_commit_count > 0) {
+            DEBUG_MSG_P(PSTR("[MAIN] Commits done: %lu\n"), _eeprom_commit_count);
+            DEBUG_MSG_P(PSTR("[MAIN]  Last result: %s\n"), _eeprom_last_commit_result ? "OK" : "ERROR");
+        }
         DEBUG_MSG_P(PSTR("+OK\n"));
     });
 
+    settingsRegisterCommand(F("EEPROM.COMMIT"), [](Embedis* e) {
+        const bool res = _eepromCommit();
+        if (res) {
+            DEBUG_MSG_P(PSTR("+OK\n"));
+        } else {
+            DEBUG_MSG_P(PSTR("-ERROR\n"));
+        }
+    });
+
     settingsRegisterCommand(F("EEPROM.DUMP"), [](Embedis* e) {
         EEPROMr.dump(settingsSerial());
         DEBUG_MSG_P(PSTR("\n+OK\n"));
@@ -69,6 +103,13 @@ void _eepromInitCommands() {
 
 // -----------------------------------------------------------------------------
 
+void eepromLoop() {
+    if (_eeprom_commit) {
+        _eepromCommit();
+        _eeprom_commit = false;
+    }
+}
+
 void eepromSetup() {
 
     #ifdef EEPROM_ROTATE_SECTORS
@@ -92,4 +133,6 @@ void eepromSetup() {
         _eepromInitCommands();
     #endif
 
+    espurnaRegisterLoop(eepromLoop);
+
 }

code/espurna/mqtt.ino

@@ -291,7 +291,7 @@ unsigned long _mqttNextMessageId() {
         EEPROMr.write(EEPROM_MESSAGE_ID + 1, (id >> 16) & 0xFF);
         EEPROMr.write(EEPROM_MESSAGE_ID + 2, (id >>  8) & 0xFF);
         EEPROMr.write(EEPROM_MESSAGE_ID + 3, (id >>  0) & 0xFF);
-        saveSettings();
+        eepromCommit();
     }
 
     id++;

code/espurna/relay.ino

@@ -418,7 +418,7 @@ void relaySave(bool do_commit) {
 
         // We are actually enqueuing the commit so it will be
         // executed on the main loop, in case this is called from a callback
-        saveSettings();
+        eepromCommit();
 
     }
 
@@ -548,7 +548,7 @@ void _relayBoot() {
     // Save if there is any relay in the RELAY_BOOT_TOGGLE mode
     if (trigger_save) {
         EEPROMr.write(EEPROM_RELAY_STATUS, mask);
-        saveSettings();
+        eepromCommit();
     }
 
     _relayRecursive = false;

code/espurna/settings.ino

@@ -22,8 +22,6 @@ EmbedisWrap embedis(_serial, TERMINAL_BUFFER_SIZE);
 #endif // SERIAL_RX_ENABLED
 #endif // TERMINAL_SUPPORT
 
-bool _settings_save = false;
-
 // -----------------------------------------------------------------------------
 // Reverse engineering EEPROM storage format
 // -----------------------------------------------------------------------------
@@ -189,6 +187,7 @@ void _settingsInitCommands() {
     settingsRegisterCommand(F("ERASE.CONFIG"), [](Embedis* e) {
         DEBUG_MSG_P(PSTR("+OK\n"));
         resetReason(CUSTOM_RESET_TERMINAL);
+        _eepromCommit();
         ESP.eraseConfig();
         *((int*) 0) = 0; // see https://github.com/esp8266/Arduino/issues/1494
     });
@@ -310,7 +309,7 @@ void _settingsInitCommands() {
 
     #if not SETTINGS_AUTOSAVE
         settingsRegisterCommand(F("SAVE"), [](Embedis* e) {
-            _settings_save = true;
+            eepromCommit();
             DEBUG_MSG_P(PSTR("\n+OK\n"));
         });
     #endif
@@ -367,7 +366,7 @@ bool hasSetting(const String& key, unsigned int index) {
 
 void saveSettings() {
     #if not SETTINGS_AUTOSAVE
-        _settings_save = true;
+        eepromCommit();
     #endif
 }
 
@@ -464,7 +463,7 @@ void settingsSetup() {
         [](size_t pos) -> char { return EEPROMr.read(pos); },
         [](size_t pos, char value) { EEPROMr.write(pos, value); },
         #if SETTINGS_AUTOSAVE
-            []() { _settings_save = true; }
+            []() { eepromCommit(); }
         #else
             []() {}
         #endif
@@ -485,12 +484,6 @@ void settingsSetup() {
 
 void settingsLoop() {
 
-    if (_settings_save) {
-        EEPROMr.commit();
-        _settings_save = false;
-    }
-
-
     #if TERMINAL_SUPPORT
 
         #if DEBUG_SERIAL_SUPPORT

code/espurna/system.ino

@@ -42,7 +42,7 @@ void systemCheck(bool stable) {
         }
     }
     EEPROMr.write(EEPROM_CRASH_COUNTER, value);
-    EEPROMr.commit();
+    eepromCommit();
 }
 
 bool systemCheck() {
@@ -77,6 +77,14 @@ unsigned long systemLoadAverage() {
 
 void systemLoop() {
 
+    // -------------------------------------------------------------------------
+    // User requested reset
+    // -------------------------------------------------------------------------
+
+    if (checkNeedsReset()) {
+        reset();
+    }
+
     // -------------------------------------------------------------------------
     // Check system stability
     // -------------------------------------------------------------------------

code/espurna/utils.ino

@@ -9,6 +9,8 @@ Copyright (C) 2017-2018 by Xose Pérez <xose dot perez at gmail dot com>
 #include <Ticker.h>
 Ticker _defer_reset;
 
+uint8_t _reset_reason = 0;
+
 String getIdentifier() {
     char buffer[20];
     snprintf_P(buffer, sizeof(buffer), PSTR("%s-%06X"), APP_NAME, ESP.getChipId());
@@ -347,8 +349,7 @@ void info() {
 
     // -------------------------------------------------------------------------
 
-    DEBUG_MSG_P(PSTR("[MAIN] EEPROM sectors: %s\n"), (char *) eepromSectors().c_str());
-    DEBUG_MSG_P(PSTR("[MAIN] EEPROM current: %lu\n"), eepromCurrent());
+    eepromSectorsDebug();
     DEBUG_MSG_P(PSTR("\n"));
 
     // -------------------------------------------------------------------------
@@ -464,8 +465,9 @@ unsigned char resetReason() {
 }
 
 void resetReason(unsigned char reason) {
+    _reset_reason = reason;
     EEPROMr.write(EEPROM_CUSTOM_RESET, reason);
-    EEPROMr.commit();
+    eepromCommit();
 }
 
 void reset() {
@@ -473,8 +475,11 @@ void reset() {
 }
 
 void deferredReset(unsigned long delay, unsigned char reason) {
-    resetReason(reason);
-    _defer_reset.once_ms(delay, reset);
+    _defer_reset.once_ms(delay, resetReason, reason);
+}
+
+bool checkNeedsReset() {
+    return _reset_reason > 0;
 }
 
 // -----------------------------------------------------------------------------