From 32ac396a41223c99e2c3c71429a1d6c920ad138a Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Mon, 14 Mar 2022 16:49:30 -0700 Subject: [PATCH] Further refine autoreload This unifies the delay into the post-run delay that also waits for user input and fake sleep. This ensures we always delay. Previous code would only delay if the code.py was running when autoreload was triggered. Now it will always delay. We also now suspend autoreload when a USB write starts and then resume on completion. This should prevent reloading in between sectors of a single write. --- locale/circuitpython.pot | 13 +++---- main.c | 39 ++++++++------------- ports/stm/supervisor/port.c | 2 -- shared-module/displayio/__init__.c | 2 +- supervisor/shared/bluetooth/file_transfer.c | 15 +++----- supervisor/shared/reload.c | 39 +++++++++++++++++---- supervisor/shared/reload.h | 25 +++++++++---- supervisor/shared/usb/usb_msc_flash.c | 4 ++- 8 files changed, 82 insertions(+), 57 deletions(-) diff --git a/locale/circuitpython.pot b/locale/circuitpython.pot index 336dd49daf..886edad28d 100644 --- a/locale/circuitpython.pot +++ b/locale/circuitpython.pot @@ -25,7 +25,7 @@ msgstr "" #: main.c msgid "" "\n" -"Code stopped by auto-reload.\n" +"Code stopped by auto-reload. Reloading soon.\n" msgstr "" #: supervisor/shared/safe_mode.c @@ -584,10 +584,6 @@ msgstr "" msgid "Brightness must be 0-1.0" msgstr "" -#: shared-bindings/supervisor/__init__.c -msgid "Brightness must be between 0 and 255" -msgstr "" - #: shared-bindings/displayio/Display.c #: shared-bindings/framebufferio/FramebufferDisplay.c msgid "Brightness not adjustable" @@ -688,6 +684,7 @@ msgstr "" msgid "Can only alarm on two low pins from deep sleep." msgstr "" +#: ports/espressif/common-hal/_bleio/Characteristic.c #: ports/nrf/common-hal/_bleio/Characteristic.c msgid "Can't set CCCD on local Characteristic" msgstr "" @@ -1605,6 +1602,7 @@ msgstr "" msgid "Nimble out of memory" msgstr "" +#: ports/espressif/common-hal/_bleio/Characteristic.c #: ports/nrf/common-hal/_bleio/Characteristic.c msgid "No CCCD for this Characteristic" msgstr "" @@ -3642,7 +3640,7 @@ msgstr "" msgid "matrix is not positive definite" msgstr "" -#: shared-bindings/wifi/Radio.c +#: ports/espressif/common-hal/wifi/Radio.c msgid "max_connections must be between 0 and 10" msgstr "" @@ -4061,6 +4059,7 @@ msgstr "" #: ports/espressif/boards/adafruit_metro_esp32s2/mpconfigboard.h #: ports/espressif/boards/adafruit_qtpy_esp32s2/mpconfigboard.h #: ports/espressif/boards/adafruit_qtpy_esp32s3_nopsram/mpconfigboard.h +#: ports/espressif/boards/ai_thinker_esp32-c3s-2m/mpconfigboard.h #: ports/espressif/boards/ai_thinker_esp32-c3s/mpconfigboard.h #: ports/espressif/boards/ai_thinker_esp_12k_nodemcu/mpconfigboard.h #: ports/espressif/boards/artisense_rd00/mpconfigboard.h @@ -4068,6 +4067,7 @@ msgstr "" #: ports/espressif/boards/crumpspace_crumps2/mpconfigboard.h #: ports/espressif/boards/electroniccats_bastwifi/mpconfigboard.h #: ports/espressif/boards/espressif_esp32c3_devkitm_1_n4/mpconfigboard.h +#: ports/espressif/boards/espressif_esp32s2_devkitc_1_n4r2/mpconfigboard.h #: ports/espressif/boards/espressif_esp32s3_box/mpconfigboard.h #: ports/espressif/boards/espressif_esp32s3_devkitc_1_n8/mpconfigboard.h #: ports/espressif/boards/espressif_esp32s3_devkitc_1_n8r2/mpconfigboard.h @@ -4083,6 +4083,7 @@ msgstr "" #: ports/espressif/boards/gravitech_cucumber_ms/mpconfigboard.h #: ports/espressif/boards/gravitech_cucumber_r/mpconfigboard.h #: ports/espressif/boards/gravitech_cucumber_rs/mpconfigboard.h +#: ports/espressif/boards/hiibot_iots2/mpconfigboard.h #: ports/espressif/boards/lilygo_ttgo_t8_s2_st7789/mpconfigboard.h #: ports/espressif/boards/lolin_s2_mini/mpconfigboard.h #: ports/espressif/boards/lolin_s2_pico/mpconfigboard.h diff --git a/main.c b/main.c index 9f31153877..070fb211d8 100644 --- a/main.c +++ b/main.c @@ -124,7 +124,6 @@ static void reset_devices(void) { } STATIC void start_mp(supervisor_allocation *heap, bool first_run) { - autoreload_reset(); supervisor_workflow_reset(); // Stack limit should be less than real stack size, so we have a chance @@ -336,7 +335,13 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re // Collects stickiness bits that apply in the current situation. uint8_t next_code_stickiness_situation = SUPERVISOR_NEXT_CODE_OPT_NEWLY_SET; + // Do the filesystem flush check before reload in case another write comes + // in while we're doing the flush. if (safe_mode == NO_SAFE_MODE) { + stack_resize(); + filesystem_flush(); + } + if (safe_mode == NO_SAFE_MODE && !autoreload_pending()) { static const char *const supported_filenames[] = STRING_LIST( "code.txt", "code.py", "main.py", "main.txt"); #if CIRCUITPY_FULL_BUILD @@ -345,8 +350,6 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re "main.txt.py", "main.py.txt", "main.txt.txt","main.py.py"); #endif - stack_resize(); - filesystem_flush(); supervisor_allocation *heap = allocate_remaining_memory(); // Prepare the VM state. Includes an alarm check/reset for sleep. @@ -390,22 +393,7 @@ 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 ((result.return_code & PYEXEC_RELOAD) && supervisor_get_run_reason() == RUN_REASON_AUTO_RELOAD) { - serial_write_compressed(translate("\nCode stopped by auto-reload.\n")); - - // Wait for autoreload interval before reloading - uint64_t start_ticks = 0; - do { - // Start waiting, or restart interval if another reload request was initiated - // while we were waiting. - if (reload_requested) { - reload_requested = false; - start_ticks = supervisor_ticks_ms64(); - } - RUN_BACKGROUND_TASKS; - } while (supervisor_ticks_ms64() - start_ticks < CIRCUITPY_AUTORELOAD_DELAY_MS); - - // Restore request for use below. - reload_requested = true; + serial_write_compressed(translate("\nCode stopped by auto-reload. Reloading soon.\n")); } else { serial_write_compressed(translate("\nCode done running.\n")); } @@ -425,8 +413,6 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re 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) { @@ -484,6 +470,8 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re size_t total_time = blink_time + LED_SLEEP_TIME_MS; #endif + // This loop is waits after code completes. It waits for fake sleeps to + // finish, user input or autoreloads. #if CIRCUITPY_ALARM bool fake_sleeping = false; #endif @@ -491,15 +479,18 @@ 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 (autoreload_ready()) { 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 // "no" for now, mainly because it's a bit less code. At this point, we have both a // success or error and a reload, so let's have both of the respective options take // effect (in OR combination). - reload_requested = false; skip_repl = true; + // We're kicking off the autoreload process so reset now. If any + // other reloads trigger after this, then we'll want another wait + // period. + autoreload_reset(); break; } @@ -526,7 +517,7 @@ STATIC bool run_code_py(safe_mode_t safe_mode, bool first_run, bool *simulate_re #endif // If messages haven't been printed yet, print them - if (!printed_press_any_key && serial_connected()) { + if (!printed_press_any_key && serial_connected() && !autoreload_pending()) { if (!serial_connected_at_start) { print_code_py_status_message(safe_mode); } diff --git a/ports/stm/supervisor/port.c b/ports/stm/supervisor/port.c index a158ade6eb..e4cce571ea 100644 --- a/ports/stm/supervisor/port.c +++ b/ports/stm/supervisor/port.c @@ -346,8 +346,6 @@ void port_enable_tick(void) { stm32_peripherals_rtc_assign_wkup_callback(supervisor_tick); stm32_peripherals_rtc_enable_wakeup_timer(); } -// TODO: what is this? can I get rid of it? -extern volatile uint32_t autoreload_delay_ms; // Disable 1/1024 second tick. void port_disable_tick(void) { diff --git a/shared-module/displayio/__init__.c b/shared-module/displayio/__init__.c index 04171198cf..87962dfc60 100644 --- a/shared-module/displayio/__init__.c +++ b/shared-module/displayio/__init__.c @@ -82,7 +82,7 @@ void displayio_background(void) { if (mp_hal_is_interrupted()) { return; } - if (reload_requested) { + if (autoreload_ready()) { // Reload is about to happen, so don't redisplay. return; } diff --git a/supervisor/shared/bluetooth/file_transfer.c b/supervisor/shared/bluetooth/file_transfer.c index 6715ee961b..6c206f35c0 100644 --- a/supervisor/shared/bluetooth/file_transfer.c +++ b/supervisor/shared/bluetooth/file_transfer.c @@ -325,8 +325,7 @@ STATIC uint8_t _process_write(const uint8_t *raw_buf, size_t command_len) { if (chunk_size == 0) { // Don't reload until everything is written out of the packet buffer. common_hal_bleio_packet_buffer_flush(&_transfer_packet_buffer); - // Trigger an autoreload - autoreload_start(); + autoreload_trigger(); return ANY_COMMAND; } @@ -383,8 +382,7 @@ STATIC uint8_t _process_write_data(const uint8_t *raw_buf, size_t command_len) { #endif // Don't reload until everything is written out of the packet buffer. common_hal_bleio_packet_buffer_flush(&_transfer_packet_buffer); - // Trigger an autoreload - autoreload_start(); + autoreload_trigger(); return ANY_COMMAND; } return WRITE_DATA; @@ -465,8 +463,7 @@ STATIC uint8_t _process_delete(const uint8_t *raw_buf, size_t command_len) { if (result == FR_OK) { // Don't reload until everything is written out of the packet buffer. common_hal_bleio_packet_buffer_flush(&_transfer_packet_buffer); - // Trigger an autoreload - autoreload_start(); + autoreload_trigger(); } return ANY_COMMAND; } @@ -520,8 +517,7 @@ STATIC uint8_t _process_mkdir(const uint8_t *raw_buf, size_t command_len) { if (result == FR_OK) { // Don't reload until everything is written out of the packet buffer. common_hal_bleio_packet_buffer_flush(&_transfer_packet_buffer); - // Trigger an autoreload - autoreload_start(); + autoreload_trigger(); } return ANY_COMMAND; } @@ -668,8 +664,7 @@ STATIC uint8_t _process_move(const uint8_t *raw_buf, size_t command_len) { if (result == FR_OK) { // Don't reload until everything is written out of the packet buffer. common_hal_bleio_packet_buffer_flush(&_transfer_packet_buffer); - // Trigger an autoreload - autoreload_start(); + autoreload_trigger(); } return ANY_COMMAND; } diff --git a/supervisor/shared/reload.c b/supervisor/shared/reload.c index b774074df6..f30d4249e2 100644 --- a/supervisor/shared/reload.c +++ b/supervisor/shared/reload.c @@ -40,11 +40,9 @@ static bool autoreload_enabled = false; // Non-zero if autoreload is temporarily off, due to an AUTORELOAD_SUSPEND_... reason. static uint32_t autoreload_suspended = 0; -// True if something has requested a reload/restart. -volatile bool reload_requested = false; +volatile uint32_t last_autoreload_trigger = 0; void reload_initiate(supervisor_run_reason_t run_reason) { - reload_requested = true; supervisor_set_run_reason(run_reason); // Raise reload exception, in case code is running. @@ -57,12 +55,12 @@ void reload_initiate(supervisor_run_reason_t run_reason) { } void autoreload_reset() { - reload_requested = false; + last_autoreload_trigger = 0; } void autoreload_enable() { autoreload_enabled = true; - reload_requested = false; + last_autoreload_trigger = 0; } void autoreload_disable() { @@ -81,8 +79,35 @@ inline bool autoreload_is_enabled() { return autoreload_enabled; } -void autoreload_start() { - if (autoreload_enabled && autoreload_suspended == 0) { +void autoreload_trigger() { + if (autoreload_enabled) { + last_autoreload_trigger = supervisor_ticks_ms32(); + // Guard against the rare time that ticks is 0; + if (last_autoreload_trigger == 0) { + last_autoreload_trigger += 1; + } + // Initiate a reload of the VM immediately. Later code will pause to + // wait for the autoreload to become ready. Doing the VM exit + // immediately is clearer for the user. reload_initiate(RUN_REASON_AUTO_RELOAD); } } + +bool autoreload_ready() { + if (last_autoreload_trigger == 0 || autoreload_suspended != 0) { + return false; + } + // Wait for autoreload interval before reloading + uint32_t now = supervisor_ticks_ms32(); + uint32_t diff; + if (now >= last_autoreload_trigger) { + diff = now - last_autoreload_trigger; + } else { + diff = now + (0xffffffff - last_autoreload_trigger); + } + return diff > CIRCUITPY_AUTORELOAD_DELAY_MS; +} + +bool autoreload_pending(void) { + return last_autoreload_trigger != 0; +} diff --git a/supervisor/shared/reload.h b/supervisor/shared/reload.h index 10b4bea00c..cb3385e7ca 100644 --- a/supervisor/shared/reload.h +++ b/supervisor/shared/reload.h @@ -42,7 +42,8 @@ enum { enum { AUTORELOAD_SUSPEND_REPL = 0x1, - AUTORELOAD_SUSPEND_BLE = 0x2 + AUTORELOAD_SUSPEND_BLE = 0x2, + AUTORELOAD_SUSPEND_USB = 0x4 }; typedef struct { @@ -52,17 +53,29 @@ typedef struct { extern supervisor_allocation *next_code_allocation; -extern volatile bool reload_requested; - +// Helper for exiting the VM and reloading immediately. void reload_initiate(supervisor_run_reason_t run_reason); -void autoreload_start(void); -void autoreload_reset(void); +// Enabled state is user controllable and very sticky. We don't reset it. void autoreload_enable(void); void autoreload_disable(void); bool autoreload_is_enabled(void); -// Temporarily turn autoreload off, for the given reason(s). +// Start the autoreload process. +void autoreload_trigger(void); +// True when the autoreload should occur. (A trigger happened and the delay has +// passed.) +bool autoreload_ready(void); +// Reset the autoreload timer in preparation for another trigger. Call when the +// last trigger starts being executed. +void autoreload_reset(void); +// True when a trigger has occurred but we're still delaying in case another +// trigger occurs. +bool autoreload_pending(void); + +// Temporarily turn autoreload off, for the given reason(s). Autoreload triggers +// will still be tracked so resuming with autoreload ready with cause an +// immediate reload. // Used during the REPL or during parts of BLE workflow. void autoreload_suspend(uint32_t suspend_reason_mask); // Allow autoreloads again, for the given reason(s). diff --git a/supervisor/shared/usb/usb_msc_flash.c b/supervisor/shared/usb/usb_msc_flash.c index fbc9c2a1b4..67d57bceb5 100644 --- a/supervisor/shared/usb/usb_msc_flash.c +++ b/supervisor/shared/usb/usb_msc_flash.c @@ -185,6 +185,7 @@ int32_t tud_msc_read10_cb(uint8_t lun, uint32_t lba, uint32_t offset, void *buff int32_t tud_msc_write10_cb(uint8_t lun, uint32_t lba, uint32_t offset, uint8_t *buffer, uint32_t bufsize) { (void)lun; (void)offset; + autoreload_suspend(AUTORELOAD_SUSPEND_USB); const uint32_t block_count = bufsize / MSC_FLASH_BLOCK_SIZE; @@ -215,7 +216,8 @@ void tud_msc_write10_complete_cb(uint8_t lun) { (void)lun; // This write is complete; initiate an autoreload. - autoreload_start(); + autoreload_trigger(); + autoreload_resume(AUTORELOAD_SUSPEND_USB); } // Invoked when received SCSI_CMD_INQUIRY