From ba5770ce9bfb6028e337e5064fafe337ad5bc05b Mon Sep 17 00:00:00 2001 From: Chris Vig Date: Tue, 9 Sep 2025 16:20:41 -0500 Subject: [PATCH] Refactor storage system to support double buffering. --- src/main/application/config.c | 61 ++++-- src/main/application/config.h | 3 +- src/main/application/intf_types.h | 4 +- src/main/application/storage.c | 312 +++++++++++++++++++++++------- src/main/application/storage.h | 24 ++- src/main/utility/utility.h | 7 + 6 files changed, 314 insertions(+), 97 deletions(-) diff --git a/src/main/application/config.c b/src/main/application/config.c index 15723c4..5e392c7 100644 --- a/src/main/application/config.c +++ b/src/main/application/config.c @@ -24,24 +24,53 @@ /* --------------------------------------------------- CONSTANTS ---------------------------------------------------- */ -/** - * @def CONFIG_VERSION_CURRENT - * @brief The currently active configuration version. - */ -#define CONFIG_VERSION_CURRENT ( 1 ) - /** * @def MINIMUM_SAVE_PERIOD * @brief Minimum elapsed time between saving config to storage. */ #define MINIMUM_SAVE_PERIOD ( 5 * TICKS_PER_SEC ) +/* ----------------------------------------------------- TYPES ------------------------------------------------------ */ + +/** + * @typedef config_version_t + * @brief Enumeration of the supported configuration versions. + * @note This is to support backwards compatibility with saved configuration when the software changes. + */ +typedef uint8_t config_version_t; +enum +{ + CONFIG_VERSION_0, /**< Configuration version 0. */ + + CONFIG_VERSION_COUNT, /**< Number of valid configuration versions.*/ + + CONFIG_VERSION_CURRENT /**< Current configuration version. */ + = CONFIG_VERSION_0 +}; + +/** + * @typedef config_storage_t + * @brief Struct defining data saved in storage for unit configuration. + */ +typedef struct +{ + config_version_t version; /**< Saved version number. */ + union + { + config_t current; /**< Configuration data as current value. */ + }; + +} config_storage_t; + /* ----------------------------------------------------- MACROS ----------------------------------------------------- */ _Static_assert( _CONFIG_DFLT_BUZZER_FREQUENCY >= BUZZER_MINIMUM_FREQUENCY && _CONFIG_DFLT_BUZZER_FREQUENCY <= BUZZER_MAXIMUM_FREQUENCY, "Invalid default buzzer frequency!" ); +_Static_assert( STORAGE_CONFIG_SIZE >= sizeof( config_storage_t ), + "Not enough storage allocated for configuration!" ); + /* --------------------------------------------------- VARIABLES ---------------------------------------------------- */ static config_t s_config; /**< Currently active app configuration. */ @@ -129,13 +158,17 @@ void config_init( void ) { // Try to get configuration from storage, and restore defaults if that fails. // In this case, any existing configuration will be lost the next time config is saved. - config_t config; - if( ! storage_get_config( CONFIG_VERSION_CURRENT, sizeof( config_t ), & config ) || - ! validate_config( & config ) ) - config_default( & config ); + // In the future, this could do migration from previous configuration versions. + config_storage_t storage; + if( ! storage_get_config( & storage, sizeof( storage ) ) || + storage.version != CONFIG_VERSION_CURRENT || + ! validate_config( & storage.current ) ) + { + config_default( & storage.current ); + } // Set local copy - s_config = config; + s_config = storage.current; } /* config_init() */ @@ -170,7 +203,11 @@ void config_tick( tick_t tick ) static void flush( tick_t tick ) { - storage_set_config( CONFIG_VERSION_CURRENT, sizeof( config_t ), & s_config ); + config_storage_t storage; + storage.version = CONFIG_VERSION_CURRENT; + storage.current = s_config; + + storage_set_config( & storage, sizeof( storage ) ); s_modified = false; s_save_tick = tick; diff --git a/src/main/application/config.h b/src/main/application/config.h index 18d111d..616ef68 100644 --- a/src/main/application/config.h +++ b/src/main/application/config.h @@ -24,6 +24,7 @@ #include "application/keyer.h" #include "application/led.h" #include "application/wpm.h" +#include "utility/utility.h" /* ----------------------------------------------------- TYPES ------------------------------------------------------ */ @@ -60,7 +61,7 @@ typedef struct /** If set to `true`, the keyer will emit dashes from the left paddle and dots from the right paddle. */ bool keyer_paddle_invert; -} __attribute__((packed)) config_t; +} PACKED_STRUCT config_t; /** * @typedef config_version_t diff --git a/src/main/application/intf_types.h b/src/main/application/intf_types.h index 60a6f13..2602b56 100644 --- a/src/main/application/intf_types.h +++ b/src/main/application/intf_types.h @@ -17,6 +17,8 @@ #include +#include "utility/utility.h" + /* --------------------------------------------------- CONSTANTS ---------------------------------------------------- */ /** @@ -91,6 +93,6 @@ typedef struct uint16_t size; /**< Total size of message payload. */ uint16_t crc; /**< 16-bit CRC of message payload. */ -} __attribute__((packed)) intf_header_t; +} PACKED_STRUCT intf_header_t; #endif /* !defined( APPLICATION_INTF_TYPES_H ) */ diff --git a/src/main/application/storage.c b/src/main/application/storage.c index 3e3cb90..d89e2f6 100644 --- a/src/main/application/storage.c +++ b/src/main/application/storage.c @@ -9,122 +9,288 @@ /* ---------------------------------------------------- INCLUDES ---------------------------------------------------- */ +#include +#include #include +#include #include #include "application/storage.h" #include "drivers/eeprom.h" #include "utility/crc.h" +#include "utility/debug.h" #include "utility/types.h" - -/* --------------------------------------------------- CONSTANTS ---------------------------------------------------- */ - -// Same for all layout verions -#define EEPROM_ADDR_RESERVED_START 0x0000 -#define EEPROM_ADDR_LAYOUT_VERSION ( EEPROM_ADDR_RESERVED_START ) -#define EEPROM_ADDR_RESERVED_END EEPROM_ADDR_RESERVED_START + 0x0100 - -// EEPROM addresses for version 1 -#define EEPROM_ADDR_V1_CONFIG_BASE EEPROM_ADDR_RESERVED_END -#define EEPROM_ADDR_V1_CONFIG_VERSION ( EEPROM_ADDR_V1_CONFIG_BASE ) -#define EEPROM_ADDR_V1_CONFIG_SIZE ( EEPROM_ADDR_V1_CONFIG_VERSION + sizeof( config_version_t ) ) -#define EEPROM_ADDR_V1_CONFIG_CRC16 ( EEPROM_ADDR_V1_CONFIG_SIZE + sizeof( size_t ) ) -#define EEPROM_ADDR_V1_CONFIG_DATA ( EEPROM_ADDR_V1_CONFIG_CRC16 + sizeof( crc16_t ) ) -#define EEPROM_ADDR_V1_CONFIG_END EEPROM_ADDR_V1_CONFIG_BASE + 0x0100 -_Static_assert( EEPROM_ADDR_V1_CONFIG_DATA + sizeof( config_t ) < EEPROM_ADDR_V1_CONFIG_END, - "Not enough space allocated!" ); - -// EEPROM addresses for current version -#define EEPROM_ADDR_CONFIG_VERSION EEPROM_ADDR_V1_CONFIG_VERSION -#define EEPROM_ADDR_CONFIG_SIZE EEPROM_ADDR_V1_CONFIG_SIZE -#define EEPROM_ADDR_CONFIG_CRC16 EEPROM_ADDR_V1_CONFIG_CRC16 -#define EEPROM_ADDR_CONFIG_DATA EEPROM_ADDR_V1_CONFIG_DATA +#include "utility/utility.h" /* ----------------------------------------------------- TYPES ------------------------------------------------------ */ /** - * @typedef layout_version_t - * @brief Enumeration of supported storage layout versions. + * @typedef layout_t + * @brief Enumeration of the known storage layouts. */ -typedef uint8_t layout_version_t; +typedef uint8_t layout_t; enum { - LAYOUT_VERSION_INVALID_0 = 0x00, - LAYOUT_VERSION_INVALID_1 = 0xFF, + LAYOUT_V1 = 0x01, - LAYOUT_VERSION_1 = 0x01, + LAYOUT_CURRENT = LAYOUT_V1, - LAYOUT_VERSION_CURRENT - = LAYOUT_VERSION_1, + LAYOUT_INVALID_0x00 = 0x00, + LAYOUT_INVALID_0xFF = 0xFF, }; -/* ---------------------------------------------- PROCEDURE PROTOTYPES ---------------------------------------------- */ +/** + * @typedef slot_t + * @brief Enumeration of the data "slots" that the storage system supports. + * @note The intent of the slot system is to double-buffer stored data, so that a power loss or other failure while + * writing will not corrupt the valid data in the other slot. + */ +typedef uint8_t slot_t; +enum +{ + SLOT_0, /**< First slot. */ + SLOT_1, /**< Second slot. */ + + SLOT_COUNT, /**< Number of slots. */ +}; /** - * @fn reinit_layout( void ) - * @brief Re-initializes the storage layout. + * @struct storage_header_t + * @brief Struct defining the layout for a storage header. + * @note This type is not intended to be instantiated - it is used only to calculate offsets. */ -static void reinit_layout( void ); +typedef struct +{ + bool valid; /**< Is data in this slot valid? */ + size_t size; /**< Size of data in this slot. */ + crc16_t crc; /**< CRC for this slot. */ -/* --------------------------------------------------- PROCEDURES --------------------------------------------------- */ +} PACKED_STRUCT storage_header_t; -bool storage_get_config( config_version_t version, size_t size, void * config ) +/** + * @struct storage_config_t + * @brief Struct defining the layout for each configuration slot. + * @note This type is not intended to be instantiated - it is used only to calculate offsets. + */ +typedef struct { - // Validate version - config_version_t check_version; - eeprom_read( EEPROM_ADDR_CONFIG_VERSION, & check_version, sizeof( config_version_t ) ); - if( check_version != version ) - return( false ); + storage_header_t header; /**< Header for this slot. */ + byte_t data[ STORAGE_CONFIG_SIZE ]; /**< Data for this slot. */ - // Validate size - size_t check_size; - eeprom_read( EEPROM_ADDR_CONFIG_SIZE, & check_size, sizeof( size_t ) ); - if( check_size != size ) - return( false ); +} PACKED_STRUCT storage_config_t; - // Read config - eeprom_read( EEPROM_ADDR_CONFIG_DATA, config, size ); +/** + * @struct storage_t + * @brief Struct defining the overall layout for the entire EEPROM storage. + * @note This type is not intended to be instantiated - it is used only to calculate offsets. + */ +typedef struct +{ + layout_t layout; /**< Storage layout version. */ + storage_config_t config[ SLOT_COUNT ]; /**< Slots for configuration storage. */ - // Validate CRC - crc16_t check_crc; - eeprom_read( EEPROM_ADDR_CONFIG_CRC16, & check_crc, sizeof( crc16_t ) ); - if( check_crc != crc_calc_crc16( config, sizeof( config_t ) ) ) - { - memset( config, 0, size ); - return( false ); - } +} PACKED_STRUCT storage_t; - return( true ); +/* ----------------------------------------------------- MACROS ----------------------------------------------------- */ + +_Static_assert( sizeof( storage_t ) <= EEPROM_COUNT, "Too much storage allocated!" ); + +/** + * @def ADDR( _field ) + * @brief Returns the EEPROM address for the specified field defined in `storage_t`. + */ +#define ADDR( _field ) \ + ( ( eeprom_addr_t )( offsetof( storage_t, _field ) ) ) + +/** + * @def OTHER_SLOT( _slot ) + * @brief Returns the opposite slot index. + */ +#define OTHER_SLOT( _slot ) \ + ( ( _slot ) == SLOT_0 ? SLOT_1 : SLOT_0 ) + +/* ---------------------------------------------- PROCEDURE PROTOTYPES ---------------------------------------------- */ + +/** + * @fn config_addresses( slot_t, eeprom_addr_t *, eeprom_addr_t *, eeprom_addr_t *, eeprom_addr_t * ) + * @brief Gets the EEPROM addresses for the configuration data in the specified slot. + */ +static void config_addresses( slot_t slot, + eeprom_addr_t * valid_addr, + eeprom_addr_t * size_addr, + eeprom_addr_t * crc_addr, + eeprom_addr_t * data_addr ); + +/** + * @fn init_layout( void ) + * @brief Initializes the EEPROM layout, invalidating any data currently present. + */ +static void init_layout( void ); + +/** + * @fn is_config_valid( slot_t ) + * @brief Returns `true` if the `valid` flag is set to `true` (i.e., `1`) for the specified configuration data slot. + * @note This only checks the `valid` flag - it does not validate the size or CRC. + */ +static bool is_config_valid( slot_t slot ); + +/** + * @fn read_config( slot_t, void *, size_t ) + * @brief Reads configuration data from the specified slot. + * @returns `true` if the data was successfully read into the buffer. + */ +static bool read_config( slot_t slot, void * data, size_t size ); + +/** + * @fn write_config( slot_t, void const *, size_t ) + * @brief Writes the specified configuration data to the specified slot. + */ +static void write_config( slot_t slot, void const * data, size_t size ); + +/* --------------------------------------------------- PROCEDURES --------------------------------------------------- */ + +bool storage_get_config( void * data, size_t size ) +{ + for( slot_t slot = 0; slot < SLOT_COUNT; slot++ ) + if( read_config( slot, data, size ) ) + return( true ); + + return( false ); } /* storage_get_config() */ void storage_init( void ) { - // Confirm we have the right layout version - // If not... welp? No other versions are known, so nothing to do but reset the entire storage. - layout_version_t layout = ( layout_version_t )eeprom_read_byte( EEPROM_ADDR_LAYOUT_VERSION ); - if( layout != LAYOUT_VERSION_CURRENT ) - reinit_layout(); + // Reinitialize the storage layout if incorrect + layout_t layout = ( layout_t )eeprom_read_byte( ADDR( layout ) ); + if( layout != LAYOUT_CURRENT ) + init_layout(); } /* storage_init() */ -void storage_set_config( config_version_t version, size_t size, void * config ) +void storage_set_config( void const * data, size_t size ) { - crc16_t crc = crc_calc_crc16( config, size ); + // Write to first empty slot + for( slot_t slot = 0; slot < SLOT_COUNT; slot++ ) + { + if( is_config_valid( slot ) ) + continue; + write_config( slot, data, size ); + return; + } - eeprom_write( EEPROM_ADDR_CONFIG_VERSION, & version, sizeof( config_version_t ) ); - eeprom_write( EEPROM_ADDR_CONFIG_SIZE, & size, sizeof( size_t ) ); - eeprom_write( EEPROM_ADDR_CONFIG_CRC16, & crc, sizeof( crc16_t ) ); - eeprom_write( EEPROM_ADDR_CONFIG_DATA, config, size ); + // If no empty slot found, overwrite first slot + write_config( SLOT_0, data, size ); } /* storage_set_config() */ -static void reinit_layout( void ) +static void config_addresses( slot_t slot, + eeprom_addr_t * valid_addr, + eeprom_addr_t * size_addr, + eeprom_addr_t * crc_addr, + eeprom_addr_t * data_addr ) +{ + // Calculate base address + eeprom_addr_t base = ADDR( config ); + base += sizeof( storage_config_t ) * slot; + + // Assign individual addresses + if( valid_addr ) + * valid_addr = base + offsetof( storage_config_t, header.valid ); + if( size_addr ) + * size_addr = base + offsetof( storage_config_t, header.size ); + if( crc_addr ) + * crc_addr = base + offsetof( storage_config_t, header.crc ); + if( data_addr ) + * data_addr = base + offsetof( storage_config_t, data ); + +} /* config_addresses() */ + + +static void init_layout( void ) { - // Write layout byte to required address - eeprom_write_byte( EEPROM_ADDR_LAYOUT_VERSION, ( byte_t )LAYOUT_VERSION_CURRENT ); + // Clear all validity flags + eeprom_write_byte( ADDR( config[ SLOT_0 ].header.valid ), ( byte_t )false ); + eeprom_write_byte( ADDR( config[ SLOT_1 ].header.valid ), ( byte_t )false ); + + // Write layout byte + eeprom_write_byte( ADDR( layout ), ( byte_t )LAYOUT_CURRENT ); + +} /* init_layout() */ + + +static bool is_config_valid( slot_t slot ) +{ + eeprom_addr_t valid_addr = EEPROM_COUNT; + config_addresses( slot, & valid_addr, NULL, NULL, NULL ); + + bool valid = false; + eeprom_read( valid_addr, & valid, sizeof( valid ) ); + + // We do an explicit comparison against true (i.e., 1) here. + // This is to prevent the empty value of 0xFF from being interpreted as a "valid" flag. + return( valid == true ); + +} /* is_config_valid() */ + + +static bool read_config( slot_t slot, void * data, size_t size ) +{ + // Read and check validity flag + if( ! is_config_valid( slot ) ) + return( false ); + + // Get other addresses we need + eeprom_addr_t size_addr, crc_addr, data_addr; + config_addresses( slot, NULL, & size_addr, & crc_addr, & data_addr ); + + // Read and check stored size + size_t stored_size = 0; + eeprom_read( size_addr, & stored_size, sizeof( stored_size ) ); + if( stored_size > size ) + return( false ); + + // Read data into buffer + eeprom_read( data_addr, data, size ); + + // Calculate CRC for data we read out + crc16_t crc = crc_calc_crc16( data, size ); + + // Read and check stored CRC + crc16_t stored_crc = 0; + eeprom_read( crc_addr, & stored_crc, sizeof( stored_crc ) ); + if( stored_crc != crc ) + return( false ); + + return( true ); + +} /* read_config() */ + + +static void write_config( slot_t slot, void const * data, size_t size ) +{ + // Get EEPROM addresses + eeprom_addr_t valid_addr, size_addr, crc_addr, data_addr, other_valid_addr; + config_addresses( slot, & valid_addr, & size_addr, & crc_addr, & data_addr ); + config_addresses( OTHER_SLOT( slot ), & other_valid_addr, NULL, NULL, NULL ); + + // Get metadata + bool valid = true; + crc16_t crc = crc_calc_crc16( data, size ); + + // Write data buffer + eeprom_write( data_addr, data, size ); + + // Write metadata, setting the validity flag last + eeprom_write( size_addr, & size, sizeof( size ) ); + eeprom_write( crc_addr, & crc, sizeof( crc ) ); + eeprom_write( valid_addr, & valid, sizeof( valid ) ); + + // Finally, clear the validity byte for the opposite-side config + valid = false; + eeprom_write( other_valid_addr, & valid, sizeof( valid ) ); -} /* reinit_layout() */ +} /* write_config() */ diff --git a/src/main/application/storage.h b/src/main/application/storage.h index fb6524b..aa5217d 100644 --- a/src/main/application/storage.h +++ b/src/main/application/storage.h @@ -19,18 +19,22 @@ #include #include -#include "application/config.h" +/* --------------------------------------------------- CONSTANTS ---------------------------------------------------- */ + +/** + * @def STORAGE_CONFIG_SIZE + * @brief The number of bytes allocated for each configuration data slot. + * @note The config module should fail a static assert if this size is not large enough. + */ +#define STORAGE_CONFIG_SIZE ( 64 ) /* ---------------------------------------------- PROCEDURE PROTOTYPES ---------------------------------------------- */ /** - * @fn storage_get_config( config_t * ) - * @brief Gets the stored application configuration from storage. - * @returns `true` if the configuration was successfully retrieved. - * @note This will return `false` if the version number does not match the specified value, if the stored size is - * invalid, or if the CRC does not match. + * @fn storage_get_config( void * data, size_t size ) + * @brief Gets the stored configuration from non-volatile memory. */ -bool storage_get_config( config_version_t version, size_t size, void * config ); +bool storage_get_config( void * data, size_t size ); /** * @fn storage_init( void ) @@ -40,9 +44,9 @@ bool storage_get_config( config_version_t version, size_t size, void * config ); void storage_init( void ); /** - * @fn storage_set_config( config_t const * ) - * @brief Stores the specified configuration. + * @fn storage_set_config( void const *, size_t ) + * @brief Sets the stored configuration in non-volatile memory. */ -void storage_set_config( config_version_t version, size_t size, void * config ); +void storage_set_config( void const * data, size_t size ); #endif /* !defined( APPLICATION_STORAGE_H ) */ diff --git a/src/main/utility/utility.h b/src/main/utility/utility.h index 14fb0ba..be5d1ae 100644 --- a/src/main/utility/utility.h +++ b/src/main/utility/utility.h @@ -36,6 +36,13 @@ #define FUNC_NEVER_RETURNS \ __attribute__((noreturn)) +/** + * @def PACKED_STRUCT + * @brief May be placed after a struct definition to ensure that no pad bytes are added. + */ +#define PACKED_STRUCT \ + __attribute__((packed)) + /** * @def array_count( _a ) * @brief Returns the number of elements in the specified array.