platform commands, make serial-settings lighter

* remove hardware/platform specific logic into Platform
* reduce dependencies and hardware stuff from serial-settings

serial-settings/Cargo.toml

@@ -6,11 +6,7 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-embedded-storage = "0.3"
 miniconf = "0.9"
 menu = {version = "0.4", features = ["echo"]}
-cortex-m = "0.7"
 heapless = "0.8"
-postcard = "1"
 embedded-io = "0.6"
-serde = { version = "1", default-features=false }

serial-settings/src/lib.rs

@@ -50,27 +50,11 @@
 
 use core::fmt::Write;
 use embedded_io::{Read, ReadReady, Write as EioWrite, WriteReady};
-use embedded_storage::nor_flash::NorFlash;
 use miniconf::{JsonCoreSlash, TreeKey};
-use serde::{Deserialize, Serialize};
-
-#[derive(Debug)]
-enum Error<F> {
-    Postcard(postcard::Error),
-    Flash(F),
-}
-
-impl<F> From<postcard::Error> for Error<F> {
-    fn from(e: postcard::Error) -> Self {
-        Self::Postcard(e)
-    }
-}
 
 /// Specifies the API required for objects that are used as settings with the serial terminal
 /// interface.
-pub trait Settings:
-    for<'a> JsonCoreSlash<'a> + Serialize + Clone + for<'a> Deserialize<'a>
-{
+pub trait Settings: for<'a> JsonCoreSlash<'a> + Clone {
     /// Reset the settings to their default values.
     fn reset(&mut self) {}
 }
@@ -85,35 +69,20 @@ pub trait Platform: Sized {
     /// Specifies the settings that are used on the device.
     type Settings: Settings;
 
-    /// Specifies the type of storage used for persisting settings between device boots.
-    type Storage: NorFlash;
+    type Error: core::fmt::Debug;
 
     /// Execute a platform specific command.
-    fn cmd(&mut self, cmd: &str) {
-        match cmd {
-            "reboot" => cortex_m::peripheral::SCB::sys_reset(),
-            "service" => {
-                writeln!(self.interface_mut(), "Service data not available")
-            }
-            "dfu" => {
-                writeln!(self.interface_mut(), "Reset to DFU is not supported")
-            }
-            _ => writeln!(
-                self.interface_mut(),
-                "Invalid platform command `{cmd}`"
-            ),
-        }
-        .ok();
-    }
-
+    fn cmd(&mut self, cmd: &str);
     /// Return a mutable reference to the `Interface`.
     fn interface_mut(&mut self) -> &mut Self::Interface;
     /// Return a reference to the `Settings`
     fn settings(&self) -> &Self::Settings;
     /// Return a mutable reference to the `Settings`.
     fn settings_mut(&mut self) -> &mut Self::Settings;
-    /// Return a mutable referenc to the `Storage`.
-    fn storage_mut(&mut self) -> &mut Self::Storage;
+    /// Load the settings from storage
+    fn load(&mut self, buffer: &mut [u8]) -> Result<(), Self::Error>;
+    /// Save the settings to storage
+    fn save(&mut self, buffer: &mut [u8]) -> Result<(), Self::Error>;
 }
 
 struct Context<'a, P: Platform> {
@@ -122,25 +91,6 @@ struct Context<'a, P: Platform> {
 }
 
 impl<'a, P: Platform> Context<'a, P> {
-    fn save(
-        &mut self,
-    ) -> Result<
-        (),
-        Error<<P::Storage as embedded_storage::nor_flash::ErrorType>::Error>,
-    > {
-        let serialized =
-            postcard::to_slice(&self.platform.settings(), self.buffer)?;
-        self.platform
-            .storage_mut()
-            .erase(0, serialized.len() as u32)
-            .map_err(Error::Flash)?;
-        self.platform
-            .storage_mut()
-            .write(0, serialized)
-            .map_err(Error::Flash)?;
-        Ok(())
-    }
-
     fn handle_platform(
         _menu: &menu::Menu<Self>,
         item: &menu::Item<Self>,
@@ -194,7 +144,7 @@ impl<'a, P: Platform> Context<'a, P> {
         context: &mut Self,
     ) {
         context.platform.settings_mut().reset();
-        match context.save() {
+        match context.platform.save(context.buffer) {
             Ok(_) => {
                 writeln!(context, "Settings reset to default").unwrap();
             }
@@ -242,7 +192,7 @@ impl<'a, P: Platform> Context<'a, P> {
             .settings_mut()
             .set_json(key, value.as_bytes())
         {
-            Ok(_) => match context.save() {
+            Ok(_) => match context.platform.save(context.buffer) {
                 Ok(_) => {
                     writeln!(
                             context,
@@ -342,50 +292,42 @@ impl<'a, P: Platform> core::fmt::Write for Context<'a, P> {
 }
 
 /// The serial settings management object.
-pub struct SerialSettings<'a, P: Platform> {
-    menu: menu::Runner<'a, Context<'a, P>>,
-}
+pub struct Runner<'a, P: Platform>(menu::Runner<'a, Context<'a, P>>);
 
-impl<'a, P: Platform> SerialSettings<'a, P> {
+impl<'a, P: Platform> Runner<'a, P> {
     /// Constructor
     ///
     /// # Args
     /// * `platform` - The platform associated with the serial settings, providing the necessary
     /// context and API to manage device settings.
-    /// * `interface` - The interface to read/write data to/from serially (via text) to the user.
-    /// * `flash` - The storage mechanism used to persist settings to between boots.
-    /// * `settings_callback` - A function called after the settings are loaded from memory. If no
-    /// settings were found, `None` is provided. This function should provide the initial device
-    /// settings.
     /// * `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.
     pub fn new(
-        platform: P,
+        mut platform: P,
         line_buf: &'a mut [u8],
         serialize_buf: &'a mut [u8],
-    ) -> Result<
-        Self,
-        <P::Storage as embedded_storage::nor_flash::ErrorType>::Error,
-    > {
-        let context = Context {
-            platform,
-            buffer: serialize_buf,
-        };
-
-        let menu = menu::Runner::new(Context::menu(), line_buf, context);
-        Ok(Self { menu })
+    ) -> Result<Self, P::Error> {
+        platform.load(serialize_buf)?;
+        Ok(Self(menu::Runner::new(
+            Context::menu(),
+            line_buf,
+            Context {
+                platform,
+                buffer: serialize_buf,
+            },
+        )))
     }
 
     /// Get the current device settings.
     pub fn settings(&self) -> &P::Settings {
-        self.menu.context.platform.settings()
+        self.0.context.platform.settings()
     }
 
     /// Get the device communication interface
     pub fn interface_mut(&mut self) -> &mut P::Interface {
-        self.menu.context.platform.interface_mut()
+        self.0.context.platform.interface_mut()
     }
 
     /// Must be called periodically to process user input.
@@ -396,7 +338,7 @@ impl<'a, P: Platform> SerialSettings<'a, P> {
             let mut buffer = [0u8; 64];
             let count = self.interface_mut().read(&mut buffer)?;
             for &value in &buffer[..count] {
-                self.menu.input_byte(value);
+                self.0.input_byte(value);
             }
         }
 

src/hardware/flash.rs

@@ -1,5 +1,7 @@
 use core::fmt::Write;
+use embedded_storage::nor_flash::{NorFlash, ReadNorFlash};
 use stm32h7xx_hal::flash::LockedFlashBank;
+use usbd_serial::embedded_io::Write as EioWrite;
 
 #[derive(Clone, serde::Serialize, serde::Deserialize, miniconf::Tree)]
 pub struct Settings {
@@ -68,3 +70,77 @@ impl embedded_storage::nor_flash::NorFlash for Flash {
         bank.write(offset, bytes)
     }
 }
+
+#[derive(Debug)]
+pub enum Error<F> {
+    Postcard(postcard::Error),
+    Flash(F),
+}
+
+impl<F> From<postcard::Error> for Error<F> {
+    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: usbd_serial::SerialPort<'static, super::UsbBus>,
+    /// The Settings structure.
+    pub settings: Settings,
+    /// The storage mechanism used to persist settings to between boots.
+    pub storage: Flash,
+}
+
+impl serial_settings::Platform for SerialSettingsPlatform {
+    type Interface = usbd_serial::SerialPort<'static, super::UsbBus>;
+    type Settings = Settings;
+    type Error =
+        Error<<Flash as embedded_storage::nor_flash::ErrorType>::Error>;
+
+    fn load(&mut self, buffer: &mut [u8]) -> Result<(), Self::Error> {
+        self.storage.read(0, buffer).map_err(Self::Error::Flash)?;
+        self.settings = match postcard::from_bytes::<Self::Settings>(buffer) {
+            Ok(mut settings) => {
+                settings.mac = self.settings.mac;
+                settings
+            }
+            Err(_) => Self::Settings::new(self.settings.mac),
+        };
+        Ok(())
+    }
+
+    fn save(&mut self, buf: &mut [u8]) -> Result<(), Self::Error> {
+        let serialized = postcard::to_slice(self.settings(), buf)?;
+        self.storage
+            .erase(0, serialized.len() as u32)
+            .map_err(Self::Error::Flash)?;
+        self.storage
+            .write(0, serialized)
+            .map_err(Self::Error::Flash)?;
+        Ok(())
+    }
+
+    fn cmd(&mut self, cmd: &str) {
+        match cmd {
+            "reboot" => cortex_m::peripheral::SCB::sys_reset(),
+            _ => writeln!(
+                self.interface_mut(),
+                "Invalid platform command `{cmd}`"
+            ),
+        }
+        .ok();
+    }
+
+    fn settings(&self) -> &Self::Settings {
+        &self.settings
+    }
+
+    fn settings_mut(&mut self) -> &mut Self::Settings {
+        &mut self.settings
+    }
+
+    fn interface_mut(&mut self) -> &mut Self::Interface {
+        &mut self.interface
+    }
+}

src/hardware/mod.rs

@@ -81,33 +81,8 @@ pub type I2c1 = hal::i2c::I2c<hal::stm32::I2C1>;
 pub type I2c1Proxy =
     shared_bus::I2cProxy<'static, shared_bus::AtomicCheckMutex<I2c1>>;
 
-pub struct SerialSettingsPlatform {
-    interface: usbd_serial::SerialPort<'static, UsbBus>,
-    settings: flash::Settings,
-    storage: flash::Flash,
-}
-
-impl serial_settings::Platform for SerialSettingsPlatform {
-    type Interface = usbd_serial::SerialPort<'static, UsbBus>;
-    type Storage = flash::Flash;
-    type Settings = flash::Settings;
-
-    fn settings(&self) -> &Self::Settings {
-        &self.settings
-    }
-    fn settings_mut(&mut self) -> &mut Self::Settings {
-        &mut self.settings
-    }
-    fn interface_mut(&mut self) -> &mut Self::Interface {
-        &mut self.interface
-    }
-    fn storage_mut(&mut self) -> &mut Self::Storage {
-        &mut self.storage
-    }
-}
-
 pub type SerialTerminal =
-    serial_settings::SerialSettings<'static, SerialSettingsPlatform>;
+    serial_settings::Runner<'static, flash::SerialSettingsPlatform>;
 
 #[inline(never)]
 #[panic_handler]

src/hardware/setup.rs

@@ -3,7 +3,6 @@
 //! This file contains all of the hardware-specific configuration of Stabilizer.
 use core::sync::atomic::{self, AtomicBool, Ordering};
 use core::{fmt::Write, ptr, slice};
-use embedded_storage::nor_flash::ReadNorFlash;
 use stm32h7xx_hal::{
     self as hal,
     ethernet::{self, PHY},
@@ -1076,29 +1075,19 @@ pub fn setup(
 
     let usb_serial = {
         let (_, flash_bank2) = device.FLASH.split();
-        let mut flash_bank2 = flash_bank2.unwrap();
 
         let input_buffer =
             cortex_m::singleton!(: [u8; 256] = [0u8; 256]).unwrap();
         let serialize_buffer =
             cortex_m::singleton!(: [u8; 512] = [0u8; 512]).unwrap();
 
-        flash_bank2.read(0, &mut serialize_buffer[..]).unwrap();
-        let settings = match postcard::from_bytes::<super::flash::Settings>(
-            serialize_buffer,
-        ) {
-            Ok(mut settings) => {
-                settings.mac = network_devices.mac_address;
-                settings
-            }
-            Err(_) => super::flash::Settings::new(network_devices.mac_address),
-        };
-
-        serial_settings::SerialSettings::new(
-            super::SerialSettingsPlatform {
+        serial_settings::Runner::new(
+            super::flash::SerialSettingsPlatform {
                 interface: usb_serial,
-                settings,
-                storage: super::flash::Flash(flash_bank2),
+                storage: super::flash::Flash(flash_bank2.unwrap()),
+                settings: super::flash::Settings::new(
+                    network_devices.mac_address,
+                ),
             },
             input_buffer,
             serialize_buffer,