pbio/sys/storage: Use status to block duplicate downloads.

This is technically the same as before, but implemented with a status flag.

This lets us visually disable the download button in Pybricks Code. The status is immediately set on the first download, so this is safe against race conditions even if the visual update arrives a little bit later.

Author: laurensvalk

lib/pbio/include/pbio/protocol.h

@@ -363,6 +363,12 @@ typedef enum {
      * @since Pybricks Profile v1.5.0
      */
     PBIO_PYBRICKS_STATUS_USB_HOST_CONNECTED = 12,
+    /**
+     * Hub is receiving, transmitting, or moving a file.
+     *
+     * @since Pybricks Profile v1.5.0
+     */
+    PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS = 13,
     /** Total number of indications. */
     NUM_PBIO_PYBRICKS_STATUS,
 } pbio_pybricks_status_flags_t;

lib/pbio/sys/core.c

@@ -39,6 +39,7 @@ static pbio_error_t pbsys_system_poll_process_thread(pbio_os_state_t *state, voi
         pbsys_battery_poll();
         pbsys_program_stop_poll();
         pbsys_status_light_poll();
+        pbsys_storage_poll();
 
         // keep the hub from resetting itself
         pbdrv_watchdog_update();

lib/pbio/sys/status.c

@@ -84,7 +84,7 @@ uint32_t pbsys_status_get_status_report(uint8_t *buf) {
  */
 void pbsys_status_increment_selected_slot(bool increment) {
     #if PBSYS_CONFIG_HMI_NUM_SLOTS
-    if (!pbsys_storage_slot_change_is_allowed()) {
+    if (pbsys_status_test(PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS)) {
         return;
     }
     if (increment && pbsys_status.slot + 1 < PBSYS_CONFIG_HMI_NUM_SLOTS) {

lib/pbio/sys/storage.c

@@ -22,20 +22,32 @@
 #include <pbsys/status.h>
 
 #include "hmi.h"
-#include "storage.h"
 
 /**
  * State of incoming program data.
  */
 static struct {
     /** Where incoming program data is placed. */
     uint8_t slot;
-    /** Currently downloading. */
-    bool busy;
-    /** Latest incoming message time. Only relevant while busy. */
+    /** Latest incoming message time. */
     pbio_os_timer_t timer;
 } download_state;
 
+/**
+ * Polls storage system for timeouts.
+ *
+ * This should be called periodically.
+ */
+void pbsys_storage_poll(void) {
+    // If we are receiving a program, check for timeout and clear
+    // busy state if timed out.
+    if (pbsys_status_test(PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS)) {
+        if (pbio_os_timer_is_expired(&download_state.timer)) {
+            pbsys_status_clear(PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS);
+        }
+    }
+}
+
 /**
  * Map of loaded data. This is kept in memory between successive runs of the
  * end user application (like MicroPython).
@@ -252,19 +264,25 @@ pbio_error_t pbsys_storage_set_program_size(uint32_t new_size) {
     // Pybricks Code sends size 0 to clear the state before sending the new
     // program, then sends the size on completion.
     if (new_size == 0) {
-        if (!pbsys_storage_slot_change_is_allowed()) {
+        if (pbsys_status_test(PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS)) {
             // Cannot start a new download while another is in progress.
             return PBIO_ERROR_INVALID_OP;
         }
 
         pbsys_storage_prepare_receive();
 
-        // Keep track of busy state to disallow some operations while busy.
-        download_state.busy = true;
+        // Set busy status to disallow some operations while busy.
+        pbsys_status_set(PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS);
         pbio_os_timer_set(&download_state.timer, 1000);
         return PBIO_SUCCESS;
     }
 
+    // This is also called on completion of the program download, in which
+    // case we expect that a download is in progress.
+    if (!pbsys_status_test(PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS)) {
+        return PBIO_ERROR_INVALID_OP;
+    }
+
     // Expecting that the slot has been cleared as per the above.
     if (download_state.slot >= PBSYS_CONFIG_STORAGE_NUM_SLOTS || map->slot_info[download_state.slot].size != 0) {
         return PBIO_ERROR_FAILED;
@@ -278,29 +296,11 @@ pbio_error_t pbsys_storage_set_program_size(uint32_t new_size) {
 
     // Program download complete, so request saving on poweroff.
     pbsys_storage_request_write();
-    download_state.busy = false;
 
-    return PBIO_SUCCESS;
-}
+    // Clear busy status.
+    pbsys_status_clear(PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS);
 
-/**
- * Tests whether a slot change is allowed.
- *
- * @return True when allowed, false if not, i.e. when new a new program is being downloaded.
- */
-bool pbsys_storage_slot_change_is_allowed(void) {
-    #if !PBSYS_CONFIG_HMI_NUM_SLOTS
-    return false;
-    #endif
-
-    if (download_state.busy && pbio_os_timer_is_expired(&download_state.timer)) {
-        // Download must have been interrupted midway through. Give up
-        // and unblock anything waiting on download to complete.
-        download_state.busy = false;
-    }
-
-    // Slot change is allowed when no longer busy.
-    return !download_state.busy;
+    return PBIO_SUCCESS;
 }
 
 /**
@@ -328,7 +328,7 @@ pbio_error_t pbsys_storage_set_program_data(uint32_t offset, const void *data, u
     }
 
     // A program transfer should have been started.
-    if (!download_state.busy) {
+    if (!pbsys_status_test(PBIO_PYBRICKS_STATUS_FILE_IO_IN_PROGRESS)) {
         return PBIO_ERROR_INVALID_OP;
     }
 

lib/pbio/sys/storage.h

@@ -15,9 +15,9 @@
 
 void pbsys_storage_init(void);
 void pbsys_storage_deinit(void);
+void pbsys_storage_poll(void);
 pbio_error_t pbsys_storage_set_program_size(uint32_t size);
 pbio_error_t pbsys_storage_set_program_data(uint32_t offset, const void *data, uint32_t size);
-bool pbsys_storage_slot_change_is_allowed(void);
 void pbsys_storage_get_program_data(pbsys_main_program_t *program);
 pbsys_storage_settings_t *pbsys_storage_settings_get_settings(void);
 
@@ -26,6 +26,8 @@ static inline void pbsys_storage_init(void) {
 }
 static inline void pbsys_storage_deinit(void) {
 }
+static inline void pbsys_storage_poll(void) {
+}
 static inline pbsys_storage_settings_t *pbsys_storage_settings_get_settings(void) {
     return NULL;
 }
@@ -35,9 +37,6 @@ static inline pbio_error_t pbsys_storage_set_program_size(uint32_t size) {
 static inline pbio_error_t pbsys_storage_set_program_data(uint32_t offset, const void *data, uint32_t size) {
     return PBIO_ERROR_NOT_SUPPORTED;
 }
-static inline bool pbsys_storage_slot_change_is_allowed(void) {
-    return false;
-}
 static inline void pbsys_storage_get_program_data(pbsys_main_program_t *program) {
     program->code_start = NULL;
     program->code_end = NULL;