From 41c32a202744d3a27b4b824220708eea318fedbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 15 Jul 2024 11:50:25 +0200 Subject: [PATCH 01/12] serial-settings: subtree list, clear, get --- serial-settings/src/lib.rs | 246 +++++++++++++++++++++++-------------- 1 file changed, 154 insertions(+), 92 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index c729e776b..5d2904918 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -122,83 +122,122 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface.platform.cmd(key) } + fn iter_root( + key: Option<&str>, + interface: &mut Self, + settings: &mut P::Settings, + mut func: F, + ) where + F: FnMut( + &Path, '/'>, + &mut Self, + &mut P::Settings, + ) -> Result<(), ()>, + { + let mut iter = P::Settings::nodes::, '/'>>(); + if let Some(key) = key { + match iter.root(&Path::<_, '/'>::from(key)) { + Ok(it) => iter = it, + Err(e) => { + writeln!(interface, "Failed to list keys: {e}").unwrap(); + return; + } + }; + } + + for key in iter { + let key = match key { + Ok((key, node)) => { + assert!(node.is_leaf()); + key + } + Err(depth) => { + writeln!( + interface, + "Failed to get path: no space at depth {depth}" + ) + .unwrap(); + return; + } + }; + match func(&key, interface, settings) { + Ok(()) => {} + Err(()) => break, + } + } + } + fn handle_list( _menu: &menu::Menu, - _item: &menu::Item, - _args: &[&str], + item: &menu::Item, + args: &[&str], interface: &mut Self, settings: &mut P::Settings, ) { let mut defaults = settings.clone(); defaults.reset(); + let key = menu::argument_finder(item, args, "item").unwrap(); + Self::iter_root( + key, + interface, + settings, + |path, interface, settings| { + let value = match settings + .get_json_by_key(path, interface.buffer) + { + Err(e) => { + writeln!( + interface, + "Failed to read {}: {e}", + path.as_str() + ) + .unwrap(); + return Ok(()); + } + Ok(len) => { + core::str::from_utf8(&interface.buffer[..len]).unwrap() + } + }; - for path in P::Settings::nodes::, '/'>>() { - match path { - Err(depth) => writeln!( - interface, - "Failed to get path: no space at depth {depth}" - ), - Ok((path, _node)) => { - let value = match settings - .get_json_by_key(&path, interface.buffer) - { - Err(e) => { - writeln!( - interface, - "Failed to read {}: {e}", - path.as_str() - ) - .unwrap(); - continue; - } - Ok(len) => { - core::str::from_utf8(&interface.buffer[..len]) - .unwrap() - } - }; - - write!( - interface.platform.interface_mut(), - "{}: {value}", - path.as_str(), - ) - .unwrap(); + write!( + interface.platform.interface_mut(), + "{}: {value}", + path.as_str(), + ) + .unwrap(); - let value_hash: u64 = yafnv::fnv1a(value); + let value_hash: u64 = yafnv::fnv1a(value); - let default_value = match defaults - .get_json_by_key(&path, interface.buffer) - { - Err(e) => { - writeln!( - interface, - "[default serialization error: {e}]" - ) - .unwrap(); - continue; - } - Ok(len) => { - core::str::from_utf8(&interface.buffer[..len]) - .unwrap() - } - }; - - let default_hash: u64 = yafnv::fnv1a(default_value); - if default_hash != value_hash { + let default_value = match defaults + .get_json_by_key(path, interface.buffer) + { + Err(e) => { writeln!( - interface.platform.interface_mut(), - " [default: {default_value}]" - ) - } else { - writeln!( - interface.platform.interface_mut(), - " [default]" + interface, + "[default serialization error: {e}]" ) + .unwrap(); + return Ok(()); + } + Ok(len) => { + core::str::from_utf8(&interface.buffer[..len]).unwrap() } + }; + + let default_hash: u64 = yafnv::fnv1a(default_value); + if default_hash != value_hash { + writeln!( + interface.platform.interface_mut(), + " [default: {default_value}]" + ) + .unwrap(); + } else { + writeln!(interface.platform.interface_mut(), " [default]") + .unwrap(); } - } - .unwrap() - } + Ok(()) + }, + ); } fn handle_clear( @@ -213,23 +252,39 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { let mut defaults = settings.clone(); defaults.reset(); - let len = match defaults.get_json(key, interface.buffer) { - Err(e) => { - writeln!(interface, "Failed to clear `{key}`: {e:?}") - .unwrap(); - return; - } + Self::iter_root( + Some(key), + interface, + settings, + |path, interface, settings| { + let len = match defaults + .get_json_by_key(path, interface.buffer) + { + Err(e) => { + writeln!( + interface, + "Failed to clear `{key}`: {e:?}" + ) + .unwrap(); + return Ok(()); + } - Ok(len) => len, - }; + Ok(len) => len, + }; - if let Err(e) = settings.set_json(key, &interface.buffer[..len]) { - writeln!(interface, "Failed to update {key}: {e:?}").unwrap(); - return; - } + if let Err(e) = + settings.set_json(key, &interface.buffer[..len]) + { + writeln!(interface, "Failed to update {key}: {e:?}") + .unwrap(); + return Ok(()); + } - interface.updated = true; - writeln!(interface, "{key} cleared to default").unwrap(); + interface.updated = true; + writeln!(interface, "{key} cleared to default").unwrap(); + Ok(()) + }, + ); } else { settings.reset(); interface.updated = true; @@ -246,20 +301,24 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface: &mut Self, settings: &mut P::Settings, ) { - let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); - match settings.get_json(key, interface.buffer) { - Err(e) => { - writeln!(interface, "Failed to read {key}: {e}") - } - Ok(len) => { - writeln!( - interface.platform.interface_mut(), - "{key}: {}", - core::str::from_utf8(&interface.buffer[..len]).unwrap() - ) + let key = menu::argument_finder(item, args, "item").unwrap(); + Self::iter_root(key, interface, settings, |key, interface, settings| { + match settings.get_json_by_key(key, interface.buffer) { + Err(e) => { + writeln!(interface, "Failed to read {}: {e}", &key.0) + } + Ok(len) => { + writeln!( + interface.platform.interface_mut(), + "{}: {}", + &key.0, + core::str::from_utf8(&interface.buffer[..len]).unwrap() + ) + } } - } - .unwrap(); + .unwrap(); + Ok(()) + }) } fn handle_save( @@ -317,7 +376,10 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { help: Some("List all available settings and their current values."), item_type: menu::ItemType::Callback { function: Self::handle_list, - parameters: &[], + parameters: &[menu::Parameter::Optional { + parameter_name: "item", + help: Some("The path below which to list settings."), + },], }, }, &menu::Item { @@ -325,7 +387,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { help: Some("Read a setting_from the device."), item_type: menu::ItemType::Callback { function: Self::handle_get, - parameters: &[menu::Parameter::Mandatory { + parameters: &[menu::Parameter::Optional { parameter_name: "item", help: Some("The name of the setting to read."), }] From d417c5ba30e9cfee0dc584cd5d6fc93dc9477f53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 15 Jul 2024 11:16:05 +0000 Subject: [PATCH 02/12] serial-settings: remove redundant list command --- serial-settings/src/lib.rs | 129 ++++++++++++++----------------------- 1 file changed, 48 insertions(+), 81 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 5d2904918..a68a818d7 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -167,7 +167,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { } } - fn handle_list( + fn handle_get( _menu: &menu::Menu, item: &menu::Item, args: &[&str], @@ -176,20 +176,20 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { ) { let mut defaults = settings.clone(); defaults.reset(); - let key = menu::argument_finder(item, args, "item").unwrap(); + let key = menu::argument_finder(item, args, "path").unwrap(); Self::iter_root( key, interface, settings, - |path, interface, settings| { + |key, interface, settings| { let value = match settings - .get_json_by_key(path, interface.buffer) + .get_json_by_key(key, interface.buffer) { Err(e) => { writeln!( interface, "Failed to read {}: {e}", - path.as_str() + key.as_str() ) .unwrap(); return Ok(()); @@ -202,14 +202,14 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { write!( interface.platform.interface_mut(), "{}: {value}", - path.as_str(), + key.as_str(), ) .unwrap(); let value_hash: u64 = yafnv::fnv1a(value); let default_value = match defaults - .get_json_by_key(path, interface.buffer) + .get_json_by_key(key, interface.buffer) { Err(e) => { writeln!( @@ -247,7 +247,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface: &mut Self, settings: &mut P::Settings, ) { - let maybe_key = menu::argument_finder(item, args, "item").unwrap(); + let maybe_key = menu::argument_finder(item, args, "path").unwrap(); if let Some(key) = maybe_key { let mut defaults = settings.clone(); defaults.reset(); @@ -256,69 +256,47 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { Some(key), interface, settings, - |path, interface, settings| { - let len = match defaults - .get_json_by_key(path, interface.buffer) - { - Err(e) => { - writeln!( - interface, - "Failed to clear `{key}`: {e:?}" - ) - .unwrap(); - return Ok(()); - } - - Ok(len) => len, - }; + |key, interface, settings| { + let len = + match defaults.get_json_by_key(key, interface.buffer) { + Err(e) => { + writeln!( + interface, + "Failed to clear `{}`: {e:?}", + key.as_str() + ) + .unwrap(); + return Ok(()); + } + + Ok(len) => len, + }; if let Err(e) = - settings.set_json(key, &interface.buffer[..len]) + settings.set_json_by_key(key, &interface.buffer[..len]) { - writeln!(interface, "Failed to update {key}: {e:?}") - .unwrap(); + writeln!( + interface, + "Failed to update {}: {e:?}", + key.as_str() + ) + .unwrap(); return Ok(()); } + interface.platform.clear(interface.buffer, Some(key)); interface.updated = true; - writeln!(interface, "{key} cleared to default").unwrap(); + writeln!(interface, "{} cleared to default", key.as_str()) + .unwrap(); Ok(()) }, ); } else { settings.reset(); + interface.platform.clear(interface.buffer, None); interface.updated = true; writeln!(interface, "All settings cleared").unwrap(); } - - interface.platform.clear(interface.buffer, maybe_key); - } - - fn handle_get( - _menu: &menu::Menu, - item: &menu::Item, - args: &[&str], - interface: &mut Self, - settings: &mut P::Settings, - ) { - let key = menu::argument_finder(item, args, "item").unwrap(); - Self::iter_root(key, interface, settings, |key, interface, settings| { - match settings.get_json_by_key(key, interface.buffer) { - Err(e) => { - writeln!(interface, "Failed to read {}: {e}", &key.0) - } - Ok(len) => { - writeln!( - interface.platform.interface_mut(), - "{}: {}", - &key.0, - core::str::from_utf8(&interface.buffer[..len]).unwrap() - ) - } - } - .unwrap(); - Ok(()) - }) } fn handle_save( @@ -328,7 +306,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface: &mut Self, settings: &mut P::Settings, ) { - match interface.platform.save(interface.buffer, menu::argument_finder(item, args, "item").unwrap(), settings) { + match interface.platform.save(interface.buffer, menu::argument_finder(item, args, "path").unwrap(), settings) { Ok(_) => writeln!( interface, "Settings saved. You may need to reboot for the settings to be applied" @@ -347,7 +325,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface: &mut Self, settings: &mut P::Settings, ) { - let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); + let key = menu::argument_finder(item, args, "path").unwrap().unwrap(); let value = menu::argument_finder(item, args, "value").unwrap().unwrap(); @@ -371,37 +349,26 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { menu::Menu { label: "settings", items: &[ - &menu::Item { - command: "list", - help: Some("List all available settings and their current values."), - item_type: menu::ItemType::Callback { - function: Self::handle_list, - parameters: &[menu::Parameter::Optional { - parameter_name: "item", - help: Some("The path below which to list settings."), - },], - }, - }, &menu::Item { command: "get", - help: Some("Read a setting_from the device."), + help: Some("List paths and read settings values"), item_type: menu::ItemType::Callback { function: Self::handle_get, parameters: &[menu::Parameter::Optional { - parameter_name: "item", - help: Some("The name of the setting to read."), + parameter_name: "path", + help: Some("The path of the setting to list/read"), }] }, }, &menu::Item { command: "set", - help: Some("Update a a setting in the device."), + help: Some("Update a setting"), item_type: menu::ItemType::Callback { function: Self::handle_set, parameters: &[ menu::Parameter::Mandatory { - parameter_name: "item", - help: Some("The name of the setting to write."), + parameter_name: "path", + help: Some("The path of the setting to set"), }, menu::Parameter::Mandatory { parameter_name: "value", @@ -412,26 +379,26 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { }, &menu::Item { command: "save", - help: Some("Save settings to the device."), + help: Some("Save settings to memory"), item_type: menu::ItemType::Callback { function: Self::handle_save, parameters: &[ menu::Parameter::Optional { - parameter_name: "item", - help: Some("The name of the setting to clear."), + parameter_name: "path", + help: Some("The path of the setting to save"), }, ] }, }, &menu::Item { command: "clear", - help: Some("Clear the device settings to default values."), + help: Some("Clear settings to default values"), item_type: menu::ItemType::Callback { function: Self::handle_clear, parameters: &[ menu::Parameter::Optional { - parameter_name: "item", - help: Some("The name of the setting to clear."), + parameter_name: "path", + help: Some("The path of the setting to clear"), }, ] }, From 2621d7b116dc7540066d5d97859781078e0806cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 15 Jul 2024 12:18:27 +0000 Subject: [PATCH 03/12] serial-settings: flatten Interface iters --- serial-settings/src/lib.rs | 141 +++++++++++++++++--------------- src/settings.rs | 159 ++++++++++++++++--------------------- 2 files changed, 148 insertions(+), 152 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index a68a818d7..98454bfb1 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -51,7 +51,7 @@ use embedded_io::{ErrorType, Read, ReadReady, Write}; use heapless::String; -use miniconf::{JsonCoreSlash, Path, TreeKey}; +use miniconf::{JsonCoreSlash, Path, Traversal, TreeKey}; mod interface; @@ -78,11 +78,11 @@ pub trait Platform: Sized { /// `save()` Error type type Error: core::fmt::Debug; - /// Save the settings to storage + /// Save the setting to storage fn save( &mut self, buffer: &mut [u8], - key: Option<&str>, + key: &str, settings: &Self::Settings, ) -> Result<(), Self::Error>; @@ -97,8 +97,8 @@ pub trait Platform: Sized { /// /// # Arguments /// * `buffer` The element serialization buffer. - /// * `key` The name of the setting to be cleared. If `None`, all settings are cleared. - fn clear(&mut self, buffer: &mut [u8], key: Option<&str>); + /// * `key` The name of the setting to be cleared. + fn clear(&mut self, buffer: &mut [u8], key: &str); /// Return a mutable reference to the `Interface`. fn interface_mut(&mut self) -> &mut Self::Interface; @@ -185,6 +185,9 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { let value = match settings .get_json_by_key(key, interface.buffer) { + Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { + return Ok(()); + } Err(e) => { writeln!( interface, @@ -211,6 +214,10 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { let default_value = match defaults .get_json_by_key(key, interface.buffer) { + Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { + writeln!(interface, "[default: absent]").unwrap(); + return Ok(()); + } Err(e) => { writeln!( interface, @@ -247,56 +254,49 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface: &mut Self, settings: &mut P::Settings, ) { - let maybe_key = menu::argument_finder(item, args, "path").unwrap(); - if let Some(key) = maybe_key { - let mut defaults = settings.clone(); - defaults.reset(); - - Self::iter_root( - Some(key), - interface, - settings, - |key, interface, settings| { - let len = - match defaults.get_json_by_key(key, interface.buffer) { - Err(e) => { - writeln!( - interface, - "Failed to clear `{}`: {e:?}", - key.as_str() - ) - .unwrap(); - return Ok(()); - } - - Ok(len) => len, - }; - - if let Err(e) = - settings.set_json_by_key(key, &interface.buffer[..len]) - { - writeln!( - interface, - "Failed to update {}: {e:?}", - key.as_str() - ) + let key = menu::argument_finder(item, args, "path").unwrap(); + let mut defaults = settings.clone(); + defaults.reset(); + Self::iter_root(key, interface, settings, |key, interface, settings| { + let len = match defaults.get_json_by_key(key, interface.buffer) { + Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { + writeln!(interface, "{} default is absent", key.as_str()) .unwrap(); - return Ok(()); - } + return Ok(()); + } + Err(e) => { + writeln!( + interface, + "Failed to get default `{}`: {e:?}", + key.as_str() + ) + .unwrap(); + return Ok(()); + } + Ok(len) => len, + }; - interface.platform.clear(interface.buffer, Some(key)); - interface.updated = true; - writeln!(interface, "{} cleared to default", key.as_str()) - .unwrap(); - Ok(()) - }, - ); - } else { - settings.reset(); - interface.platform.clear(interface.buffer, None); + match settings.set_json_by_key(key, &interface.buffer[..len]) { + Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { + return Ok(()); + } + Err(e) => { + writeln!( + interface, + "Failed to update {}: {e:?}", + key.as_str() + ) + .unwrap(); + return Ok(()); + } + Ok(_len) => {} + } + + interface.platform.clear(interface.buffer, key); interface.updated = true; - writeln!(interface, "All settings cleared").unwrap(); - } + writeln!(interface, "{} cleared", key.as_str()).unwrap(); + Ok(()) + }) } fn handle_save( @@ -306,16 +306,33 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface: &mut Self, settings: &mut P::Settings, ) { - match interface.platform.save(interface.buffer, menu::argument_finder(item, args, "path").unwrap(), settings) { - Ok(_) => writeln!( - interface, - "Settings saved. You may need to reboot for the settings to be applied" - ) - .unwrap(), - Err(e) => { - writeln!(interface, "Failed to save settings: {e:?}").unwrap() - } - } + let key = menu::argument_finder(item, args, "path").unwrap(); + let mut defaults = settings.clone(); + defaults.reset(); + Self::iter_root( + key, + interface, + settings, + |key, interface, settings| { + match interface.platform.save(interface.buffer, key, settings) { + Ok(_) => { + writeln!(interface, "{} saved", key.as_str()).unwrap() + } + Err(e) => writeln!( + interface, + "Failed to save {}: {e:?}", + key.as_str() + ) + .unwrap(), + } + Ok(()) + }, + ); + writeln!( + interface, + "Saved settings may require reboot to become active" + ) + .unwrap(); } fn handle_set( diff --git a/src/settings.rs b/src/settings.rs index 6e48cbacd..d243fc790 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -209,64 +209,55 @@ where fn save( &mut self, buf: &mut [u8], - key: Option<&str>, + path: &str, settings: &Self::Settings, ) -> Result<(), Self::Error> { - let mut save_setting = |path| { - let path = SettingsKey(path); - - let mut data = Vec::new(); - data.resize(data.capacity(), 0).unwrap(); - let flavor = postcard::ser_flavors::Slice::new(&mut data); - - let len = match settings.get_postcard_by_key(&path.0, flavor) { - Err(e) => { - log::warn!( - "Failed to save `{}` to flash: {e:?}", - path.0.as_str() - ); - return Ok::<_, Self::Error>(()); - } - Ok(slice) => slice.len(), - }; - data.truncate(len); - - let range = self.storage.range(); - - // Check if the settings has changed from what's currently in flash (or if it doesn't - // yet exist). - if block_on(fetch_item( + let path = SettingsKey(String::try_from(path).unwrap().into()); + + let mut data = Vec::new(); + data.resize(data.capacity(), 0).unwrap(); + let flavor = postcard::ser_flavors::Slice::new(&mut data); + + let len = match settings.get_postcard_by_key(&path.0, flavor) { + Err(miniconf::Error::Traversal(miniconf::Traversal::Absent(_))) => { + return Ok(()); + }, + Err(e) => { + log::warn!( + "Failed to save `{}` to flash: {e:?}", + path.0.as_str() + ); + return Ok(()); + } + Ok(slice) => slice.len(), + }; + data.truncate(len); + + let range = self.storage.range(); + + // Check if the settings has changed from what's currently in flash (or if it doesn't + // yet exist). + if block_on(fetch_item( + &mut self.storage, + range.clone(), + &mut NoCache::new(), + buf, + path.clone(), + )) + .unwrap() + .map(|old: SettingsItem| old.0 != data) + .unwrap_or(true) + { + log::info!("Storing `{}` to flash", path.0.as_str()); + block_on(store_item( &mut self.storage, - range.clone(), + range, &mut NoCache::new(), buf, - path.clone(), + path, + &SettingsItem(data), )) - .unwrap() - .map(|old: SettingsItem| old.0 != data) - .unwrap_or(true) - { - log::info!("Storing `{}` to flash", path.0.as_str()); - block_on(store_item( - &mut self.storage, - range, - &mut NoCache::new(), - buf, - path, - &SettingsItem(data), - )) - .unwrap(); - } - - Ok(()) - }; - - if let Some(key) = key { - save_setting(String::try_from(key).unwrap().into())?; - } else { - for path in Self::Settings::nodes() { - save_setting(path.unwrap().0)?; - } + .unwrap(); } Ok(()) @@ -324,49 +315,37 @@ where &mut self.interface } - fn clear(&mut self, buf: &mut [u8], key: Option<&str>) { - let mut erase_setting = |path| { - let path = SettingsKey(path); - let range = self.storage.range(); - - // Check if there's an entry for this item in our flash map. The item might be a - // sentinel value indicating "erased". Because we can't write flash memory twice, we - // instead append a sentry "erased" value to the map where the serialized value is - // empty. - let maybe_item: Option = block_on(fetch_item( + fn clear(&mut self, buf: &mut [u8], path: &str) { + let path = SettingsKey(String::try_from(path).unwrap().into()); + let range = self.storage.range(); + + // Check if there's an entry for this item in our flash map. The item might be a + // sentinel value indicating "erased". Because we can't write flash memory twice, we + // instead append a sentry "erased" value to the map where the serialized value is + // empty. + let maybe_item: Option = block_on(fetch_item( + &mut self.storage, + range.clone(), + &mut NoCache::new(), + buf, + path.clone(), + )) + .unwrap(); + + // An entry may exist in the map with no data as a sentinel that this path was + // previously erased. If we find this, there's no need to store a duplicate "item is + // erased" sentinel in flash. We only need to logically erase the path from the map if + // it existed there in the first place. + if matches!(maybe_item, Some(item) if !item.0.is_empty()) { + block_on(store_item( &mut self.storage, - range.clone(), + range, &mut NoCache::new(), buf, - path.clone(), + path, + &SettingsItem(Vec::new()), )) .unwrap(); - - // An entry may exist in the map with no data as a sentinel that this path was - // previously erased. If we find this, there's no need to store a duplicate "item is - // erased" sentinel in flash. We only need to logically erase the path from the map if - // it existed there in the first place. - if matches!(maybe_item, Some(item) if !item.0.is_empty()) { - block_on(store_item( - &mut self.storage, - range, - &mut NoCache::new(), - buf, - path, - &SettingsItem(Vec::new()), - )) - .unwrap(); - } - - Ok::<_, Self::Error>(()) - }; - - if let Some(key) = key { - erase_setting(String::try_from(key).unwrap().into()).unwrap(); - } else { - for path in Self::Settings::nodes() { - erase_setting(path.unwrap().0).unwrap(); - } } } } From 8997cbe6acfe08ec4f75c7743d0463405459d076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 15 Jul 2024 14:34:22 +0000 Subject: [PATCH 04/12] serial-settings platform redesign --- Cargo.lock | 2 + serial-settings/Cargo.toml | 5 +- serial-settings/src/lib.rs | 232 ++++++++++++++++++++++--------------- src/settings.rs | 214 +++++++++++++++++++--------------- 4 files changed, 267 insertions(+), 186 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a07b77b19..3866fc03b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1205,8 +1205,10 @@ version = "0.1.0" dependencies = [ "embedded-io", "heapless 0.8.0", + "log", "menu", "miniconf", + "postcard", "yafnv", ] diff --git a/serial-settings/Cargo.toml b/serial-settings/Cargo.toml index ac21f5049..bf9abad0f 100644 --- a/serial-settings/Cargo.toml +++ b/serial-settings/Cargo.toml @@ -6,8 +6,11 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -miniconf = { version = "0.13", features = ["json-core"] } +miniconf = { version = "0.13", features = ["json-core", "postcard"] } menu = { version = "0.5", features = ["echo"] } heapless = "0.8" embedded-io = "0.6" yafnv = "2.0" +postcard = "1" +log = { version = "0.4", features = ["max_level_trace", "release_max_level_info"] } + diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 98454bfb1..7ff00a715 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -51,7 +51,7 @@ use embedded_io::{ErrorType, Read, ReadReady, Write}; use heapless::String; -use miniconf::{JsonCoreSlash, Path, Traversal, TreeKey}; +use miniconf::{JsonCoreSlash, Path, Postcard, Traversal, TreeKey}; mod interface; @@ -66,45 +66,39 @@ pub trait Settings: fn reset(&mut self) {} } -pub trait Platform: Sized { +pub trait Platform { /// This type specifies the interface to the user, for example, a USB CDC-ACM serial port. type Interface: embedded_io::Read + embedded_io::ReadReady + embedded_io::Write; - /// Specifies the settings that are used on the device. - type Settings: Settings; - /// `save()` Error type type Error: core::fmt::Debug; + type Settings: Settings; + + fn fetch<'a>( + &mut self, + buf: &'a mut [u8], + key: &[u8], + ) -> Result, Self::Error>; + /// Save the setting to storage - fn save( + fn store( &mut self, - buffer: &mut [u8], - key: &str, - settings: &Self::Settings, + buf: &mut [u8], + key: &[u8], + value: &[u8], ) -> Result<(), Self::Error>; /// Execute a platform specific command. fn cmd(&mut self, cmd: &str); - /// Handle clearing a settings key. - /// - /// # Note - /// The run-time setting will have already been updated when this is called. This is intended - /// to remove settings from permanent storage if necessary. - /// - /// # Arguments - /// * `buffer` The element serialization buffer. - /// * `key` The name of the setting to be cleared. - fn clear(&mut self, buffer: &mut [u8], key: &str); - /// Return a mutable reference to the `Interface`. fn interface_mut(&mut self) -> &mut Self::Interface; } -struct Interface<'a, P: Platform, const Y: usize> { +struct Interface<'a, P, const Y: usize> { platform: P, buffer: &'a mut [u8], updated: bool, @@ -128,41 +122,33 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { settings: &mut P::Settings, mut func: F, ) where - F: FnMut( - &Path, '/'>, - &mut Self, - &mut P::Settings, - ) -> Result<(), ()>, + F: FnMut(&Path, '/'>, &mut Self, &mut P::Settings), { let mut iter = P::Settings::nodes::, '/'>>(); if let Some(key) = key { match iter.root(&Path::<_, '/'>::from(key)) { Ok(it) => iter = it, Err(e) => { - writeln!(interface, "Failed to list keys: {e}").unwrap(); + writeln!(interface, "Failed to locate `{key}`: {e}") + .unwrap(); return; } }; } for key in iter { - let key = match key { + match key { Ok((key, node)) => { assert!(node.is_leaf()); - key + func(&key, interface, settings) } Err(depth) => { writeln!( interface, - "Failed to get path: no space at depth {depth}" + "Failed to build path: no space at depth {depth}" ) .unwrap(); - return; } - }; - match func(&key, interface, settings) { - Ok(()) => {} - Err(()) => break, } } } @@ -182,67 +168,63 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface, settings, |key, interface, settings| { - let value = match settings + let check: u32 = match settings .get_json_by_key(key, interface.buffer) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { - return Ok(()); + return; } Err(e) => { writeln!( interface, - "Failed to read {}: {e}", + "Failed to get `{}`: {e}", key.as_str() ) .unwrap(); - return Ok(()); + return; } Ok(len) => { - core::str::from_utf8(&interface.buffer[..len]).unwrap() + write!( + interface.platform.interface_mut(), + "{}: {}", + key.as_str(), + core::str::from_utf8(&interface.buffer[..len]) + .unwrap() + ) + .unwrap(); + yafnv::fnv1a(&interface.buffer[..len]) } }; - write!( - interface.platform.interface_mut(), - "{}: {value}", - key.as_str(), - ) - .unwrap(); - - let value_hash: u64 = yafnv::fnv1a(value); - - let default_value = match defaults - .get_json_by_key(key, interface.buffer) - { + match defaults.get_json_by_key(key, interface.buffer) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { - writeln!(interface, "[default: absent]").unwrap(); - return Ok(()); + writeln!(interface, "[default: absent]") } Err(e) => { writeln!( interface, "[default serialization error: {e}]" ) - .unwrap(); - return Ok(()); } Ok(len) => { - core::str::from_utf8(&interface.buffer[..len]).unwrap() + if yafnv::fnv1a::(&interface.buffer[..len]) + != check + { + writeln!( + interface.platform.interface_mut(), + " [default: {}]", + core::str::from_utf8(&interface.buffer[..len]) + .unwrap() + ) + } else { + writeln!( + interface.platform.interface_mut(), + " [default]" + ) + } } - }; - - let default_hash: u64 = yafnv::fnv1a(default_value); - if default_hash != value_hash { - writeln!( - interface.platform.interface_mut(), - " [default: {default_value}]" - ) - .unwrap(); - } else { - writeln!(interface.platform.interface_mut(), " [default]") - .unwrap(); } - Ok(()) + .unwrap(); }, ); } @@ -259,10 +241,10 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { defaults.reset(); Self::iter_root(key, interface, settings, |key, interface, settings| { let len = match defaults.get_json_by_key(key, interface.buffer) { + Ok(len) => len, Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { - writeln!(interface, "{} default is absent", key.as_str()) - .unwrap(); - return Ok(()); + log::warn!("Default absent: `{}`", key.as_str()); + return; } Err(e) => { writeln!( @@ -271,14 +253,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { key.as_str() ) .unwrap(); - return Ok(()); + return; } - Ok(len) => len, }; match settings.set_json_by_key(key, &interface.buffer[..len]) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { - return Ok(()); + return; } Err(e) => { writeln!( @@ -287,15 +268,14 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { key.as_str() ) .unwrap(); - return Ok(()); + return; } Ok(_len) => {} } - interface.platform.clear(interface.buffer, key); + //interface.platform.clear(interface.buffer, key); interface.updated = true; writeln!(interface, "{} cleared", key.as_str()).unwrap(); - Ok(()) }) } @@ -314,18 +294,85 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface, settings, |key, interface, settings| { - match interface.platform.save(interface.buffer, key, settings) { - Ok(_) => { - writeln!(interface, "{} saved", key.as_str()).unwrap() + let slic = postcard::ser_flavors::Slice::new(interface.buffer); + let mut check = match defaults.get_postcard_by_key(key, slic) { + Ok(slic) => yafnv::fnv1a::(slic), + Err(miniconf::Error::Traversal(Traversal::Absent( + _depth, + ))) => { + log::warn!("Default absent: `{}`", key.as_str()); + return; } + Err(e) => { + writeln!( + interface, + "Failed to get `{}` default: {e:?}", + key.as_str() + ) + .unwrap(); + return; + } + }; + match interface.platform.fetch(interface.buffer, key.as_bytes()) + { + Ok(None) => {} + Ok(Some(stored)) => { + let stored = yafnv::fnv1a::(stored); + if stored != check { + log::debug!( + "Stored differs from default: `{}`", + key.as_str() + ); + check = stored; + } else { + log::debug!( + "Stored matches default: `{}`", + key.as_str() + ); + } + } + Err(e) => { + writeln!( + interface, + "Could not load `{}`: {e:?}", + key.as_str() + ) + .unwrap(); + } + } + let slic = postcard::ser_flavors::Slice::new(interface.buffer); + let value = match settings.get_postcard_by_key(key, slic) { + Ok(value) => value, + Err(miniconf::Error::Traversal(Traversal::Absent( + _depth, + ))) => { + return; + } + Err(e) => { + writeln!( + interface, + "Could not get {}: {e:?}", + key.as_str() + ) + .unwrap(); + return; + } + }; + if yafnv::fnv1a::(&value) == check { + log::debug!("Not saving default {}", key.as_str()); + return; + } + let len = value.len(); + let (value, rest) = interface.buffer.split_at_mut(len); + match interface.platform.store(rest, key.as_bytes(), value) { + Ok(_) => writeln!(interface, "{} saved", key.as_str()), Err(e) => writeln!( interface, "Failed to save {}: {e:?}", key.as_str() - ) - .unwrap(), + ), } - Ok(()) + .unwrap(); }, ); writeln!( @@ -347,19 +394,16 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { menu::argument_finder(item, args, "value").unwrap().unwrap(); // Now, write the new value into memory. - match settings.set_json(key, value.as_bytes()) - { + match settings.set_json(key, value.as_bytes()) { Ok(_) => { interface.updated = true; - writeln!( - interface, - "Settings updated. You may need to reboot for the setting to be applied" - ) - }, + writeln!(interface, "Set. May require reboot to activate.") + } Err(e) => { - writeln!(interface, "Failed to update {key}: {e:?}") + writeln!(interface, "Failed to set {key}: {e:?}") } - }.unwrap(); + } + .unwrap(); } fn menu() -> menu::Menu<'a, Self, P::Settings> { diff --git a/src/settings.rs b/src/settings.rs index d243fc790..5d696aa4c 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -90,7 +90,7 @@ pub fn load_from_flash JsonCoreSlash<'d, Y>, const Y: usize>( storage.range(), &mut NoCache::new(), &mut buffer, - SettingsKey(path.clone()), + SettingsKey(Vec::try_from(path.as_bytes()).unwrap()), )) { Err(e) => { log::warn!( @@ -111,7 +111,7 @@ pub fn load_from_flash JsonCoreSlash<'d, Y>, const Y: usize>( log::info!("Loading initial `{}` from flash", path.as_str()); - let flavor = postcard::de_flavors::Slice::new(&item.0); + let flavor = postcard::de_flavors::Slice::new(item.0); if let Err(e) = structure.set_postcard_by_key(&path, flavor) { log::warn!( "Failed to deserialize `{}` from flash: {e:?}", @@ -124,7 +124,7 @@ pub fn load_from_flash JsonCoreSlash<'d, Y>, const Y: usize>( #[derive( Default, serde::Serialize, serde::Deserialize, Clone, PartialEq, Eq, )] -pub struct SettingsKey(Path, '/'>); +pub struct SettingsKey(Vec); impl sequential_storage::map::Key for SettingsKey { fn serialize_into( @@ -149,15 +149,15 @@ impl sequential_storage::map::Key for SettingsKey { #[derive( Default, serde::Serialize, serde::Deserialize, Clone, PartialEq, Eq, )] -pub struct SettingsItem(Vec); +pub struct SettingsItem<'a>(&'a [u8]); -impl<'a> sequential_storage::map::Value<'a> for SettingsItem { +impl<'a> sequential_storage::map::Value<'a> for SettingsItem<'a> { fn serialize_into( &self, buffer: &mut [u8], ) -> Result { if let Some(buf) = buffer.get_mut(..self.0.len()) { - buf.copy_from_slice(&self.0); + buf.copy_from_slice(self.0); Ok(self.0.len()) } else { Err(SerializationError::BufferTooSmall) @@ -165,9 +165,7 @@ impl<'a> sequential_storage::map::Value<'a> for SettingsItem { } fn deserialize_from(buffer: &'a [u8]) -> Result { - Vec::from_slice(buffer) - .map_err(|_| SerializationError::BufferTooSmall) - .map(Self) + Ok(Self(buffer)) } } @@ -202,67 +200,135 @@ where { type Interface = BestEffortInterface; type Settings = C; - type Error = Error< + type Error = sequential_storage::Error< ::Error, >; - fn save( + fn fetch<'a>( + &mut self, + buf: &'a mut [u8], + key: &[u8], + ) -> Result, Self::Error> { + let range = self.storage.range(); + let key = SettingsKey(Vec::try_from(key).unwrap()); + let maybe_item: Option = block_on(fetch_item( + &mut self.storage, + range, + &mut NoCache::new(), + buf, + key, + ))?; + Ok(maybe_item.map(|v| v.0)) + } + + fn store( &mut self, buf: &mut [u8], - path: &str, - settings: &Self::Settings, + key: &[u8], + value: &[u8], ) -> Result<(), Self::Error> { - let path = SettingsKey(String::try_from(path).unwrap().into()); - - let mut data = Vec::new(); - data.resize(data.capacity(), 0).unwrap(); - let flavor = postcard::ser_flavors::Slice::new(&mut data); - - let len = match settings.get_postcard_by_key(&path.0, flavor) { - Err(miniconf::Error::Traversal(miniconf::Traversal::Absent(_))) => { - return Ok(()); - }, - Err(e) => { - log::warn!( - "Failed to save `{}` to flash: {e:?}", - path.0.as_str() - ); - return Ok(()); - } - Ok(slice) => slice.len(), - }; - data.truncate(len); - let range = self.storage.range(); - - // Check if the settings has changed from what's currently in flash (or if it doesn't - // yet exist). - if block_on(fetch_item( + block_on(store_item( &mut self.storage, - range.clone(), + range, &mut NoCache::new(), buf, - path.clone(), + SettingsKey(Vec::try_from(key).unwrap()), + &SettingsItem(value), )) - .unwrap() - .map(|old: SettingsItem| old.0 != data) - .unwrap_or(true) - { - log::info!("Storing `{}` to flash", path.0.as_str()); - block_on(store_item( - &mut self.storage, - range, - &mut NoCache::new(), - buf, - path, - &SettingsItem(data), - )) - .unwrap(); - } - - Ok(()) } + // fn clear(&mut self, buf: &mut [u8], path: &str) { + // let path = SettingsKey(String::try_from(path).unwrap().into()); + // let range = self.storage.range(); + + // // Check if there's an entry for this item in our flash map. The item might be a + // // sentinel value indicating "erased". Because we can't write flash memory twice, we + // // instead append a sentry "erased" value to the map where the serialized value is + // // empty. + // let maybe_item: Option = block_on(fetch_item( + // &mut self.storage, + // range.clone(), + // &mut NoCache::new(), + // buf, + // path.clone(), + // )) + // .unwrap(); + + // // An entry may exist in the map with no data as a sentinel that this path was + // // previously erased. If we find this, there's no need to store a duplicate "item is + // // erased" sentinel in flash. We only need to logically erase the path from the map if + // // it existed there in the first place. + // if matches!(maybe_item, Some(item) if !item.0.is_empty()) { + // block_on(store_item( + // &mut self.storage, + // range, + // &mut NoCache::new(), + // buf, + // path, + // &SettingsItem(Vec::new()), + // )) + // .unwrap(); + // } + // } + + // fn save( + // &mut self, + // buf: &mut [u8], + // path: &str, + // settings: &Self::Settings, + // ) -> Result<(), Self::Error> { + // let path = SettingsKey(String::try_from(path).unwrap().into()); + + // let mut data = Vec::new(); + // data.resize(data.capacity(), 0).unwrap(); + // let flavor = postcard::ser_flavors::Slice::new(&mut data); + + // let len = match settings.get_postcard_by_key(&path.0, flavor) { + // Err(miniconf::Error::Traversal(miniconf::Traversal::Absent(_))) => { + // return Ok(()); + // }, + // Err(e) => { + // log::warn!( + // "Failed to save `{}` to flash: {e:?}", + // path.0.as_str() + // ); + // return Ok(()); + // } + // Ok(slice) => slice.len(), + // }; + // data.truncate(len); + + // let range = self.storage.range(); + + // // Check if the settings has changed from what's currently in flash (or if it doesn't + // // yet exist). + // if block_on(fetch_item( + // &mut self.storage, + // range.clone(), + // &mut NoCache::new(), + // buf, + // path.clone(), + // )) + // .unwrap() + // .map(|old: SettingsItem| old.0 != data) + // .unwrap_or(true) + // { + // log::info!("Storing `{}` to flash", path.0.as_str()); + // block_on(store_item( + // &mut self.storage, + // range, + // &mut NoCache::new(), + // buf, + // path, + // &SettingsItem(data), + // )) + // .unwrap(); + // } + + // Ok(()) + // } + fn cmd(&mut self, cmd: &str) { match cmd { "reboot" => cortex_m::peripheral::SCB::sys_reset(), @@ -314,38 +380,4 @@ where fn interface_mut(&mut self) -> &mut Self::Interface { &mut self.interface } - - fn clear(&mut self, buf: &mut [u8], path: &str) { - let path = SettingsKey(String::try_from(path).unwrap().into()); - let range = self.storage.range(); - - // Check if there's an entry for this item in our flash map. The item might be a - // sentinel value indicating "erased". Because we can't write flash memory twice, we - // instead append a sentry "erased" value to the map where the serialized value is - // empty. - let maybe_item: Option = block_on(fetch_item( - &mut self.storage, - range.clone(), - &mut NoCache::new(), - buf, - path.clone(), - )) - .unwrap(); - - // An entry may exist in the map with no data as a sentinel that this path was - // previously erased. If we find this, there's no need to store a duplicate "item is - // erased" sentinel in flash. We only need to logically erase the path from the map if - // it existed there in the first place. - if matches!(maybe_item, Some(item) if !item.0.is_empty()) { - block_on(store_item( - &mut self.storage, - range, - &mut NoCache::new(), - buf, - path, - &SettingsItem(Vec::new()), - )) - .unwrap(); - } - } } From 869e1b52abb93e05b50c86feb65f62ff726cf92a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 15 Jul 2024 15:43:36 +0000 Subject: [PATCH 05/12] actually discard in BestEffortInterface --- serial-settings/src/interface.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serial-settings/src/interface.rs b/serial-settings/src/interface.rs index dc4162fff..f3441016f 100644 --- a/serial-settings/src/interface.rs +++ b/serial-settings/src/interface.rs @@ -36,7 +36,7 @@ where if let Ok(true) = self.0.write_ready() { self.0.write(buf) } else { - Ok(0) + Ok(buf.len()) // discard! } } From 8ac1dfbd277249ccc2442db805d71743ca7fd3f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 15 Jul 2024 16:15:54 +0000 Subject: [PATCH 06/12] serial-settings: save->store, get shows stored as well --- Cargo.lock | 4 +- serial-settings/Cargo.toml | 3 +- serial-settings/src/lib.rs | 264 +++++++++++++++++++++++++------------ src/settings.rs | 149 ++------------------- 4 files changed, 199 insertions(+), 221 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3866fc03b..429d47071 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1584,9 +1584,9 @@ checksum = "1e9df38ee2d2c3c5948ea468a8406ff0db0b29ae1ffde1bcf20ef305bcc95c51" [[package]] name = "yafnv" -version = "2.0.0" +version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66aad9090f952997c6570d94039d80b5b5e26d182c5f39ef7b05d64535fb5f88" +checksum = "a98cd19b5e3fbcda406d58efc9f5cb2d6e82e381708d52f357b3b3cfb19559b1" dependencies = [ "num-traits", ] diff --git a/serial-settings/Cargo.toml b/serial-settings/Cargo.toml index bf9abad0f..c081b88eb 100644 --- a/serial-settings/Cargo.toml +++ b/serial-settings/Cargo.toml @@ -10,7 +10,6 @@ miniconf = { version = "0.13", features = ["json-core", "postcard"] } menu = { version = "0.5", features = ["echo"] } heapless = "0.8" embedded-io = "0.6" -yafnv = "2.0" +yafnv = "3.0" postcard = "1" log = { version = "0.4", features = ["max_level_trace", "release_max_level_info"] } - diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 7ff00a715..a903ffcdf 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -1,7 +1,7 @@ //! Persistent Settings Management Serial Interface //! //! # Description -//! This crate provides a simple means to load, configure, and save device settings over a serial +//! This crate provides a simple means to load, configure, and store device settings over a serial //! (i.e. text-based) interface. It is ideal to be used with serial ports and terminal emulators, //! and exposes a simple way to allow users to configure device operation. //! @@ -19,11 +19,10 @@ //! ``` //!> help //! AVAILABLE ITEMS: -//! list -//! get -//! set -//! save [item] -//! clear [item] +//! get [path] +//! set +//! store [path] +//! clear [path] //! platform //! help [ ] //! @@ -33,10 +32,10 @@ //! > plaform service //! Service data not available //! -//! > list +//! > get //! Available settings: -//! /broker: "test" [default: "mqtt"] -//! /id: "04-91-62-d2-a8-6f" [default: "04-91-62-d2-a8-6f"] +//! /broker: "test" [default: "mqtt"] [not stored] +//! /id: "04-91-62-d2-a8-6f" [default: "04-91-62-d2-a8-6f"] [not stored] //! ``` //! //! # Design @@ -72,7 +71,6 @@ pub trait Platform { + embedded_io::ReadReady + embedded_io::Write; - /// `save()` Error type type Error: core::fmt::Debug; type Settings: Settings; @@ -83,7 +81,7 @@ pub trait Platform { key: &[u8], ) -> Result, Self::Error>; - /// Save the setting to storage + /// Store the setting fn store( &mut self, buf: &mut [u8], @@ -91,6 +89,8 @@ pub trait Platform { value: &[u8], ) -> Result<(), Self::Error>; + fn clear(&mut self, buf: &mut [u8], key: &[u8]) -> Result<(), Self::Error>; + /// Execute a platform specific command. fn cmd(&mut self, cmd: &str); @@ -122,7 +122,12 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { settings: &mut P::Settings, mut func: F, ) where - F: FnMut(&Path, '/'>, &mut Self, &mut P::Settings), + F: FnMut( + &Path, '/'>, + &mut Self, + &mut P::Settings, + &mut P::Settings, + ), { let mut iter = P::Settings::nodes::, '/'>>(); if let Some(key) = key { @@ -136,11 +141,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { }; } + let mut defaults = settings.clone(); + defaults.reset(); for key in iter { match key { Ok((key, node)) => { assert!(node.is_leaf()); - func(&key, interface, settings) + func(&key, interface, settings, &mut defaults) } Err(depth) => { writeln!( @@ -160,15 +167,14 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface: &mut Self, settings: &mut P::Settings, ) { - let mut defaults = settings.clone(); - defaults.reset(); let key = menu::argument_finder(item, args, "path").unwrap(); Self::iter_root( key, interface, settings, - |key, interface, settings| { - let check: u32 = match settings + |key, interface, settings, defaults| { + // Get current + let check = match settings .get_json_by_key(key, interface.buffer) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { @@ -192,39 +198,76 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { .unwrap() ) .unwrap(); - yafnv::fnv1a(&interface.buffer[..len]) + yafnv::fnv1a::(&interface.buffer[..len]) } }; + // Get default and compare match defaults.get_json_by_key(key, interface.buffer) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { - writeln!(interface, "[default: absent]") + write!(interface, " [default: absent]") } Err(e) => { - writeln!( - interface, - "[default serialization error: {e}]" - ) + write!(interface, " [default serialization error: {e}]") } Ok(len) => { - if yafnv::fnv1a::(&interface.buffer[..len]) + if yafnv::fnv1a::(&interface.buffer[..len]) != check { - writeln!( + write!( interface.platform.interface_mut(), " [default: {}]", core::str::from_utf8(&interface.buffer[..len]) .unwrap() ) } else { - writeln!( - interface.platform.interface_mut(), - " [default]" - ) + write!(interface, " [default]") } } } .unwrap(); + + // Get stored and compare + match interface.platform.fetch(interface.buffer, key.as_bytes()) + { + Err(e) => write!( + interface, + " [fetch error: {e:?}]" + ), + Ok(None) => + write!(interface, " [not stored]"), + Ok(Some(stored)) => { + let slic = postcard::de_flavors::Slice::new(stored); + // Temporary reload into default for conversion + match defaults.set_postcard_by_key(key, slic) { + Err(e) => write!( + interface, + " [stored deserialize error: {e}]" + ), + Ok(_rest) => + match defaults.get_json_by_key(key, interface.buffer) { + Err(e) => write!( + interface, + " [stored serialization error: {e}]" + ), + Ok(len) => { + if yafnv::fnv1a::(&interface.buffer[..len]) != check { + write!( + interface.platform.interface_mut(), + " [stored: {}]", + core::str::from_utf8(&interface.buffer[..len]).unwrap()) + } else { + write!( + interface, + " [stored]" + ) + } + }, + } + } + } + }.unwrap(); + writeln!(interface).unwrap(); }, ); } @@ -237,49 +280,101 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { settings: &mut P::Settings, ) { let key = menu::argument_finder(item, args, "path").unwrap(); - let mut defaults = settings.clone(); - defaults.reset(); - Self::iter_root(key, interface, settings, |key, interface, settings| { - let len = match defaults.get_json_by_key(key, interface.buffer) { - Ok(len) => len, - Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { - log::warn!("Default absent: `{}`", key.as_str()); - return; - } - Err(e) => { - writeln!( - interface, - "Failed to get default `{}`: {e:?}", - key.as_str() - ) - .unwrap(); - return; - } - }; + Self::iter_root( + key, + interface, + settings, + |key, interface, settings, defaults| { + // Get default + let slic = postcard::ser_flavors::Slice::new(interface.buffer); + let slic = match defaults.get_postcard_by_key(key, slic) { + Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { + log::warn!( + "Can't clear. Default absent: `{}`", + key.as_str() + ); + return; + } + Err(e) => { + writeln!( + interface, + "Failed to get default `{}`: {e}", + key.as_str() + ) + .unwrap(); + return; + } + Ok(slic) => slic, + }; - match settings.set_json_by_key(key, &interface.buffer[..len]) { - Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { - return; - } - Err(e) => { - writeln!( + // Set default + let slic = postcard::de_flavors::Slice::new(slic); + match settings.set_postcard_by_key(key, slic) { + Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { + return; + } + Err(e) => { + writeln!( + interface, + "Failed to set {}: {e:?}", + key.as_str() + ) + .unwrap(); + return; + } + Ok(_rest) => writeln!( interface, - "Failed to update {}: {e:?}", + "Cleared current `{}`", key.as_str() ) - .unwrap(); - return; + .unwrap(), } - Ok(_len) => {} - } - //interface.platform.clear(interface.buffer, key); - interface.updated = true; - writeln!(interface, "{} cleared", key.as_str()).unwrap(); - }) + // Check for stored + match interface.platform.fetch(interface.buffer, key.as_bytes()) + { + Err(e) => { + writeln!( + interface, + "Failed to fetch `{}`: {e:?}", + key.as_str() + ) + .unwrap(); + } + Ok(None) => {} + // Clear stored + Ok(Some(_stored)) => match interface + .platform + .clear(interface.buffer, key.as_bytes()) + { + Ok(()) => { + writeln!( + interface, + "Clear stored `{}`", + key.as_str() + ) + } + Err(e) => { + writeln!( + interface, + "Failed to clear `{}` from storage: {e:?}", + key.as_str() + ) + } + } + .unwrap(), + } + }, + ); + interface.updated = true; + writeln!( + interface, + "Stored settings may require reboot to become active" + ) + .unwrap(); } - fn handle_save( + fn handle_store( _menu: &menu::Menu, item: &menu::Item, args: &[&str], @@ -287,16 +382,15 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { settings: &mut P::Settings, ) { let key = menu::argument_finder(item, args, "path").unwrap(); - let mut defaults = settings.clone(); - defaults.reset(); Self::iter_root( key, interface, settings, - |key, interface, settings| { + |key, interface, settings, defaults| { + // Get default value checksum let slic = postcard::ser_flavors::Slice::new(interface.buffer); let mut check = match defaults.get_postcard_by_key(key, slic) { - Ok(slic) => yafnv::fnv1a::(slic), + Ok(slic) => yafnv::fnv1a::(slic), Err(miniconf::Error::Traversal(Traversal::Absent( _depth, ))) => { @@ -313,11 +407,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { return; } }; + + // Get stored value checksum match interface.platform.fetch(interface.buffer, key.as_bytes()) { Ok(None) => {} Ok(Some(stored)) => { - let stored = yafnv::fnv1a::(stored); + let stored = yafnv::fnv1a::(stored); if stored != check { log::debug!( "Stored differs from default: `{}`", @@ -334,12 +430,14 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { Err(e) => { writeln!( interface, - "Could not load `{}`: {e:?}", + "Failed to fetch `{}`: {e:?}", key.as_str() ) .unwrap(); } } + + // Get value let slic = postcard::ser_flavors::Slice::new(interface.buffer); let value = match settings.get_postcard_by_key(key, slic) { Ok(value) => value, @@ -351,24 +449,28 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { Err(e) => { writeln!( interface, - "Could not get {}: {e:?}", + "Could not get `{}`: {e}", key.as_str() ) .unwrap(); return; } }; - if yafnv::fnv1a::(&value) == check { - log::debug!("Not saving default {}", key.as_str()); + + // Check for mismatch + if yafnv::fnv1a::(value) == check { + log::debug!("Not saving default/stored `{}`", key.as_str()); return; } let len = value.len(); let (value, rest) = interface.buffer.split_at_mut(len); + + // Store match interface.platform.store(rest, key.as_bytes(), value) { - Ok(_) => writeln!(interface, "{} saved", key.as_str()), + Ok(_) => writeln!(interface, "`{}` stored", key.as_str()), Err(e) => writeln!( interface, - "Failed to save {}: {e:?}", + "Failed to store `{}`: {e:?}", key.as_str() ), } @@ -377,7 +479,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { ); writeln!( interface, - "Saved settings may require reboot to become active" + "Stored settings may require reboot to become active" ) .unwrap(); } @@ -439,21 +541,21 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { }, }, &menu::Item { - command: "save", - help: Some("Save settings to memory"), + command: "store", + help: Some("Store settings in memory"), item_type: menu::ItemType::Callback { - function: Self::handle_save, + function: Self::handle_store, parameters: &[ menu::Parameter::Optional { parameter_name: "path", - help: Some("The path of the setting to save"), + help: Some("The path of the setting to store"), }, ] }, }, &menu::Item { command: "clear", - help: Some("Clear settings to default values"), + help: Some("Clear active and stored settings to default values"), item_type: menu::ItemType::Callback { function: Self::handle_clear, parameters: &[ diff --git a/src/settings.rs b/src/settings.rs index 5d696aa4c..d41ae1e00 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -85,7 +85,7 @@ pub fn load_from_flash JsonCoreSlash<'d, Y>, const Y: usize>( let (path, _node) = path.unwrap(); // Try to fetch the setting from flash. - let item: SettingsItem = match block_on(fetch_item( + let value: &[u8] = match block_on(fetch_item( storage, storage.range(), &mut NoCache::new(), @@ -99,19 +99,19 @@ pub fn load_from_flash JsonCoreSlash<'d, Y>, const Y: usize>( ); continue; } - Ok(Some(item)) => item, - _ => continue, + Ok(Some(value)) => value, + Ok(None) => continue, }; // An empty vector may be saved to flash to "erase" a setting, since the H7 doesn't support // multi-write NOR flash. If we see an empty vector, ignore this entry. - if item.0.is_empty() { + if value.is_empty() { continue; } log::info!("Loading initial `{}` from flash", path.as_str()); - let flavor = postcard::de_flavors::Slice::new(item.0); + let flavor = postcard::de_flavors::Slice::new(value); if let Err(e) = structure.set_postcard_by_key(&path, flavor) { log::warn!( "Failed to deserialize `{}` from flash: {e:?}", @@ -146,41 +146,6 @@ impl sequential_storage::map::Key for SettingsKey { } } -#[derive( - Default, serde::Serialize, serde::Deserialize, Clone, PartialEq, Eq, -)] -pub struct SettingsItem<'a>(&'a [u8]); - -impl<'a> sequential_storage::map::Value<'a> for SettingsItem<'a> { - fn serialize_into( - &self, - buffer: &mut [u8], - ) -> Result { - if let Some(buf) = buffer.get_mut(..self.0.len()) { - buf.copy_from_slice(self.0); - Ok(self.0.len()) - } else { - Err(SerializationError::BufferTooSmall) - } - } - - fn deserialize_from(buffer: &'a [u8]) -> Result { - Ok(Self(buffer)) - } -} - -#[derive(Debug)] -pub enum Error { - Postcard(postcard::Error), - Flash(F), -} - -impl From for Error { - fn from(e: postcard::Error) -> Self { - Self::Postcard(e) - } -} - pub struct SerialSettingsPlatform { /// The interface to read/write data to/from serially (via text) to the user. pub interface: BestEffortInterface, @@ -210,15 +175,14 @@ where key: &[u8], ) -> Result, Self::Error> { let range = self.storage.range(); - let key = SettingsKey(Vec::try_from(key).unwrap()); - let maybe_item: Option = block_on(fetch_item( + block_on(fetch_item( &mut self.storage, range, &mut NoCache::new(), buf, - key, - ))?; - Ok(maybe_item.map(|v| v.0)) + SettingsKey(Vec::try_from(key).unwrap()), + )) + .map(|v| v.filter(|v: &&[u8]| !v.is_empty())) } fn store( @@ -234,100 +198,13 @@ where &mut NoCache::new(), buf, SettingsKey(Vec::try_from(key).unwrap()), - &SettingsItem(value), + &value, )) } - // fn clear(&mut self, buf: &mut [u8], path: &str) { - // let path = SettingsKey(String::try_from(path).unwrap().into()); - // let range = self.storage.range(); - - // // Check if there's an entry for this item in our flash map. The item might be a - // // sentinel value indicating "erased". Because we can't write flash memory twice, we - // // instead append a sentry "erased" value to the map where the serialized value is - // // empty. - // let maybe_item: Option = block_on(fetch_item( - // &mut self.storage, - // range.clone(), - // &mut NoCache::new(), - // buf, - // path.clone(), - // )) - // .unwrap(); - - // // An entry may exist in the map with no data as a sentinel that this path was - // // previously erased. If we find this, there's no need to store a duplicate "item is - // // erased" sentinel in flash. We only need to logically erase the path from the map if - // // it existed there in the first place. - // if matches!(maybe_item, Some(item) if !item.0.is_empty()) { - // block_on(store_item( - // &mut self.storage, - // range, - // &mut NoCache::new(), - // buf, - // path, - // &SettingsItem(Vec::new()), - // )) - // .unwrap(); - // } - // } - - // fn save( - // &mut self, - // buf: &mut [u8], - // path: &str, - // settings: &Self::Settings, - // ) -> Result<(), Self::Error> { - // let path = SettingsKey(String::try_from(path).unwrap().into()); - - // let mut data = Vec::new(); - // data.resize(data.capacity(), 0).unwrap(); - // let flavor = postcard::ser_flavors::Slice::new(&mut data); - - // let len = match settings.get_postcard_by_key(&path.0, flavor) { - // Err(miniconf::Error::Traversal(miniconf::Traversal::Absent(_))) => { - // return Ok(()); - // }, - // Err(e) => { - // log::warn!( - // "Failed to save `{}` to flash: {e:?}", - // path.0.as_str() - // ); - // return Ok(()); - // } - // Ok(slice) => slice.len(), - // }; - // data.truncate(len); - - // let range = self.storage.range(); - - // // Check if the settings has changed from what's currently in flash (or if it doesn't - // // yet exist). - // if block_on(fetch_item( - // &mut self.storage, - // range.clone(), - // &mut NoCache::new(), - // buf, - // path.clone(), - // )) - // .unwrap() - // .map(|old: SettingsItem| old.0 != data) - // .unwrap_or(true) - // { - // log::info!("Storing `{}` to flash", path.0.as_str()); - // block_on(store_item( - // &mut self.storage, - // range, - // &mut NoCache::new(), - // buf, - // path, - // &SettingsItem(data), - // )) - // .unwrap(); - // } - - // Ok(()) - // } + fn clear(&mut self, buf: &mut [u8], key: &[u8]) -> Result<(), Self::Error> { + self.store(buf, key, b"") + } fn cmd(&mut self, cmd: &str) { match cmd { From aca61ad504e289102f4999cbd391a727fcbba1fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 15 Jul 2024 21:29:50 +0000 Subject: [PATCH 07/12] move load_from_flash into inherent Platform impl --- src/hardware/setup.rs | 5 ++- src/settings.rs | 94 ++++++++++++++++++++++--------------------- 2 files changed, 52 insertions(+), 47 deletions(-) diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index b7323d30d..aa9d07ddd 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -644,7 +644,10 @@ where }; let mut settings = C::new(NetSettings::new(mac_addr)); - crate::settings::load_from_flash(&mut settings, &mut flash); + crate::settings::SerialSettingsPlatform::load( + &mut settings, + &mut flash, + ); (flash, settings) }; diff --git a/src/settings.rs b/src/settings.rs index d41ae1e00..919ac85d4 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -75,52 +75,6 @@ pub trait AppSettings { fn net(&self) -> &NetSettings; } -pub fn load_from_flash JsonCoreSlash<'d, Y>, const Y: usize>( - structure: &mut T, - storage: &mut Flash, -) { - // Loop over flash and read settings - let mut buffer = [0u8; 512]; - for path in T::nodes::, '/'>>() { - let (path, _node) = path.unwrap(); - - // Try to fetch the setting from flash. - let value: &[u8] = match block_on(fetch_item( - storage, - storage.range(), - &mut NoCache::new(), - &mut buffer, - SettingsKey(Vec::try_from(path.as_bytes()).unwrap()), - )) { - Err(e) => { - log::warn!( - "Failed to fetch `{}` from flash: {e:?}", - path.as_str() - ); - continue; - } - Ok(Some(value)) => value, - Ok(None) => continue, - }; - - // An empty vector may be saved to flash to "erase" a setting, since the H7 doesn't support - // multi-write NOR flash. If we see an empty vector, ignore this entry. - if value.is_empty() { - continue; - } - - log::info!("Loading initial `{}` from flash", path.as_str()); - - let flavor = postcard::de_flavors::Slice::new(value); - if let Err(e) = structure.set_postcard_by_key(&path, flavor) { - log::warn!( - "Failed to deserialize `{}` from flash: {e:?}", - path.as_str() - ); - } - } -} - #[derive( Default, serde::Serialize, serde::Deserialize, Clone, PartialEq, Eq, )] @@ -159,6 +113,54 @@ pub struct SerialSettingsPlatform { pub metadata: &'static ApplicationMetadata, } +impl SerialSettingsPlatform +where + C: for<'d> JsonCoreSlash<'d, Y>, +{ + pub fn load(structure: &mut C, storage: &mut Flash) { + // Loop over flash and read settings + let mut buffer = [0u8; 512]; + for path in C::nodes::, '/'>>() { + let (path, _node) = path.unwrap(); + + // Try to fetch the setting from flash. + let value: &[u8] = match block_on(fetch_item( + storage, + storage.range(), + &mut NoCache::new(), + &mut buffer, + SettingsKey(Vec::try_from(path.as_bytes()).unwrap()), + )) { + Err(e) => { + log::warn!( + "Failed to fetch `{}` from flash: {e:?}", + path.as_str() + ); + continue; + } + Ok(Some(value)) => value, + Ok(None) => continue, + }; + + // An empty vector may be saved to flash to "erase" a setting, since the H7 doesn't support + // multi-write NOR flash. If we see an empty vector, ignore this entry. + if value.is_empty() { + continue; + } + + log::info!("Loading initial `{}` from flash", path.as_str()); + + let flavor = postcard::de_flavors::Slice::new(value); + if let Err(e) = structure.set_postcard_by_key(&path, flavor) { + log::warn!( + "Failed to deserialize `{}` from flash: {e:?}", + path.as_str() + ); + } + } + } +} + impl Platform for SerialSettingsPlatform where C: Settings, From 1e6630368ecf941690ad3bc27046f3af36ee9645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 16 Jul 2024 08:13:59 +0000 Subject: [PATCH 08/12] serial-settings: refine messages, fix logic --- serial-settings/src/lib.rs | 104 ++++++++++++++++++++----------------- 1 file changed, 56 insertions(+), 48 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index a903ffcdf..c6b776c0c 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -146,7 +146,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { for key in iter { match key { Ok((key, node)) => { - assert!(node.is_leaf()); + debug_assert!(node.is_leaf()); func(&key, interface, settings, &mut defaults) } Err(depth) => { @@ -290,10 +290,10 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { let slic = match defaults.get_postcard_by_key(key, slic) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { log::warn!( - "Can't clear. Default absent: `{}`", + "Can't clear. Default is absent: `{}`", key.as_str() ); - return; + None } Err(e) => { writeln!( @@ -304,30 +304,37 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { .unwrap(); return; } - Ok(slic) => slic, + Ok(slic) => Some(slic), }; // Set default - let slic = postcard::de_flavors::Slice::new(slic); - match settings.set_postcard_by_key(key, slic) { - Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { - return; - } - Err(e) => { - writeln!( - interface, - "Failed to set {}: {e:?}", - key.as_str() - ) - .unwrap(); - return; + if let Some(slic) = slic { + let slic = postcard::de_flavors::Slice::new(slic); + match settings.set_postcard_by_key(key, slic) { + Err(miniconf::Error::Traversal(Traversal::Absent( + _, + ))) => { + return; + } + Err(e) => { + writeln!( + interface, + "Failed to set {}: {e:?}", + key.as_str() + ) + .unwrap(); + return; + } + Ok(_rest) => { + interface.updated = true; + writeln!( + interface, + "Cleared current `{}`", + key.as_str() + ) + .unwrap() + } } - Ok(_rest) => writeln!( - interface, - "Cleared current `{}`", - key.as_str() - ) - .unwrap(), } // Check for stored @@ -367,11 +374,8 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { }, ); interface.updated = true; - writeln!( - interface, - "Stored settings may require reboot to become active" - ) - .unwrap(); + writeln!(interface, "Some values may require reboot to become active") + .unwrap(); } fn handle_store( @@ -389,6 +393,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { |key, interface, settings, defaults| { // Get default value checksum let slic = postcard::ser_flavors::Slice::new(interface.buffer); + // Could in theory serialize directly into the hasher let mut check = match defaults.get_postcard_by_key(key, slic) { Ok(slic) => yafnv::fnv1a::(slic), Err(miniconf::Error::Traversal(Traversal::Absent( @@ -419,13 +424,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { "Stored differs from default: `{}`", key.as_str() ); - check = stored; } else { log::debug!( "Stored matches default: `{}`", key.as_str() ); } + check = stored; } Err(e) => { writeln!( @@ -459,7 +464,10 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { // Check for mismatch if yafnv::fnv1a::(value) == check { - log::debug!("Not saving default/stored `{}`", key.as_str()); + log::debug!( + "Not saving matching default/stored `{}`", + key.as_str() + ); return; } let len = value.len(); @@ -477,11 +485,8 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { .unwrap(); }, ); - writeln!( - interface, - "Stored settings may require reboot to become active" - ) - .unwrap(); + writeln!(interface, "Some values may require reboot to become active") + .unwrap(); } fn handle_set( @@ -499,10 +504,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { match settings.set_json(key, value.as_bytes()) { Ok(_) => { interface.updated = true; - writeln!(interface, "Set. May require reboot to activate.") + writeln!( + interface, + "Set but not stored. May require store and reboot to activate." + ) } Err(e) => { - writeln!(interface, "Failed to set {key}: {e:?}") + writeln!(interface, "Failed to set `{key}`: {e:?}") } } .unwrap(); @@ -514,54 +522,54 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { items: &[ &menu::Item { command: "get", - help: Some("List paths and read settings values"), + help: Some("List paths and read current, default, and stored values"), item_type: menu::ItemType::Callback { function: Self::handle_get, parameters: &[menu::Parameter::Optional { parameter_name: "path", - help: Some("The path of the setting to list/read"), + help: Some("The path of the value or subtree to list/read."), }] }, }, &menu::Item { command: "set", - help: Some("Update a setting"), + help: Some("Update a value"), item_type: menu::ItemType::Callback { function: Self::handle_set, parameters: &[ menu::Parameter::Mandatory { parameter_name: "path", - help: Some("The path of the setting to set"), + help: Some("The path to set"), }, menu::Parameter::Mandatory { parameter_name: "value", - help: Some("Specifies the value to be written. Values must be JSON-encoded"), + help: Some("The value to be written, JSON-encoded"), }, ] }, }, &menu::Item { command: "store", - help: Some("Store settings in memory"), + help: Some("Store values that differ from defaults"), item_type: menu::ItemType::Callback { function: Self::handle_store, parameters: &[ menu::Parameter::Optional { parameter_name: "path", - help: Some("The path of the setting to store"), + help: Some("The path of the value or subtree to store."), }, ] }, }, &menu::Item { command: "clear", - help: Some("Clear active and stored settings to default values"), + help: Some("Clear active to defaults and remove all stored values"), item_type: menu::ItemType::Callback { function: Self::handle_clear, parameters: &[ menu::Parameter::Optional { parameter_name: "path", - help: Some("The path of the setting to clear"), + help: Some("The path of the value or subtree to clear"), }, ] }, @@ -609,7 +617,7 @@ impl<'a, P: Platform, const Y: usize> Write for Interface<'a, P, Y> { } } -/// The serial settings management object. +// The Menu runner pub struct Runner<'a, P: Platform, const Y: usize>( menu::Runner<'a, Interface<'a, P, Y>, P::Settings>, ); @@ -623,7 +631,7 @@ impl<'a, P: Platform, const Y: usize> Runner<'a, P, Y> { /// * `line_buf` - A buffer used for maintaining the serial menu input line. It should be at /// least as long as the longest user input. /// * `serialize_buf` - A buffer used for serializing and deserializing settings. This buffer - /// needs to be at least as big as the entire serialized settings structure. + /// needs to be at least as big as twice the biggest serialized setting plus its path. pub fn new( platform: P, line_buf: &'a mut [u8], From bd722757ae41200c99fbda641768d20627df2960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 16 Jul 2024 13:25:15 +0200 Subject: [PATCH 09/12] serial-settings: allow force-store --- serial-settings/src/lib.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index c6b776c0c..314de430c 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -386,6 +386,9 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { settings: &mut P::Settings, ) { let key = menu::argument_finder(item, args, "path").unwrap(); + let force = menu::argument_finder(item, args, "force") + .unwrap() + .is_some(); Self::iter_root( key, interface, @@ -463,7 +466,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { }; // Check for mismatch - if yafnv::fnv1a::(value) == check { + if yafnv::fnv1a::(value) == check && !force { log::debug!( "Not saving matching default/stored `{}`", key.as_str() @@ -554,10 +557,15 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { item_type: menu::ItemType::Callback { function: Self::handle_store, parameters: &[ + menu::Parameter::Named { + parameter_name: "force", + help: Some("Also store values that match defaults"), + }, menu::Parameter::Optional { parameter_name: "path", help: Some("The path of the value or subtree to store."), }, + ] }, }, From ec487afc21f59e3fdf9d2bbd7ef8c8fc741b44e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 16 Jul 2024 13:32:14 +0200 Subject: [PATCH 10/12] serial-settings: don't clear current if it is already default --- serial-settings/src/lib.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 314de430c..485926161 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -285,6 +285,23 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface, settings, |key, interface, settings, defaults| { + let slic = postcard::ser_flavors::Slice::new(interface.buffer); + let check = match settings.get_postcard_by_key(key, slic) { + Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { + return; + } + Err(e) => { + writeln!( + interface, + "Failed to get {}: {e:?}", + key.as_str() + ) + .unwrap(); + return; + } + Ok(slic) => yafnv::fnv1a::(slic), + }; + // Get default let slic = postcard::ser_flavors::Slice::new(interface.buffer); let slic = match defaults.get_postcard_by_key(key, slic) { @@ -304,7 +321,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { .unwrap(); return; } - Ok(slic) => Some(slic), + Ok(slic) => { + if yafnv::fnv1a::(slic) != check { + Some(slic) + } else { + None + } + } }; // Set default From f8e5318448637ca311ed93184a12d38e774502d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 16 Jul 2024 12:36:57 +0000 Subject: [PATCH 11/12] serial-settings: path length 128, less type leakage --- serial-settings/src/lib.rs | 104 +++++++++++++++---------------------- 1 file changed, 42 insertions(+), 62 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 485926161..378e4b8b0 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -123,13 +123,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { mut func: F, ) where F: FnMut( - &Path, '/'>, + Path<&str, '/'>, &mut Self, &mut P::Settings, &mut P::Settings, ), { - let mut iter = P::Settings::nodes::, '/'>>(); + let mut iter = P::Settings::nodes::, '/'>>(); if let Some(key) = key { match iter.root(&Path::<_, '/'>::from(key)) { Ok(it) => iter = it, @@ -147,7 +147,12 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { match key { Ok((key, node)) => { debug_assert!(node.is_leaf()); - func(&key, interface, settings, &mut defaults) + func( + key.as_str().into(), + interface, + settings, + &mut defaults, + ) } Err(depth) => { writeln!( @@ -175,25 +180,21 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { |key, interface, settings, defaults| { // Get current let check = match settings - .get_json_by_key(key, interface.buffer) + .get_json_by_key(&key, interface.buffer) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { return; } Err(e) => { - writeln!( - interface, - "Failed to get `{}`: {e}", - key.as_str() - ) - .unwrap(); + writeln!(interface, "Failed to get `{}`: {e}", *key) + .unwrap(); return; } Ok(len) => { write!( interface.platform.interface_mut(), "{}: {}", - key.as_str(), + *key, core::str::from_utf8(&interface.buffer[..len]) .unwrap() ) @@ -203,7 +204,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { }; // Get default and compare - match defaults.get_json_by_key(key, interface.buffer) { + match defaults.get_json_by_key(&key, interface.buffer) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { write!(interface, " [default: absent]") } @@ -239,13 +240,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { Ok(Some(stored)) => { let slic = postcard::de_flavors::Slice::new(stored); // Temporary reload into default for conversion - match defaults.set_postcard_by_key(key, slic) { + match defaults.set_postcard_by_key(&key, slic) { Err(e) => write!( interface, " [stored deserialize error: {e}]" ), Ok(_rest) => - match defaults.get_json_by_key(key, interface.buffer) { + match defaults.get_json_by_key(&key, interface.buffer) { Err(e) => write!( interface, " [stored serialization error: {e}]" @@ -286,17 +287,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { settings, |key, interface, settings, defaults| { let slic = postcard::ser_flavors::Slice::new(interface.buffer); - let check = match settings.get_postcard_by_key(key, slic) { + let check = match settings.get_postcard_by_key(&key, slic) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { return; } Err(e) => { - writeln!( - interface, - "Failed to get {}: {e:?}", - key.as_str() - ) - .unwrap(); + writeln!(interface, "Failed to get {}: {e:?}", *key) + .unwrap(); return; } Ok(slic) => yafnv::fnv1a::(slic), @@ -304,11 +301,11 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { // Get default let slic = postcard::ser_flavors::Slice::new(interface.buffer); - let slic = match defaults.get_postcard_by_key(key, slic) { + let slic = match defaults.get_postcard_by_key(&key, slic) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { log::warn!( "Can't clear. Default is absent: `{}`", - key.as_str() + *key ); None } @@ -316,7 +313,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { writeln!( interface, "Failed to get default `{}`: {e}", - key.as_str() + *key ) .unwrap(); return; @@ -333,7 +330,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { // Set default if let Some(slic) = slic { let slic = postcard::de_flavors::Slice::new(slic); - match settings.set_postcard_by_key(key, slic) { + match settings.set_postcard_by_key(&key, slic) { Err(miniconf::Error::Traversal(Traversal::Absent( _, ))) => { @@ -343,19 +340,15 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { writeln!( interface, "Failed to set {}: {e:?}", - key.as_str() + *key ) .unwrap(); return; } Ok(_rest) => { interface.updated = true; - writeln!( - interface, - "Cleared current `{}`", - key.as_str() - ) - .unwrap() + writeln!(interface, "Cleared current `{}`", *key) + .unwrap() } } } @@ -367,7 +360,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { writeln!( interface, "Failed to fetch `{}`: {e:?}", - key.as_str() + *key ) .unwrap(); } @@ -378,17 +371,13 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { .clear(interface.buffer, key.as_bytes()) { Ok(()) => { - writeln!( - interface, - "Clear stored `{}`", - key.as_str() - ) + writeln!(interface, "Clear stored `{}`", *key) } Err(e) => { writeln!( interface, "Failed to clear `{}` from storage: {e:?}", - key.as_str() + *key ) } } @@ -420,19 +409,19 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { // Get default value checksum let slic = postcard::ser_flavors::Slice::new(interface.buffer); // Could in theory serialize directly into the hasher - let mut check = match defaults.get_postcard_by_key(key, slic) { + let mut check = match defaults.get_postcard_by_key(&key, slic) { Ok(slic) => yafnv::fnv1a::(slic), Err(miniconf::Error::Traversal(Traversal::Absent( _depth, ))) => { - log::warn!("Default absent: `{}`", key.as_str()); + log::warn!("Default absent: `{}`", *key); return; } Err(e) => { writeln!( interface, "Failed to get `{}` default: {e:?}", - key.as_str() + *key ) .unwrap(); return; @@ -448,13 +437,10 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { if stored != check { log::debug!( "Stored differs from default: `{}`", - key.as_str() + *key ); } else { - log::debug!( - "Stored matches default: `{}`", - key.as_str() - ); + log::debug!("Stored matches default: `{}`", *key); } check = stored; } @@ -462,7 +448,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { writeln!( interface, "Failed to fetch `{}`: {e:?}", - key.as_str() + *key ) .unwrap(); } @@ -470,7 +456,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { // Get value let slic = postcard::ser_flavors::Slice::new(interface.buffer); - let value = match settings.get_postcard_by_key(key, slic) { + let value = match settings.get_postcard_by_key(&key, slic) { Ok(value) => value, Err(miniconf::Error::Traversal(Traversal::Absent( _depth, @@ -478,12 +464,8 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { return; } Err(e) => { - writeln!( - interface, - "Could not get `{}`: {e}", - key.as_str() - ) - .unwrap(); + writeln!(interface, "Could not get `{}`: {e}", *key) + .unwrap(); return; } }; @@ -492,7 +474,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { if yafnv::fnv1a::(value) == check && !force { log::debug!( "Not saving matching default/stored `{}`", - key.as_str() + *key ); return; } @@ -501,12 +483,10 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { // Store match interface.platform.store(rest, key.as_bytes(), value) { - Ok(_) => writeln!(interface, "`{}` stored", key.as_str()), - Err(e) => writeln!( - interface, - "Failed to store `{}`: {e:?}", - key.as_str() - ), + Ok(_) => writeln!(interface, "`{}` stored", *key), + Err(e) => { + writeln!(interface, "Failed to store `{}`: {e:?}", *key) + } } .unwrap(); }, From 8c4385f41b90c044da63ecf7193beca70f854b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 16 Jul 2024 15:55:29 +0000 Subject: [PATCH 12/12] be more explicit about trait bounds --- serial-settings/src/lib.rs | 23 ++++++++++++++++------- src/bin/dual-iir.rs | 2 +- src/bin/lockin.rs | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 378e4b8b0..d4223d348 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -53,18 +53,24 @@ use heapless::String; use miniconf::{JsonCoreSlash, Path, Postcard, Traversal, TreeKey}; mod interface; - pub use interface::BestEffortInterface; /// Specifies the API required for objects that are used as settings with the serial terminal /// interface. pub trait Settings: - for<'a> JsonCoreSlash<'a, Y> + Clone + for<'de> JsonCoreSlash<'de, Y> + for<'de> Postcard<'de, Y> + Clone { /// Reset the settings to their default values. fn reset(&mut self) {} } +/// Platform support for serial settings. +/// +/// Covers platform-specific commands, persistent key-value storage and the +/// Read/Write interface for interaction. +/// +/// Assuming there are no unit fields in the `Settings`, the empty value can be +/// used to mark the "cleared" state. pub trait Platform { /// This type specifies the interface to the user, for example, a USB CDC-ACM serial port. type Interface: embedded_io::Read @@ -75,13 +81,14 @@ pub trait Platform { type Settings: Settings; + /// Fetch a value from persisten storage fn fetch<'a>( &mut self, buf: &'a mut [u8], key: &[u8], ) -> Result, Self::Error>; - /// Store the setting + /// Store a value to persistent storage fn store( &mut self, buf: &mut [u8], @@ -89,6 +96,7 @@ pub trait Platform { value: &[u8], ) -> Result<(), Self::Error>; + /// Remove a key from storage. fn clear(&mut self, buf: &mut [u8], key: &[u8]) -> Result<(), Self::Error>; /// Execute a platform specific command. @@ -239,7 +247,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { write!(interface, " [not stored]"), Ok(Some(stored)) => { let slic = postcard::de_flavors::Slice::new(stored); - // Temporary reload into default for conversion + // Use defaults as scratch space for postcard->json conversion match defaults.set_postcard_by_key(&key, slic) { Err(e) => write!( interface, @@ -286,6 +294,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { interface, settings, |key, interface, settings, defaults| { + // Get current value checksum let slic = postcard::ser_flavors::Slice::new(interface.buffer); let check = match settings.get_postcard_by_key(&key, slic) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { @@ -299,7 +308,7 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { Ok(slic) => yafnv::fnv1a::(slic), }; - // Get default + // Get default if different let slic = postcard::ser_flavors::Slice::new(interface.buffer); let slic = match defaults.get_postcard_by_key(&key, slic) { Err(miniconf::Error::Traversal(Traversal::Absent(_))) => { @@ -408,8 +417,8 @@ impl<'a, P: Platform, const Y: usize> Interface<'a, P, Y> { |key, interface, settings, defaults| { // Get default value checksum let slic = postcard::ser_flavors::Slice::new(interface.buffer); - // Could in theory serialize directly into the hasher let mut check = match defaults.get_postcard_by_key(&key, slic) { + // Could also serialize directly into the hasher for all these checksum calcs Ok(slic) => yafnv::fnv1a::(slic), Err(miniconf::Error::Traversal(Traversal::Absent( _depth, @@ -678,7 +687,7 @@ impl<'a, P: Platform, const Y: usize> Runner<'a, P, Y> { /// /// # Returns /// A boolean indicating true if the settings were modified. - pub fn process( + pub fn poll( &mut self, settings: &mut P::Settings, ) -> Result::Error> { diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index c5724a8dd..5dc1c7a8e 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -529,7 +529,7 @@ mod app { }); c.shared.settings.lock(|settings| { - if c.local.usb_terminal.process(settings).unwrap() { + if c.local.usb_terminal.poll(settings).unwrap() { settings_update::spawn().unwrap() } }); diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index f317a2116..77924c843 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -573,7 +573,7 @@ mod app { }); c.shared.settings.lock(|settings| { - if c.local.usb_terminal.process(settings).unwrap() { + if c.local.usb_terminal.poll(settings).unwrap() { settings_update::spawn().unwrap() } });