Skip to content

Commit

Permalink
Support reading and writing data larger than shared buffer to EEPROM (#…
Browse files Browse the repository at this point in the history 
…18)

* another crack at progressive import/export

* bump node

* bump thingset node

* code review fixes

* extend test to also cover progressive read/write

* move config to right place

* fix testcase yaml

---------

Co-authored-by: Gareth Potter <gareth.potter@brillpower.com>
  • Loading branch information
@garethpotter
garethpotter and garethpotter authored Mar 26, 2024
1 parent c0805aa commit bd8d916
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 26 deletions.
8 changes: 8 additions & 0 deletions src/Kconfig.storage
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,12 @@ config THINGSET_STORAGE_DATA_VERSION
Warning: This will discard previously stored data in order to prevent data corruption. Try
to avoid changing data object IDs used previously.

config THINGSET_STORAGE_EEPROM_PROGRESSIVE_IMPORT_EXPORT
bool "Enable progressive import/export for EEPROM storage."
select THINGSET_PROGRESSIVE_IMPORT_EXPORT
default n
help
When enabled, allows the loading and saving of data larger than the shared buffer
in EEPROM.

endif # THINGSET_STORAGE
138 changes: 114 additions & 24 deletions src/storage_eeprom.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,35 +65,82 @@ int thingset_storage_load()

struct shared_buffer *sbuf = thingset_sdk_shared_buffer();

k_sem_take(&sbuf->lock, K_FOREVER);

if (header.data_len > sbuf->size) {
LOG_ERR("EEPROM buffer too small (%d bytes required)", header.data_len);
return -ENOMEM;
}
#ifdef CONFIG_THINGSET_STORAGE_EEPROM_PROGRESSIVE_IMPORT_EXPORT
uint32_t calculated_crc = 0x0;
uint32_t last_id = 0;
size_t processed_size = 0;
size_t total_read_size = sizeof(header);
size_t len = header.data_len;
do {
int size = len > sbuf->size ? sbuf->size : len;
LOG_DBG("Reading %d bytes starting at offset %d", size, total_read_size);
err = eeprom_read(eeprom_dev, total_read_size, sbuf->data, size);
if (err) {
LOG_ERR("Error %d reading EEPROM.", -err);
break;
}

err =
thingset_import_data_progressively(&ts, sbuf->data, size, THINGSET_BIN_IDS_VALUES,
THINGSET_WRITE_MASK, &last_id, &processed_size);
calculated_crc = crc32_ieee_update(calculated_crc, sbuf->data, processed_size);
total_read_size += processed_size;
len -= processed_size;
} while (len > 0 && err > 0);
LOG_DBG("Finished processing %d bytes; calculated CRC %.8x",
total_read_size - sizeof(header), calculated_crc);
if (!err) {
thingset_import_data_progressively_end(&ts);
}

k_sem_take(&sbuf->lock, K_FOREVER);
if (calculated_crc == header.crc) {
if (!err) {
LOG_DBG("EEPROM read and data successfully updated");
}
else {
LOG_ERR("Importing data failed with ThingSet response code 0x%X", -err);
err = -EINVAL;
}
}
else {
LOG_ERR("EEPROM data CRC invalid, expected 0x%x and data_len %d", header.crc, len);
err = -EINVAL;
}

