pw_kvs: Alignment handling adjustments
- Rename multi-span Write to WriteAligned to reflect that the number of
bytes written may be larger than the input due to padding.
- Make alignment_bytes_ a member of FlashPartition instead of a virtual
function.
- Enable tests that now pass.
Change-Id: Iad985988ef97607b38fb0c84589cabe752de71a2
diff --git a/pw_kvs/flash_memory.cc b/pw_kvs/flash_memory.cc
index b5b72ae..ca5f5ff 100644
--- a/pw_kvs/flash_memory.cc
+++ b/pw_kvs/flash_memory.cc
@@ -47,7 +47,7 @@
return flash_.Write(PartitionToFlashAddress(address), data);
}
-StatusWithSize FlashPartition::Write(
+StatusWithSize FlashPartition::WriteAligned(
const Address start_address, std::initializer_list<span<const byte>> data) {
byte buffer[64]; // TODO: Configure this?
@@ -89,7 +89,7 @@
!status.ok()) {
return StatusWithSize(status, bytes_written());
}
- address += remaining_write_size;
+ address += remaining_write_size; // Include padding bytes in the total.
}
return StatusWithSize(bytes_written());
diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index 5f5bddc..5663091 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -139,8 +139,6 @@
Status KeyValueStore::LoadEntry(Address entry_address,
Address* next_entry_address) {
- const size_t alignment_bytes = partition_.alignment_bytes();
-
EntryHeader header;
TRY(ReadEntryHeader(entry_address, &header));
// TODO: Should likely add a "LogHeader" method or similar.
@@ -151,8 +149,7 @@
DBG(" Key length = 0x%zx", size_t(header.key_length()));
DBG(" Value length = 0x%zx", size_t(header.value_length()));
DBG(" Entry size = 0x%zx", size_t(header.size()));
- DBG(" Padded size = 0x%zx",
- size_t(AlignUp(header.size(), alignment_bytes)));
+ DBG(" Alignment = 0x%zx", size_t(header.alignment_bytes()));
if (HeaderLooksLikeUnwrittenData(header)) {
return Status::NOT_FOUND;
@@ -741,8 +738,8 @@
// Handles writing multiple concatenated buffers, while breaking up the writes
// into alignment-sized blocks.
TRY_ASSIGN(
- size_t written,
- partition_.Write(
+ const size_t written,
+ partition_.WriteAligned(
address, {as_bytes(span(&header, 1)), as_bytes(span(key)), value}));
if (options_.verify_on_write) {
diff --git a/pw_kvs/key_value_store_test.cc b/pw_kvs/key_value_store_test.cc
index 27ce09a..c2848d7 100644
--- a/pw_kvs/key_value_store_test.cc
+++ b/pw_kvs/key_value_store_test.cc
@@ -282,8 +282,7 @@
} // namespace
-TEST_F(KeyValueStoreTest,
- DISABLED_Put_SameKeySameValueRepeatedly_AlignedEntries) {
+TEST_F(KeyValueStoreTest, Put_SameKeySameValueRepeatedly_AlignedEntries) {
std::array<char, 8> value{'v', 'a', 'l', 'u', 'e', '6', '7', '\0'};
for (int i = 0; i < 1000; ++i) {
@@ -291,8 +290,7 @@
}
}
-TEST_F(KeyValueStoreTest,
- DISABLED_Put_SameKeySameValueRepeatedly_UnalignedEntries) {
+TEST_F(KeyValueStoreTest, Put_SameKeySameValueRepeatedly_UnalignedEntries) {
std::array<char, 7> value{'v', 'a', 'l', 'u', 'e', '6', '\0'};
for (int i = 0; i < 1000; ++i) {
@@ -300,9 +298,13 @@
}
}
-TEST_F(KeyValueStoreTest, DISABLED_Put_SameKeyDifferentValueRepeatedly) {
- for (uint64_t i = 0; i < 1000u; ++i) {
- ASSERT_EQ(Status::OK, kvs_.Put("The Key!", i));
+TEST_F(KeyValueStoreTest, Put_SameKeyDifferentValuesRepeatedly) {
+ std::array<char, 10> value{'v', 'a', 'l', 'u', 'e', '6', '7', '8', '9', '\0'};
+
+ for (int i = 0; i < 100; ++i) {
+ for (unsigned size = 0; size < value.size(); ++size) {
+ ASSERT_EQ(Status::OK, kvs_.Put("The Key!", i));
+ }
}
}
diff --git a/pw_kvs/public/pw_kvs/flash_memory.h b/pw_kvs/public/pw_kvs/flash_memory.h
index 01c0120..3bc2a2b 100644
--- a/pw_kvs/public/pw_kvs/flash_memory.h
+++ b/pw_kvs/public/pw_kvs/flash_memory.h
@@ -57,7 +57,10 @@
alignment_(alignment),
start_address_(start_address),
start_sector_(sector_start),
- erased_memory_content_(erased_memory_content) {}
+ erased_memory_content_(erased_memory_content) {
+ // TODO: The smallest possible alignment is 1 B; 0 is invalid.
+ // DCHECK_NE(alignment_, 0);
+ }
virtual ~FlashMemory() = default;
@@ -138,24 +141,15 @@
FlashMemory* flash,
uint32_t start_sector_index,
uint32_t sector_count,
+ uint32_t alignment_bytes = 0, // Defaults to flash alignment
PartitionPermission permission = PartitionPermission::kReadAndWrite)
: flash_(*flash),
start_sector_index_(start_sector_index),
sector_count_(sector_count),
+ alignment_bytes_(alignment_bytes == 0 ? flash_.alignment_bytes()
+ : alignment_bytes),
permission_(permission) {}
-#if 0
- constexpr FlashPartition(
- FlashMemory* flash,
- uint32_t start_sector_index,
- uint32_t end_sector_index,
- PartitionPermission permission = PartitionPermission::kReadAndWrite)
- : flash_(*flash),
- start_sector_index_(start_sector_index),
- sector_count_(end_sector_index - start_sector_index + 1),
- permission_(permission) {}
-#endif
-
virtual ~FlashPartition() = default;
// Performs any required partition or flash-level initialization.
@@ -191,8 +185,9 @@
// UNKNOWN, on HAL error
virtual StatusWithSize Write(Address address, span<const std::byte> data);
- StatusWithSize Write(Address start_address,
- std::initializer_list<span<const std::byte>> data);
+ // Returns the total number of bytes written, including any padding.
+ StatusWithSize WriteAligned(
+ Address start_address, std::initializer_list<span<const std::byte>> data);
// Check to see if chunk of flash memory is erased. Address and len need to
// be aligned with FlashMemory.
@@ -214,9 +209,9 @@
size_t size_bytes() const { return sector_count() * sector_size_bytes(); }
- virtual size_t alignment_bytes() const { return flash_.alignment_bytes(); }
+ size_t alignment_bytes() const { return alignment_bytes_; }
- virtual size_t sector_count() const { return sector_count_; }
+ size_t sector_count() const { return sector_count_; }
// Convert a FlashMemory::Address to an MCU pointer, this can be used for
// memory mapped reads. Return NULL if the memory is not memory mapped.
@@ -247,6 +242,7 @@
FlashMemory& flash_;
const uint32_t start_sector_index_;
const uint32_t sector_count_;
+ const uint32_t alignment_bytes_;
const PartitionPermission permission_;
};
