Fix flash write error handling; clean up safe mode message printing

dhalbert · dhalbert · commit 7889b999ccb3 · 2019-12-12T14:41:49.000-05:00
diff --git a/main.c b/main.c
@@ -264,9 +264,7 @@ bool run_code_py(safe_mode_t safe_mode) {
     rgb_status_animation_t animation;
     prep_rgb_status_animation(&result, found_main, safe_mode, &animation);
     while (true) {
-        #ifdef MICROPY_VM_HOOK_LOOP
-            MICROPY_VM_HOOK_LOOP
-        #endif
+        RUN_BACKGROUND_TASKS;
         if (reload_requested) {
             reload_requested = false;
             return true;
diff --git a/ports/nrf/common-hal/nvm/ByteArray.c b/ports/nrf/common-hal/nvm/ByteArray.c
@@ -24,6 +24,7 @@
  * THE SOFTWARE.
  */
 
+#include "py/runtime.h"
 #include "common-hal/nvm/ByteArray.h"
 
 #include <stdio.h>
@@ -43,16 +44,20 @@ uint32_t common_hal_nvm_bytearray_get_length(nvm_bytearray_obj_t *self) {
 }
 
 static void write_page(uint32_t page_addr, uint32_t offset, uint32_t len, uint8_t *bytes) {
-    // Write a whole page to flash, buffering it first and then erasing and rewriting 
+    // Write a whole page to flash, buffering it first and then erasing and rewriting
     // it since we can only clear a whole page at a time.
 
+    bool status;
     if (offset == 0 && len == FLASH_PAGE_SIZE) {
-        nrf_nvm_safe_flash_page_write(page_addr, bytes);
+        status = nrf_nvm_safe_flash_page_write(page_addr, bytes);
     } else {
         uint8_t buffer[FLASH_PAGE_SIZE];
         memcpy(buffer, (uint8_t *)page_addr, FLASH_PAGE_SIZE);
         memcpy(buffer + offset, bytes, len);
-        nrf_nvm_safe_flash_page_write(page_addr, buffer);
+        status = nrf_nvm_safe_flash_page_write(page_addr, buffer);
+    }
+    if (!status) {
+        mp_raise_OSError_msg(translate("Flash write failed"));
     }
 }
 
diff --git a/ports/nrf/peripherals/nrf/nvm.c b/ports/nrf/peripherals/nrf/nvm.c
@@ -50,7 +50,7 @@ STATIC sd_flash_operation_status_t sd_flash_operation_wait_until_done(void) {
 }
 #endif
 
-void nrf_nvm_safe_flash_page_write(uint32_t page_addr, uint8_t *data) {
+bool nrf_nvm_safe_flash_page_write(uint32_t page_addr, uint8_t *data) {
     #ifdef BLUETOOTH_SD
         uint8_t sd_en = 0;
         (void) sd_softdevice_is_enabled(&sd_en);
@@ -61,11 +61,11 @@ void nrf_nvm_safe_flash_page_write(uint32_t page_addr, uint8_t *data) {
             sd_flash_operation_start();
             err_code = sd_flash_page_erase(page_addr / FLASH_PAGE_SIZE);
             if (err_code != NRF_SUCCESS) {
-                mp_raise_OSError_msg_varg(translate("Flash erase failed to start, err 0x%04x"), err_code);
+                return false;
             }
             status = sd_flash_operation_wait_until_done();
             if (status == SD_FLASH_OPERATION_ERROR) {
-                mp_raise_OSError_msg(translate("Flash erase failed"));
+                return false;
             }
 
             // Divide a full page into parts, because writing a full page causes an assertion failure.
@@ -78,18 +78,19 @@ void nrf_nvm_safe_flash_page_write(uint32_t page_addr, uint8_t *data) {
                                           (uint32_t *)data + i * words_to_write,
                                           words_to_write);
                 if (err_code != NRF_SUCCESS) {
-                    mp_raise_OSError_msg_varg(translate("Flash write failed to start, err 0x%04x"), err_code);
+                    return false;
                 }
                 status = sd_flash_operation_wait_until_done();
                 if (status == SD_FLASH_OPERATION_ERROR) {
-                    mp_raise_OSError_msg(translate("Flash write failed"));
+                    return false;
                 }
             }
 
-            return;
+            return true;
         }
     #endif
 
     nrf_nvmc_page_erase(page_addr);
     nrf_nvmc_write_bytes(page_addr, data, FLASH_PAGE_SIZE);
-}    
+    return true;
+}
diff --git a/ports/nrf/peripherals/nrf/nvm.h b/ports/nrf/peripherals/nrf/nvm.h
@@ -31,4 +31,4 @@
 #define CIRCUITPY_INTERNAL_NVM_SIZE (0)
 #endif
 
-void nrf_nvm_safe_flash_page_write(uint32_t page_addr, uint8_t *data);
+bool nrf_nvm_safe_flash_page_write(uint32_t page_addr, uint8_t *data);
diff --git a/ports/nrf/supervisor/internal_flash.c b/ports/nrf/supervisor/internal_flash.c
@@ -34,6 +34,7 @@
 #include "py/obj.h"
 #include "py/runtime.h"
 #include "lib/oofatfs/ff.h"
+#include "supervisor/shared/safe_mode.h"
 
 #include "peripherals/nrf/nvm.h"
 
@@ -75,7 +76,9 @@ void supervisor_flash_flush(void) {
 
     // Skip if data is the same
     if (memcmp(_flash_cache, (void *)_flash_page_addr, FLASH_PAGE_SIZE) != 0) {
-        nrf_nvm_safe_flash_page_write(_flash_page_addr, _flash_cache);
+        if (!nrf_nvm_safe_flash_page_write(_flash_page_addr, _flash_cache)) {
+            reset_into_safe_mode(FLASH_WRITE_FAIL);
+        }
     }
 }
 
@@ -120,4 +123,3 @@ mp_uint_t supervisor_flash_write_blocks(const uint8_t *src, uint32_t lba, uint32
 
 void supervisor_flash_release_cache(void) {
 }
-
diff --git a/supervisor/shared/safe_mode.c b/supervisor/shared/safe_mode.c
@@ -94,44 +94,72 @@ void __attribute__((noinline,)) reset_into_safe_mode(safe_mode_t reason) {
     reset_cpu();
 }
 
+
+
+#define FILE_AN_ISSUE translate("\r\nPlease file an issue with the contents of your CIRCUITPY drive at \nhttps://github.com/adafruit/circuitpython/issues\r\n")
+
 void print_safe_mode_message(safe_mode_t reason) {
     if (reason == NO_SAFE_MODE) {
         return;
     }
     serial_write("\r\n");
-    // Output a user safe mode string if its set.
+    // Output a user safe mode string if it's set.
     #ifdef BOARD_USER_SAFE_MODE
     if (reason == USER_SAFE_MODE) {
         serial_write_compressed(translate("You requested starting safe mode by "));
         serial_write(BOARD_USER_SAFE_MODE_ACTION);
-        serial_write("\r\n");
-        serial_write_compressed(translate("To exit, please reset the board without "));
+        serial_write_compressed(translate("\r\nTo exit, please reset the board without "));
         serial_write(BOARD_USER_SAFE_MODE_ACTION);
         serial_write("\r\n");
     } else
     #endif
-    if (reason == MANUAL_SAFE_MODE) {
-        serial_write_compressed(translate("The reset button was pressed while booting CircuitPython. Press again to exit safe mode.\n"));
-    } else if (reason == PROGRAMMATIC_SAFE_MODE) {
-        serial_write_compressed(translate("The `microcontroller` module was used to boot into safe mode. Press reset to exit safe mode.\n"));
-    } else {
-        serial_write_compressed(translate("You are running in safe mode which means something unanticipated happened.\n"));
-        if (reason == HARD_CRASH || reason == MICROPY_NLR_JUMP_FAIL || reason == MICROPY_FATAL_ERROR || reason == GC_ALLOC_OUTSIDE_VM) {
-            serial_write_compressed(translate("Looks like our core CircuitPython code crashed hard. Whoops!\nPlease file an issue at https://github.com/adafruit/circuitpython/issues\n with the contents of your CIRCUITPY drive and this message:\n"));
-            if (reason == HARD_CRASH) {
-                serial_write_compressed(translate("Crash into the HardFault_Handler.\n"));
-            } else if (reason == MICROPY_NLR_JUMP_FAIL) {
-                serial_write_compressed(translate("MicroPython NLR jump failed. Likely memory corruption.\n"));
-            } else if (reason == MICROPY_FATAL_ERROR) {
-                serial_write_compressed(translate("MicroPython fatal error.\n"));
-            } else if (reason == GC_ALLOC_OUTSIDE_VM) {
-                serial_write_compressed(translate("Attempted heap allocation when MicroPython VM not running.\n"));
-            }
-        } else if (reason == BROWNOUT) {
-            serial_write_compressed(translate("The microcontroller's power dipped. Please make sure your power supply provides\nenough power for the whole circuit and press reset (after ejecting CIRCUITPY).\n"));
-        } else if (reason == HEAP_OVERWRITTEN) {
-            serial_write_compressed(translate("The CircuitPython heap was corrupted because the stack was too small.\nPlease increase stack size limits and press reset (after ejecting CIRCUITPY).\nIf you didn't change the stack, then file an issue here with the contents of your CIRCUITPY drive:\n"));
-            serial_write("https://github.com/adafruit/circuitpython/issues\r\n");
+        switch (reason) {
+            case MANUAL_SAFE_MODE:
+                serial_write_compressed(translate("CircuitPython is in safe mode because you pressed the reset button during boot. Press again to exit safe mode.\r\n"));
+                return;
+            case PROGRAMMATIC_SAFE_MODE:
+                serial_write_compressed(translate("The `microcontroller` module was used to boot into safe mode. Press reset to exit safe mode.\r\n"));
+                return;
+            default:
+                break;
         }
-    }
+
+        serial_write_compressed(translate("You are in safe mode: something unanticipated happened.\r\n"));
+        switch (reason) {
+            case BROWNOUT:
+                serial_write_compressed(translate("The microcontroller's power dipped. Make sure your power supply provides\r\nenough power for the whole circuit and press reset (after ejecting CIRCUITPY).\r\n"));
+                return;
+            case HEAP_OVERWRITTEN:
+                serial_write_compressed(translate("The CircuitPython heap was corrupted because the stack was too small.\r\nPlease increase the stack size if you know how, or if not:"));
+                serial_write_compressed(FILE_AN_ISSUE);
+                return;
+            default:
+                break;
+        }
+
+        serial_write_compressed(translate("CircuitPython core code crashed hard. Whoops!\r\n"));
+        switch (reason) {
+            case HARD_CRASH:
+                serial_write_compressed(translate("Crash into the HardFault_Handler."));
+                return;
+            case MICROPY_NLR_JUMP_FAIL:
+                serial_write_compressed(translate("MicroPython NLR jump failed. Likely memory corruption."));
+                return;
+            case MICROPY_FATAL_ERROR:
+                serial_write_compressed(translate("MicroPython fatal error."));
+                break;
+            case GC_ALLOC_OUTSIDE_VM:
+                serial_write_compressed(translate("Attempted heap allocation when MicroPython VM not running."));
+                break;
+            case NORDIC_SOFT_DEVICE_ASSERT:
+                serial_write_compressed(translate("Nordic Soft Device failure assertion."));
+                break;
+            case FLASH_WRITE_FAIL:
+                serial_write_compressed(translate("Failed to write internal flash."));
+                break;
+            default:
+                serial_write_compressed(translate("Unknown reason."));
+                break;
+        }
+        serial_write_compressed(FILE_AN_ISSUE);
 }
diff --git a/supervisor/shared/safe_mode.h b/supervisor/shared/safe_mode.h
@@ -38,7 +38,8 @@ typedef enum {
   MICROPY_FATAL_ERROR,
   GC_ALLOC_OUTSIDE_VM,
   PROGRAMMATIC_SAFE_MODE,
-  NORDIC_SOFT_DEVICE_ASSERT
+  NORDIC_SOFT_DEVICE_ASSERT,
+  FLASH_WRITE_FAIL,
 } safe_mode_t;
 
 safe_mode_t wait_for_safe_mode_reset(void);

Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,7 @@`
`34`	`34`	`#include "py/obj.h"`
`35`	`35`	`#include "py/runtime.h"`
`36`	`36`	`#include "lib/oofatfs/ff.h"`
	`37`	`+#include "supervisor/shared/safe_mode.h"`
`37`	`38`
`38`	`39`	`#include "peripherals/nrf/nvm.h"`
`39`	`40`
`@@ -75,7 +76,9 @@ void supervisor_flash_flush(void) {`
`75`	`76`
`76`	`77`	`// Skip if data is the same`
`77`	`78`	`if (memcmp(_flash_cache, (void *)_flash_page_addr, FLASH_PAGE_SIZE) != 0) {`
`78`		`- nrf_nvm_safe_flash_page_write(_flash_page_addr, _flash_cache);`
	`79`	`+ if (!nrf_nvm_safe_flash_page_write(_flash_page_addr, _flash_cache)) {`
	`80`	`+ reset_into_safe_mode(FLASH_WRITE_FAIL);`
	`81`	`+ }`
`79`	`82`	`}`
`80`	`83`	`}`
`81`	`84`
`@@ -120,4 +123,3 @@ mp_uint_t supervisor_flash_write_blocks(const uint8_t *src, uint32_t lba, uint32`
`120`	`123`
`121`	`124`	`void supervisor_flash_release_cache(void) {`
`122`	`125`	`}`
`123`		`-`