err = eeprom_read(eeprom_dev, sizeof(header), sbuf->data, header.data_len);
if (err != 0) {
LOG_ERR("EEPROM read failed: %d", err);
goto out;
#else
LOG_ERR("EEPROM buffer too small (%d bytes required)", header.data_len);
err = -ENOMEM;
goto out;
#endif /* CONFIG_THINGSET_STORAGE_EEPROM_PROGRESSIVE_IMPORT_EXPORT */
}
else {
err = eeprom_read(eeprom_dev, sizeof(header), sbuf->data, header.data_len);
if (err != 0) {
LOG_ERR("EEPROM read failed: %d", err);
goto out;
}

if (crc32_ieee(sbuf->data, header.data_len) == header.crc) {
int status = thingset_import_data(&ts, sbuf->data, header.data_len, THINGSET_WRITE_MASK,
THINGSET_BIN_IDS_VALUES);
if (status == 0) {
LOG_DBG("EEPROM read and data successfully updated");
if (crc32_ieee(sbuf->data, header.data_len) == header.crc) {
int status = thingset_import_data(&ts, sbuf->data, header.data_len, THINGSET_WRITE_MASK,
THINGSET_BIN_IDS_VALUES);
if (status == 0) {
LOG_DBG("EEPROM read and data successfully updated");
}
else {
LOG_ERR("Importing data failed with ThingSet response code 0x%X", -status);
err = -EINVAL;
}
}
else {
LOG_ERR("Importing data failed with ThingSet response code 0x%X", -status);
LOG_ERR("EEPROM data CRC invalid, expected 0x%x and data_len %d", header.crc,
header.data_len);
err = -EINVAL;
}
}
else {
LOG_ERR("EEPROM data CRC invalid, expected 0x%x and data_len %d", header.crc,
header.data_len);
err = -EINVAL;
}

out:
k_sem_give(&sbuf->lock);
Expand All @@ -113,16 +160,59 @@ int thingset_storage_save()
struct shared_buffer *sbuf = thingset_sdk_shared_buffer();
k_sem_take(&sbuf->lock, K_FOREVER);

struct thingset_eeprom_header header = { .version = CONFIG_THINGSET_STORAGE_DATA_VERSION };

#ifdef CONFIG_THINGSET_STORAGE_EEPROM_PROGRESSIVE_IMPORT_EXPORT
LOG_DBG("Initialising with buffer of size %d", sbuf->size);

int rtn;
int i = 0;
size_t size;
size_t total_size = sizeof(header);
uint32_t crc = 0x0;
do {
rtn = thingset_export_subsets_progressively(&ts, sbuf->data, sbuf->size, TS_SUBSET_NVM,
THINGSET_BIN_IDS_VALUES, &i, &size);
if (rtn < 0) {
LOG_ERR("ThingSet data export error 0x%x", -rtn);
err = -EINVAL;
break;
}
crc = crc32_ieee_update(crc, sbuf->data, size);
LOG_DBG("Writing %d bytes to EEPROM", size);
err = eeprom_write(eeprom_dev, total_size, sbuf->data, size);
if (err) {
LOG_ERR("EEPROM write error %d", err);
break;
}
total_size += size;
} while (rtn > 0 && err == 0);
if (!err) {
total_size -= sizeof(header);
LOG_DBG("Wrote a total of %d bytes comprising %d items with checksum %.8x; writing "
"header",
total_size, i, crc);

/* now write the header */
header.data_len = (uint16_t)total_size;
header.crc = crc;
err = eeprom_write(eeprom_dev, 0, &header, sizeof(header));
LOG_DBG("EEPROM data successfully stored");
}
else {
LOG_ERR("EEPROM write error %d", -err);
err = -EINVAL;
}

goto out;
#else
int len = thingset_export_subsets(&ts, sbuf->data, sbuf->size, TS_SUBSET_NVM,
THINGSET_BIN_IDS_VALUES);
if (len > 0) {
uint32_t crc = crc32_ieee(sbuf->data, len);

struct thingset_eeprom_header header = {
.version = CONFIG_THINGSET_STORAGE_DATA_VERSION,
.data_len = (uint16_t)len,
.crc = crc,
};
header.data_len = (uint16_t)len;
header.crc = crc;

LOG_DBG("EEPROM header: ver %d, len %d, CRC %.8x", CONFIG_THINGSET_STORAGE_DATA_VERSION,
len, crc);
Expand All @@ -145,7 +235,7 @@ int thingset_storage_save()
LOG_ERR("Exporting data failed with ThingSet response code 0x%X", -len);
err = -EINVAL;
}

#endif /* CONFIG_THINGSET_STORAGE_EEPROM_PROGRESSIVE_IMPORT_EXPORT */
out:
k_sem_give(&sbuf->lock);

Expand Down
11 changes: 10 additions & 1 deletion tests/storage_eeprom/testcase.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
# SPDX-License-Identifier: Apache-2.0

tests:
thingset_sdk.storage_eeprom:
thingset_sdk.storage_eeprom.default:
integration_platforms:
- native_posix_64
platform_exclude:
# native sim with at24 emul EEPROM currently fails for unknown reasons
- native_sim
extra_args: EXTRA_CFLAGS=-Werror
thingset_sdk.storage_eeprom.progressive:
integration_platforms:
- native_posix_64
platform_exclude:
# native sim with at24 emul EEPROM currently fails for unknown reasons
- native_sim
extra_args: EXTRA_CFLAGS=-Werror
extra_configs:
- CONFIG_THINGSET_STORAGE_EEPROM_PROGRESSIVE_IMPORT_EXPORT=y
2 changes: 1 addition & 1 deletion west.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ manifest:
- picolibc
- name: thingset-node-c
remote: thingset
revision: ac06ae5832d76582053ab82a650c9c25e80378de
revision: 8de7f185eb62bed1346de52e4a2d5f5d616a7a61
path: modules/thingset-node-c
import: true

0 comments on commit bd8d916

Please sign in to comment.