pw_blob_store: Add blob open and read

- Add support to open blobs saved to flash
- Add support to stream read blobs

Change-Id: I97c587df34c98b6be03dba582837cd637e8c6448
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/18300
Commit-Queue: David Rogers <davidrogers@google.com>
Reviewed-by: Ewout van Bekkum <ewout@google.com>
diff --git a/pw_blob_store/BUILD b/pw_blob_store/BUILD
index 188ede7..0ff8d86 100644
--- a/pw_blob_store/BUILD
+++ b/pw_blob_store/BUILD
@@ -55,6 +55,21 @@
 )
 
 pw_cc_test(
+    name = "blob_store_chunk_write_test",
+    srcs = [
+        "blob_store_chunk_write_test.cc",
+    ],
+    deps = [
+        ":pw_blob_store",
+        "//pw_kvs:crc16",
+        "//pw_kvs:fake_flash",
+        "//pw_kvs:fake_flash_test_key_value_store",
+        "//pw_log",
+        "//pw_random",
+        "//pw_unit_test",
+    ],
+)
+pw_cc_test(
     name = "blob_store_deferred_write_test",
     srcs = [
         "blob_store_deferred_write_test.cc",
diff --git a/pw_blob_store/BUILD.gn b/pw_blob_store/BUILD.gn
index 99c2e03..d6274c6 100644
--- a/pw_blob_store/BUILD.gn
+++ b/pw_blob_store/BUILD.gn
@@ -42,6 +42,7 @@
   tests = [
     ":blob_store_test",
     ":blob_store_deferred_write_test",
+    ":blob_store_chunk_write_test",
   ]
 }
 
@@ -57,6 +58,18 @@
   sources = [ "blob_store_test.cc" ]
 }
 
+pw_test("blob_store_chunk_write_test") {
+  deps = [
+    ":pw_blob_store",
+    "$dir_pw_kvs:crc16",
+    "$dir_pw_kvs:fake_flash",
+    "$dir_pw_kvs:fake_flash_test_key_value_store",
+    dir_pw_log,
+    dir_pw_random,
+  ]
+  sources = [ "blob_store_chunk_write_test.cc" ]
+}
+
 pw_test("blob_store_deferred_write_test") {
   deps = [
     ":pw_blob_store",
diff --git a/pw_blob_store/blob_store.cc b/pw_blob_store/blob_store.cc
index d594d3f..7744a4b 100644
--- a/pw_blob_store/blob_store.cc
+++ b/pw_blob_store/blob_store.cc
@@ -26,6 +26,8 @@
     return Status::OK;
   }
 
+  PW_LOG_INFO("Init BlobStore");
+
   const size_t write_buffer_size_alignment =
       write_buffer_.size_bytes() % partition_.alignment_bytes();
   PW_CHECK_UINT_EQ((write_buffer_size_alignment), 0);
@@ -34,24 +36,53 @@
   PW_CHECK_UINT_GE(write_buffer_.size_bytes(), flash_write_size_bytes_);
   PW_CHECK_UINT_GE(flash_write_size_bytes_, partition_.alignment_bytes());
 
+  ResetChecksum();
+  initialized_ = true;
+
+  if (LoadMetadata().ok()) {
+    valid_data_ = true;
+    write_address_ = metadata_.data_size_bytes;
+    flash_address_ = metadata_.data_size_bytes;
+
+    PW_LOG_DEBUG("BlobStore init - Have valid blob of %u bytes",
+                 static_cast<unsigned>(write_address_));
+    return Status::OK;
+  }
+
+  // No saved blob, check for flash being erased.
   bool erased = false;
   if (partition_.IsErased(&erased).ok() && erased) {
     flash_erased_ = true;
+
+    // Blob data is considered valid as soon as the flash is erased. Even though
+    // there are 0 bytes written, they are valid.
     valid_data_ = true;
+    PW_LOG_DEBUG("BlobStore init - is erased");
+  } else {
+    PW_LOG_DEBUG("BlobStore init - not erased");
+  }
+  return Status::OK;
+}
+
+Status BlobStore::LoadMetadata() {
+  if (!kvs_.Get(MetadataKey(), &metadata_).ok()) {
+    // If no metadata was read, make sure the metadata is reset.
+    metadata_.reset();
+    return Status::NOT_FOUND;
   }
 
-  ResetChecksum();
+  if (!ValidateChecksum().ok()) {
+    PW_LOG_ERROR("BlobStore init - Invalidating blob with invalid checksum");
+    Invalidate();
+    return Status::DATA_LOSS;
+  }
 
-  // TODO: Add checking for already written valid blob.
-
-  initialized_ = true;
   return Status::OK;
 }
 
 size_t BlobStore::MaxDataSizeBytes() const { return partition_.size_bytes(); }
 
 Status BlobStore::OpenWrite() {
-  // TODO: should Open auto initialize?
   if (!initialized_) {
     return Status::FAILED_PRECONDITION;
   }
@@ -62,12 +93,11 @@
     return Status::UNAVAILABLE;
   }
 
+  PW_LOG_DEBUG("Blob writer open");
+
   writer_open_ = true;
 
-  write_address_ = 0;
-  flash_address_ = 0;
-
-  ResetChecksum();
+  Invalidate();
 
   return Status::OK;
 }
@@ -77,17 +107,18 @@
     return Status::FAILED_PRECONDITION;
   }
 
-  // If there is no open writer, then once the reader opens there can not be
-  // one. So check that there is actully valid data.
-  // TODO: Move this validation to Init and CloseWrite
-  if (!writer_open_) {
-    if (!ValidateChecksum().ok()) {
-      PW_LOG_ERROR("Validation check failed, invalidate blob");
-      Invalidate();
-      return Status::FAILED_PRECONDITION;
-    }
+  // Reader can only be opened if there is no writer open.
+  if (writer_open_) {
+    return Status::UNAVAILABLE;
   }
 
+  if (!ValidToRead()) {
+    PW_LOG_ERROR("Blob reader unable open without valid data");
+    return Status::FAILED_PRECONDITION;
+  }
+
+  PW_LOG_DEBUG("Blob reader open");
+
   readers_open_++;
   return Status::OK;
 }
@@ -104,8 +135,11 @@
       return Status::OK;
     }
 
-    PW_LOG_DEBUG("Close with %u bytes in write buffer",
-                 static_cast<unsigned>(WriteBufferBytesUsed()));
+    PW_LOG_DEBUG(
+        "Blob writer close of %u byte blob, with %u bytes still in write "
+        "buffer",
+        static_cast<unsigned>(write_address_),
+        static_cast<unsigned>(WriteBufferBytesUsed()));
 
     // Do a Flush of any flash_write_size_bytes_ sized chunks so any remaining
     // bytes in the write buffer are less than flash_write_size_bytes_.
@@ -117,14 +151,10 @@
     if (!WriteBufferEmpty()) {
       PW_TRY(FlushFinalPartialChunk());
     }
+    PW_DCHECK(WriteBufferEmpty());
 
     // If things are still good, save the blob metadata.
-    // Should not be completing a blob write when it has completed metadata.
-    PW_DCHECK(WriteBufferEmpty());
-    PW_DCHECK(!metadata_.complete);
-
-    metadata_ = {
-        .checksum = 0, .data_size_bytes = flash_address_, .complete = true};
+    metadata_ = {.checksum = 0, .data_size_bytes = flash_address_};
     if (checksum_algo_ != nullptr) {
       ConstByteSpan checksum = checksum_algo_->Finish();
       std::memcpy(&metadata_.checksum,
@@ -132,8 +162,14 @@
                   std::min(checksum.size(), sizeof(metadata_.checksum)));
     }
 
-    PW_TRY(kvs_.Put(MetadataKey(), metadata_));
-    PW_TRY(ValidateChecksum());
+    if (!ValidateChecksum().ok()) {
+      Invalidate();
+      return Status::DATA_LOSS;
+    }
+
+    if (!kvs_.Put(MetadataKey(), metadata_).ok()) {
+      return Status::DATA_LOSS;
+    }
 
     return Status::OK;
   };
@@ -151,6 +187,7 @@
 Status BlobStore::CloseRead() {
   PW_CHECK_UINT_GT(readers_open_, 0);
   readers_open_--;
+  PW_LOG_DEBUG("Blob reader close");
   return Status::OK;
 }
 
@@ -338,7 +375,6 @@
     data_bytes = source.size_bytes();
   }
   flash_erased_ = false;
-  valid_data_ = true;
   StatusWithSize result = partition_.Write(flash_address_, source);
   flash_address_ += data_bytes;
   if (checksum_algo_ != nullptr) {
@@ -373,14 +409,22 @@
   return Status::OK;
 }
 
