Repository: quartiq/stabilizer (language: rust)

Issue:
Add ability to save active settings to flash
Add functionality to save the currently-operational settings into device flash memory via the USB interface. This was identified in https://github.com/quartiq/stabilizer/pull/857#discussion_r1539080660


Discussion / hints:
When playing with it I noticed that after the change to a shared settings struct the behavior is not anymore (after the very positive changes to shared settings) what I'd have expected (without reading docs/code).

* `set` saves "everything" (in the sense that what is doesn't save is unchanged from default). At least now it should save only what's been set. But arguably it should not save at all anymore (since it already affects live application settings).
* There is no way to "save the settings to flash" without actively setting one. Since over USB we now share the MQTT view, I'd want a `save` command to make the currently active (and visible) settings the initial settings.

I think I closed this too early.

Fix patch:
diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs
index 7c41ddf6c..9d381a63a 100644
--- a/serial-settings/src/lib.rs
+++ b/serial-settings/src/lib.rs
@@ -22,7 +22,8 @@
 //!   list
 //!   get <item>
 //!   set <item> <value>
-//!   clear
+//!   save [item]
+//!   clear [item]
 //!   platform <cmd>
 //!   help [ <command> ]
 //!
@@ -83,6 +84,7 @@ pub trait Platform<const Y: usize>: Sized {
     fn save(
         &mut self,
         buffer: &mut [u8],
+        key: Option<&str>,
         settings: &Self::Settings,
     ) -> Result<(), Self::Error>;
 
@@ -195,7 +197,8 @@ impl<'a, P: Platform<Y>, const Y: usize> Interface<'a, P, Y> {
         interface: &mut Self,
         settings: &mut P::Settings,
     ) {
-        if let Some(key) = menu::argument_finder(item, args, "item").unwrap() {
+        let maybe_key = menu::argument_finder(item, args, "item").unwrap();
+        if let Some(key) = maybe_key {
             let mut defaults = settings.clone();
             defaults.reset();
 
@@ -222,9 +225,9 @@ impl<'a, P: Platform<Y>, const Y: usize> Interface<'a, P, Y> {
             writeln!(interface, "All settings cleared").unwrap();
         }
 
-        match interface.platform.save(interface.buffer, settings) {
+        match interface.platform.save(interface.buffer, maybe_key, settings) {
             Ok(_) => {
-                writeln!(interface, "Settings saved. Reboot device (`platform reboot`) to apply.")
+                writeln!(interface, "Settings saved. You may need to reboot for the settings to be applied")
             }
             Err(e) => {
                 writeln!(interface, "Failed to clear settings: {e:?}")
@@ -256,6 +259,25 @@ impl<'a, P: Platform<Y>, const Y: usize> Interface<'a, P, Y> {
         .unwrap();
     }
 
+    fn handle_save(
+        _menu: &menu::Menu<Self, P::Settings>,
+        item: &menu::Item<Self, P::Settings>,
+        args: &[&str],
+        interface: &mut Self,
+        settings: &mut P::Settings,
+    ) {
+        match interface.platform.save(interface.buffer, menu::argument_finder(item, args, "item").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()
+            }
+        }
+    }
+
     fn handle_set(
         _menu: &menu::Menu<Self, P::Settings>,
         item: &menu::Item<Self, P::Settings>,
@@ -268,22 +290,14 @@ impl<'a, P: Platform<Y>, const Y: usize> Interface<'a, P, Y> {
             menu::argument_finder(item, args, "value").unwrap().unwrap();
 
         // Now, write the new value into memory.
-        // TODO: Validate it first?
         match settings.set_json(key, value.as_bytes())
         {
             Ok(_) => {
                 interface.updated = true;
-                match interface.platform.save(interface.buffer, settings) {
-                    Ok(_) => {
-                        writeln!(
-                                interface,
-                                "Settings saved. Reboot device (`platform reboot`) to apply."
-                            )
-                    }
-                    Err(e) => {
-                        writeln!(interface, "Failed to save settings: {e:?}")
-                    }
-                }
+                writeln!(
+                        interface,
+                        "Settings updated. You may need to reboot for the setting to be applied"
+                    )
             },
             Err(e) => {
                 writeln!(interface, "Failed to update {key}: {e:?}")
@@ -331,6 +345,19 @@ impl<'a, P: Platform<Y>, const Y: usize> Interface<'a, P, Y> {
                     ]
                 },
             },
+            &menu::Item {
+                command: "save",
+                help: Some("Save settings to the device."),
+                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."),
+                        },
+                    ]
+                },
+            },
             &menu::Item {
                 command: "clear",
                 help: Some("Clear the device settings to default values."),
diff --git a/src/settings.rs b/src/settings.rs
index eaeb8b7da..0838a9ed2 100644
--- a/src/settings.rs
+++ b/src/settings.rs
@@ -167,11 +167,12 @@ where
     fn save(
         &mut self,
         buf: &mut [u8],
+        key: Option<&str>,
         settings: &Self::Settings,
     ) -> Result<(), Self::Error> {
-        for path in Self::Settings::iter_paths::<String<64>>("/") {
+        let mut save_setting = |path| -> Result<(), Self::Error> {
             let mut item = SettingsItem {
-                path: path.unwrap(),
+                path,
                 ..Default::default()
             };
 
@@ -187,7 +188,7 @@ where
                         "Failed to save `{}` to flash: {e:?}",
                         item.path
                     );
-                    continue;
+                    return Ok(());
                 }
                 Ok(slice) => slice.len(),
             };
@@ -210,6 +211,17 @@ where
                 log::info!("Storing `{}` to flash", item.path);
                 map::store_item(&mut self.storage, range, buf, item).unwrap();
             }
+
+            Ok(())
+        };
+
+        if let Some(key) = key {
+            save_setting(heapless::String::from(key))?;
+        } else {
+            for path in Self::Settings::iter_paths::<heapless::String<64>>("/")
+            {
+                save_setting(path.unwrap())?;
+            }
         }
 
         Ok(())


Test patch: