Skip to content

Commit d79c44a

Browse files
committed
Remove the by-ref trait impls as concurrent access to the key-value storage might leave it in an inconsistent state
1 parent 6f6c2c8 commit d79c44a

1 file changed

Lines changed: 8 additions & 53 deletions

File tree

src/nvs.rs

Lines changed: 8 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -738,21 +738,14 @@ impl RawHandle for EspNvs<NvsEncrypted> {
738738
}
739739

740740
/// A specialized key-value storage wrapper around `EspNvs` that provides a simplified interface
741-
/// for storing and retrieving arbitrary data as byte arrays.
741+
/// for storing and retrieving arbitrary data as byte (`u8`) slices.
742742
///
743-
/// This struct was introduced to solve issue [#585](https://github.com/esp-rs/esp-idf-svc/issues/585)
744-
/// where the `contains()` method in `EspNvs` incorrectly returned `false` for string values
745-
/// that actually existed in NVS partitions. The root cause was that `EspNvs` implements
746-
/// two different storage strategies:
747-
///
748-
/// 1. **Native ESP-IDF NVS API**: Direct access to ESP-IDF's native types (u8, u16, u32, u64, i8, i16, i32, i64, str, blob)
749-
/// 2. **Serialized storage**: Everything stored as either u64 (≤7 bytes) or blob (>7 bytes) for compatibility with serde
750-
///
751-
/// `EspKeyValueStorage` focuses on the second approach, providing a clean interface for:
743+
/// `EspKeyValueStorage` provides an interface for:
752744
/// - Storing any data that can be represented as `&[u8]`
753-
/// - Automatic optimization: values ≤7 bytes stored as u64, larger values as blobs
745+
/// - Automatic optimization: values ≤7 bytes stored as ESP-NVS `u64` values, larger values as ESP-NVS blobs
754746
/// - Consistent `contains()` method that works correctly with this storage strategy
755-
/// - Full compatibility with Rust serde implementations (postcard, json, etc.)
747+
/// - Full compatibility with Rust serde implementations (postcard, json, etc.) in that these can naturally do serde
748+
/// over byte slices
756749
///
757750
/// ## Usage
758751
///
@@ -778,28 +771,13 @@ impl RawHandle for EspNvs<NvsEncrypted> {
778771
/// # Ok(())
779772
/// # }
780773
/// ```
781-
///
782-
/// ## Performance Characteristics
783-
///
784-
/// - **Small values (≤7 bytes)**: Stored as u64 for efficiency
785-
/// - **Large values (>7 bytes)**: Stored as ESP-IDF blobs
786-
/// - **Memory efficient**: No unnecessary allocations for small values
787-
/// - **Flash efficient**: Optimized storage format reduces wear on flash memory
788774
pub struct EspKeyValueStorage<T: NvsPartitionId>(EspNvs<T>);
789775

790776
impl<T: NvsPartitionId> EspKeyValueStorage<T> {
791777
pub const fn new(nvs: EspNvs<T>) -> Self {
792778
Self(nvs)
793779
}
794780

795-
pub const fn esp_nvs(&self) -> &EspNvs<T> {
796-
&self.0
797-
}
798-
799-
pub fn partition(&self) -> &EspNvsPartition<T> {
800-
&self.0 .0
801-
}
802-
803781
pub fn contains(&self, name: &str) -> Result<bool, EspError> {
804782
self.len(name).map(|v| v.is_some())
805783
}
@@ -852,6 +830,9 @@ impl<T: NvsPartitionId> EspKeyValueStorage<T> {
852830

853831
pub fn set_raw(&self, name: &str, buf: &[u8]) -> Result<bool, EspError> {
854832
// start by just clearing this key, ignoring the result since it may not exist
833+
// TODO: This is not optimal, because if the chip is shut-down right after
834+
// the call to `remove`, the key will be gone forever.
835+
855836
_ = self.0.remove(name);
856837

857838
if buf.len() < 8 {
@@ -899,29 +880,3 @@ impl<T: NvsPartitionId> RawStorage for EspKeyValueStorage<T> {
899880
EspKeyValueStorage::set_raw(self, name, buf)
900881
}
901882
}
902-
903-
impl<T: NvsPartitionId> StorageBase for &EspKeyValueStorage<T> {
904-
type Error = EspError;
905-
906-
fn contains(&self, name: &str) -> Result<bool, Self::Error> {
907-
EspKeyValueStorage::contains(*self, name)
908-
}
909-
910-
fn remove(&mut self, name: &str) -> Result<bool, Self::Error> {
911-
EspKeyValueStorage::remove(*self, name)
912-
}
913-
}
914-
915-
impl<T: NvsPartitionId> RawStorage for &EspKeyValueStorage<T> {
916-
fn len(&self, name: &str) -> Result<Option<usize>, Self::Error> {
917-
EspKeyValueStorage::len(*self, name)
918-
}
919-
920-
fn get_raw<'a>(&self, name: &str, buf: &'a mut [u8]) -> Result<Option<&'a [u8]>, Self::Error> {
921-
EspKeyValueStorage::get_raw(*self, name, buf)
922-
}
923-
924-
fn set_raw(&mut self, name: &str, buf: &[u8]) -> Result<bool, Self::Error> {
925-
EspKeyValueStorage::set_raw(*self, name, buf)
926-
}
927-
}

0 commit comments

Comments
 (0)