Merge pull request #6132 from dhalbert/fix-autoreload

tannewt · web-flow · commit 0957c1546ff9 · 2022-03-09T14:02:15.000-08:00
Make autoreload checking more robust
diff --git a/main.c b/main.c
@@ -124,7 +124,7 @@ static void reset_devices(void) {
 }
 
 STATIC void start_mp(supervisor_allocation *heap, bool first_run) {
-    autoreload_stop();
+    autoreload_reset();
     supervisor_workflow_reset();
 
     // Stack limit should be less than real stack size, so we have a chance
@@ -329,7 +329,7 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re
     result.exception = MP_OBJ_NULL;
     result.exception_line = 0;
 
-    bool skip_repl;
+    bool skip_repl = false;
     bool skip_wait = false;
     bool found_main = false;
     uint8_t next_code_options = 0;
@@ -389,13 +389,13 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re
 
         // Print done before resetting everything so that we get the message over
         // BLE before it is reset and we have a delay before reconnect.
-        if (reload_requested && result.return_code == PYEXEC_EXCEPTION) {
+        if (result.return_code == PYEXEC_RELOAD) {
             serial_write_compressed(translate("\nCode stopped by auto-reload.\n"));
         } else {
             serial_write_compressed(translate("\nCode done running.\n"));
         }
 
-        // Finished executing python code. Cleanup includes a board reset.
+        // Finished executing python code. Cleanup includes filesystem flush and a board reset.
         cleanup_after_vm(heap, result.exception);
 
         // If a new next code file was set, that is a reason to keep it (obviously). Stuff this into
@@ -407,8 +407,10 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re
             next_code_options |= SUPERVISOR_NEXT_CODE_OPT_NEWLY_SET;
         }
 
-        if (reload_requested) {
+        if (result.return_code & PYEXEC_RELOAD) {
             next_code_stickiness_situation |= SUPERVISOR_NEXT_CODE_OPT_STICKY_ON_RELOAD;
+            skip_repl = true;
+            skip_wait = true;
         } else if (result.return_code == 0) {
             next_code_stickiness_situation |= SUPERVISOR_NEXT_CODE_OPT_STICKY_ON_SUCCESS;
             if (next_code_options & SUPERVISOR_NEXT_CODE_OPT_RELOAD_ON_SUCCESS) {
@@ -426,7 +428,7 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re
             }
         }
         if (result.return_code & PYEXEC_FORCED_EXIT) {
-            skip_repl = reload_requested;
+            skip_repl = false;
             skip_wait = true;
         }
     }
@@ -473,7 +475,7 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re
         RUN_BACKGROUND_TASKS;
 
         // If a reload was requested by the supervisor or autoreload, return
-        if (reload_requested) {
+        if (result.return_code & PYEXEC_RELOAD) {
             next_code_stickiness_situation |= SUPERVISOR_NEXT_CODE_OPT_STICKY_ON_RELOAD;
             // Should the STICKY_ON_SUCCESS and STICKY_ON_ERROR bits be cleared in
             // next_code_stickiness_situation? I can see arguments either way, but I'm deciding
@@ -627,13 +629,14 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re
         }
     }
 
+    // Done waiting, start the board back up.
+
     // free code allocation if unused
     if ((next_code_options & next_code_stickiness_situation) == 0) {
         free_memory(next_code_allocation);
         next_code_allocation = NULL;
     }
 
-    // Done waiting, start the board back up.
     #if CIRCUITPY_STATUS_LED
     if (led_active) {
         new_status_color(BLACK);
@@ -757,7 +760,7 @@ STATIC int run_repl(bool first_run) {
     usb_setup_with_vm();
     #endif
 
-    autoreload_suspend(AUTORELOAD_LOCK_REPL);
+    autoreload_suspend(AUTORELOAD_SUSPEND_REPL);
 
     // Set the status LED to the REPL color before running the REPL. For
     // NeoPixels and DotStars this will be sticky but for PWM or single LED it
@@ -787,7 +790,7 @@ STATIC int run_repl(bool first_run) {
     status_led_deinit();
     #endif
 
-    autoreload_resume(AUTORELOAD_LOCK_REPL);
+    autoreload_resume(AUTORELOAD_SUSPEND_REPL);
     return exit_code;
 }
 
diff --git a/ports/atmel-samd/supervisor/port.c b/ports/atmel-samd/supervisor/port.c
@@ -129,8 +129,9 @@
 #include "common-hal/_pew/PewPew.h"
 #endif
 static volatile bool sleep_ok = true;
+
 #ifdef SAMD21
-static uint8_t _tick_event_channel = 0;
+uint8_t _tick_event_channel;
 
 // Sleeping requires a register write that can stall interrupt handling. Turning
 // off sleeps allows for more accurate interrupt timing. (Python still thinks
@@ -142,7 +143,13 @@ void rtc_start_pulse(void) {
 void rtc_end_pulse(void) {
     sleep_ok = true;
 }
-#endif
+#endif // SAMD21
+
+static void reset_ticks(void) {
+    #ifdef SAMD21
+    _tick_event_channel = EVSYS_SYNCH_NUM;
+    #endif
+}
 
 extern volatile bool mp_msc_enabled;
 
@@ -426,9 +433,7 @@ void reset_port(void) {
     #endif
 
     reset_event_system();
-    #ifdef SAMD21
-    _tick_event_channel = EVSYS_SYNCH_NUM;
-    #endif
+    reset_ticks();
 
     reset_all_pins();
 
@@ -498,7 +503,7 @@ uint32_t port_get_saved_word(void) {
 static volatile uint64_t overflowed_ticks = 0;
 
 static uint32_t _get_count(uint64_t *overflow_count) {
-    while(1) {
+    while (1) {
         // Disable interrupts so we can grab the count and the overflow atomically.
         common_hal_mcu_disable_interrupts();
 
@@ -521,7 +526,7 @@ static uint32_t _get_count(uint64_t *overflow_count) {
             return count;
         }
 
-    // Try again if overflow hasn't been processed yet.
+        // Try again if overflow hasn't been processed yet.
     }
 }
 
@@ -620,7 +625,7 @@ void port_enable_tick(void) {
     RTC->MODE0.INTENSET.reg = RTC_MODE0_INTENSET_PER2;
     #endif
     #ifdef SAMD21
-    // SAMD21 ticks won't survive port_reset(). This *should* be ok since it'll
+    // SAMD21 ticks won't survive reset_port(). This *should* be ok since it'll
     // be triggered by ticks and no Python will be running.
     if (_tick_event_channel >= EVSYS_SYNCH_NUM) {
         turn_on_event_system();
@@ -653,6 +658,7 @@ void port_disable_tick(void) {
         uint8_t value = 1 << _tick_event_channel;
         EVSYS->INTENCLR.reg = EVSYS_INTENSET_EVD(value);
     }
+    _tick_event_channel = EVSYS_SYNCH_NUM;
     #endif
 }
 
diff --git a/ports/nrf/boards/aramcon2_badge/mpconfigboard.h b/ports/nrf/boards/aramcon2_badge/mpconfigboard.h
@@ -54,8 +54,6 @@
 
 #define BOARD_USER_SAFE_MODE_ACTION translate("pressing the left button at start up\n")
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS 500
-
 #define CIRCUITPY_INTERNAL_NVM_SIZE (4096)
 
 #define BOARD_FLASH_SIZE (FLASH_SIZE - 0x4000 - CIRCUITPY_INTERNAL_NVM_SIZE)
diff --git a/ports/nrf/boards/arduino_nano_33_ble/mpconfigboard.h b/ports/nrf/boards/arduino_nano_33_ble/mpconfigboard.h
@@ -3,8 +3,6 @@
 #define MICROPY_HW_BOARD_NAME       "Arduino Nano 33 BLE"
 #define MICROPY_HW_MCU_NAME         "nRF52840"
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS 500
-
 #define DEFAULT_I2C_BUS_SCL         (&pin_P0_02)
 #define DEFAULT_I2C_BUS_SDA         (&pin_P0_31)
 
diff --git a/ports/nrf/boards/bastble/mpconfigboard.h b/ports/nrf/boards/bastble/mpconfigboard.h
@@ -3,8 +3,6 @@
 #define MICROPY_HW_BOARD_NAME       "BastBLE"
 #define MICROPY_HW_MCU_NAME         "nRF52840"
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS 500
-
 #if QSPI_FLASH_FILESYSTEM
 #define MICROPY_QSPI_DATA0                NRF_GPIO_PIN_MAP(0, 30)
 #define MICROPY_QSPI_DATA1                NRF_GPIO_PIN_MAP(0, 29)
diff --git a/ports/nrf/boards/feather_bluefruit_sense/mpconfigboard.h b/ports/nrf/boards/feather_bluefruit_sense/mpconfigboard.h
@@ -53,8 +53,6 @@
 #define SPI_FLASH_CS_PIN &pin_P0_20
 #endif
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS 500
-
 #define CIRCUITPY_INTERNAL_NVM_SIZE (4096)
 
 #define BOARD_FLASH_SIZE (FLASH_SIZE - 0x4000 - CIRCUITPY_INTERNAL_NVM_SIZE)
diff --git a/ports/nrf/boards/ikigaisense_vita/mpconfigboard.h b/ports/nrf/boards/ikigaisense_vita/mpconfigboard.h
@@ -5,8 +5,6 @@
 
 #define MICROPY_HW_LED_STATUS       (&pin_P0_27)
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS 500
-
 #define CIRCUITPY_INTERNAL_NVM_SIZE (4096)
 
 #define BOARD_FLASH_SIZE (FLASH_SIZE - 0x4000 - CIRCUITPY_INTERNAL_NVM_SIZE)
diff --git a/ports/nrf/boards/itsybitsy_nrf52840_express/mpconfigboard.h b/ports/nrf/boards/itsybitsy_nrf52840_express/mpconfigboard.h
@@ -24,8 +24,6 @@
 #define SPI_FLASH_CS_PIN &pin_P0_23
 #endif
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS 500
-
 #define CIRCUITPY_INTERNAL_NVM_SIZE (4096)
 
 #define BOARD_FLASH_SIZE (FLASH_SIZE - 0x4000 - CIRCUITPY_INTERNAL_NVM_SIZE)
diff --git a/ports/nrf/boards/warmbit_bluepixel/mpconfigboard.h b/ports/nrf/boards/warmbit_bluepixel/mpconfigboard.h
@@ -5,8 +5,6 @@
 
 #define MICROPY_HW_LED_STATUS       (&pin_P0_12)
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS 500
-
 #define CIRCUITPY_INTERNAL_NVM_SIZE (4096)
 
 #define BOARD_FLASH_SIZE (FLASH_SIZE - 0x4000 - CIRCUITPY_INTERNAL_NVM_SIZE)
diff --git a/ports/stm/boards/pyb_nano_v2/mpconfigboard.h b/ports/stm/boards/pyb_nano_v2/mpconfigboard.h
@@ -42,8 +42,6 @@
 #define SPI_FLASH_SCK_PIN           (&pin_PB13)
 #define SPI_FLASH_CS_PIN            (&pin_PB12)
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS (500)
-
 #define BOARD_FLASH_SIZE (FLASH_SIZE - 0x2000 - 0xC000)
 
 #define AUTORESET_DELAY_MS (500)
diff --git a/ports/stm/boards/stm32f411ce_blackpill/mpconfigboard.h b/ports/stm/boards/stm32f411ce_blackpill/mpconfigboard.h
@@ -45,8 +45,6 @@
 #define DEFAULT_I2C_BUS_SCL (&pin_PB06)
 #define DEFAULT_I2C_BUS_SDA (&pin_PB07)
 
-#define CIRCUITPY_AUTORELOAD_DELAY_MS (500)
-
 #define BOARD_FLASH_SIZE (FLASH_SIZE - 0x2000 - 0xC000)
 
 #define AUTORESET_DELAY_MS (500)
diff --git a/ports/stm/boards/stm32f411ce_blackpill_with_flash/mpconfigboard.h b/ports/stm/boards/stm32f411ce_blackpill_with_flash/mpconfigboard.h
@@ -49,6 +49,4 @@
 
 #define BOARD_FLASH_SIZE (FLASH_SIZE - 0x2000 - 0xC000)
 
-#define AUTORESET_DELAY_MS (500)
-
 #define MICROPY_FATFS_EXFAT 0
diff --git a/shared-bindings/supervisor/__init__.c b/shared-bindings/supervisor/__init__.c
@@ -95,7 +95,6 @@ MP_DEFINE_CONST_FUN_OBJ_1(supervisor_set_rgb_status_brightness_obj, supervisor_s
 //|     ...
 //|
 STATIC mp_obj_t supervisor_reload(void) {
-    reload_requested = true;
     supervisor_set_run_reason(RUN_REASON_SUPERVISOR_RELOAD);
     mp_raise_reload_exception();
     return mp_const_none;
diff --git a/shared/runtime/pyexec.c b/shared/runtime/pyexec.c
@@ -174,12 +174,13 @@ STATIC int parse_compile_execute(const void *source, mp_parse_input_kind_t input
         } else if (mp_obj_is_subclass_fast(MP_OBJ_FROM_PTR(mp_obj_get_type((mp_obj_t)nlr.ret_val)), &mp_type_DeepSleepRequest)) {
             ret = PYEXEC_DEEP_SLEEP;
         #endif
+        } else if ((mp_obj_t)nlr.ret_val == MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_reload_exception))) {
+            ret = PYEXEC_RELOAD;
         } else {
-            if ((mp_obj_t)nlr.ret_val != MP_OBJ_FROM_PTR(&MP_STATE_VM(mp_reload_exception))) {
-                mp_obj_print_exception(&mp_plat_print, MP_OBJ_FROM_PTR(nlr.ret_val));
-            }
+            mp_obj_print_exception(&mp_plat_print, MP_OBJ_FROM_PTR(nlr.ret_val));
             ret = PYEXEC_EXCEPTION;
         }
+
     }
     if (result != NULL) {
         result->return_code = ret;
diff --git a/shared/runtime/pyexec.h b/shared/runtime/pyexec.h
@@ -49,6 +49,7 @@ extern int pyexec_system_exit;
 #define PYEXEC_FORCED_EXIT (0x100)
 #define PYEXEC_EXCEPTION   (0x200)
 #define PYEXEC_DEEP_SLEEP  (0x400)
+#define PYEXEC_RELOAD      (0x800)
 
 int pyexec_raw_repl(void);
 int pyexec_friendly_repl(void);
diff --git a/supervisor/shared/autoreload.c b/supervisor/shared/autoreload.c
@@ -33,67 +33,76 @@
 supervisor_allocation *next_code_allocation;
 #include "shared-bindings/supervisor/Runtime.h"
 
-static volatile uint32_t autoreload_delay_ms = 0;
+static volatile uint32_t autoreload_countdown_ms = 0;
+
+// True if user has disabled autoreload.
 static bool autoreload_enabled = false;
-static size_t autoreload_suspended = 0;
 
+// Non-zero if autoreload is temporarily off, due to an AUTORELOAD_SUSPEND_... reason.
+static uint32_t autoreload_suspended = 0;
+
+// True if autoreload has been triggered. Wait for CIRCUITPY_AUTORELOAD_DELAY_MS before doing the
+// autoreload, in case further writes arrive.
+static bool autoreload_countdown = false;
+
+// True if something has requested a reload/restart.
 volatile bool reload_requested = false;
 
+void autoreload_reset() {
+    if (autoreload_countdown) {
+        supervisor_disable_tick();
+        autoreload_countdown = false;
+    }
+    autoreload_countdown_ms = 0;
+    reload_requested = false;
+}
+
 inline void autoreload_tick() {
-    if (autoreload_delay_ms == 0) {
+    if (!autoreload_countdown) {
         return;
     }
-    if (autoreload_delay_ms == 1 && autoreload_enabled &&
+    if (autoreload_countdown_ms > 0) {
+        autoreload_countdown_ms--;
+    }
+    if (autoreload_countdown_ms == 0 && autoreload_enabled &&
         autoreload_suspended == 0 && !reload_requested) {
-        mp_raise_reload_exception();
         reload_requested = true;
-        supervisor_set_run_reason(RUN_REASON_AUTO_RELOAD);
+        autoreload_countdown = false;
         supervisor_disable_tick();
+        supervisor_set_run_reason(RUN_REASON_AUTO_RELOAD);
+        mp_raise_reload_exception();
     }
-    autoreload_delay_ms--;
 }
 
 void autoreload_enable() {
     autoreload_enabled = true;
     reload_requested = false;
+    autoreload_countdown = false;
 }
 
 void autoreload_disable() {
     autoreload_enabled = false;
+    autoreload_countdown = false;
 }
 
-void autoreload_suspend(size_t lock_mask) {
-    autoreload_suspended |= lock_mask;
+void autoreload_suspend(uint32_t suspend_reason_mask) {
+    autoreload_suspended |= suspend_reason_mask;
 }
 
-void autoreload_resume(size_t lock_mask) {
-    autoreload_suspended &= ~lock_mask;
+void autoreload_resume(uint32_t suspend_reason_mask) {
+    autoreload_suspended &= ~suspend_reason_mask;
 }
 
 inline bool autoreload_is_enabled() {
     return autoreload_enabled;
 }
 
-void autoreload_start() {
-    // Enable ticks if we haven't been tracking an autoreload delay. We check
-    // our current state so that we only turn ticks on once. Multiple starts
-    // can occur before we reload and then turn ticks off.
-    if (autoreload_delay_ms == 0) {
+void autoreload_start_countdown() {
+    // Avoid multiple tick enables.
+    if (!autoreload_countdown) {
         supervisor_enable_tick();
+        autoreload_countdown = true;
     }
-    autoreload_delay_ms = CIRCUITPY_AUTORELOAD_DELAY_MS;
-}
-
-void autoreload_stop() {
-    autoreload_delay_ms = 0;
-    reload_requested = false;
-}
-
-void autoreload_now() {
-    if (!autoreload_enabled || autoreload_suspended || reload_requested) {
-        return;
-    }
-    mp_raise_reload_exception();
-    reload_requested = true;
-    supervisor_set_run_reason(RUN_REASON_AUTO_RELOAD);
+    // Start or restart the countdown interval.
+    autoreload_countdown_ms = CIRCUITPY_AUTORELOAD_DELAY_MS;
 }
diff --git a/supervisor/shared/autoreload.h b/supervisor/shared/autoreload.h
diff --git a/supervisor/shared/bluetooth/file_transfer.c b/supervisor/shared/bluetooth/file_transfer.c
diff --git a/supervisor/shared/flash.c b/supervisor/shared/flash.c
diff --git a/supervisor/shared/tick.c b/supervisor/shared/tick.c
diff --git a/supervisor/shared/usb/usb_msc_flash.c b/supervisor/shared/usb/usb_msc_flash.c
