Skip to content

Commit 19d2f7e

Browse files
authored
Rework with new trait (#28)
This is a fairly large PR, but in broad strokes, it: * Introduces the trait described by #25 * Implements the behavior and "data format" described by #25 * One note: we are now able to store the whole path as the key, instead of just a hash * Implements a non-sequential-storage based test impl of the trait, allowing for more direct testing The trait lifetimes got ROUGH, and due to that, I ended up needing to compromise on some interface niceness, but I was able to make it okay-ish with some macros. End-users won't see this grossness, only folks that try to implement the trait, and if they use the blanket sequential-storage impl: they won't ever need to touch it. Also as a note, I had hoped at least the test impl of the trait would be `Send`, but there are still issues (cc rust-lang/rust#100013), so I've switched to just using `LocalSet` for all testing to not require `Send` in testing.
1 parent 1e7a8c5 commit 19d2f7e

File tree

13 files changed

+2982
-2973
lines changed

13 files changed

+2982
-2973
lines changed

.github/workflows/miri.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,3 @@ jobs:
2424
cargo miri setup
2525
- name: Test with Miri
2626
run: cargo miri test --features std
27-
- name: Run with Miri
28-
run: cargo miri run --features std

Cargo.lock

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,6 @@ edition = "2024"
33
name = "cfg-noodle"
44
version = "0.1.0"
55

6-
[[bin]]
7-
name = "main"
8-
path = "src/main.rs"
9-
required-features = ["std"]
10-
116
[[test]]
127
name = "integration_tests"
138
required-features = ["std"]
@@ -54,6 +49,7 @@ defmt = { version = "1.0.1", optional = true }
5449
embassy-futures = "0.1.1"
5550

5651
arbitrary = { version = "1", optional = true, features = ["derive"] }
52+
crc = "3.3.0"
5753

5854
[dev-dependencies]
5955
approx = "0.5.1"

src/error.rs

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
//! Error types used for the list
2-
use sequential_storage::Error as SecStorError;
3-
4-
use crate::intrusive::{KEY_LEN, State};
2+
use crate::storage_node::State;
53

64
/// General error that is not specific to the flash implementation
75
#[derive(Debug)]
@@ -11,34 +9,35 @@ pub enum Error {
119
Deserialization,
1210
/// Serializing a node into the buffer failed
1311
Serialization,
14-
/// Old version of the node was found. Returned when the node does not have the latest
15-
/// counter value.
16-
OldData,
1712
/// The key hash already exists in the list
1813
DuplicateKey,
14+
/// Recoverable error to tell the caller that the list needs reading first.
15+
NeedsRead,
1916
/// Node is in a state that is invalid at the particular point of operation.
2017
/// Contains a tuple of the node key hash and the state that was deemed invalid.
21-
InvalidState([u8; KEY_LEN], State),
18+
InvalidState(&'static str, State),
2219
}
2320

24-
// TODO @James: I have created this error because it uses the generic <F>. Adding generics
25-
// to the generall error struct was super awkward. Is there any better way of wrapping errors
26-
// from other crates when they require generics?
27-
2821
#[derive(Debug)]
2922
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
3023
/// Errors during loading from and storing to flash.
3124
///
3225
/// Sometimes specific to the flash implementation.
33-
pub enum LoadStoreError<F> {
34-
/// Writing to flash has failed. Contains the error returned by sequential storage.
35-
FlashWrite(SecStorError<F>),
36-
/// Reading from flash has failed. Contains the error returned by sequential storage.
37-
FlashRead(SecStorError<F>),
26+
pub enum LoadStoreError<T> {
27+
/// Needs Initial read processed
28+
NeedsFirstRead,
29+
/// Writing to flash has failed. Contains the error returned by the storage impl.
30+
FlashWrite(T),
31+
/// Reading from flash has failed. Contains the error returned by the storage impl.
32+
FlashRead(T),
3833
/// Value read back from the flash during verification did not match serialized list node.
3934
WriteVerificationFailed,
40-
/// Recoverable error to tell the caller that the list needs reading first.
41-
NeedsRead,
4235
/// Application-level error that occurred during list operations.
4336
AppError(Error),
4437
}
38+
39+
impl<T> From<Error> for LoadStoreError<T> {
40+
fn from(value: Error) -> Self {
41+
Self::AppError(value)
42+
}
43+
}

src/flash.rs

Lines changed: 168 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,57 @@
22
//!
33
//! This module provides a `Flash` struct that wraps a MultiwriteNorFlash device
44
//! and exposes async methods for queue-like operations on persistent storage.
5+
use core::{num::NonZeroU32, ops::Deref};
56

6-
use embedded_storage_async::nor_flash::MultiwriteNorFlash;
7+
use embedded_storage_async::nor_flash::{ErrorType, MultiwriteNorFlash};
78
use sequential_storage::{
8-
Error as SeqStorError,
99
cache::CacheImpl,
10-
queue::{self, QueueIterator},
10+
queue::{self, QueueIterator, QueueIteratorEntry},
1111
};
1212

13+
use crate::{Elem, NdlDataStorage, NdlElemIter, NdlElemIterNode, SerData, consts};
14+
1315
/// Owns a flash and the range reserved for the `StorageList`
1416
pub struct Flash<T: MultiwriteNorFlash, C: CacheImpl> {
1517
flash: T,
1618
range: core::ops::Range<u32>,
1719
cache: C,
1820
}
1921

22+
/// An iterator over a [`Flash`]
23+
pub struct FlashIter<'flash, T: MultiwriteNorFlash, C: CacheImpl> {
24+
iter: QueueIterator<'flash, T, C>,
25+
}
26+
27+
/// A single position in the flash queue iterator
28+
///
29+
/// This represents a well-decoded element, where `half` contains the decoded
30+
/// contents that correspond with `qit`.
31+
pub struct FlashNode<'flash, 'iter, 'buf, T: MultiwriteNorFlash, C: CacheImpl> {
32+
half: HalfElem,
33+
qit: QueueIteratorEntry<'flash, 'buf, 'iter, T, C>,
34+
}
35+
36+
/// A partially decoded element
37+
///
38+
/// This is a helper type that mimics [`Elem`], so we don't need to re-decode it
39+
/// on every access.
40+
#[derive(Clone, Copy)]
41+
enum HalfElem {
42+
Start { seq_no: NonZeroU32 },
43+
Data,
44+
End { seq_no: NonZeroU32, calc_crc: u32 },
45+
}
46+
47+
// ---- impl Flash ----
48+
2049
impl<T: MultiwriteNorFlash, C: CacheImpl> Flash<T, C> {
2150
/// Creates a new Flash instance with the given flash device and address range.
2251
///
2352
/// # Arguments
2453
/// * `flash` - The MultiwriteNorFlash device to use for storage operations
2554
/// * `range` - The address range within the flash device reserved for this storage
55+
/// * `cache` - the cache to use with this flash access
2656
pub fn new(flash: T, range: core::ops::Range<u32>, cache: C) -> Self {
2757
Self {
2858
flash,
@@ -35,36 +65,153 @@ impl<T: MultiwriteNorFlash, C: CacheImpl> Flash<T, C> {
3565
pub fn flash(&mut self) -> &mut T {
3666
&mut self.flash
3767
}
68+
}
69+
70+
impl<T: MultiwriteNorFlash + 'static, C: CacheImpl + 'static> NdlDataStorage for Flash<T, C> {
71+
type Iter<'this>
72+
= FlashIter<'this, T, C>
73+
where
74+
Self: 'this;
75+
76+
type Error = sequential_storage::Error<<T as ErrorType>::Error>;
77+
78+
async fn iter_elems<'this>(
79+
&'this mut self,
80+
) -> Result<Self::Iter<'this>, <Self::Iter<'this> as NdlElemIter>::Error> {
81+
Ok(FlashIter {
82+
iter: queue::iter(&mut self.flash, self.range.clone(), &mut self.cache).await?,
83+
})
84+
}
85+
86+
async fn push(&mut self, data: &Elem<'_>) -> Result<(), Self::Error> {
87+
// scratch buffer used if this is start/end
88+
let mut buf = [0u8; 9];
89+
let used = match data {
90+
Elem::Start { seq_no } => {
91+
let buf = &mut buf[..5];
92+
buf[0] = consts::ELEM_START;
93+
buf[1..5].copy_from_slice(&seq_no.get().to_le_bytes());
94+
buf
95+
}
96+
Elem::Data { data } => data.hdr_key_val,
97+
Elem::End { seq_no, calc_crc } => {
98+
buf[0] = consts::ELEM_END;
99+
buf[1..5].copy_from_slice(&seq_no.get().to_le_bytes());
100+
buf[5..9].copy_from_slice(&calc_crc.to_le_bytes());
101+
buf.as_slice()
102+
}
103+
};
38104

39-
/// Pushes data to the sequential storage queue.
40-
pub async fn push(&mut self, data: &[u8]) -> Result<(), SeqStorError<T::Error>> {
105+
// Push data to the underlying queue
41106
queue::push(
42107
&mut self.flash,
43108
self.range.clone(),
44109
&mut self.cache,
45-
data,
110+
used,
46111
false,
47112
)
48113
.await
49114
}
115+
}
116+
117+
// ---- impl FlashIter ----
50118

51-
/// Creates an iterator over the sequential storage queue.
52-
pub async fn iter(&mut self) -> Result<QueueIterator<T, C>, SeqStorError<T::Error>> {
53-
queue::iter(&mut self.flash, self.range.clone(), &mut self.cache).await
119+
impl<'flash, T: MultiwriteNorFlash + 'static, C: CacheImpl + 'static> NdlElemIter
120+
for FlashIter<'flash, T, C>
121+
{
122+
type Item<'this, 'buf>
123+
= FlashNode<'flash, 'this, 'buf, T, C>
124+
where
125+
Self: 'this,
126+
Self: 'buf;
127+
128+
type Error = sequential_storage::Error<<T as ErrorType>::Error>;
129+
130+
async fn next<'iter, 'buf>(
131+
&'iter mut self,
132+
buf: &'buf mut [u8],
133+
) -> Result<Option<Option<Self::Item<'iter, 'buf>>>, Self::Error>
134+
where
135+
Self: 'buf,
136+
Self: 'iter,
137+
{
138+
// Attempt to get the next item
139+
let nxt: Option<QueueIteratorEntry<'flash, 'buf, 'iter, T, C>> =
140+
self.iter.next(buf).await?;
141+
142+
// No data? all done.
143+
let Some(nxt) = nxt else { return Ok(None) };
144+
145+
// Can we decode this as an element?
146+
if let Some(elem) = HalfElem::from_bytes(&nxt) {
147+
Ok(Some(Some(FlashNode {
148+
half: elem,
149+
qit: nxt,
150+
})))
151+
} else {
152+
Ok(Some(None))
153+
}
54154
}
155+
}
156+
157+
// ---- impl FlashIterNode ----
158+
159+
impl<T: MultiwriteNorFlash, C: CacheImpl> NdlElemIterNode for FlashNode<'_, '_, '_, T, C> {
160+
type Error = sequential_storage::Error<<T as ErrorType>::Error>;
55161

56-
/// Pops data from the sequential storage queue.
57-
pub async fn pop<'a>(
58-
&mut self,
59-
data: &'a mut [u8],
60-
) -> Result<Option<&'a mut [u8]>, SeqStorError<T::Error>> {
61-
queue::pop(&mut self.flash, self.range.clone(), &mut self.cache, data).await
162+
fn data(&self) -> Elem<'_> {
163+
match self.half {
164+
HalfElem::Start { seq_no } => Elem::Start { seq_no },
165+
HalfElem::Data => Elem::Data {
166+
data: SerData::from_existing(self.qit.deref()).unwrap(),
167+
},
168+
HalfElem::End { seq_no, calc_crc } => Elem::End { seq_no, calc_crc },
169+
}
170+
}
171+
172+
async fn invalidate(self) -> Result<(), Self::Error> {
173+
self.qit.pop().await?;
174+
Ok(())
62175
}
63-
/// Peeks at data from the sequential storage queue.
64-
pub async fn peek<'a>(
65-
&mut self,
66-
data: &'a mut [u8],
67-
) -> Result<Option<&'a mut [u8]>, SeqStorError<T::Error>> {
68-
queue::peek(&mut self.flash, self.range.clone(), &mut self.cache, data).await
176+
}
177+
178+
// ---- impl HalfElem ----
179+
180+
impl HalfElem {
181+
fn from_bytes(data: &[u8]) -> Option<Self> {
182+
let (first, rest) = data.split_first()?;
183+
match *first {
184+
consts::ELEM_START => {
185+
if rest.len() != 4 {
186+
return None;
187+
}
188+
let mut bytes = [0u8; 4];
189+
bytes.copy_from_slice(rest);
190+
Some(HalfElem::Start {
191+
seq_no: NonZeroU32::new(u32::from_le_bytes(bytes))?,
192+
})
193+
}
194+
consts::ELEM_DATA => {
195+
if rest.is_empty() {
196+
None
197+
} else {
198+
Some(HalfElem::Data)
199+
}
200+
}
201+
consts::ELEM_END => {
202+
if rest.len() != 8 {
203+
return None;
204+
}
205+
let mut seq_bytes = [0u8; 4];
206+
seq_bytes.copy_from_slice(&rest[..4]);
207+
let mut crc_bytes = [0u8; 4];
208+
crc_bytes.copy_from_slice(&rest[4..8]);
209+
Some(HalfElem::End {
210+
seq_no: NonZeroU32::new(u32::from_le_bytes(seq_bytes))?,
211+
calc_crc: u32::from_le_bytes(crc_bytes),
212+
})
213+
}
214+
_ => None,
215+
}
69216
}
70217
}

0 commit comments

Comments
 (0)