Update crash handler (#1796)

* Update crash handler

- limit recorder stack size by 128 bytes
- offset settings a bit more
- add possibility to disable crash crash recoder completely

* nicer setting names, comment layout change

code/espurna/crash.ino

@@ -29,7 +29,8 @@ extern "C" {
  *  8. depc
  *  9. adress of stack start
  * 10. adress of stack end
- * 11. stack trace bytes
+ * 11. stack trace size
+ * 12. stack trace bytes
  *     ...
  */
 #define SAVE_CRASH_CRASH_TIME       0x00  // 4 bytes
@@ -42,12 +43,19 @@ extern "C" {
 #define SAVE_CRASH_DEPC             0x16  // 4 bytes
 #define SAVE_CRASH_STACK_START      0x1A  // 4 bytes
 #define SAVE_CRASH_STACK_END        0x1E  // 4 bytes
-#define SAVE_CRASH_STACK_TRACE      0x22  // variable
+#define SAVE_CRASH_STACK_SIZE       0x22  // 2 bytes
+#define SAVE_CRASH_STACK_TRACE      0x24  // variable
+
+#define SAVE_CRASH_STACK_TRACE_MAX  0x80  // limit at 128 bytes (increment/decrement by 16)
+
+uint16_t _save_crash_stack_trace_max = SAVE_CRASH_STACK_TRACE_MAX;
+uint16_t _save_crash_enabled = true;
 
 /**
  * Save crash information in EEPROM
  * This function is called automatically if ESP8266 suffers an exception
  * It should be kept quick / consise to be able to execute before hardware wdt may kick in
+ * This method assumes EEPROM has already been initialized, which is the first thing ESPurna does
  */
 extern "C" void custom_crash_callback(struct rst_info * rst_info, uint32_t stack_start, uint32_t stack_end ) {
 
@@ -56,37 +64,44 @@ extern "C" void custom_crash_callback(struct rst_info * rst_info, uint32_t stack
         return;
     }
 
-    // This method assumes EEPROM has already been initialized
-    // which is the first thing ESPurna does
+    // Check if runtime setting disabled this callback
+    if (!_save_crash_enabled) {
+        return;
+    }
 
-    // write crash time to EEPROM
+    // write crash time to EEPROM, which we will later use as a marker that there was a crash
     uint32_t crash_time = millis();
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_CRASH_TIME, crash_time);
 
-    // write reset info to EEPROM
+    // rst_info::reason and ::exccause are uint32_t, but are holding small values
     EEPROMr.write(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_RESTART_REASON, rst_info->reason);
     EEPROMr.write(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EXCEPTION_CAUSE, rst_info->exccause);
 
-    // write epc1, epc2, epc3, excvaddr and depc to EEPROM
+    // write epc1, epc2, epc3, excvaddr and depc to EEPROM as uint32_t
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EPC1, rst_info->epc1);
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EPC2, rst_info->epc2);
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EPC3, rst_info->epc3);
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_EXCVADDR, rst_info->excvaddr);
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_DEPC, rst_info->depc);
 
-    // write stack start and end address to EEPROM
+    // EEPROM size is limited, write as little as possible.
+    // we sometimes want to avoid big stack traces, e.g. if stack_end == 0x3fffffb0, we are in SYS context.
+    // but still should get enough relevant info and it is possible to set needed size at build/runtime
+    const uint16_t stack_size = constrain((stack_end - stack_start), 0, _save_crash_stack_trace_max);
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_START, stack_start);
     EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_END, stack_end);
+    EEPROMr.put(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_SIZE, stack_size);
 
-    // starting address of Embedis data plus reserve
-    const uint16_t settings_start = SPI_FLASH_SEC_SIZE - settingsSize() - 0x10;
+    // starting EEPROM address of Embedis data plus reserve
+    const uint16_t settings_start = (
+        ((SPI_FLASH_SEC_SIZE - settingsSize() + 31) & -32) - 0x20);
 
     // 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);
