Merge pull request #156 from quartiq/rj/iter-path-write

be generic about the thing to write the path into

CHANGELOG.md

@@ -20,9 +20,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   to provide the previously existing functionality.
 * The only required change for most downstream crates to adapt to the above is to
   make sure the `SerDe` trait is in scope (`use miniconf::SerDe`).
-* Paths now start with the path separator (unless they are empty). This affects the `miniconf_derive`
-  crate and both `Miniconf` implementation pairs for `Option`/`Array`.
+* [breaking] Paths now start with the path separator (unless they are empty).
+  This affects the `Miniconf` derive macro and the `Miniconf` implementation pairs
+  for `Option`/`Array`.
+  Downstram crates should ensure non-empty paths start with the separator and
+  expect `next_path` paths to start with the separator or be empty.
 * The path iterator does not need to be `Peekable` anymore.
+* [breaking] `iter_paths`/`MiniconfIter` is now generic over the type
+  to write the path into. Downstream crates should replace `iter_paths::<L, TS>()` with
+  `iter_paths::<L, heapless::String<TS>>()`.
 
 ## [0.7.1] (https://github.com/quartiq/miniconf/compare/v0.7.0...v0.7.1)
 

README.md

@@ -16,7 +16,7 @@ client](MqttClient) and a Python reference implementation to ineract with it.
 
 ## Example
 ```rust
-use miniconf::{Error, Miniconf, SerDe};
+use miniconf::{heapless::String, Error, Miniconf, SerDe};
 use serde::{Deserialize, Serialize};
 
 #[derive(Deserialize, Serialize, Copy, Clone, Default)]
@@ -97,13 +97,13 @@ let len = settings.get("/struct_", &mut buf)?;
 assert_eq!(&buf[..len], br#"{"a":3,"b":3}"#);
 
 // Iterating over and serializing all paths
-for path in Settings::iter_paths::<3, 32>().unwrap() {
+for path in Settings::iter_paths::<3, String<32>>().unwrap() {
     match settings.get(&path, &mut buf) {
         Ok(len) => {
             settings.set(&path, &buf[..len]).unwrap();
         }
         // Some settings are still `None` and thus their paths are expected to be absent
-        Err(Error::PathAbsent) => {},
+        Err(Error::PathAbsent) => {}
         e => {
             e.unwrap();
         }

examples/readback.rs

@@ -1,4 +1,4 @@
-use miniconf::{Miniconf, SerDe};
+use miniconf::{heapless::String, Miniconf, SerDe};
 
 #[derive(Debug, Default, Miniconf)]
 struct AdditionalSettings {
@@ -23,7 +23,7 @@ fn main() {
     };
 
     // Maintains our state of iteration.
-    let mut settings_iter = Settings::iter_paths::<5, 128>().unwrap();
+    let mut settings_iter = Settings::iter_paths::<5, String<128>>().unwrap();
 
     // Just get one topic/value from the iterator
     if let Some(topic) = settings_iter.next() {

src/iter.rs

@@ -1,13 +1,11 @@
-use super::{IterError, Metadata, Miniconf, SerDe};
-use core::marker::PhantomData;
-use heapless::String;
+use super::{IterError, Metadata, SerDe};
+use core::{fmt::Write, marker::PhantomData};
 
 /// An iterator over the paths in a Miniconf namespace.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub struct MiniconfIter<M: ?Sized, const L: usize, const TS: usize, S> {
+pub struct MiniconfIter<M, S, const L: usize, P> {
     /// Zero-size marker field to allow being generic over M and gaining access to M.
-    miniconf: PhantomData<M>,
-    spec: PhantomData<S>,
+    marker: PhantomData<(M, S, P)>,
 
     /// The iteration state.
     ///
@@ -24,26 +22,19 @@ pub struct MiniconfIter<M: ?Sized, const L: usize, const TS: usize, S> {
     count: Option<usize>,
 }
 
-impl<M: ?Sized, const L: usize, const TS: usize, S> Default for MiniconfIter<M, L, TS, S> {
+impl<M, S, const L: usize, P> Default for MiniconfIter<M, S, L, P> {
     fn default() -> Self {
         Self {
             count: None,
-            miniconf: PhantomData,
-            spec: PhantomData,
+            marker: PhantomData,
             state: [0; L],
         }
     }
 }
 
-impl<M: ?Sized + Miniconf + SerDe<S>, const L: usize, const TS: usize, S>
-    MiniconfIter<M, L, TS, S>
-{
+impl<M: SerDe<S>, S, const L: usize, P> MiniconfIter<M, S, L, P> {
     pub fn metadata() -> Result<Metadata, IterError> {
         let meta = M::metadata(M::SEPARATOR.len_utf8());
-        if TS < meta.max_length {
-            return Err(IterError::Length);
-        }
-
         if L < meta.max_depth {
             return Err(IterError::Depth);
         }
@@ -59,13 +50,11 @@ impl<M: ?Sized + Miniconf + SerDe<S>, const L: usize, const TS: usize, S>
     }
 }
 
-impl<M: Miniconf + SerDe<S> + ?Sized, const L: usize, const TS: usize, S> Iterator
-    for MiniconfIter<M, L, TS, S>
-{
-    type Item = String<TS>;
+impl<M: SerDe<S>, S, const L: usize, P: Write + Default> Iterator for MiniconfIter<M, S, L, P> {
+    type Item = P;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let mut path = Self::Item::new();
+        let mut path = Self::Item::default();
 
         loop {
             match M::next_path(&self.state, 0, &mut path, M::SEPARATOR) {
@@ -79,7 +68,7 @@ impl<M: Miniconf + SerDe<S> + ?Sized, const L: usize, const TS: usize, S> Iterat
                     return None;
                 }
                 Err(IterError::Next(depth)) => {
-                    path.clear();
+                    path = Self::Item::default();
                     self.state[depth] = 0;
                     self.state[depth - 1] += 1;
                 }

src/lib.rs

@@ -181,7 +181,7 @@ pub trait Miniconf {
 
 /// Trait for implementing a specific way of serialization/deserialization into/from a slice
 /// and splitting/joining the path with a separator.
-pub trait SerDe<S>: Miniconf {
+pub trait SerDe<S>: Miniconf + Sized {
     /// The path hierarchy separator.
     ///
     /// This is passed to [Miniconf::next_path] by [MiniconfIter] and
@@ -201,12 +201,11 @@ pub trait SerDe<S>: Miniconf {
     ///
     /// # Generics
     /// * `L`  - The maximum depth of the path, i.e. number of separators plus 1.
-    /// * `TS` - The maximum length of the path in bytes.
+    /// * `P`  - The type to hold the path.
     ///
     /// # Returns
-    /// A [MiniconfIter] of paths or an [IterError] if `L` or `TS` are insufficient.
-    fn iter_paths<const L: usize, const TS: usize>(
-    ) -> Result<iter::MiniconfIter<Self, L, TS, S>, IterError> {
+    /// A [MiniconfIter] of paths or an [IterError] if `L` is insufficient.
+    fn iter_paths<const L: usize, P>() -> Result<iter::MiniconfIter<Self, S, L, P>, IterError> {
         MiniconfIter::new()
     }
 
@@ -221,11 +220,11 @@ pub trait SerDe<S>: Miniconf {
     ///
     /// # Generics
     /// * `L`  - The maximum depth of the path, i.e. number of separators plus 1.
-    /// * `TS` - The maximum length of the path in bytes.
+    /// * `P`  - The type to hold the path.
     ///
     /// # Returns
-    /// A [MiniconfIter] of paths or an [IterError] if `L` or `TS` are insufficient.
-    fn unchecked_iter_paths<const L: usize, const TS: usize>() -> MiniconfIter<Self, L, TS, S> {
+    /// A [MiniconfIter] of paths or an [IterError] if `L` is insufficient.
+    fn unchecked_iter_paths<const L: usize, P>() -> MiniconfIter<Self, S, L, P> {
         MiniconfIter::default()
     }
 
@@ -250,7 +249,7 @@ pub trait SerDe<S>: Miniconf {
     fn get(&self, path: &str, data: &mut [u8]) -> Result<usize, Error<Self::SerError>>;
 }
 
-/// Marker struct for [SerDe].
+/// Marker struct for the "JSON and `/`" [SerDe] specification.
 ///
 /// Access items with `'/'` as path separator and JSON (from `serde-json-core`)
 /// as serialization/deserialization payload format.

src/mqtt_client/mod.rs

@@ -23,7 +23,8 @@ const MAX_RECURSION_DEPTH: usize = 8;
 // republished.
 const REPUBLISH_TIMEOUT_SECONDS: u32 = 2;
 
-type MiniconfIter<M> = crate::MiniconfIter<M, MAX_RECURSION_DEPTH, MAX_TOPIC_LENGTH, JsonCoreSlash>;
+type MiniconfIter<M> =
+    crate::MiniconfIter<M, JsonCoreSlash, MAX_RECURSION_DEPTH, String<MAX_TOPIC_LENGTH>>;
 
 mod sm {
     use minimq::embedded_time::{self, duration::Extensions, Instant};
@@ -50,7 +51,7 @@ mod sm {
         }
     }
 
-    pub struct Context<C: embedded_time::Clock, M: super::Miniconf + ?Sized> {
+    pub struct Context<C: embedded_time::Clock, M: super::Miniconf> {
         clock: C,
         timeout: Option<Instant<C>>,
         pub republish_state: super::MiniconfIter<M>,

tests/arrays.rs

@@ -1,4 +1,4 @@
-use miniconf::{Array, Error, Miniconf, SerDe};
+use miniconf::{heapless::String, Array, Error, Miniconf, SerDe};
 use serde::Deserialize;
 
 #[derive(Debug, Default, Miniconf, Deserialize)]
@@ -192,7 +192,7 @@ fn null_array() {
         #[miniconf(defer)]
         data: [u32; 0],
     }
-    assert!(S::iter_paths::<2, 7>().unwrap().next().is_none());
+    assert!(S::iter_paths::<2, String<7>>().unwrap().next().is_none());
 }
 
 #[test]
@@ -206,5 +206,5 @@ fn null_miniconf_array() {
         #[miniconf(defer)]
         data: Array<I, 0>,
     }
-    assert!(S::iter_paths::<3, 9>().unwrap().next().is_none());
+    assert!(S::iter_paths::<3, String<9>>().unwrap().next().is_none());
 }

tests/iter.rs

@@ -1,4 +1,4 @@
-use miniconf::{Miniconf, SerDe};
+use miniconf::{heapless::String, Miniconf, SerDe};
 
 #[derive(Miniconf, Default)]
 struct Inner {
@@ -21,10 +21,7 @@ fn insufficient_space() {
     assert_eq!(meta.count, 3);
 
     // Ensure that we can't iterate if we make a state vector that is too small.
-    assert!(Settings::iter_paths::<1, 256>().is_err());
-
-    // Ensure that we can't iterate if the topic buffer is too small.
-    assert!(Settings::iter_paths::<10, 1>().is_err());
+    assert!(Settings::iter_paths::<1, String<256>>().is_err());
 }
 
 #[test]
@@ -35,7 +32,7 @@ fn test_iteration() {
         ("/c/inner".to_string(), false),
     ]);
 
-    for field in Settings::iter_paths::<32, 256>().unwrap() {
+    for field in Settings::iter_paths::<32, String<256>>().unwrap() {
         assert!(iterated.contains_key(&field.as_str().to_string()));
         iterated.insert(field.as_str().to_string(), true);
     }
@@ -54,7 +51,7 @@ fn test_array_iteration() {
 
     let mut settings = Settings::default();
 
-    for field in Settings::iter_paths::<32, 256>().unwrap() {
+    for field in Settings::iter_paths::<32, String<256>>().unwrap() {
         settings.set(&field, b"true").unwrap();
     }
 

tests/option.rs

@@ -1,4 +1,4 @@
-use miniconf::{Miniconf, SerDe};
+use miniconf::{heapless::String, Miniconf, SerDe};
 
 #[derive(PartialEq, Debug, Clone, Default, Miniconf)]
 struct Inner {
@@ -13,7 +13,7 @@ struct Settings {
 
 #[test]
 fn just_option() {
-    let mut it = Option::<u32>::iter_paths::<1, 0>().unwrap();
+    let mut it = Option::<u32>::iter_paths::<1, String<0>>().unwrap();
     assert_eq!(it.next(), Some("".into()));
     assert_eq!(it.next(), None);
 }
@@ -60,13 +60,13 @@ fn option_iterate_some_none() {
 
     // When the value is None, it will still be iterated over as a topic but may not exist at runtime.
     settings.value.take();
-    let mut iterator = Settings::iter_paths::<10, 128>().unwrap();
+    let mut iterator = Settings::iter_paths::<10, String<128>>().unwrap();
     assert_eq!(iterator.next().unwrap(), "/value/data");
     assert!(iterator.next().is_none());
 
     // When the value is Some, it should be iterated over.
     settings.value.replace(Inner { data: 5 });
-    let mut iterator = Settings::iter_paths::<10, 128>().unwrap();
+    let mut iterator = Settings::iter_paths::<10, String<128>>().unwrap();
     assert_eq!(iterator.next().unwrap(), "/value/data");
     assert!(iterator.next().is_none());
 }
@@ -81,14 +81,14 @@ fn option_test_normal_option() {
     let mut s = S::default();
     assert!(s.data.is_none());
 
-    let mut iterator = S::iter_paths::<10, 128>().unwrap();
+    let mut iterator = S::iter_paths::<10, String<128>>().unwrap();
     assert_eq!(iterator.next(), Some("/data".into()));
     assert!(iterator.next().is_none());
 
     s.set("/data", b"7").unwrap();
     assert_eq!(s.data, Some(7));
 
-    let mut iterator = S::iter_paths::<10, 128>().unwrap();
+    let mut iterator = S::iter_paths::<10, String<128>>().unwrap();
     assert_eq!(iterator.next(), Some("/data".into()));
     assert!(iterator.next().is_none());
 
@@ -107,7 +107,7 @@ fn option_test_defer_option() {
     let mut s = S::default();
     assert!(s.data.is_none());
 
-    let mut iterator = S::iter_paths::<10, 128>().unwrap();
+    let mut iterator = S::iter_paths::<10, String<128>>().unwrap();
     assert_eq!(iterator.next(), Some("/data".into()));
     assert!(iterator.next().is_none());
 
@@ -116,7 +116,7 @@ fn option_test_defer_option() {
     s.set("/data", b"7").unwrap();
     assert_eq!(s.data, Some(7));
 
-    let mut iterator = S::iter_paths::<10, 128>().unwrap();
+    let mut iterator = S::iter_paths::<10, String<128>>().unwrap();
     assert_eq!(iterator.next(), Some("/data".into()));
     assert!(iterator.next().is_none());
 

tests/structs.rs

@@ -1,4 +1,4 @@
-use miniconf::{Miniconf, SerDe};
+use miniconf::{heapless::String, Miniconf, SerDe};
 use serde::{Deserialize, Serialize};
 
 #[test]
@@ -97,5 +97,8 @@ fn struct_with_string() {
 fn empty_struct() {
     #[derive(Miniconf, Default)]
     struct Settings {}
-    assert!(Settings::iter_paths::<1, 0>().unwrap().next().is_none());
+    assert!(Settings::iter_paths::<1, String<0>>()
+        .unwrap()
+        .next()
+        .is_none());
 }