pbio/sys/storage: Implement persistent storage.

CHANGELOG.md

@@ -7,9 +7,16 @@
 ### Added
 
 - Allow Bluetooth to be toggled off and on with the Bluetooth button on the
-  Prime Hub and the Inventor Hub ([support#1615]).
+  Prime Hub and the Inventor Hub ([support#1615]), and have this state persist
+  between reboots.
+
+### Changed
+
+- When upgrading the firmware to a new version, the user program will now
+  be erased. This avoids issues with incompatible program files ([support#1622]).
 
 [support#1615]: https://github.com/pybricks/support/issues/1615
+[support#1622]: https://github.com/pybricks/support/issues/1622
 
 ## [3.5.0] - 2024-04-11
 

bricks/_common/sources.mk

@@ -233,6 +233,7 @@ PBIO_SRC_C = $(addprefix lib/pbio/,\
 	sys/program_stop.c \
 	sys/status.c \
 	sys/storage.c \
+	sys/storage_settings.c \
 	sys/supervisor.c \
 	sys/user_program.c \
 	)

lib/pbio/include/pbsys/bluetooth.h

@@ -32,8 +32,6 @@ uint32_t pbsys_bluetooth_rx_get_available(void);
 pbio_error_t pbsys_bluetooth_rx(uint8_t *data, uint32_t *size);
 pbio_error_t pbsys_bluetooth_tx(const uint8_t *data, uint32_t *size);
 bool pbsys_bluetooth_tx_is_idle(void);
-bool pbsys_bluetooth_is_user_enabled(void);
-void pbsys_bluetooth_is_user_enabled_request_toggle(void);
 
 #else // PBSYS_CONFIG_BLUETOOTH
 
@@ -51,11 +49,6 @@ static inline pbio_error_t pbsys_bluetooth_tx(const uint8_t *data, uint32_t *siz
 static inline bool pbsys_bluetooth_tx_is_idle(void) {
     return false;
 }
-static inline void pbsys_bluetooth_is_user_enabled_request_toggle(void) {
-}
-static inline bool pbsys_bluetooth_is_user_enabled(void) {
-    return true;
-}
 
 #endif // PBSYS_CONFIG_BLUETOOTH
 

lib/pbio/include/pbsys/storage.h

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: MIT
-// Copyright (c) 2022 The Pybricks Authors
+// Copyright (c) 2022-2024 The Pybricks Authors
 
 /**
- * @addtogroup SysProgramLoad System: Load user programs.
+ * @addtogroup SysStorage System: Load user programs, data, and settings.
  *
- * Configuration for loading data.
+ * Load user programs, data, and settings.
  *
  * @{
  */
@@ -15,6 +15,7 @@
 #include <stdint.h>
 
 #include <pbsys/config.h>
+#include <pbsys/storage_settings.h>
 
 #if PBSYS_CONFIG_STORAGE
 
@@ -24,7 +25,7 @@
 #endif
 
 /**
- * Header of loaded data. All data types are little-endian.
+ * Map of loaded data. All data types are little-endian.
  */
 typedef struct _pbsys_storage_data_map_t {
     /**
@@ -42,9 +43,21 @@ typedef struct _pbsys_storage_data_map_t {
     volatile uint32_t checksum_complement;
     #endif
     /**
-     * End-user read-write accessible data.
+     * Firmware version used to create the stored data. See pbio/version.
+     * Human-readable when printed as hex. If this value does not match
+     * the version of the running firmware, user data will be reset to 0.
+     */
+    uint32_t stored_firmware_version;
+    /**
+     * End-user read-write accessible data. Everything after this is also
+     * user-readable but not writable.
      */
     uint8_t user_data[PBSYS_CONFIG_STORAGE_USER_DATA_SIZE];
+    /**
+     * System settings. Settings will be reset to defaults when the firmware
+     * version changes due to an update.
+     */
+    pbsys_storage_settings_t settings;
     /**
      * Size of the application program (size of code only).
      */
@@ -68,13 +81,11 @@ pbio_error_t pbsys_storage_get_user_data(uint32_t offset, uint8_t **data, uint32
 static inline pbio_error_t pbsys_storage_set_user_data(uint32_t offset, const uint8_t *data, uint32_t size) {
     return PBIO_ERROR_NOT_SUPPORTED;
 }
-
 static inline pbio_error_t pbsys_storage_get_user_data(uint32_t offset, uint8_t **data, uint32_t size) {
     *data = NULL;
     return PBIO_ERROR_NOT_SUPPORTED;
 }
 
-
 #endif // PBSYS_CONFIG_STORAGE
 
 #endif // _PBSYS_STORAGE_H_

lib/pbio/include/pbsys/storage_settings.h

@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2024 The Pybricks Authors
+
+/**
+ * @addtogroup SysStorageSettings System: Load user settings.
+ *
+ * Interface for reading and storing user system settings.
+ *
+ * @{
+ */
+
+#ifndef _PBSYS_STORAGE_SETTINGS_H_
+#define _PBSYS_STORAGE_SETTINGS_H_
+
+#include <stdint.h>
+
+#include <pbio/error.h>
+
+#include <pbsys/config.h>
+
+/**
+ * System setting flags
+ */
+typedef enum {
+    /**
+     * Bluetooth is enabled by the user (defaults to true).
+     */
+    PBSYS_STORAGE_SETTINGS_FLAGS_BLUETOOTH_ENABLED = (1 << 0),
+} pbsys_storage_settings_flags_t;
+
+/**
+ * System settings. All data types are little-endian.
+ */
+typedef struct _pbsys_storage_settings_t {
+    uint32_t flags;
+} pbsys_storage_settings_t;
+
+#if PBSYS_CONFIG_STORAGE
+
+void pbsys_storage_set_default_settings(pbsys_storage_settings_t *settings);
+
+bool pbsys_storage_settings_bluetooth_enabled(void);
+
+void pbsys_storage_settings_bluetooth_enabled_request_toggle(void);
+
+#else
+
+static inline void pbsys_storage_set_default_settings(pbsys_storage_settings_t *settings) {
+}
+static inline bool pbsys_storage_settings_bluetooth_enabled(void) {
+    return true;
+}
+static inline void pbsys_storage_settings_bluetooth_enabled_request_toggle(void) {
+}
+
+#endif // PBSYS_CONFIG_STORAGE
+
+#endif // _PBSYS_STORAGE_SETTINGS_H_
+
+/** @} */

lib/pbio/sys/bluetooth.c

@@ -21,8 +21,10 @@
 #include <pbsys/bluetooth.h>
 #include <pbsys/command.h>
 #include <pbsys/status.h>
+#include <pbsys/storage.h>
 
 #include "light.h"
+#include "storage.h"
 
 // REVISIT: this can be the negotiated MTU - 3 to allow for better throughput
 #define MAX_CHAR_SIZE 20
@@ -45,6 +47,10 @@ static bool send_busy;
 
 PROCESS(pbsys_bluetooth_process, "Bluetooth");
 
+void pbsys_bluetooth_process_poll(void) {
+    process_poll(&pbsys_bluetooth_process);
+}
+
 // Internal API
 
 /** Initializes Bluetooth. */
@@ -178,7 +184,7 @@ pbio_error_t pbsys_bluetooth_tx(const uint8_t *data, uint32_t *size) {
 
     // poke the process to start tx soon-ish. This way, we can accumulate up to
     // MAX_CHAR_SIZE bytes before actually transmitting
-    process_poll(&pbsys_bluetooth_process);
+    pbsys_bluetooth_process_poll();
 
     return PBIO_SUCCESS;
 }
@@ -198,39 +204,8 @@ bool pbsys_bluetooth_tx_is_idle(void) {
     return !send_busy && lwrb_get_full(&stdout_ring_buf) == 0;
 }
 
-static bool pbsys_bluetooth_user_enabled = true;
-
-bool pbsys_bluetooth_is_user_enabled(void) {
-    return pbsys_bluetooth_user_enabled;
-}
-
-void pbsys_bluetooth_is_user_enabled_request_toggle(void) {
-
-    // Ignore toggle request in all but idle system status.
-    if (pbsys_status_test(PBIO_PYBRICKS_STATUS_USER_PROGRAM_RUNNING)
-        || pbsys_status_test(PBIO_PYBRICKS_STATUS_POWER_BUTTON_PRESSED)
-        || pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN)
-        || pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN_REQUEST)
-        // Ignore toggle is Bluetooth is currently being used in a connection.
-        || pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_LE)
-        || pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_PERIPHERAL)
-        // Ignore if last request not yet finished processing.
-        || pbsys_bluetooth_user_enabled != pbdrv_bluetooth_is_ready()
-        ) {
-        return;
-    }
-
-    // Toggle the user enabled state and poll process to take action.
-    pbsys_bluetooth_user_enabled = !pbsys_bluetooth_user_enabled;
-    process_poll(&pbsys_bluetooth_process);
-}
-
 // Contiki process
 
-static void on_event(void) {
-    process_poll(&pbsys_bluetooth_process);
-}
-
 static pbio_pybricks_error_t handle_receive(pbdrv_bluetooth_connection_t connection, const uint8_t *data, uint32_t size) {
     if (connection == PBDRV_BLUETOOTH_CONNECTION_PYBRICKS) {
         return pbsys_command(data, size);
@@ -256,7 +231,7 @@ static void send_done(void) {
     }
 
     send_busy = false;
-    process_poll(&pbsys_bluetooth_process);
+    pbsys_bluetooth_process_poll();
 }
 
 // drain all buffers and queues and reset global state
@@ -316,16 +291,28 @@ PROCESS_THREAD(pbsys_bluetooth_process, ev, data) {
 
     PROCESS_BEGIN();
 
-    pbdrv_bluetooth_set_on_event(on_event);
+    // Ensures Bluetooth preferences are loaded before we read them.
+    #if PBSYS_CONFIG_STORAGE && PBSYS_CONFIG_BLUETOOTH_TOGGLE
+    PROCESS_WAIT_EVENT_UNTIL(pbsys_storage_get_settings() != NULL);
+    #endif // PBSYS_CONFIG_STORAGE && PBSYS_CONFIG_BLUETOOTH_TOGGLE
+
+    pbdrv_bluetooth_set_on_event(pbsys_bluetooth_process_poll);
     pbdrv_bluetooth_set_receive_handler(handle_receive);
 
     while (!pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN)) {
+
+        // Show inactive status only if user requested Bluetooth as disabled to
+        // avoid always flashing red in between program runs when disconnected.
+        if (!pbsys_storage_settings_bluetooth_enabled()) {
+            pbsys_status_light_bluetooth_set_color(PBIO_COLOR_RED);
+        }
+
         // make sure the Bluetooth chip is in reset long enough to actually reset
         etimer_set(&timer, 150);
         PROCESS_WAIT_EVENT_UNTIL(ev == PROCESS_EVENT_TIMER && etimer_expired(&timer));
 
         // Wait until Bluetooth enabled requested by user, but stop waiting on shutdown.
-        PROCESS_WAIT_UNTIL(pbsys_bluetooth_user_enabled || pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN));
+        PROCESS_WAIT_UNTIL(pbsys_storage_settings_bluetooth_enabled() || pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN));
         if (pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN)) {
             break;
         }
@@ -346,7 +333,7 @@ PROCESS_THREAD(pbsys_bluetooth_process, ev, data) {
             pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_LE)
             || pbsys_status_test(PBIO_PYBRICKS_STATUS_USER_PROGRAM_RUNNING)
             || pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN)
-            || !pbsys_bluetooth_user_enabled);
+            || !pbsys_storage_settings_bluetooth_enabled());
 
         // Now change the state depending on which of the above was triggered.
 
@@ -365,7 +352,7 @@ PROCESS_THREAD(pbsys_bluetooth_process, ev, data) {
         // The Bluetooth enabled flag can only change while disconnected and
         // while no program is running. So here it just serves to skip the
         // Bluetooth loop below and go directly to the disable step below it.
-        while (pbsys_bluetooth_user_enabled
+        while (pbsys_storage_settings_bluetooth_enabled()
                && pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_LE)
                && !pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN)) {
 
@@ -404,12 +391,6 @@ PROCESS_THREAD(pbsys_bluetooth_process, ev, data) {
         reset_all();
         PROCESS_WAIT_WHILE(pbsys_status_test(PBIO_PYBRICKS_STATUS_USER_PROGRAM_RUNNING));
 
-        // Indicate status only if user requested Bluetooth to be disabled to
-        // avoid always flashing red in between program runs when disconnected.
-        if (!pbsys_bluetooth_user_enabled) {
-            pbsys_status_light_bluetooth_set_color(PBIO_COLOR_RED);
-        }
-
         // reset Bluetooth chip
         pbdrv_bluetooth_power_on(false);
         PROCESS_WAIT_WHILE(pbdrv_bluetooth_is_ready());

lib/pbio/sys/bluetooth.h

@@ -8,5 +8,6 @@
 
 uint32_t pbsys_bluetooth_rx_get_free(void);
 void pbsys_bluetooth_rx_write(const uint8_t *data, uint32_t size);
+void pbsys_bluetooth_process_poll(void);
 
 #endif // _PBSYS_SYS_BLUETOOTH_H_

lib/pbio/sys/command.c

@@ -7,6 +7,8 @@
 #include <pbdrv/reset.h>
 #include <pbio/protocol.h>
 
+#include <pbsys/storage.h>
+
 #include "./bluetooth.h"
 #include "./storage.h"
 #include "./program_stop.h"

lib/pbio/sys/hmi.c

@@ -19,9 +19,9 @@
 #include <pbio/color.h>
 #include <pbio/event.h>
 #include <pbio/light.h>
-#include <pbsys/bluetooth.h>
 #include <pbsys/config.h>
 #include <pbsys/status.h>
+#include <pbsys/storage_settings.h>
 
 #include "light_matrix.h"
 #include "light.h"
@@ -81,7 +81,7 @@ static PT_THREAD(update_bluetooth_button_wait_state(bool button_pressed)) {
         // button may still be pressed during user program
         PT_WAIT_UNTIL(pt, !button_pressed);
         PT_WAIT_UNTIL(pt, button_pressed);
-        pbsys_bluetooth_is_user_enabled_request_toggle();
+        pbsys_storage_settings_bluetooth_enabled_request_toggle();
     }
 
     PT_END(pt);