+    int16_t eeprom_addr = SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_TRACE;
+    for (uint32_t* addr = (uint32_t*)stack_start; addr < (uint32_t*)(stack_start + stack_size); addr++) {
+        if (eeprom_addr >= settings_start) break;
+        EEPROMr.put(eeprom_addr, *addr);
+        eeprom_addr += sizeof(uint32_t);
     }
 
     EEPROMr.commit();
@@ -129,19 +144,22 @@ void crashDump() {
     DEBUG_MSG_P(PSTR("[DEBUG] excvaddr=0x%08x depc=0x%08x\n"), excvaddr, depc);
 
     uint32_t stack_start, stack_end;
+    uint16_t stack_size;
+
     EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_START, stack_start);
     EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_END, stack_end);
+    EEPROMr.get(SAVE_CRASH_EEPROM_OFFSET + SAVE_CRASH_STACK_SIZE, stack_size);
 
-    DEBUG_MSG_P(PSTR("[DEBUG] sp=0x%08x end=0x%08x\n"), stack_start, stack_end);
+    DEBUG_MSG_P(PSTR("sp=0x%08x end=0x%08x saved=0x%04x\n\n"), stack_start, stack_end, stack_size);
+    if (0xFFFF == stack_size) return;
 
     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) {
+    for (int16_t i = 0; i < stack_size; i += 0x10) {
         DEBUG_MSG_P(PSTR("%08x: "), stack_start + i);
         for (byte j = 0; j < 4; j++) {
             EEPROMr.get(current_address, stack_trace);
@@ -154,4 +172,23 @@ void crashDump() {
 
 }
 
+void crashSetup() {
+
+    #if TERMINAL_SUPPORT
+        terminalRegisterCommand(F("CRASH"), [](Embedis* e) {
+            crashDump();
+            crashClear();
+            terminalOK();
+        });
+    #endif
+
+    // Minumum of 16 and align for column formatter in crashDump()
+    _save_crash_stack_trace_max = getSetting("sysTraceMax", SAVE_CRASH_STACK_TRACE_MAX).toInt();
+    _save_crash_stack_trace_max = (_save_crash_stack_trace_max + 15) & -16;
+    setSetting("sysScTraceMax", _save_crash_stack_trace_max);
+
+    _save_crash_enabled = getSetting("sysCrashSave", 1).toInt() == 1;
+
+}
+
 #endif // DEBUG_SUPPORT

code/espurna/espurna.ino

@@ -83,9 +83,13 @@ void setup() {
     // Init persistance
     settingsSetup();
 
+    // Init crash recorder
+    crashSetup();
+
     // Return bogus free heap value for broken devices
     // XXX: device is likely to trigger other bugs! tread carefuly
     wtfHeap(getSetting("wtfHeap", 0).toInt());
+
     // Init Serial, SPIFFS and system check
     systemSetup();
 

code/espurna/system.ino

@@ -164,7 +164,9 @@ void _systemSetupHeartbeat() {
 
 #if WEB_SUPPORT
     bool _systemWebSocketOnReceive(const char * key, JsonVariant& value) {
-        return (strncmp(key, "hb", 2) == 0);
+        if (strncmp(key, "sys", 3) == 0) return true;
+        if (strncmp(key, "hb", 2) == 0) return true;
+        return false;
     }
 #endif
 

code/espurna/terminal.ino

@@ -81,14 +81,6 @@ void _terminalKeysCommand() {
 
 void _terminalInitCommand() {
 
-    #if DEBUG_SUPPORT
-        terminalRegisterCommand(F("CRASH"), [](Embedis* e) {
-            crashDump();
-            crashClear();
-            terminalOK();
-        });
-    #endif
-
     terminalRegisterCommand(F("COMMANDS"), [](Embedis* e) {
         _terminalHelpCommand();
         terminalOK();