From d71a0db7f23ecff70f13ce5384cc44ca4507470e Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Tue, 3 Sep 2024 11:26:26 +0200 Subject: [PATCH] pbio/sys/storage: Make map private. This was only public so that we could use a #define based on the struct size. We'll use a function instead. The maximum program size may not be constant when we add slots. --- bricks/_common/micropython.c | 2 +- .../drv/bluetooth/bluetooth_stm32_bluenrg.c | 2 +- .../drv/bluetooth/bluetooth_stm32_cc2640.c | 2 +- .../drv/bluetooth/pybricks_service_server.c | 2 +- lib/pbio/include/pbsys/storage.h | 55 ++-------------- lib/pbio/sys/storage.c | 64 ++++++++++++++++++- 6 files changed, 69 insertions(+), 58 deletions(-) diff --git a/bricks/_common/micropython.c b/bricks/_common/micropython.c index 6ddc1084d..37fa6aea4 100644 --- a/bricks/_common/micropython.c +++ b/bricks/_common/micropython.c @@ -321,7 +321,7 @@ pbio_error_t pbsys_main_program_validate(pbsys_main_program_t *program) { // If requesting a user program, ensure that it exists and is valid. uint32_t program_size = program->code_end - program->code_start; - if (program_size == 0 || program_size > PBSYS_STORAGE_MAX_PROGRAM_SIZE) { + if (program_size == 0 || program_size > pbsys_storage_get_maximum_program_size()) { return PBIO_ERROR_NOT_SUPPORTED; } diff --git a/lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c b/lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c index 2d080ae6b..58130ef61 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c +++ b/lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c @@ -1078,7 +1078,7 @@ static PT_THREAD(init_pybricks_service(struct pt *pt)) { PT_WAIT_WHILE(pt, write_xfer_size); { uint8_t buf[PBIO_PYBRICKS_HUB_CAPABILITIES_VALUE_SIZE]; - pbio_pybricks_hub_capabilities(buf, ATT_MTU - 3, PBSYS_CONFIG_APP_FEATURE_FLAGS, PBSYS_STORAGE_MAX_PROGRAM_SIZE); + pbio_pybricks_hub_capabilities(buf, ATT_MTU - 3, PBSYS_CONFIG_APP_FEATURE_FLAGS, pbsys_storage_get_maximum_program_size()); aci_gatt_update_char_value_begin(pybricks_service_handle, pybricks_hub_capabilities_char_handle, 0, PBIO_PYBRICKS_HUB_CAPABILITIES_VALUE_SIZE, buf); } diff --git a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c index 817f2cdd5..eefe05c44 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c +++ b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c @@ -1480,7 +1480,7 @@ static void handle_event(uint8_t *packet) { uint8_t buf[PBIO_PYBRICKS_HUB_CAPABILITIES_VALUE_SIZE]; // REVISIT: this assumes connection_handle == conn_handle - pbio_pybricks_hub_capabilities(buf, conn_mtu - 3, PBSYS_CONFIG_APP_FEATURE_FLAGS, PBSYS_STORAGE_MAX_PROGRAM_SIZE); + pbio_pybricks_hub_capabilities(buf, conn_mtu - 3, PBSYS_CONFIG_APP_FEATURE_FLAGS, pbsys_storage_get_maximum_program_size()); rsp.len = sizeof(buf); rsp.pValue = buf; ATT_ReadRsp(connection_handle, &rsp); diff --git a/lib/pbio/drv/bluetooth/pybricks_service_server.c b/lib/pbio/drv/bluetooth/pybricks_service_server.c index 6121279ed..57cd80ec3 100644 --- a/lib/pbio/drv/bluetooth/pybricks_service_server.c +++ b/lib/pbio/drv/bluetooth/pybricks_service_server.c @@ -89,7 +89,7 @@ static uint16_t pybricks_service_read_callback(hci_con_handle_t con_handle, uint if (attribute_handle == pybricks_hub_capabilities_value_handle) { if (buffer && buffer_size >= PBIO_PYBRICKS_HUB_CAPABILITIES_VALUE_SIZE) { pbio_pybricks_hub_capabilities(buffer, pbio_int_math_min(att_server_get_mtu(con_handle) - 3, 512), - PBSYS_CONFIG_APP_FEATURE_FLAGS, PBSYS_STORAGE_MAX_PROGRAM_SIZE); + PBSYS_CONFIG_APP_FEATURE_FLAGS, pbsys_storage_get_maximum_program_size()); } return PBIO_PYBRICKS_HUB_CAPABILITIES_VALUE_SIZE; } diff --git a/lib/pbio/include/pbsys/storage.h b/lib/pbio/include/pbsys/storage.h index c93246ba0..1d94cffd9 100644 --- a/lib/pbio/include/pbsys/storage.h +++ b/lib/pbio/include/pbsys/storage.h @@ -19,56 +19,7 @@ #if PBSYS_CONFIG_STORAGE -// Sanity check that application RAM is enough to load ROM and still do something useful -#if PBSYS_CONFIG_STORAGE_RAM_SIZE < PBSYS_CONFIG_STORAGE_ROM_SIZE + 2048 -#error "Application RAM must be at least ROM size + 2K." -#endif - -/** - * Map of loaded data. All data types are little-endian. - */ -typedef struct _pbsys_storage_data_map_t { - /** - * How much to write on shutdown (and how much to load on boot). This is - * reset to 0 in RAM on load, and should be set whenever any data is - * updated. This must always remain the first element of this structure. - */ - uint32_t write_size; - #if PBSYS_CONFIG_STORAGE_OVERLAPS_BOOTLOADER_CHECKSUM - /** - * Checksum complement to satisfy bootloader requirements. This ensures - * that words in the scanned area still add up to precisely 0 after user - * data was written. - */ - 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. - */ - 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). - */ - uint32_t program_size; - /** - * Data of the application program (code + heap). - */ - uint8_t program_data[] __attribute__((aligned(sizeof(void *)))); -} pbsys_storage_data_map_t; - -#define PBSYS_STORAGE_MAX_PROGRAM_SIZE (PBSYS_CONFIG_STORAGE_ROM_SIZE - sizeof(pbsys_storage_data_map_t)) +uint32_t pbsys_storage_get_maximum_program_size(void); pbio_error_t pbsys_storage_set_user_data(uint32_t offset, const uint8_t *data, uint32_t size); @@ -76,7 +27,9 @@ pbio_error_t pbsys_storage_get_user_data(uint32_t offset, uint8_t **data, uint32 #else -#define PBSYS_STORAGE_MAX_PROGRAM_SIZE (0) +static inline uint32_t pbsys_storage_get_maximum_program_size(void) { + return 0; +} 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; diff --git a/lib/pbio/sys/storage.c b/lib/pbio/sys/storage.c index d53154424..cac5b31c7 100644 --- a/lib/pbio/sys/storage.c +++ b/lib/pbio/sys/storage.c @@ -21,16 +21,74 @@ #include "core.h" +/** + * Map of loaded data. All data types are little-endian. + */ +typedef struct { + /** + * How much to write on shutdown (and how much to load on boot). This is + * reset to 0 in RAM on load, and should be set whenever any data is + * updated. This must always remain the first element of this structure. + */ + uint32_t write_size; + #if PBSYS_CONFIG_STORAGE_OVERLAPS_BOOTLOADER_CHECKSUM + /** + * Checksum complement to satisfy bootloader requirements. This ensures + * that words in the scanned area still add up to precisely 0 after user + * data was written. + */ + 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. + */ + 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). + */ + uint32_t program_size; + /** + * Data of the application program (code + heap). + */ + uint8_t program_data[] __attribute__((aligned(sizeof(void *)))); +} pbsys_storage_data_map_t; + /** * Map of loaded data sits at the start of user RAM. */ -union { +static union { pbsys_storage_data_map_t data_map; uint8_t data[PBSYS_CONFIG_STORAGE_RAM_SIZE]; } pbsys_user_ram_data_map __attribute__((section(".noinit"), used)); +// Application RAM must enough to load ROM and still do something useful. +#if PBSYS_CONFIG_STORAGE_RAM_SIZE < PBSYS_CONFIG_STORAGE_ROM_SIZE + 2048 +#error "Application RAM must be at least ROM size + 2K." +#endif + static pbsys_storage_data_map_t *map = &pbsys_user_ram_data_map.data_map; +/** + * Gets the maximum size of a program that can be downloaded to the hub. + * + * @returns Maximum program size in bytes. + */ +uint32_t pbsys_storage_get_maximum_program_size(void) { + return PBSYS_CONFIG_STORAGE_ROM_SIZE - sizeof(pbsys_storage_data_map_t); +} + static bool data_map_is_loaded = false; pbsys_storage_settings_t *pbsys_storage_settings_get_settings(void) { @@ -161,7 +219,7 @@ pbio_error_t pbsys_storage_set_program_size(uint32_t size) { * Otherwise ::PBIO_SUCCESS. */ pbio_error_t pbsys_storage_set_program_data(uint32_t offset, const void *data, uint32_t size) { - if (offset + size > PBSYS_STORAGE_MAX_PROGRAM_SIZE) { + if (offset + size > pbsys_storage_get_maximum_program_size()) { return PBIO_ERROR_INVALID_ARG; } @@ -183,7 +241,7 @@ pbio_error_t pbsys_storage_set_program_data(uint32_t offset, const void *data, u void pbsys_storage_get_program_data(pbsys_main_program_t *program) { program->code_start = map->program_data; program->code_end = map->program_data + map->program_size; - program->data_end = map->program_data + PBSYS_STORAGE_MAX_PROGRAM_SIZE; + program->data_end = map->program_data + pbsys_storage_get_maximum_program_size(); } PROCESS(pbsys_storage_process, "storage");