From ed10485de948cbaf7add5a5eef2e5e1e248ebeb5 Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Tue, 17 Sep 2024 14:48:12 +0200 Subject: [PATCH] pbio/sys/storage: Delete user data on different builds. We were already deleting it across firmware releases, but this makes it delete data between different local builds, which is helpful for development and testing. --- bricks/_common/micropython.c | 7 +++++++ lib/pbio/include/pbsys/main.h | 12 ++++++++++++ lib/pbio/sys/storage.c | 16 +++++++++------- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/bricks/_common/micropython.c b/bricks/_common/micropython.c index c5fcd6bd9..acf44d76e 100644 --- a/bricks/_common/micropython.c +++ b/bricks/_common/micropython.c @@ -18,6 +18,7 @@ #include #include +#include "genhdr/mpversion.h" #include "shared/readline/readline.h" #include "shared/runtime/gchelper.h" #include "shared/runtime/interrupt_char.h" @@ -326,6 +327,12 @@ pbio_error_t pbsys_main_program_validate(pbsys_main_program_t *program) { return PBIO_SUCCESS; } +const char *pbsys_main_get_application_version_hash(void) { + // This is (somewhat confusingly) passed in as the MICROPY_GIT_HASH. + // REVISIT: Make PYBRICKS_GIT_HASH available in a pbio header via a build step. + return MICROPY_GIT_HASH; +} + // Runs MicroPython with the given program data. void pbsys_main_run_program(pbsys_main_program_t *program) { diff --git a/lib/pbio/include/pbsys/main.h b/lib/pbio/include/pbsys/main.h index e1a47059b..0ce129bd2 100644 --- a/lib/pbio/include/pbsys/main.h +++ b/lib/pbio/include/pbsys/main.h @@ -90,6 +90,13 @@ void pbsys_main_stop_program(bool force_stop); */ bool pbsys_main_stdin_event(uint8_t c); +/** + * Gets the 9-symbol git hash of pybricks. + * + * @returns Git hash string. + */ +const char *pbsys_main_get_application_version_hash(void); + #else // PBSYS_CONFIG_MAIN static inline pbio_error_t pbsys_main_program_request_start(pbio_pybricks_user_program_id_t id) { @@ -110,6 +117,11 @@ static inline bool pbsys_main_stdin_event(uint8_t c) { return false; } +static inline const char *pbsys_main_get_application_version_hash(void) { + return NULL; +} + + #endif // PBSYS_CONFIG_MAIN diff --git a/lib/pbio/sys/storage.c b/lib/pbio/sys/storage.c index 747c72d4f..e7e359ad3 100644 --- a/lib/pbio/sys/storage.c +++ b/lib/pbio/sys/storage.c @@ -66,11 +66,11 @@ typedef struct { volatile uint32_t checksum_complement; #endif /** - * 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. + * First 8 symbols of the git hash of the firmware version used to create + * this data map. If this does not match the version of the running + * firmware, user data will be reset to 0. */ - uint32_t stored_firmware_version; + char stored_firmware_hash[8]; /** * End-user read-write accessible data. Everything after this is also * user-readable but not writable. @@ -437,15 +437,17 @@ PROCESS_THREAD(pbsys_storage_process, ev, data) { // Read the available data into RAM. PROCESS_PT_SPAWN(&pt, pbdrv_block_device_read(&pt, 0, (uint8_t *)map, map->saved_data_size, &err)); - // Test that storage matches current firmware version. - if (err != PBIO_SUCCESS || map->stored_firmware_version != PBIO_HEXVERSION) { + bool is_bad_version = strncmp(map->stored_firmware_hash, pbsys_main_get_application_version_hash(), sizeof(map->stored_firmware_hash)); + + // Test that storage successfully loaded and matches current firmware. + if (err != PBIO_SUCCESS || is_bad_version) { // Reset storage except for program data. It is sufficient to set its // size to 0, which is what happens here since it is in the map. memset(map, 0, sizeof(pbsys_storage_data_map_t)); pbsys_storage_settings_set_defaults(&map->settings); // Set firmware version used to create current storage map. - map->stored_firmware_version = PBIO_HEXVERSION; + strncpy(map->stored_firmware_hash, pbsys_main_get_application_version_hash(), sizeof(map->stored_firmware_hash)); // Ensure new firmware version and default settings are written. pbsys_storage_request_write();