-StatusWithSize BlobStore::Read(size_t offset, ByteSpan dest) {
-  (void)offset;
-  (void)dest;
-  return StatusWithSize::UNIMPLEMENTED;
+StatusWithSize BlobStore::Read(size_t offset, ByteSpan dest) const {
+  if (!ValidToRead()) {
+    return StatusWithSize::FAILED_PRECONDITION;
+  }
+  if (offset >= ReadableDataBytes()) {
+    return StatusWithSize::OUT_OF_RANGE;
+  }
+
+  size_t available_bytes = ReadableDataBytes() - offset;
+  size_t read_size = std::min(available_bytes, dest.size_bytes());
+
+  return partition_.Read(offset, dest.first(read_size));
 }
 
 Result<ByteSpan> BlobStore::GetMemoryMappedBlob() const {
-  if (flash_address_ == 0 || !valid_data_) {
+  if (!ValidToRead()) {
     return Status::FAILED_PRECONDITION;
   }
 
@@ -391,20 +435,21 @@
   return std::span(mcu_address, ReadableDataBytes());
 }
 
-size_t BlobStore::ReadableDataBytes() const { return flash_address_; }
+size_t BlobStore::ReadableDataBytes() const {
+  // TODO: clean up state related to readable bytes.
+  return flash_address_;
+}
 
 Status BlobStore::Erase() {
-  // Can't erase if any readers are open, since this would erase flash right out
-  // from under them.
-  if (readers_open_ != 0) {
-    return Status::UNAVAILABLE;
-  }
-
   // If already erased our work here is done.
   if (flash_erased_) {
-    // The write buffer might have bytes when the erase happens.
+    // The write buffer might already have bytes when this call happens, due to
+    // a deferred write.
     PW_DCHECK_UINT_LE(write_address_, write_buffer_.size_bytes());
     PW_DCHECK_UINT_EQ(flash_address_, 0);
+
+    // Erased blobs should be valid as soon as the flash is erased. Even though
+    // there are 0 bytes written, they are valid.
     PW_DCHECK(valid_data_);
     return Status::OK;
   }
@@ -415,32 +460,37 @@
 
   if (status.ok()) {
     flash_erased_ = true;
+
+    // Blob data is considered valid as soon as the flash is erased. Even though
+    // there are 0 bytes written, they are valid.
     valid_data_ = true;
   }
   return status;
 }
 
 Status BlobStore::Invalidate() {
-  if (readers_open_ != 0) {
-    return Status::UNAVAILABLE;
-  }
+  metadata_.reset();
 
-  Status status = kvs_.Delete(MetadataKey());
-  valid_data_ = false;
+  // Blob data is considered if the flash is erased. Even though
+  // there are 0 bytes written, they are valid.
+  valid_data_ = flash_erased_;
   ResetChecksum();
   write_address_ = 0;
   flash_address_ = 0;
 
-  return status;
+  return kvs_.Delete(MetadataKey());
 }
 
 Status BlobStore::ValidateChecksum() {
-  if (!metadata_.complete) {
+  if (metadata_.data_size_bytes == 0) {
+    PW_LOG_INFO("Blob unable to validate checksum of an empty blob");
     return Status::UNAVAILABLE;
   }
 
   if (checksum_algo_ == nullptr) {
     if (metadata_.checksum != 0) {
+      PW_LOG_ERROR(
+          "Blob invalid to have a checkum value with no checksum algo");
       return Status::DATA_LOSS;
     }
 
@@ -450,16 +500,16 @@
   PW_LOG_DEBUG("Validate checksum of 0x%08x in flash for blob of %u bytes",
                static_cast<unsigned>(metadata_.checksum),
                static_cast<unsigned>(metadata_.data_size_bytes));
-  auto result = CalculateChecksumFromFlash(metadata_.data_size_bytes);
-  if (!result.ok()) {
-    return result.status();
-  }
+  PW_TRY(CalculateChecksumFromFlash(metadata_.data_size_bytes));
 
-  return checksum_algo_->Verify(as_bytes(std::span(&metadata_.checksum, 1)));
+  Status status =
+      checksum_algo_->Verify(as_bytes(std::span(&metadata_.checksum, 1)));
+  PW_LOG_DEBUG("  checksum verify of %s", status.str());
+
+  return status;
 }
 
-Result<BlobStore::ChecksumValue> BlobStore::CalculateChecksumFromFlash(
-    size_t bytes_to_check) {
+Status BlobStore::CalculateChecksumFromFlash(size_t bytes_to_check) {
   if (checksum_algo_ == nullptr) {
     return Status::OK;
   }
@@ -473,23 +523,16 @@
   std::array<std::byte, kReadBufferSizeBytes> buffer;
   while (address < end) {
     const size_t read_size = std::min(size_t(end - address), buffer.size());
-    StatusWithSize status =
-        partition_.Read(address, std::span(buffer).first(read_size));
-    // TODO: switch to TRY once available.
-    if (!status.ok()) {
-      return status.status();
-    }
+    PW_TRY(partition_.Read(address, std::span(buffer).first(read_size)));
 
     checksum_algo_->Update(buffer.data(), read_size);
     address += read_size;
   }
 
-  ChecksumValue checksum_value;
-  std::span checksum = checksum_algo_->Finish();
-  std::memcpy(&checksum_value,
-              checksum.data(),
-              std::min(checksum.size(), sizeof(checksum_value)));
-  return checksum_value;
+  // Safe to ignore the return from Finish, checksum_algo_ keeps the state
+  // information that it needs.
+  checksum_algo_->Finish();
+  return Status::OK;
 }
 
 }  // namespace pw::blob_store
diff --git a/pw_blob_store/blob_store_chunk_write_test.cc b/pw_blob_store/blob_store_chunk_write_test.cc
new file mode 100644
index 0000000..42dac62
--- /dev/null
+++ b/pw_blob_store/blob_store_chunk_write_test.cc
@@ -0,0 +1,171 @@
+// Copyright 2020 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+#include <array>
+#include <cstddef>
+#include <cstring>
+#include <span>
+
+#include "gtest/gtest.h"
+#include "pw_blob_store/blob_store.h"
+#include "pw_kvs/crc16_checksum.h"
+#include "pw_kvs/fake_flash_memory.h"
+#include "pw_kvs/flash_memory.h"
+#include "pw_kvs/test_key_value_store.h"
+#include "pw_log/log.h"
+#include "pw_random/xor_shift.h"
+
+namespace pw::blob_store {
+namespace {
+
+class BlobStoreChunkTest : public ::testing::Test {
+ protected:
+  BlobStoreChunkTest() : flash_(kFlashAlignment), partition_(&flash_) {}
+
+  void InitFlashTo(std::span<const std::byte> contents) {
+    partition_.Erase();
+    std::memcpy(flash_.buffer().data(), contents.data(), contents.size());
+  }
+
+  void InitSourceBufferToRandom(uint64_t seed) {
+    partition_.Erase();
+    random::XorShiftStarRng64 rng(seed);
+    rng.Get(source_buffer_);
+  }
+
+  void InitSourceBufferToFill(char fill) {
+    partition_.Erase();
+    std::memset(source_buffer_.data(), fill, source_buffer_.size());
+  }
+
+  // Fill the source buffer with random pattern based on given seed, written to
+  // BlobStore in specified chunk size.
+  void ChunkWriteTest(size_t chunk_size) {
+    constexpr size_t kBufferSize = 256;
+    kvs::ChecksumCrc16 checksum;
+
+    char name[16] = {};
+    snprintf(name, sizeof(name), "Blob%u", static_cast<unsigned>(chunk_size));
+
+    BlobStoreBuffer<kBufferSize> blob(
+        name, &partition_, &checksum, kvs::TestKvs());
+    EXPECT_EQ(Status::OK, blob.Init());
+
+    BlobStore::BlobWriter writer(blob);
+    EXPECT_EQ(Status::OK, writer.Open());
+    EXPECT_EQ(Status::OK, writer.Erase());
+
+    ByteSpan source = source_buffer_;
+    while (source.size_bytes() > 0) {
+      const size_t write_size = std::min(source.size_bytes(), chunk_size);
+
+      PW_LOG_DEBUG("Do write of %u bytes, %u bytes remain",
+                   static_cast<unsigned>(write_size),
+                   static_cast<unsigned>(source.size_bytes()));
+
+      ASSERT_EQ(Status::OK, writer.Write(source.first(write_size)));
+
+      source = source.subspan(write_size);
+    }
+
+    EXPECT_EQ(Status::OK, writer.Close());
+
+    // Use reader to check for valid data.
+    BlobStore::BlobReader reader(blob);
+    ASSERT_EQ(Status::OK, reader.Open());
+    Result<ByteSpan> result = reader.GetMemoryMappedBlob();
+    ASSERT_TRUE(result.ok());
+    VerifyFlash(result.value());
+    EXPECT_EQ(Status::OK, reader.Close());
+  }
+
+  void VerifyFlash(ByteSpan verify_bytes) {
+    // Should be defined as same size.
+    EXPECT_EQ(source_buffer_.size(), flash_.buffer().size_bytes());
+
+    // Can't allow it to march off the end of source_buffer_.
+    ASSERT_LE(verify_bytes.size_bytes(), source_buffer_.size());
+
+    for (size_t i = 0; i < verify_bytes.size_bytes(); i++) {
+      EXPECT_EQ(source_buffer_[i], verify_bytes[i]);
+    }
+  }
+
+  static constexpr size_t kFlashAlignment = 16;
+  static constexpr size_t kSectorSize = 2048;
+  static constexpr size_t kSectorCount = 2;
+  static constexpr size_t kBlobDataSize = (kSectorCount * kSectorSize);
+
+  kvs::FakeFlashMemoryBuffer<kSectorSize, kSectorCount> flash_;
+  kvs::FlashPartition partition_;
+  std::array<std::byte, kBlobDataSize> source_buffer_;
+};
+
+TEST_F(BlobStoreChunkTest, ChunkWrite1) {
+  InitSourceBufferToRandom(0x8675309);
+  ChunkWriteTest(1);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite2) {
+  InitSourceBufferToRandom(0x8675);
+  ChunkWriteTest(2);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite3) {
+  InitSourceBufferToFill(0);
+  ChunkWriteTest(3);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite4) {
+  InitSourceBufferToFill(1);
+  ChunkWriteTest(4);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite5) {
+  InitSourceBufferToFill(0xff);
+  ChunkWriteTest(5);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite16) {
+  InitSourceBufferToRandom(0x86);
+  ChunkWriteTest(16);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite64) {
+  InitSourceBufferToRandom(0x9);
+  ChunkWriteTest(64);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite256) {
+  InitSourceBufferToRandom(0x12345678);
+  ChunkWriteTest(256);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite512) {
+  InitSourceBufferToRandom(0x42);
+  ChunkWriteTest(512);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWrite4096) {
+  InitSourceBufferToRandom(0x89);
+  ChunkWriteTest(4096);
+}
+
+TEST_F(BlobStoreChunkTest, ChunkWriteSingleFull) {
+  InitSourceBufferToRandom(0x98765);
+  ChunkWriteTest(kBlobDataSize);
+}
+
+}  // namespace
+}  // namespace pw::blob_store
diff --git a/pw_blob_store/blob_store_deferred_write_test.cc b/pw_blob_store/blob_store_deferred_write_test.cc
index 8100f2a..a957308 100644
--- a/pw_blob_store/blob_store_deferred_write_test.cc
+++ b/pw_blob_store/blob_store_deferred_write_test.cc
@@ -76,7 +76,7 @@
                    static_cast<unsigned>(write_size),
                    static_cast<unsigned>(source.size_bytes()));
 
-      EXPECT_EQ(Status::OK, writer.Write(source.first(write_size)));
+      ASSERT_EQ(Status::OK, writer.Write(source.first(write_size)));
       // TODO: Add check that the write did not go to flash yet.
 
       source = source.subspan(write_size);
@@ -84,15 +84,15 @@
 
       if (bytes_since_flush >= flush_interval) {
         bytes_since_flush = 0;
-        EXPECT_EQ(Status::OK, writer.Flush());
+        ASSERT_EQ(Status::OK, writer.Flush());
       }
     }
 
     EXPECT_EQ(Status::OK, writer.Close());
 
     // Use reader to check for valid data.
-    BlobStore::BlobReader reader(blob, 0);
-    EXPECT_EQ(Status::OK, reader.Open());
+    BlobStore::BlobReader reader(blob);
+    ASSERT_EQ(Status::OK, reader.Open());
     Result<ByteSpan> result = reader.GetMemoryMappedBlob();
     ASSERT_TRUE(result.ok());
     VerifyFlash(result.value());
diff --git a/pw_blob_store/blob_store_test.cc b/pw_blob_store/blob_store_test.cc
index 97e8ce4..55f98f1 100644
--- a/pw_blob_store/blob_store_test.cc
+++ b/pw_blob_store/blob_store_test.cc
@@ -52,12 +52,12 @@
 
   // Fill the source buffer with random pattern based on given seed, written to
   // BlobStore in specified chunk size.
-  void ChunkWriteTest(size_t chunk_size) {
+  void WriteTestBlock() {
     constexpr size_t kBufferSize = 256;
     kvs::ChecksumCrc16 checksum;
 
     char name[16] = {};
-    snprintf(name, sizeof(name), "Blob%u", static_cast<unsigned>(chunk_size));
+    snprintf(name, sizeof(name), "TestBlobBlock");
 
     BlobStoreBuffer<kBufferSize> blob(
         name, &partition_, &checksum, kvs::TestKvs());
@@ -65,50 +65,80 @@
 
     BlobStore::BlobWriter writer(blob);
     EXPECT_EQ(Status::OK, writer.Open());
-
-    ByteSpan source = source_buffer_;
-    while (source.size_bytes() > 0) {
-      const size_t write_size = std::min(source.size_bytes(), chunk_size);
-
-      PW_LOG_DEBUG("Do write of %u bytes, %u bytes remain",
-                   static_cast<unsigned>(write_size),
-                   static_cast<unsigned>(source.size_bytes()));
-
-      EXPECT_EQ(Status::OK, writer.Write(source.first(write_size)));
-
-      source = source.subspan(write_size);
-    }
-
+    EXPECT_EQ(Status::OK, writer.Erase());
+    ASSERT_EQ(Status::OK, writer.Write(source_buffer_));
     EXPECT_EQ(Status::OK, writer.Close());
 
     // Use reader to check for valid data.
-    BlobStore::BlobReader reader(blob, 0);
-    EXPECT_EQ(Status::OK, reader.Open());
+    BlobStore::BlobReader reader(blob);
+    ASSERT_EQ(Status::OK, reader.Open());
     Result<ByteSpan> result = reader.GetMemoryMappedBlob();
     ASSERT_TRUE(result.ok());
     VerifyFlash(result.value());
     EXPECT_EQ(Status::OK, reader.Close());
   }
 
-  void VerifyFlash(ByteSpan verify_bytes) {
+  // Open a new blob instance and read the blob using the given read chunk size.
+  void ChunkReadTest(size_t read_chunk_size) {
+    kvs::ChecksumCrc16 checksum;
+
+    VerifyFlash(flash_.buffer());
+
+    char name[16] = "TestBlobBlock";
+    BlobStoreBuffer<16> blob(name, &partition_, &checksum, kvs::TestKvs());
+    EXPECT_EQ(Status::OK, blob.Init());
+
+    // Use reader to check for valid data.
+    BlobStore::BlobReader reader1(blob);
+    ASSERT_EQ(Status::OK, reader1.Open());
+    Result<ByteSpan> result = reader1.GetMemoryMappedBlob();
+    ASSERT_TRUE(result.ok());
+    VerifyFlash(result.value());
+    EXPECT_EQ(Status::OK, reader1.Close());
+
+    BlobStore::BlobReader reader(blob);
+    ASSERT_EQ(Status::OK, reader.Open());
+
+    std::array<std::byte, kBlobDataSize> read_buffer_;
+
+    ByteSpan read_span = read_buffer_;
+    while (read_span.size_bytes() > 0) {
+      size_t read_size = std::min(read_span.size_bytes(), read_chunk_size);
+
+      PW_LOG_DEBUG("Do write of %u bytes, %u bytes remain",
+                   static_cast<unsigned>(read_size),
+                   static_cast<unsigned>(read_span.size_bytes()));
+
+      ASSERT_EQ(read_span.size_bytes(), reader.ConservativeReadLimit());
+      auto result = reader.Read(read_span.first(read_size));
+      ASSERT_EQ(result.status(), Status::OK);
+      read_span = read_span.subspan(read_size);
+    }
+    EXPECT_EQ(Status::OK, reader.Close());
+
+    VerifyFlash(read_buffer_);
+  }
+
+  void VerifyFlash(ByteSpan verify_bytes, size_t offset = 0) {
     // Should be defined as same size.
     EXPECT_EQ(source_buffer_.size(), flash_.buffer().size_bytes());
 
     // Can't allow it to march off the end of source_buffer_.
-    ASSERT_LE(verify_bytes.size_bytes(), source_buffer_.size());
+    ASSERT_LE((verify_bytes.size_bytes() + offset), source_buffer_.size());
 
     for (size_t i = 0; i < verify_bytes.size_bytes(); i++) {
-      EXPECT_EQ(source_buffer_[i], verify_bytes[i]);
+      ASSERT_EQ(source_buffer_[i + offset], verify_bytes[i]);
     }
   }
 
   static constexpr size_t kFlashAlignment = 16;
   static constexpr size_t kSectorSize = 2048;
   static constexpr size_t kSectorCount = 2;
+  static constexpr size_t kBlobDataSize = (kSectorCount * kSectorSize);
 
   kvs::FakeFlashMemoryBuffer<kSectorSize, kSectorCount> flash_;
   kvs::FlashPartition partition_;
-  std::array<std::byte, kSectorCount * kSectorSize> source_buffer_;
+  std::array<std::byte, kBlobDataSize> source_buffer_;
 };
 
 TEST_F(BlobStoreTest, Init_Ok) {
@@ -128,59 +158,86 @@
   EXPECT_EQ(Status::OK, writer.Erase());
 }
 
-TEST_F(BlobStoreTest, ChunkWrite1) {
+TEST_F(BlobStoreTest, OffsetRead) {
+  InitSourceBufferToRandom(0x11309);
+  WriteTestBlock();
+
+  constexpr size_t kOffset = 10;
+  ASSERT_LT(kOffset, kBlobDataSize);
+
+  kvs::ChecksumCrc16 checksum;
+
+  char name[16] = "TestBlobBlock";
+  BlobStoreBuffer<16> blob(name, &partition_, &checksum, kvs::TestKvs());
+  EXPECT_EQ(Status::OK, blob.Init());
+  BlobStore::BlobReader reader(blob);
+  ASSERT_EQ(Status::OK, reader.Open(kOffset));
+
+  std::array<std::byte, kBlobDataSize - kOffset> read_buffer_;
+  ByteSpan read_span = read_buffer_;
+  ASSERT_EQ(read_span.size_bytes(), reader.ConservativeReadLimit());
+
+  auto result = reader.Read(read_span);
+  ASSERT_EQ(result.status(), Status::OK);
+  EXPECT_EQ(Status::OK, reader.Close());
+  VerifyFlash(read_buffer_, kOffset);
+}
+
+TEST_F(BlobStoreTest, InvalidReadOffset) {
+  InitSourceBufferToRandom(0x11309);
+  WriteTestBlock();
+
+  constexpr size_t kOffset = kBlobDataSize;
+
+  kvs::ChecksumCrc16 checksum;
+
+  char name[16] = "TestBlobBlock";
+  BlobStoreBuffer<16> blob(name, &partition_, &checksum, kvs::TestKvs());
+  EXPECT_EQ(Status::OK, blob.Init());
+  BlobStore::BlobReader reader(blob);
+  ASSERT_EQ(Status::INVALID_ARGUMENT, reader.Open(kOffset));
+}
+
+TEST_F(BlobStoreTest, ChunkRead1) {
   InitSourceBufferToRandom(0x8675309);
-  ChunkWriteTest(1);
+  WriteTestBlock();
+  ChunkReadTest(1);
 }
 
-TEST_F(BlobStoreTest, ChunkWrite2) {
-  InitSourceBufferToRandom(0x8675);
-  ChunkWriteTest(2);
-}
-
-TEST_F(BlobStoreTest, ChunkWrite3) {
+TEST_F(BlobStoreTest, ChunkRead3) {
   InitSourceBufferToFill(0);
-  ChunkWriteTest(3);
+  WriteTestBlock();
+  ChunkReadTest(3);
 }
 
-TEST_F(BlobStoreTest, ChunkWrite4) {
+TEST_F(BlobStoreTest, ChunkRead4) {
   InitSourceBufferToFill(1);
-  ChunkWriteTest(4);
+  WriteTestBlock();
+  ChunkReadTest(4);
 }
 
-TEST_F(BlobStoreTest, ChunkWrite5) {
+TEST_F(BlobStoreTest, ChunkRead5) {
   InitSourceBufferToFill(0xff);
-  ChunkWriteTest(5);
+  WriteTestBlock();
+  ChunkReadTest(5);
 }
 
-TEST_F(BlobStoreTest, ChunkWrite16) {
+TEST_F(BlobStoreTest, ChunkRead16) {
   InitSourceBufferToRandom(0x86);
-  ChunkWriteTest(16);
+  WriteTestBlock();
+  ChunkReadTest(16);
 }
 
-TEST_F(BlobStoreTest, ChunkWrite64) {
+TEST_F(BlobStoreTest, ChunkRead64) {
   InitSourceBufferToRandom(0x9);
-  ChunkWriteTest(64);
+  WriteTestBlock();
+  ChunkReadTest(64);
 }
 
-TEST_F(BlobStoreTest, ChunkWrite256) {
-  InitSourceBufferToRandom(0x12345678);
-  ChunkWriteTest(256);
-}
-
-TEST_F(BlobStoreTest, ChunkWrite512) {
-  InitSourceBufferToRandom(0x42);
-  ChunkWriteTest(512);
-}
-
-TEST_F(BlobStoreTest, ChunkWrite4096) {
-  InitSourceBufferToRandom(0x89);
-  ChunkWriteTest(4096);
-}
-
-TEST_F(BlobStoreTest, ChunkWriteSingleFull) {
-  InitSourceBufferToRandom(0x98765);
-  ChunkWriteTest((kSectorCount * kSectorSize));
+TEST_F(BlobStoreTest, ChunkReadFull) {
+  InitSourceBufferToRandom(0x9);
+  WriteTestBlock();
+  ChunkReadTest(kBlobDataSize);
 }
 
 // TODO: test that does close with bytes still in the buffer to test zero fill
diff --git a/pw_blob_store/public/pw_blob_store/blob_store.h b/pw_blob_store/public/pw_blob_store/blob_store.h
index 14a1461..4c02790 100644
--- a/pw_blob_store/public/pw_blob_store/blob_store.h
+++ b/pw_blob_store/public/pw_blob_store/blob_store.h
@@ -30,8 +30,8 @@
 //
 // Write and read are only done using the BlobWriter and BlobReader classes.
 //
-// Once a blob write is closed, reopening followed by a Discard(), Write(), or
-// Erase() will discard the previous blob.
+// Once a blob write is closed, reopening to write will discard the previous
+// blob.
 //
 // Write blob:
 //  0) Create BlobWriter instance
@@ -63,8 +63,9 @@
       }
     }
 
-    // Open a blob for writing/erasing. Can not open when already open. Only one
-    // writer is allowed to be open at a time. Returns:
+    // Open a blob for writing/erasing. Open will invalidate any existing blob
+    // that may be stored. Can not open when already open. Only one writer is
+    // allowed to be open at a time. Returns:
     //
     // OK - success.
     // UNAVAILABLE - Unable to open, another writer or reader instance is
@@ -91,7 +92,9 @@
       return store_.CloseWrite();
     }
 
-    // Erase the partition and reset state for a new blob. Returns:
+    // Erase the blob partition and reset state for a new blob. Explicit calls
+    // to Erase are optional, beginning a write will do any needed Erase.
+    // Returns:
     //
     // OK - success.
     // UNAVAILABLE - Unable to erase while reader is open.
@@ -101,8 +104,8 @@
       return store_.Erase();
     }
 
-    // Discard blob (in-progress or valid stored). Any written bytes are
-    // considered invalid. Returns:
+    // Discard the current blob. Any written bytes to this point are considered
+    // invalid. Returns:
     //
     // OK - success.
     // FAILED_PRECONDITION - not open.
@@ -172,11 +175,12 @@
     }
   };
 
-  // Implement stream::Reader interface for BlobStore.
+  // Implement stream::Reader interface for BlobStore. Multiple readers may be
+  // open at the same time, but readers may not be open with a writer open.
   class BlobReader final : public stream::Reader {
    public:
-    constexpr BlobReader(BlobStore& store, size_t offset)
-        : store_(store), open_(false), offset_(offset) {}
+    constexpr BlobReader(BlobStore& store)
+        : store_(store), open_(false), offset_(0) {}
     BlobReader(const BlobReader&) = delete;
     BlobReader& operator=(const BlobReader&) = delete;
     ~BlobReader() {
@@ -185,13 +189,19 @@
       }
     }
 
-    // Open to do a blob read. Can not open when already open. Multiple readers
-    // can be open at the same time. Returns:
+    // Open to do a blob read at the given offset in to the blob. Can not open
+    // when already open. Multiple readers can be open at the same time.
+    // Returns:
     //
     // OK - success.
     // UNAVAILABLE - Unable to open, already open.
-    Status Open() {
+    Status Open(size_t offset = 0) {
       PW_DCHECK(!open_);
+      if (offset >= store_.ReadableDataBytes()) {
+        return Status::INVALID_ARGUMENT;
+      }
+
+      offset_ = offset;
       Status status = store_.OpenRead();
       if (status.ok()) {
         open_ = true;
@@ -230,7 +240,12 @@
    private:
     StatusWithSize DoRead(ByteSpan dest) override {
       PW_DCHECK(open_);
-      return store_.Read(offset_, dest);
+      StatusWithSize status = store_.Read(offset_, dest);
+      if (status.ok()) {
+        PW_DCHECK_UINT_EQ(status.size(), dest.size_bytes());
+        offset_ += status.size();
+      }
+      return status;
     }
 
     BlobStore& store_;
@@ -287,6 +302,8 @@
  private:
   typedef uint32_t ChecksumValue;
 
+  Status LoadMetadata();
+
   // Open to do a blob write. Returns:
   //
   // OK - success.
@@ -303,7 +320,7 @@
   // Finalize a blob write. Flush all remaining buffered data to storage and
   // store blob metadata. Returns:
   //
-  // OK - success.
+  // OK - success, valid complete blob.
   // DATA_LOSS - Error during write (this close or previous write/flush). Blob
   //     is closed and marked as invalid.
   Status CloseWrite();
@@ -379,6 +396,9 @@
 
   Status EraseIfNeeded();
 
+  // Blob is valid/OK and has data to read.
+  bool ValidToRead() const { return (valid_data_ && ReadableDataBytes() > 0); }
+
   // Read valid data. Attempts to read the lesser of output.size_bytes() or
   // available bytes worth of data. Returns:
   //
@@ -390,7 +410,7 @@
   //     Try again once bytes become available.
   // OUT_OF_RANGE - Reader has been exhausted, similar to EOF. No bytes read, no
   //     more will be read.
-  StatusWithSize Read(size_t offset, ByteSpan dest);
+  StatusWithSize Read(size_t offset, ByteSpan dest) const;
 
   // Get a span with the MCU pointer and size of the data. Returns:
   //
@@ -399,8 +419,7 @@
   // UNIMPLEMENTED - Memory mapped access not supported for this blob.
   Result<ByteSpan> GetMemoryMappedBlob() const;
 
-  // Current size of blob/readable data, in bytes. For a completed write this is
-  // the size of the data blob. For all other cases this is zero bytes.
+  // Size of blob/readable data, in bytes.
   size_t ReadableDataBytes() const;
 
   size_t WriteBytesRemaining() const {
@@ -419,8 +438,7 @@
 
   Status ValidateChecksum();
 
-  Result<BlobStore::ChecksumValue> CalculateChecksumFromFlash(
-      size_t bytes_to_check);
+  Status CalculateChecksumFromFlash(size_t bytes_to_check);
 
   const std::string_view MetadataKey() { return name_; }
 
@@ -433,8 +451,12 @@
     // Number of blob data bytes stored in flash.
     size_t data_size_bytes;
 
-    // This blob is complete (the write was properly closed).
-    bool complete;
+    constexpr void reset() {
+      *this = {
+          .checksum = 0,
+          .data_size_bytes = 0,
+      };
+    }
   };
 
   std::string_view name_;
@@ -457,7 +479,7 @@
   bool initialized_;
 
   // Bytes stored are valid and good. Blob is OK to read and write to. Set as
-  // soon as is valid, even when bytes written is still 0.
+  // soon as blob is erased. Even when bytes written is still 0, they are valid.
   bool valid_data_;
 
   // Blob partition is currently erased and ready to write a new blob.