From f17a000925c9a4780887a7deb6745714f2f2ea62 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Wed, 15 Nov 2023 14:32:11 +0100 Subject: [PATCH 01/20] Moving serial settings to a separate crate --- Cargo.lock | 62 +++++-- Cargo.toml | 3 +- serial-settings/Cargo.toml | 18 ++ serial-settings/src/lib.rs | 306 ++++++++++++++++++++++++++++++++ src/bin/dual-iir.rs | 10 +- src/bin/lockin.rs | 8 +- src/hardware/flash.rs | 41 ++++- src/hardware/mod.rs | 3 +- src/hardware/serial_terminal.rs | 285 ----------------------------- src/hardware/setup.rs | 18 +- 10 files changed, 437 insertions(+), 317 deletions(-) create mode 100644 serial-settings/Cargo.toml create mode 100644 serial-settings/src/lib.rs delete mode 100644 src/hardware/serial_terminal.rs diff --git a/Cargo.lock b/Cargo.lock index 83f173eb2..6723db82e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -187,7 +187,7 @@ dependencies = [ "bare-metal 1.0.0", "cortex-m 0.7.7", "cortex-m-rtic-macros", - "heapless", + "heapless 0.7.16", "rtic-core", "rtic-monotonic", "version_check", @@ -243,7 +243,7 @@ version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "447416d161ba378782c13e82b11b267d6d2104b4913679a7c5640e7e94f96ea7" dependencies = [ - "heapless", + "heapless 0.7.16", "nb 1.1.0", "no-std-net", ] @@ -335,6 +335,15 @@ dependencies = [ "byteorder", ] +[[package]] +name = "hash32" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47d60b12902ba28e2730cd37e95b8c9223af2808df9e902d4df49588d1470606" +dependencies = [ + "byteorder", +] + [[package]] name = "hashbrown" version = "0.12.3" @@ -348,13 +357,23 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "db04bc24a18b9ea980628ecf00e6c0264f3c1426dac36c00cb49b6fbad8b0743" dependencies = [ "atomic-polyfill 0.1.11", - "hash32", + "hash32 0.2.1", "rustc_version 0.4.0", "serde", "spin", "stable_deref_trait", ] +[[package]] +name = "heapless" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0bfb9eb618601c89945a70e254898da93b13be0388091d42117462b265bb3fad" +dependencies = [ + "hash32 0.3.1", + "stable_deref_trait", +] + [[package]] name = "idsp" version = "0.13.0" @@ -431,6 +450,10 @@ dependencies = [ "paste", ] +[[package]] +name = "menu" +version = "0.3.2" + [[package]] name = "menu" version = "0.3.2" @@ -444,7 +467,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9df2d7bdba3acb28460c347b21e1e88d869f2716ebe060eb6a79f7b76b57de72" dependencies = [ "embedded-io", - "heapless", + "heapless 0.7.16", "itoa", "log", "miniconf_derive", @@ -474,7 +497,7 @@ dependencies = [ "bit_field", "embedded-nal", "embedded-time", - "heapless", + "heapless 0.7.16", "num_enum 0.7.1", "serde", "smlang", @@ -650,7 +673,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a55c51ee6c0db07e68448e336cf8ea4131a620edefebf9893e759b2d793420f8" dependencies = [ "cobs", - "heapless", + "heapless 0.7.16", "serde", ] @@ -821,7 +844,7 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3c9e1ab533c0bc414c34920ec7e5f097101d126ed5eac1a1aac711222e0bbb33" dependencies = [ - "heapless", + "heapless 0.7.16", "ryu", "serde", ] @@ -837,6 +860,22 @@ dependencies = [ "syn 2.0.38", ] +[[package]] +name = "serial-settings" +version = "0.1.0" +dependencies = [ + "bbqueue", + "cortex-m 0.7.7", + "embedded-storage", + "heapless 0.8.0", + "menu 0.3.2", + "miniconf", + "postcard", + "serde", + "usb-device", + "usbd-serial", +] + [[package]] name = "shared-bus" version = "0.2.5" @@ -890,7 +929,7 @@ dependencies = [ "bitflags 1.3.2", "byteorder", "cfg-if", - "heapless", + "heapless 0.7.16", "managed", ] @@ -902,7 +941,7 @@ checksum = "6dd2ed2f8e7643a170506863ed0f52ad1dc5762abdcff27de825dde14fc8025f" dependencies = [ "embedded-nal", "embedded-time", - "heapless", + "heapless 0.7.16", "nanorand", "shared-bus 0.2.5", "smoltcp", @@ -930,12 +969,12 @@ dependencies = [ "embedded-storage", "enum-iterator", "fugit", - "heapless", + "heapless 0.7.16", "idsp", "lm75", "log", "mcp230xx", - "menu", + "menu 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "miniconf", "minimq", "mono-clock", @@ -949,6 +988,7 @@ dependencies = [ "rtt-target", "serde", "serde-json-core", + "serial-settings", "shared-bus 0.3.1", "smoltcp-nal", "spin", diff --git a/Cargo.toml b/Cargo.toml index 9e9531bbc..2e874ba58 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,7 @@ features = [] default-target = "thumbv7em-none-eabihf" [workspace] -members = ["ad9959"] +members = ["ad9959", "serial-settings"] [dependencies] embedded-storage = "0.3" @@ -49,6 +49,7 @@ num_enum = { version = "0.7.1", default-features = false } paste = "1" idsp = "0.13" ad9959 = { path = "ad9959", version = "0.2.1" } +serial-settings = {path = "serial-settings"} mcp230xx = "1.0" mutex-trait = "0.2" fugit = "0.3" diff --git a/serial-settings/Cargo.toml b/serial-settings/Cargo.toml new file mode 100644 index 000000000..5a129bf28 --- /dev/null +++ b/serial-settings/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "serial-settings" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +embedded-storage = "0.3" +usbd-serial = "0.1.1" +bbqueue = "0.5" +usb-device = "0.2.9" +miniconf = "0.9" +menu = {path = "../../../rust-embedded-community/menu"} +cortex-m = "0.7" +heapless = "0.8" +postcard = "1" +serde = {version = "1", default-features=false} diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs new file mode 100644 index 000000000..3247259c2 --- /dev/null +++ b/serial-settings/src/lib.rs @@ -0,0 +1,306 @@ +#![no_std] + +use core::fmt::Write; +use miniconf::JsonCoreSlash; +use embedded_storage::nor_flash::NorFlash; +use serde::Serialize; + +static OUTPUT_BUFFER: bbqueue::BBBuffer<512> = bbqueue::BBBuffer::new(); + +struct Context { + output: OutputBuffer, + flash: Flash, + settings: Settings, +} + +impl Context { + fn save(&mut self) { + self.flash.erase(0, self.flash.capacity() as u32).unwrap(); + // TODO: Use a user provided buffer. + let mut buf = [0; 512]; + let serialized = postcard::to_slice(&self.settings, &mut buf).unwrap(); + self.flash.write(0, serialized).unwrap(); + } +} + +pub trait SerialSettings: for<'a> JsonCoreSlash<'a> + Serialize + Clone + 'static { + fn reset(&mut self); +} + +fn make_menu() -> menu::Menu<'static, Context> { + menu::Menu { + label: "root", + items: &[ + &menu::Item { + command: "reboot", + help: Some("Reboot the device to force new settings to take effect."), + item_type: menu::ItemType::Callback { + function: handle_reboot, + parameters: &[] + }, + }, + &menu::Item { + command: "factory-reset", + help: Some("Reset the device settings to default values."), + item_type: menu::ItemType::Callback { + function: handle_reset, + parameters: &[] + }, + }, + &menu::Item { + command: "list", + help: Some("List all available settings and their current values."), + item_type: menu::ItemType::Callback { + function: handle_list, + parameters: &[], + }, + }, + &menu::Item { + command: "get", + help: Some("Read a setting_from the device."), + item_type: menu::ItemType::Callback { + function: handle_get, + parameters: &[menu::Parameter::Mandatory { + parameter_name: "item", + help: Some("The name of the setting to read."), + }] + }, + }, + &menu::Item { + command: "set", + help: Some("Update a a setting in the device."), + item_type: menu::ItemType::Callback { + function: handle_set, + parameters: &[ + menu::Parameter::Mandatory { + parameter_name: "item", + help: Some("The name of the setting to write."), + }, + menu::Parameter::Mandatory { + parameter_name: "value", + help: Some("Specifies the value to be written. Values must be JSON-encoded"), + }, + ] + }, + }, + ], + entry: None, + exit: None, + } +} + +pub struct OutputBuffer { + producer: bbqueue::Producer<'static, 512>, +} + +impl Write for OutputBuffer { + fn write_str(&mut self, s: &str) -> core::fmt::Result { + let data = s.as_bytes(); + + // Write as much data as possible to the output buffer. + let Ok(mut grant) = self.producer.grant_max_remaining(data.len()) + else { + // Output buffer is full, silently drop the data. + return Ok(()); + }; + + let len = grant.buf().len(); + grant.buf().copy_from_slice(&data[..len]); + grant.commit(len); + Ok(()) + } +} + +impl core::fmt::Write for Context { + /// Write data to the serial terminal. + /// + /// # Note + /// The terminal uses an internal buffer. Overflows of the output buffer are silently ignored. + fn write_str(&mut self, s: &str) -> core::fmt::Result { + self.output.write_str(s) + } +} + +fn handle_list( + _menu: &menu::Menu>, + _item: &menu::Item>, + _args: &[&str], + context: &mut Context, +) { + writeln!(context, "Available items:").unwrap(); + + let mut defaults = context.settings.clone(); + defaults.reset(); + + let mut buf = [0; 256]; + let mut default_buf = [0; 256]; + for path in Settings::iter_paths::>("/") { + let path = path.unwrap(); + let current_value = { + let len = context.settings.get_json(&path, &mut buf).unwrap(); + core::str::from_utf8(&buf[..len]).unwrap() + }; + let default_value = { + let len = defaults.get_json(&path, &mut default_buf).unwrap(); + core::str::from_utf8(&default_buf[..len]).unwrap() + }; + writeln!( + context, + "{path}: {current_value} [default: {default_value}]" + ) + .unwrap(); + } +} + +fn handle_reboot( + _menu: &menu::Menu>, + _item: &menu::Item>, + _args: &[&str], + _context: &mut Context, +) { + cortex_m::peripheral::SCB::sys_reset(); +} + +fn handle_reset( + _menu: &menu::Menu>, + _item: &menu::Item>, + _args: &[&str], + context: &mut Context, +) { + context.settings.reset(); + context.save(); +} + +fn handle_get( + _menu: &menu::Menu>, + item: &menu::Item>, + args: &[&str], + context: &mut Context, +) { + let mut buf = [0u8; 256]; + let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); + let len = match context.settings.get_json(key, &mut buf) { + Err(e) => { + writeln!(context, "Failed to read {key}: {e}").unwrap(); + return; + } + Ok(len) => len, + }; + + let stringified = core::str::from_utf8(&buf[..len]).unwrap(); + writeln!(context, "{key}: {stringified}").unwrap(); +} + +fn handle_set( + _menu: &menu::Menu>, + item: &menu::Item>, + args: &[&str], + context: &mut Context, +) { + let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); + let value = menu::argument_finder(item, args, "value").unwrap().unwrap(); + + // Now, write the new value into memory. + // TODO: Validate it first? + match context.settings.set_json(key, value.as_bytes()) { + Ok(_) => { + context.save(); + writeln!( + context, + "Settings in memory may differ from currently operating settings. \ + Reset device to apply settings." + ) + .unwrap(); + } + Err(e) => { + writeln!(context, "Failed to update {key}: {e:?}").unwrap(); + } + } +} + +pub struct SerialTerminal<'a, UsbBus: usb_device::bus::UsbBus, Settings: SerialSettings, Flash: NorFlash + 'static> { + usb_device: usb_device::device::UsbDevice<'a, UsbBus>, + usb_serial: usbd_serial::SerialPort<'a, UsbBus>, + menu: menu::Runner<'a, Context>, + output: bbqueue::Consumer<'static, 512>, +} + +impl<'a, UsbBus: usb_device::bus::UsbBus, Settings: SerialSettings, Flash: NorFlash + 'static> SerialTerminal<'a, UsbBus, Settings, Flash> { + pub fn new( + usb_device: usb_device::device::UsbDevice<'static, UsbBus>, + usb_serial: usbd_serial::SerialPort<'static, UsbBus>, + input_buffer: &'a mut [u8], + settings: Settings, + flash: Flash, + ) -> Self { + let (producer, consumer) = OUTPUT_BUFFER.try_split().unwrap(); + + // TODO: Attempt to load from flash first. + + let context = Context { + settings, + output: OutputBuffer { producer }, + flash, + }; + Self { + menu: menu::Runner::new(make_menu(), input_buffer, context), + usb_device, + usb_serial, + output: consumer, + } + } + + fn flush(&mut self) { + let read = match self.output.read() { + Ok(grant) => grant, + Err(bbqueue::Error::InsufficientSize) => return, + err => err.unwrap(), + }; + + match self.usb_serial.write(read.buf()) { + Ok(count) => read.release(count), + Err(usbd_serial::UsbError::WouldBlock) => read.release(0), + Err(_) => { + let len = read.buf().len(); + read.release(len); + } + } + } + + pub fn settings(&self) -> &Settings { + &self.menu.context.settings + } + + pub fn usb_is_suspended(&self) -> bool { + self.usb_device.state() == usb_device::device::UsbDeviceState::Suspend + } + + pub fn process(&mut self) { + self.flush(); + + if !self.usb_device.poll(&mut [&mut self.usb_serial]) { + return; + } + + let mut buffer = [0u8; 64]; + match self.usb_serial.read(&mut buffer) { + Ok(count) => { + for &value in &buffer[..count] { + self.menu.input_byte(value); + } + } + + Err(usbd_serial::UsbError::WouldBlock) => {} + Err(_) => { + self.menu.prompt(true); + self.output + .read() + .map(|grant| { + let len = grant.buf().len(); + grant.release(len); + }) + .ok(); + } + } + } +} diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index 2479985a2..7ff93cb1e 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -44,7 +44,7 @@ use stabilizer::{ afe::Gain, dac::{Dac0Output, Dac1Output, DacCode}, hal, - serial_terminal::SerialTerminal, + SerialTerminal, signal_generator::{self, SignalGenerator}, timers::SamplingTimer, DigitalInput0, DigitalInput1, SystemTimer, Systick, AFE0, AFE1, @@ -208,7 +208,7 @@ mod app { let clock = SystemTimer::new(|| monotonics::now().ticks() as u32); // Configure the microcontroller - let (mut stabilizer, _pounder) = hardware::setup::setup( + let (stabilizer, _pounder) = hardware::setup::setup( c.core, c.device, clock, @@ -216,14 +216,14 @@ mod app { SAMPLE_TICKS, ); - let flash = stabilizer.usb_serial.flash(); + let settings = stabilizer.usb_serial.settings(); let mut network = NetworkUsers::new( stabilizer.net.stack, stabilizer.net.phy, clock, env!("CARGO_BIN_NAME"), - &flash.settings.broker, - &flash.settings.id, + &settings.broker, + &settings.id, ); let generator = network.configure_streaming(StreamFormat::AdcDacData); diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index e0accc39e..49ba337f4 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -47,7 +47,7 @@ use stabilizer::{ dac::{Dac0Output, Dac1Output, DacCode}, hal, input_stamper::InputStamper, - serial_terminal::SerialTerminal, + SerialTerminal, signal_generator, timers::SamplingTimer, DigitalInput0, DigitalInput1, SystemTimer, Systick, AFE0, AFE1, @@ -256,14 +256,14 @@ mod app { SAMPLE_TICKS, ); - let flash = stabilizer.usb_serial.flash(); + let settings = stabilizer.usb_serial.settings(); let mut network = NetworkUsers::new( stabilizer.net.stack, stabilizer.net.phy, clock, env!("CARGO_BIN_NAME"), - &flash.settings.broker, - &flash.settings.id, + &settings.broker, + &settings.id, ); let generator = network.configure_streaming(StreamFormat::AdcDacData); diff --git a/src/hardware/flash.rs b/src/hardware/flash.rs index e7774a538..12c5212ce 100644 --- a/src/hardware/flash.rs +++ b/src/hardware/flash.rs @@ -11,8 +11,14 @@ pub struct Settings { pub mac: smoltcp_nal::smoltcp::wire::EthernetAddress, } +impl serial_settings::SerialSettings for Settings { + fn reset(&mut self) { + *self = Self::new(self.mac) + } +} + impl Settings { - fn new(mac: smoltcp_nal::smoltcp::wire::EthernetAddress) -> Self { + pub fn new(mac: smoltcp_nal::smoltcp::wire::EthernetAddress) -> Self { let mut id = heapless::String::new(); write!(&mut id, "{mac}").unwrap(); @@ -22,9 +28,38 @@ impl Settings { mac, } } +} - pub fn reset(&mut self) { - *self = Self::new(self.mac) +pub struct Flash(pub LockedFlashBank); + +impl embedded_storage::nor_flash::ErrorType for Flash { + type Error = ::Error; +} + +impl embedded_storage::nor_flash::ReadNorFlash for Flash { + const READ_SIZE: usize = LockedFlashBank::READ_SIZE; + + fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error> { + self.0.read(offset, bytes) + } + + fn capacity(&self) -> usize { + self.0.capacity() + } +} + +impl embedded_storage::nor_flash::NorFlash for Flash { + const WRITE_SIZE: usize = stm32h7xx_hal::flash::UnlockedFlashBank::WRITE_SIZE; + const ERASE_SIZE: usize = stm32h7xx_hal::flash::UnlockedFlashBank::ERASE_SIZE; + + fn erase(&mut self, from: u32, to: u32) -> Result<(), Self::Error> { + let mut bank = self.0.unlocked(); + bank.erase(from, to) + } + + fn write(&mut self, offset: u32, bytes: &[u8]) -> Result<(), Self::Error> { + let mut bank = self.0.unlocked(); + bank.write(offset, bytes) } } diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index a75cd1f39..8f579409f 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -12,7 +12,6 @@ pub mod design_parameters; pub mod flash; pub mod input_stamper; pub mod pounder; -pub mod serial_terminal; pub mod setup; pub mod shared_adc; pub mod signal_generator; @@ -81,6 +80,8 @@ pub type I2c1 = hal::i2c::I2c; pub type I2c1Proxy = shared_bus::I2cProxy<'static, shared_bus::AtomicCheckMutex>; +pub type SerialTerminal = serial_settings::SerialTerminal<'static, UsbBus, flash::Settings, flash::Flash>; + #[inline(never)] #[panic_handler] fn panic(info: &core::panic::PanicInfo) -> ! { diff --git a/src/hardware/serial_terminal.rs b/src/hardware/serial_terminal.rs deleted file mode 100644 index eaadc436d..000000000 --- a/src/hardware/serial_terminal.rs +++ /dev/null @@ -1,285 +0,0 @@ -use super::UsbBus; -use crate::hardware::flash::FlashSettings; -use crate::hardware::flash::Settings; -use core::fmt::Write; -use miniconf::{JsonCoreSlash, TreeKey}; - -struct Context { - output: OutputBuffer, - flash: FlashSettings, -} - -const ROOT_MENU: menu::Menu = menu::Menu { - label: "root", - items: &[ - &menu::Item { - command: "reboot", - help: Some("Reboot the device to force new settings to take effect."), - item_type: menu::ItemType::Callback { - function: handle_reboot, - parameters: &[] - }, - }, - &menu::Item { - command: "factory-reset", - help: Some("Reset the device settings to default values."), - item_type: menu::ItemType::Callback { - function: handle_reset, - parameters: &[] - }, - }, - &menu::Item { - command: "list", - help: Some("List all available settings and their current values."), - item_type: menu::ItemType::Callback { - function: handle_list, - parameters: &[], - }, - }, - &menu::Item { - command: "get", - help: Some("Read a setting_from the device."), - item_type: menu::ItemType::Callback { - function: handle_get, - parameters: &[menu::Parameter::Mandatory { - parameter_name: "item", - help: Some("The name of the setting to read."), - }] - }, - }, - &menu::Item { - command: "set", - help: Some("Update a a setting in the device."), - item_type: menu::ItemType::Callback { - function: handle_set, - parameters: &[ - menu::Parameter::Mandatory { - parameter_name: "item", - help: Some("The name of the setting to write."), - }, - menu::Parameter::Mandatory { - parameter_name: "value", - help: Some("Specifies the value to be written. Values must be JSON-encoded"), - }, - ] - }, - }, - ], - entry: None, - exit: None, -}; - -static OUTPUT_BUFFER: bbqueue::BBBuffer<512> = bbqueue::BBBuffer::new(); - -pub struct OutputBuffer { - producer: bbqueue::Producer<'static, 512>, -} - -impl Write for OutputBuffer { - fn write_str(&mut self, s: &str) -> core::fmt::Result { - let data = s.as_bytes(); - - // Write as much data as possible to the output buffer. - let Ok(mut grant) = self.producer.grant_max_remaining(data.len()) - else { - // Output buffer is full, silently drop the data. - return Ok(()); - }; - - let len = grant.buf().len(); - grant.buf().copy_from_slice(&data[..len]); - grant.commit(len); - Ok(()) - } -} - -impl core::fmt::Write for Context { - /// Write data to the serial terminal. - /// - /// # Note - /// The terminal uses an internal buffer. Overflows of the output buffer are silently ignored. - fn write_str(&mut self, s: &str) -> core::fmt::Result { - self.output.write_str(s) - } -} - -fn handle_list( - _menu: &menu::Menu, - _item: &menu::Item, - _args: &[&str], - context: &mut Context, -) { - writeln!(context, "Available items:").unwrap(); - - let mut defaults = context.flash.settings.clone(); - defaults.reset(); - - let mut buf = [0; 256]; - let mut default_buf = [0; 256]; - for path in Settings::iter_paths::>("/") { - let path = path.unwrap(); - let current_value = { - let len = context.flash.settings.get_json(&path, &mut buf).unwrap(); - core::str::from_utf8(&buf[..len]).unwrap() - }; - let default_value = { - let len = defaults.get_json(&path, &mut default_buf).unwrap(); - core::str::from_utf8(&default_buf[..len]).unwrap() - }; - writeln!( - context, - "{path}: {current_value} [default: {default_value}]" - ) - .unwrap(); - } -} - -fn handle_reboot( - _menu: &menu::Menu, - _item: &menu::Item, - _args: &[&str], - _context: &mut Context, -) { - cortex_m::peripheral::SCB::sys_reset(); -} - -fn handle_reset( - _menu: &menu::Menu, - _item: &menu::Item, - _args: &[&str], - context: &mut Context, -) { - context.flash.settings.reset(); - context.flash.save(); -} - -fn handle_get( - _menu: &menu::Menu, - item: &menu::Item, - args: &[&str], - context: &mut Context, -) { - let mut buf = [0u8; 256]; - let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); - let len = match context.flash.settings.get_json(key, &mut buf) { - Err(e) => { - writeln!(context, "Failed to read {key}: {e}").unwrap(); - return; - } - Ok(len) => len, - }; - - let stringified = core::str::from_utf8(&buf[..len]).unwrap(); - writeln!(context, "{key}: {stringified}").unwrap(); -} - -fn handle_set( - _menu: &menu::Menu, - item: &menu::Item, - args: &[&str], - context: &mut Context, -) { - let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); - let value = menu::argument_finder(item, args, "value").unwrap().unwrap(); - - // Now, write the new value into memory. - // TODO: Validate it first? - match context.flash.settings.set_json(key, value.as_bytes()) { - Ok(_) => { - context.flash.save(); - writeln!( - context, - "Settings in memory may differ from currently operating settings. \ - Reset device to apply settings." - ) - .unwrap(); - } - Err(e) => { - writeln!(context, "Failed to update {key}: {e:?}").unwrap(); - } - } -} - -pub struct SerialTerminal { - usb_device: usb_device::device::UsbDevice<'static, UsbBus>, - usb_serial: usbd_serial::SerialPort<'static, UsbBus>, - menu: menu::Runner<'static, Context>, - output: bbqueue::Consumer<'static, 512>, -} - -impl SerialTerminal { - pub fn new( - usb_device: usb_device::device::UsbDevice<'static, UsbBus>, - usb_serial: usbd_serial::SerialPort<'static, UsbBus>, - flash: FlashSettings, - ) -> Self { - let (producer, consumer) = OUTPUT_BUFFER.try_split().unwrap(); - - let input_buffer = - cortex_m::singleton!(: [u8; 256] = [0; 256]).unwrap(); - let context = Context { - output: OutputBuffer { producer }, - flash, - }; - Self { - menu: menu::Runner::new(&ROOT_MENU, input_buffer, context), - usb_device, - usb_serial, - output: consumer, - } - } - - pub fn flash(&mut self) -> &mut FlashSettings { - &mut self.menu.context.flash - } - - fn flush(&mut self) { - let read = match self.output.read() { - Ok(grant) => grant, - Err(bbqueue::Error::InsufficientSize) => return, - err => err.unwrap(), - }; - - match self.usb_serial.write(read.buf()) { - Ok(count) => read.release(count), - Err(usbd_serial::UsbError::WouldBlock) => read.release(0), - Err(_) => { - let len = read.buf().len(); - read.release(len); - } - } - } - - pub fn usb_is_suspended(&self) -> bool { - self.usb_device.state() == usb_device::device::UsbDeviceState::Suspend - } - - pub fn process(&mut self) { - self.flush(); - - if !self.usb_device.poll(&mut [&mut self.usb_serial]) { - return; - } - - let mut buffer = [0u8; 64]; - match self.usb_serial.read(&mut buffer) { - Ok(count) => { - for &value in &buffer[..count] { - self.menu.input_byte(value); - } - } - - Err(usbd_serial::UsbError::WouldBlock) => {} - Err(_) => { - self.menu.prompt(true); - self.output - .read() - .map(|grant| { - let len = grant.buf().len(); - grant.release(len); - }) - .ok(); - } - } - } -} diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index 2e825c724..9a420fff6 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -14,8 +14,8 @@ use smoltcp_nal::smoltcp; use super::{ adc, afe, cpu_temp_sensor::CpuTempSensor, dac, delay, design_parameters, - eeprom, flash::FlashSettings, input_stamper::InputStamper, pounder, - pounder::dds_output::DdsOutput, serial_terminal::SerialTerminal, + eeprom, input_stamper::InputStamper, pounder, + pounder::dds_output::DdsOutput, shared_adc::SharedAdc, timers, DigitalInput0, DigitalInput1, EemDigitalInput0, EemDigitalInput1, EemDigitalOutput0, EemDigitalOutput1, EthernetPhy, NetworkStack, SystemTimer, Systick, UsbBus, AFE0, AFE1, @@ -117,7 +117,7 @@ pub struct StabilizerDevices { pub net: NetworkDevices, pub digital_inputs: (DigitalInput0, DigitalInput1), pub eem_gpio: EemGpioDevices, - pub usb_serial: SerialTerminal, + pub usb_serial: super::SerialTerminal, } /// The available Pounder-specific hardware interfaces. @@ -1070,10 +1070,14 @@ pub fn setup( (usb_device, serial) }; - let (_, flash_bank2) = device.FLASH.split(); + let usb_serial = { + let (_, flash_bank2) = device.FLASH.split(); + let settings = super::flash::Settings::new(network_devices.mac_address); - let settings = - FlashSettings::new(flash_bank2.unwrap(), network_devices.mac_address); + let input_buffer = cortex_m::singleton!(: [u8; 256] = [0u8; 256]).unwrap(); + + serial_settings::SerialTerminal::new(usb_device, usb_serial, input_buffer, settings, super::flash::Flash(flash_bank2.unwrap())) + }; let stabilizer = StabilizerDevices { systick, @@ -1089,7 +1093,7 @@ pub fn setup( timestamp_timer, digital_inputs, eem_gpio, - usb_serial: SerialTerminal::new(usb_device, usb_serial, settings), + usb_serial, }; // info!("Version {} {}", build_info::PKG_VERSION, build_info::GIT_VERSION.unwrap()); From 82639c605f781bc20ffd1e3f0816121b6d4e5328 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Wed, 15 Nov 2023 16:06:41 +0100 Subject: [PATCH 02/20] Using embedded IO traits --- Cargo.lock | 13 +- Cargo.toml | 1 + serial-settings/Cargo.toml | 4 +- serial-settings/src/lib.rs | 245 +++++++++++++++----------------- src/bin/dual-iir.rs | 15 +- src/bin/lockin.rs | 15 +- src/hardware/flash.rs | 17 ++- src/hardware/mod.rs | 8 +- src/hardware/serial_terminal.rs | 71 +++++++++ src/hardware/setup.rs | 17 ++- 10 files changed, 244 insertions(+), 162 deletions(-) create mode 100644 src/hardware/serial_terminal.rs diff --git a/Cargo.lock b/Cargo.lock index 6723db82e..55c279490 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -231,6 +231,12 @@ dependencies = [ "void", ] +[[package]] +name = "embedded-io" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "658bbadc628dc286b9ae02f0cb0f5411c056eb7487b72f0083203f115de94060" + [[package]] name = "embedded-io" version = "0.6.1" @@ -466,7 +472,7 @@ version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9df2d7bdba3acb28460c347b21e1e88d869f2716ebe060eb6a79f7b76b57de72" dependencies = [ - "embedded-io", + "embedded-io 0.6.1", "heapless 0.7.16", "itoa", "log", @@ -864,16 +870,14 @@ dependencies = [ name = "serial-settings" version = "0.1.0" dependencies = [ - "bbqueue", "cortex-m 0.7.7", + "embedded-io 0.5.0", "embedded-storage", "heapless 0.8.0", "menu 0.3.2", "miniconf", "postcard", "serde", - "usb-device", - "usbd-serial", ] [[package]] @@ -966,6 +970,7 @@ dependencies = [ "cortex-m-rt", "cortex-m-rtic", "embedded-hal", + "embedded-io 0.5.0", "embedded-storage", "enum-iterator", "fugit", diff --git a/Cargo.toml b/Cargo.toml index 2e874ba58..8d585ebb9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ default-target = "thumbv7em-none-eabihf" members = ["ad9959", "serial-settings"] [dependencies] +embedded-io = "0.5" embedded-storage = "0.3" menu = "0.3" cortex-m = { version = "0.7.7", features = ["inline-asm", "critical-section-single-core"] } diff --git a/serial-settings/Cargo.toml b/serial-settings/Cargo.toml index 5a129bf28..30512c5fb 100644 --- a/serial-settings/Cargo.toml +++ b/serial-settings/Cargo.toml @@ -7,12 +7,10 @@ edition = "2021" [dependencies] embedded-storage = "0.3" -usbd-serial = "0.1.1" -bbqueue = "0.5" -usb-device = "0.2.9" miniconf = "0.9" menu = {path = "../../../rust-embedded-community/menu"} cortex-m = "0.7" heapless = "0.8" postcard = "1" +embedded-io = "0.5" serde = {version = "1", default-features=false} diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 3247259c2..18afd63e1 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -1,33 +1,55 @@ #![no_std] use core::fmt::Write; -use miniconf::JsonCoreSlash; use embedded_storage::nor_flash::NorFlash; +use miniconf::JsonCoreSlash; use serde::Serialize; -static OUTPUT_BUFFER: bbqueue::BBBuffer<512> = bbqueue::BBBuffer::new(); +pub trait Settings: + for<'a> JsonCoreSlash<'a> + Serialize + Clone + 'static +{ + fn reset(&mut self); +} + +pub trait Interface: + embedded_io::Read + embedded_io::ReadReady + embedded_io::Write +{ +} -struct Context { - output: OutputBuffer, +struct Context<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> { flash: Flash, - settings: Settings, + settings: S, + interface: I, + buffer: &'a mut [u8], } -impl Context { - fn save(&mut self) { - self.flash.erase(0, self.flash.capacity() as u32).unwrap(); - // TODO: Use a user provided buffer. - let mut buf = [0; 512]; - let serialized = postcard::to_slice(&self.settings, &mut buf).unwrap(); - self.flash.write(0, serialized).unwrap(); +#[derive(Debug)] +enum Error { + Postcard(postcard::Error), + Flash(F), +} + +impl From for Error { + fn from(e: postcard::Error) -> Self { + Self::Postcard(e) } } -pub trait SerialSettings: for<'a> JsonCoreSlash<'a> + Serialize + Clone + 'static { - fn reset(&mut self); +impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> + Context<'a, I, S, Flash> +{ + fn save(&mut self) -> Result<(), Error> { + let serialized = postcard::to_slice(&self.settings, &mut self.buffer)?; + self.flash + .erase(0, serialized.len() as u32) + .map_err(Error::Flash)?; + self.flash.write(0, serialized).map_err(Error::Flash)?; + Ok(()) + } } -fn make_menu() -> menu::Menu<'static, Context> { +fn make_menu<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( +) -> menu::Menu<'a, Context<'a, I, S, Flash>> { menu::Menu { label: "root", items: &[ @@ -89,43 +111,24 @@ fn make_menu() -> menu::Men } } -pub struct OutputBuffer { - producer: bbqueue::Producer<'static, 512>, -} - -impl Write for OutputBuffer { - fn write_str(&mut self, s: &str) -> core::fmt::Result { - let data = s.as_bytes(); - - // Write as much data as possible to the output buffer. - let Ok(mut grant) = self.producer.grant_max_remaining(data.len()) - else { - // Output buffer is full, silently drop the data. - return Ok(()); - }; - - let len = grant.buf().len(); - grant.buf().copy_from_slice(&data[..len]); - grant.commit(len); - Ok(()) - } -} - -impl core::fmt::Write for Context { +impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> core::fmt::Write + for Context<'a, I, S, Flash> +{ /// Write data to the serial terminal. /// /// # Note /// The terminal uses an internal buffer. Overflows of the output buffer are silently ignored. fn write_str(&mut self, s: &str) -> core::fmt::Result { - self.output.write_str(s) + self.interface.write_all(s.as_bytes()).ok(); + Ok(()) } } -fn handle_list( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_list<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context, + context: &mut Context<'a, I, S, Flash>, ) { writeln!(context, "Available items:").unwrap(); @@ -134,7 +137,7 @@ fn handle_list( let mut buf = [0; 256]; let mut default_buf = [0; 256]; - for path in Settings::iter_paths::>("/") { + for path in S::iter_paths::>("/") { let path = path.unwrap(); let current_value = { let len = context.settings.get_json(&path, &mut buf).unwrap(); @@ -152,30 +155,37 @@ fn handle_list( } } -fn handle_reboot( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_reboot<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - _context: &mut Context, + _context: &mut Context<'a, I, S, Flash>, ) { cortex_m::peripheral::SCB::sys_reset(); } -fn handle_reset( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_reset<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context, + context: &mut Context<'a, I, S, Flash>, ) { context.settings.reset(); - context.save(); + match context.save() { + Ok(_) => { + writeln!(context, "Settings reset to default").unwrap(); + } + Err(e) => { + writeln!(context, "Failed to reset settings: {e:?}").unwrap(); + } + } } -fn handle_get( - _menu: &menu::Menu>, - item: &menu::Item>, +fn handle_get<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( + _menu: &menu::Menu>, + item: &menu::Item>, args: &[&str], - context: &mut Context, + context: &mut Context<'a, I, S, Flash>, ) { let mut buf = [0u8; 256]; let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); @@ -191,11 +201,11 @@ fn handle_get( writeln!(context, "{key}: {stringified}").unwrap(); } -fn handle_set( - _menu: &menu::Menu>, - item: &menu::Item>, +fn handle_set<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( + _menu: &menu::Menu>, + item: &menu::Item>, args: &[&str], - context: &mut Context, + context: &mut Context<'a, I, S, Flash>, ) { let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); let value = menu::argument_finder(item, args, "value").unwrap().unwrap(); @@ -203,103 +213,74 @@ fn handle_set( // Now, write the new value into memory. // TODO: Validate it first? match context.settings.set_json(key, value.as_bytes()) { - Ok(_) => { - context.save(); - writeln!( - context, - "Settings in memory may differ from currently operating settings. \ - Reset device to apply settings." - ) - .unwrap(); - } + Ok(_) => match context.save() { + Ok(_) => { + writeln!( + context, + "Settings in memory may differ from currently operating settings. \ +Reset device to apply settings." + ) + .unwrap(); + } + Err(e) => { + writeln!(context, "Failed to save settings: {e:?}").unwrap(); + } + }, Err(e) => { writeln!(context, "Failed to update {key}: {e:?}").unwrap(); } } } -pub struct SerialTerminal<'a, UsbBus: usb_device::bus::UsbBus, Settings: SerialSettings, Flash: NorFlash + 'static> { - usb_device: usb_device::device::UsbDevice<'a, UsbBus>, - usb_serial: usbd_serial::SerialPort<'a, UsbBus>, - menu: menu::Runner<'a, Context>, - output: bbqueue::Consumer<'static, 512>, +pub struct SerialSettings< + 'a, + I: Interface, + S: Settings, + Flash: NorFlash + 'static, +> { + menu: menu::Runner<'a, Context<'a, I, S, Flash>>, } -impl<'a, UsbBus: usb_device::bus::UsbBus, Settings: SerialSettings, Flash: NorFlash + 'static> SerialTerminal<'a, UsbBus, Settings, Flash> { +impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> + SerialSettings<'a, I, S, Flash> +{ pub fn new( - usb_device: usb_device::device::UsbDevice<'static, UsbBus>, - usb_serial: usbd_serial::SerialPort<'static, UsbBus>, - input_buffer: &'a mut [u8], - settings: Settings, + interface: I, + settings: S, flash: Flash, + line_buf: &'a mut [u8], + serialize_buf: &'a mut [u8], ) -> Self { - let (producer, consumer) = OUTPUT_BUFFER.try_split().unwrap(); - // TODO: Attempt to load from flash first. let context = Context { settings, - output: OutputBuffer { producer }, + interface, flash, + buffer: serialize_buf, }; - Self { - menu: menu::Runner::new(make_menu(), input_buffer, context), - usb_device, - usb_serial, - output: consumer, - } + let menu = menu::Runner::new(make_menu(), line_buf, context); + Self { menu } } - fn flush(&mut self) { - let read = match self.output.read() { - Ok(grant) => grant, - Err(bbqueue::Error::InsufficientSize) => return, - err => err.unwrap(), - }; - - match self.usb_serial.write(read.buf()) { - Ok(count) => read.release(count), - Err(usbd_serial::UsbError::WouldBlock) => read.release(0), - Err(_) => { - let len = read.buf().len(); - read.release(len); - } - } + pub fn settings(&self) -> &S { + &self.menu.context.settings } - pub fn settings(&self) -> &Settings { - &self.menu.context.settings + pub fn interface_mut(&mut self) -> &mut I { + &mut self.menu.context.interface } - pub fn usb_is_suspended(&self) -> bool { - self.usb_device.state() == usb_device::device::UsbDeviceState::Suspend + pub fn interface(&self) -> &I { + &self.menu.context.interface } pub fn process(&mut self) { - self.flush(); - - if !self.usb_device.poll(&mut [&mut self.usb_serial]) { - return; - } - - let mut buffer = [0u8; 64]; - match self.usb_serial.read(&mut buffer) { - Ok(count) => { - for &value in &buffer[..count] { - self.menu.input_byte(value); - } - } - - Err(usbd_serial::UsbError::WouldBlock) => {} - Err(_) => { - self.menu.prompt(true); - self.output - .read() - .map(|grant| { - let len = grant.buf().len(); - grant.release(len); - }) - .ok(); + while self.menu.context.interface.read_ready().unwrap() { + let mut buffer = [0u8; 64]; + let count = self.menu.context.interface.read(&mut buffer).unwrap(); + for &value in &buffer[..count] { + self.menu.input_byte(value); } } } diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index 7ff93cb1e..581f46906 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -44,10 +44,10 @@ use stabilizer::{ afe::Gain, dac::{Dac0Output, Dac1Output, DacCode}, hal, - SerialTerminal, signal_generator::{self, SignalGenerator}, timers::SamplingTimer, - DigitalInput0, DigitalInput1, SystemTimer, Systick, AFE0, AFE1, + DigitalInput0, DigitalInput1, SerialTerminal, SystemTimer, Systick, + AFE0, AFE1, }, net::{ data_stream::{FrameGenerator, StreamFormat, StreamTarget}, @@ -403,10 +403,9 @@ mod app { NetworkState::Updated => {} NetworkState::NoChange => { // We can't sleep if USB is not in suspend. - if c.shared - .usb_terminal - .lock(|terminal| terminal.usb_is_suspended()) - { + if c.shared.usb_terminal.lock(|terminal| { + terminal.interface().usb_is_suspended() + }) { cortex_m::asm::wfi(); } } @@ -468,7 +467,9 @@ mod app { #[task(priority = 1, shared=[usb_terminal])] fn usb(mut c: usb::Context) { // Handle the USB serial terminal. - c.shared.usb_terminal.lock(|usb| usb.process()); + c.shared + .usb_terminal + .lock(|usb| usb.interface_mut().process()); // Schedule to run this task every 10 milliseconds. usb::spawn_after(10u64.millis()).unwrap(); diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 49ba337f4..2bf6bd072 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -47,10 +47,10 @@ use stabilizer::{ dac::{Dac0Output, Dac1Output, DacCode}, hal, input_stamper::InputStamper, - SerialTerminal, signal_generator, timers::SamplingTimer, - DigitalInput0, DigitalInput1, SystemTimer, Systick, AFE0, AFE1, + DigitalInput0, DigitalInput1, SerialTerminal, SystemTimer, Systick, + AFE0, AFE1, }, net::{ data_stream::{FrameGenerator, StreamFormat, StreamTarget}, @@ -465,10 +465,9 @@ mod app { NetworkState::Updated => {} NetworkState::NoChange => { // We can't sleep if USB is not in suspend. - if c.shared - .usb_terminal - .lock(|terminal| terminal.usb_is_suspended()) - { + if c.shared.usb_terminal.lock(|terminal| { + terminal.interface().usb_is_suspended() + }) { cortex_m::asm::wfi(); } } @@ -519,7 +518,9 @@ mod app { #[task(priority = 1, shared=[usb_terminal])] fn usb(mut c: usb::Context) { // Handle the USB serial terminal. - c.shared.usb_terminal.lock(|usb| usb.process()); + c.shared + .usb_terminal + .lock(|usb| usb.interface_mut().process()); // Schedule to run this task every 10 milliseconds. usb::spawn_after(10u64.millis()).unwrap(); diff --git a/src/hardware/flash.rs b/src/hardware/flash.rs index 12c5212ce..12232a9ac 100644 --- a/src/hardware/flash.rs +++ b/src/hardware/flash.rs @@ -11,7 +11,7 @@ pub struct Settings { pub mac: smoltcp_nal::smoltcp::wire::EthernetAddress, } -impl serial_settings::SerialSettings for Settings { +impl serial_settings::Settings for Settings { fn reset(&mut self) { *self = Self::new(self.mac) } @@ -33,13 +33,18 @@ impl Settings { pub struct Flash(pub LockedFlashBank); impl embedded_storage::nor_flash::ErrorType for Flash { - type Error = ::Error; + type Error = + ::Error; } impl embedded_storage::nor_flash::ReadNorFlash for Flash { const READ_SIZE: usize = LockedFlashBank::READ_SIZE; - fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error> { + fn read( + &mut self, + offset: u32, + bytes: &mut [u8], + ) -> Result<(), Self::Error> { self.0.read(offset, bytes) } @@ -49,8 +54,10 @@ impl embedded_storage::nor_flash::ReadNorFlash for Flash { } impl embedded_storage::nor_flash::NorFlash for Flash { - const WRITE_SIZE: usize = stm32h7xx_hal::flash::UnlockedFlashBank::WRITE_SIZE; - const ERASE_SIZE: usize = stm32h7xx_hal::flash::UnlockedFlashBank::ERASE_SIZE; + const WRITE_SIZE: usize = + stm32h7xx_hal::flash::UnlockedFlashBank::WRITE_SIZE; + const ERASE_SIZE: usize = + stm32h7xx_hal::flash::UnlockedFlashBank::ERASE_SIZE; fn erase(&mut self, from: u32, to: u32) -> Result<(), Self::Error> { let mut bank = self.0.unlocked(); diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index 8f579409f..24b354dc6 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -12,6 +12,7 @@ pub mod design_parameters; pub mod flash; pub mod input_stamper; pub mod pounder; +pub mod serial_terminal; pub mod setup; pub mod shared_adc; pub mod signal_generator; @@ -80,7 +81,12 @@ pub type I2c1 = hal::i2c::I2c; pub type I2c1Proxy = shared_bus::I2cProxy<'static, shared_bus::AtomicCheckMutex>; -pub type SerialTerminal = serial_settings::SerialTerminal<'static, UsbBus, flash::Settings, flash::Flash>; +pub type SerialTerminal = serial_settings::SerialSettings< + 'static, + serial_terminal::SerialInterface, + flash::Settings, + flash::Flash, +>; #[inline(never)] #[panic_handler] diff --git a/src/hardware/serial_terminal.rs b/src/hardware/serial_terminal.rs new file mode 100644 index 000000000..8ae0aee85 --- /dev/null +++ b/src/hardware/serial_terminal.rs @@ -0,0 +1,71 @@ +use super::UsbBus; + +pub struct SerialInterface { + usb_device: usb_device::device::UsbDevice<'static, UsbBus>, + usb_serial: usbd_serial::SerialPort<'static, UsbBus>, +} + +#[derive(Debug)] +pub struct Error(usbd_serial::UsbError); + +impl From for Error { + fn from(e: usbd_serial::UsbError) -> Self { + Self(e) + } +} + +impl embedded_io::Error for Error { + fn kind(&self) -> embedded_io::ErrorKind { + embedded_io::ErrorKind::Other + } +} + +impl embedded_io::ErrorType for SerialInterface { + type Error = Error; +} + +impl embedded_io::Read for SerialInterface { + fn read(&mut self, buf: &mut [u8]) -> Result { + self.usb_serial.read(buf).map_err(From::from) + } +} + +impl embedded_io::ReadReady for SerialInterface { + fn read_ready(&mut self) -> Result { + Ok(true) + } +} + +impl embedded_io::Write for SerialInterface { + fn write(&mut self, buf: &[u8]) -> Result { + self.usb_serial.write(buf).map_err(From::from) + } + + fn flush(&mut self) -> Result<(), Self::Error> { + unimplemented!() + } +} + +impl serial_settings::Interface for SerialInterface {} + +impl SerialInterface { + pub fn new( + usb_device: usb_device::device::UsbDevice<'static, UsbBus>, + usb_serial: usbd_serial::SerialPort<'static, UsbBus>, + ) -> Self { + Self { + usb_device, + usb_serial, + } + } + + pub fn usb_is_suspended(&self) -> bool { + self.usb_device.state() == usb_device::device::UsbDeviceState::Suspend + } + + pub fn process(&mut self) { + if !self.usb_device.poll(&mut [&mut self.usb_serial]) { + return; + } + } +} diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index 9a420fff6..bde882102 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -15,7 +15,7 @@ use smoltcp_nal::smoltcp; use super::{ adc, afe, cpu_temp_sensor::CpuTempSensor, dac, delay, design_parameters, eeprom, input_stamper::InputStamper, pounder, - pounder::dds_output::DdsOutput, + pounder::dds_output::DdsOutput, serial_terminal::SerialInterface, shared_adc::SharedAdc, timers, DigitalInput0, DigitalInput1, EemDigitalInput0, EemDigitalInput1, EemDigitalOutput0, EemDigitalOutput1, EthernetPhy, NetworkStack, SystemTimer, Systick, UsbBus, AFE0, AFE1, @@ -1074,9 +1074,20 @@ pub fn setup( let (_, flash_bank2) = device.FLASH.split(); let settings = super::flash::Settings::new(network_devices.mac_address); - let input_buffer = cortex_m::singleton!(: [u8; 256] = [0u8; 256]).unwrap(); + let input_buffer = + cortex_m::singleton!(: [u8; 256] = [0u8; 256]).unwrap(); + let serialize_buffer = + cortex_m::singleton!(: [u8; 512] = [0u8; 512]).unwrap(); - serial_settings::SerialTerminal::new(usb_device, usb_serial, input_buffer, settings, super::flash::Flash(flash_bank2.unwrap())) + let usb_interface = SerialInterface::new(usb_device, usb_serial); + + serial_settings::SerialSettings::new( + usb_interface, + settings, + super::flash::Flash(flash_bank2.unwrap()), + input_buffer, + serialize_buffer, + ) }; let stabilizer = StabilizerDevices { From 0a3bfb8dc55111942817cf18786e805a8421a0c6 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Wed, 15 Nov 2023 16:13:31 +0100 Subject: [PATCH 03/20] Fixes after testing --- serial-settings/src/lib.rs | 6 ++++-- src/bin/dual-iir.rs | 5 ++++- src/bin/lockin.rs | 5 ++++- src/hardware/serial_terminal.rs | 4 +--- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 18afd63e1..6f05c6b81 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -275,13 +275,15 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> &self.menu.context.interface } - pub fn process(&mut self) { + pub fn process(&mut self) -> Result<(), I::Error>{ while self.menu.context.interface.read_ready().unwrap() { let mut buffer = [0u8; 64]; - let count = self.menu.context.interface.read(&mut buffer).unwrap(); + let count = self.menu.context.interface.read(&mut buffer)?; for &value in &buffer[..count] { self.menu.input_byte(value); } } + + Ok(()) } } diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index 581f46906..da6cfcee2 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -469,7 +469,10 @@ mod app { // Handle the USB serial terminal. c.shared .usb_terminal - .lock(|usb| usb.interface_mut().process()); + .lock(|usb| { + usb.interface_mut().process(); + usb.process().ok(); + }); // Schedule to run this task every 10 milliseconds. usb::spawn_after(10u64.millis()).unwrap(); diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 2bf6bd072..bf26a543b 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -520,7 +520,10 @@ mod app { // Handle the USB serial terminal. c.shared .usb_terminal - .lock(|usb| usb.interface_mut().process()); + .lock(|usb| { + usb.interface_mut().process(); + usb.process().ok(); + }); // Schedule to run this task every 10 milliseconds. usb::spawn_after(10u64.millis()).unwrap(); diff --git a/src/hardware/serial_terminal.rs b/src/hardware/serial_terminal.rs index 8ae0aee85..dcb093b89 100644 --- a/src/hardware/serial_terminal.rs +++ b/src/hardware/serial_terminal.rs @@ -64,8 +64,6 @@ impl SerialInterface { } pub fn process(&mut self) { - if !self.usb_device.poll(&mut [&mut self.usb_serial]) { - return; - } + self.usb_device.poll(&mut [&mut self.usb_serial]); } } From b8116337cc766961d14a043a016e940dbee4eab5 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Wed, 15 Nov 2023 16:22:58 +0100 Subject: [PATCH 04/20] Loading settings from memory --- serial-settings/src/lib.rs | 25 ++++++++++++++++--------- src/bin/dual-iir.rs | 10 ++++------ src/bin/lockin.rs | 10 ++++------ src/hardware/setup.rs | 17 +++++++++++++++-- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 6f05c6b81..ef3ce5d93 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -3,10 +3,14 @@ use core::fmt::Write; use embedded_storage::nor_flash::NorFlash; use miniconf::JsonCoreSlash; -use serde::Serialize; +use serde::{Deserialize, Serialize}; pub trait Settings: - for<'a> JsonCoreSlash<'a> + Serialize + Clone + 'static + for<'a> JsonCoreSlash<'a> + + Serialize + + Clone + + 'static + + for<'a> Deserialize<'a> { fn reset(&mut self); } @@ -246,12 +250,15 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> { pub fn new( interface: I, - settings: S, - flash: Flash, + mut flash: Flash, + settings_callback: impl FnOnce(Option) -> S, line_buf: &'a mut [u8], serialize_buf: &'a mut [u8], - ) -> Self { - // TODO: Attempt to load from flash first. + ) -> Result { + flash.read(0, &mut serialize_buf[..])?; + + let settings = + settings_callback(postcard::from_bytes::(serialize_buf).ok()); let context = Context { settings, @@ -260,7 +267,7 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> buffer: serialize_buf, }; let menu = menu::Runner::new(make_menu(), line_buf, context); - Self { menu } + Ok(Self { menu }) } pub fn settings(&self) -> &S { @@ -275,8 +282,8 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> &self.menu.context.interface } - pub fn process(&mut self) -> Result<(), I::Error>{ - while self.menu.context.interface.read_ready().unwrap() { + pub fn process(&mut self) -> Result<(), I::Error> { + while self.menu.context.interface.read_ready()? { let mut buffer = [0u8; 64]; let count = self.menu.context.interface.read(&mut buffer)?; for &value in &buffer[..count] { diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index da6cfcee2..77238d6e9 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -467,12 +467,10 @@ mod app { #[task(priority = 1, shared=[usb_terminal])] fn usb(mut c: usb::Context) { // Handle the USB serial terminal. - c.shared - .usb_terminal - .lock(|usb| { - usb.interface_mut().process(); - usb.process().ok(); - }); + c.shared.usb_terminal.lock(|usb| { + usb.interface_mut().process(); + usb.process().ok(); + }); // Schedule to run this task every 10 milliseconds. usb::spawn_after(10u64.millis()).unwrap(); diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index bf26a543b..19fa2043a 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -518,12 +518,10 @@ mod app { #[task(priority = 1, shared=[usb_terminal])] fn usb(mut c: usb::Context) { // Handle the USB serial terminal. - c.shared - .usb_terminal - .lock(|usb| { - usb.interface_mut().process(); - usb.process().ok(); - }); + c.shared.usb_terminal.lock(|usb| { + usb.interface_mut().process(); + usb.process().ok(); + }); // Schedule to run this task every 10 milliseconds. usb::spawn_after(10u64.millis()).unwrap(); diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index bde882102..b3b4f34fd 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -1072,7 +1072,6 @@ pub fn setup( let usb_serial = { let (_, flash_bank2) = device.FLASH.split(); - let settings = super::flash::Settings::new(network_devices.mac_address); let input_buffer = cortex_m::singleton!(: [u8; 256] = [0u8; 256]).unwrap(); @@ -1081,13 +1080,27 @@ pub fn setup( let usb_interface = SerialInterface::new(usb_device, usb_serial); + let settings_callback = + |maybe_settings: Option| { + match maybe_settings { + Some(mut settings) => { + settings.mac = network_devices.mac_address; + settings + } + None => { + super::flash::Settings::new(network_devices.mac_address) + } + } + }; + serial_settings::SerialSettings::new( usb_interface, - settings, super::flash::Flash(flash_bank2.unwrap()), + settings_callback, input_buffer, serialize_buffer, ) + .unwrap() }; let stabilizer = StabilizerDevices { From e9ff9c3590cf121f795b8760efd074cae23b7509 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Thu, 16 Nov 2023 11:11:39 +0100 Subject: [PATCH 05/20] Cleaning up embeedded-io impl --- Cargo.lock | 59 ++++++++++++++++++++-------- Cargo.toml | 13 +++++-- serial-settings/Cargo.toml | 6 +-- serial-settings/src/lib.rs | 21 ++++++++-- src/bin/dual-iir.rs | 20 +++++----- src/bin/lockin.rs | 20 +++++----- src/hardware/mod.rs | 4 +- src/hardware/serial_terminal.rs | 69 --------------------------------- src/hardware/setup.rs | 26 +++++++------ src/hardware/usb.rs | 22 +++++++++++ 10 files changed, 130 insertions(+), 130 deletions(-) delete mode 100644 src/hardware/serial_terminal.rs create mode 100644 src/hardware/usb.rs diff --git a/Cargo.lock b/Cargo.lock index 55c279490..c25ab9d1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -458,13 +458,9 @@ dependencies = [ [[package]] name = "menu" -version = "0.3.2" - -[[package]] -name = "menu" -version = "0.3.2" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b03d7f798bfe97329ad6df937951142eec93886b37d87010502dd25e8cc75fd5" +checksum = "d5208bd660042c7760f40d960ba0b1a9dc7a9c90775bea4c4637c3b666d2b53d" [[package]] name = "miniconf" @@ -635,6 +631,15 @@ dependencies = [ "num_enum_derive 0.5.11", ] +[[package]] +name = "num_enum" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a015b430d3c108a207fd776d2e2196aaf8b1cf8cf93253e3a097ff3085076a1" +dependencies = [ + "num_enum_derive 0.6.1", +] + [[package]] name = "num_enum" version = "0.7.1" @@ -655,6 +660,17 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "num_enum_derive" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96667db765a921f7b295ffee8b60472b686a51d4f21c2ee4ffdb94c7013b65a6" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.38", +] + [[package]] name = "num_enum_derive" version = "0.7.1" @@ -672,6 +688,12 @@ version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c" +[[package]] +name = "portable-atomic" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3bccab0e7fd7cc19f820a1c8c91720af652d0c88dc9664dd72aef2614f04af3b" + [[package]] name = "postcard" version = "1.0.8" @@ -871,10 +893,10 @@ name = "serial-settings" version = "0.1.0" dependencies = [ "cortex-m 0.7.7", - "embedded-io 0.5.0", + "embedded-io 0.6.1", "embedded-storage", "heapless 0.8.0", - "menu 0.3.2", + "menu", "miniconf", "postcard", "serde", @@ -979,7 +1001,6 @@ dependencies = [ "lm75", "log", "mcp230xx", - "menu 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "miniconf", "minimq", "mono-clock", @@ -1067,8 +1088,7 @@ dependencies = [ [[package]] name = "synopsys-usb-otg" version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "678f3707a7b1fd4863023292c42f73c6bab0e9b0096f41ae612d1af0ff221b45" +source = "git+https://github.com/quartiq/synopsys-usb-otg?branch=feature/usb-device-bump#06bd4b0827f4ae7907ce3a2f9db1444c4ad10d1c" dependencies = [ "cortex-m 0.7.7", "embedded-hal", @@ -1107,18 +1127,23 @@ checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" [[package]] name = "usb-device" -version = "0.2.9" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1f6cc3adc849b5292b4075fc0d5fdcf2f24866e88e336dd27a8943090a520508" +checksum = "e73e438f527e567fb3982f2370967821fab4f5aea84c42e218a211dd2002b6a2" +dependencies = [ + "heapless 0.7.16", + "num_enum 0.6.1", + "portable-atomic", +] [[package]] name = "usbd-serial" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db75519b86287f12dcf0d171c7cf4ecc839149fe9f3b720ac4cfce52959e1dfe" +version = "0.2.0" +source = "git+https://github.com/rust-embedded-community/usbd-serial?branch=feature/embedded-io#1b2d0fc7049640761f55a71b765e63a835637151" dependencies = [ "embedded-hal", - "nb 0.1.3", + "embedded-io 0.6.1", + "nb 1.1.0", "usb-device", ] diff --git a/Cargo.toml b/Cargo.toml index 8d585ebb9..ac3b333d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,6 @@ members = ["ad9959", "serial-settings"] [dependencies] embedded-io = "0.5" embedded-storage = "0.3" -menu = "0.3" cortex-m = { version = "0.7.7", features = ["inline-asm", "critical-section-single-core"] } cortex-m-rt = { version = "0.7", features = ["device"] } log = { version = "0.4", features = ["max_level_trace", "release_max_level_info"] } @@ -65,8 +64,8 @@ rand_xorshift = "0.3.0" rand_core = "0.6.4" minimq = "0.8.0" # patch with https://github.com/rust-embedded-community/usb-device/pull/129 -usb-device = "0.2.9" -usbd-serial = "0.1.1" +usb-device = "0.3.0" +usbd-serial = "0.2" # Keep this synced with the miniconf version in py/setup.py miniconf = "0.9.0" smoltcp-nal = { version = "0.4.1", features = ["shared-stack"]} @@ -77,6 +76,14 @@ postcard = "1" version = "0.15.1" features = ["stm32h743v", "rt", "ethernet", "xspi", "usb_hs"] +[patch.crates-io.usbd-serial] +git = "https://github.com/rust-embedded-community/usbd-serial" +branch = "feature/embedded-io" + +[patch.crates-io.synopsys-usb-otg] +git = "https://github.com/quartiq/synopsys-usb-otg" +branch = "feature/usb-device-bump" + [features] nightly = [ ] pounder_v1_0 = [ ] diff --git a/serial-settings/Cargo.toml b/serial-settings/Cargo.toml index 30512c5fb..b54779678 100644 --- a/serial-settings/Cargo.toml +++ b/serial-settings/Cargo.toml @@ -8,9 +8,9 @@ edition = "2021" [dependencies] embedded-storage = "0.3" miniconf = "0.9" -menu = {path = "../../../rust-embedded-community/menu"} +menu = "0.4" cortex-m = "0.7" heapless = "0.8" postcard = "1" -embedded-io = "0.5" -serde = {version = "1", default-features=false} +embedded-io = "0.6" +serde = { version = "1", default-features=false } diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index ef3ce5d93..09e18843a 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -1,4 +1,4 @@ -#![no_std] +#![cfg_attr(not(test), no_std)] use core::fmt::Write; use embedded_storage::nor_flash::NorFlash; @@ -16,7 +16,18 @@ pub trait Settings: } pub trait Interface: - embedded_io::Read + embedded_io::ReadReady + embedded_io::Write + embedded_io::Read + + embedded_io::ReadReady + + embedded_io::Write + + embedded_io::WriteReady +{ +} + +impl Interface for T where + T: embedded_io::Read + + embedded_io::ReadReady + + embedded_io::Write + + embedded_io::WriteReady { } @@ -43,7 +54,7 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> Context<'a, I, S, Flash> { fn save(&mut self) -> Result<(), Error> { - let serialized = postcard::to_slice(&self.settings, &mut self.buffer)?; + let serialized = postcard::to_slice(&self.settings, self.buffer)?; self.flash .erase(0, serialized.len() as u32) .map_err(Error::Flash)?; @@ -123,7 +134,9 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> core::fmt::Write /// # Note /// The terminal uses an internal buffer. Overflows of the output buffer are silently ignored. fn write_str(&mut self, s: &str) -> core::fmt::Result { - self.interface.write_all(s.as_bytes()).ok(); + if let Ok(true) = self.interface.write_ready() { + self.interface.write_all(s.as_bytes()).ok(); + } Ok(()) } } diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index 77238d6e9..8b3418f64 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -46,6 +46,7 @@ use stabilizer::{ hal, signal_generator::{self, SignalGenerator}, timers::SamplingTimer, + usb::UsbDevice, DigitalInput0, DigitalInput1, SerialTerminal, SystemTimer, Systick, AFE0, AFE1, }, @@ -183,7 +184,7 @@ mod app { #[shared] struct Shared { - usb_terminal: SerialTerminal, + usb: UsbDevice, network: NetworkUsers, settings: Settings, @@ -193,6 +194,7 @@ mod app { #[local] struct Local { + usb_terminal: SerialTerminal, sampling_timer: SamplingTimer, digital_inputs: (DigitalInput0, DigitalInput1), afes: (AFE0, AFE1), @@ -231,7 +233,7 @@ mod app { let settings = Settings::default(); let shared = Shared { - usb_terminal: stabilizer.usb_serial, + usb: stabilizer.usb_device, network, settings, telemetry: TelemetryBuffer::default(), @@ -250,6 +252,7 @@ mod app { }; let mut local = Local { + usb_terminal: stabilizer.usb_serial, sampling_timer: stabilizer.adc_dac_timer, digital_inputs: stabilizer.digital_inputs, afes: stabilizer.afes, @@ -393,7 +396,7 @@ mod app { ); } - #[idle(shared=[network, usb_terminal])] + #[idle(shared=[network, usb])] fn idle(mut c: idle::Context) -> ! { loop { match c.shared.network.lock(|net| net.update()) { @@ -403,9 +406,7 @@ mod app { NetworkState::Updated => {} NetworkState::NoChange => { // We can't sleep if USB is not in suspend. - if c.shared.usb_terminal.lock(|terminal| { - terminal.interface().usb_is_suspended() - }) { + if c.shared.usb.lock(|usb| usb.usb_is_suspended()) { cortex_m::asm::wfi(); } } @@ -464,12 +465,11 @@ mod app { .unwrap(); } - #[task(priority = 1, shared=[usb_terminal])] + #[task(priority = 1, shared=[usb], local=[usb_terminal])] fn usb(mut c: usb::Context) { // Handle the USB serial terminal. - c.shared.usb_terminal.lock(|usb| { - usb.interface_mut().process(); - usb.process().ok(); + c.shared.usb.lock(|usb| { + usb.process(c.local.usb_terminal); }); // Schedule to run this task every 10 milliseconds. diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 19fa2043a..381de9c1f 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -49,6 +49,7 @@ use stabilizer::{ input_stamper::InputStamper, signal_generator, timers::SamplingTimer, + usb::UsbDevice, DigitalInput0, DigitalInput1, SerialTerminal, SystemTimer, Systick, AFE0, AFE1, }, @@ -222,7 +223,7 @@ mod app { #[shared] struct Shared { - usb_terminal: SerialTerminal, + usb: UsbDevice, network: NetworkUsers, settings: Settings, telemetry: TelemetryBuffer, @@ -230,6 +231,7 @@ mod app { #[local] struct Local { + usb_terminal: SerialTerminal, sampling_timer: SamplingTimer, digital_inputs: (DigitalInput0, DigitalInput1), timestamper: InputStamper, @@ -270,7 +272,7 @@ mod app { let shared = Shared { network, - usb_terminal: stabilizer.usb_serial, + usb: stabilizer.usb_device, telemetry: TelemetryBuffer::default(), settings: Settings::default(), }; @@ -285,6 +287,7 @@ mod app { }; let mut local = Local { + usb_terminal: stabilizer.usb_serial, sampling_timer: stabilizer.adc_dac_timer, digital_inputs: stabilizer.digital_inputs, afes: stabilizer.afes, @@ -455,7 +458,7 @@ mod app { }); } - #[idle(shared=[network, usb_terminal])] + #[idle(shared=[network, usb])] fn idle(mut c: idle::Context) -> ! { loop { match c.shared.network.lock(|net| net.update()) { @@ -465,9 +468,7 @@ mod app { NetworkState::Updated => {} NetworkState::NoChange => { // We can't sleep if USB is not in suspend. - if c.shared.usb_terminal.lock(|terminal| { - terminal.interface().usb_is_suspended() - }) { + if c.shared.usb.lock(|usb| usb.usb_is_suspended()) { cortex_m::asm::wfi(); } } @@ -515,12 +516,11 @@ mod app { .unwrap(); } - #[task(priority = 1, shared=[usb_terminal])] + #[task(priority = 1, shared=[usb], local=[usb_terminal])] fn usb(mut c: usb::Context) { // Handle the USB serial terminal. - c.shared.usb_terminal.lock(|usb| { - usb.interface_mut().process(); - usb.process().ok(); + c.shared.usb.lock(|usb| { + usb.process(c.local.usb_terminal); }); // Schedule to run this task every 10 milliseconds. diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index 24b354dc6..c7aa1c106 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -12,11 +12,11 @@ pub mod design_parameters; pub mod flash; pub mod input_stamper; pub mod pounder; -pub mod serial_terminal; pub mod setup; pub mod shared_adc; pub mod signal_generator; pub mod timers; +pub mod usb; mod eeprom; @@ -83,7 +83,7 @@ pub type I2c1Proxy = pub type SerialTerminal = serial_settings::SerialSettings< 'static, - serial_terminal::SerialInterface, + usbd_serial::SerialPort<'static, UsbBus>, flash::Settings, flash::Flash, >; diff --git a/src/hardware/serial_terminal.rs b/src/hardware/serial_terminal.rs deleted file mode 100644 index dcb093b89..000000000 --- a/src/hardware/serial_terminal.rs +++ /dev/null @@ -1,69 +0,0 @@ -use super::UsbBus; - -pub struct SerialInterface { - usb_device: usb_device::device::UsbDevice<'static, UsbBus>, - usb_serial: usbd_serial::SerialPort<'static, UsbBus>, -} - -#[derive(Debug)] -pub struct Error(usbd_serial::UsbError); - -impl From for Error { - fn from(e: usbd_serial::UsbError) -> Self { - Self(e) - } -} - -impl embedded_io::Error for Error { - fn kind(&self) -> embedded_io::ErrorKind { - embedded_io::ErrorKind::Other - } -} - -impl embedded_io::ErrorType for SerialInterface { - type Error = Error; -} - -impl embedded_io::Read for SerialInterface { - fn read(&mut self, buf: &mut [u8]) -> Result { - self.usb_serial.read(buf).map_err(From::from) - } -} - -impl embedded_io::ReadReady for SerialInterface { - fn read_ready(&mut self) -> Result { - Ok(true) - } -} - -impl embedded_io::Write for SerialInterface { - fn write(&mut self, buf: &[u8]) -> Result { - self.usb_serial.write(buf).map_err(From::from) - } - - fn flush(&mut self) -> Result<(), Self::Error> { - unimplemented!() - } -} - -impl serial_settings::Interface for SerialInterface {} - -impl SerialInterface { - pub fn new( - usb_device: usb_device::device::UsbDevice<'static, UsbBus>, - usb_serial: usbd_serial::SerialPort<'static, UsbBus>, - ) -> Self { - Self { - usb_device, - usb_serial, - } - } - - pub fn usb_is_suspended(&self) -> bool { - self.usb_device.state() == usb_device::device::UsbDeviceState::Suspend - } - - pub fn process(&mut self) { - self.usb_device.poll(&mut [&mut self.usb_serial]); - } -} diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index b3b4f34fd..53dfa7616 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -15,10 +15,10 @@ use smoltcp_nal::smoltcp; use super::{ adc, afe, cpu_temp_sensor::CpuTempSensor, dac, delay, design_parameters, eeprom, input_stamper::InputStamper, pounder, - pounder::dds_output::DdsOutput, serial_terminal::SerialInterface, - shared_adc::SharedAdc, timers, DigitalInput0, DigitalInput1, - EemDigitalInput0, EemDigitalInput1, EemDigitalOutput0, EemDigitalOutput1, - EthernetPhy, NetworkStack, SystemTimer, Systick, UsbBus, AFE0, AFE1, + pounder::dds_output::DdsOutput, shared_adc::SharedAdc, timers, + usb::UsbDevice, DigitalInput0, DigitalInput1, EemDigitalInput0, + EemDigitalInput1, EemDigitalOutput0, EemDigitalOutput1, EthernetPhy, + NetworkStack, SerialTerminal, SystemTimer, Systick, UsbBus, AFE0, AFE1, }; const NUM_TCP_SOCKETS: usize = 4; @@ -117,7 +117,8 @@ pub struct StabilizerDevices { pub net: NetworkDevices, pub digital_inputs: (DigitalInput0, DigitalInput1), pub eem_gpio: EemGpioDevices, - pub usb_serial: super::SerialTerminal, + pub usb_serial: SerialTerminal, + pub usb_device: UsbDevice, } /// The available Pounder-specific hardware interfaces. @@ -1061,13 +1062,15 @@ pub fn setup( usb_bus.as_ref().unwrap(), usb_device::device::UsbVidPid(0x1209, 0x392F), ) - .manufacturer("ARTIQ/Sinara") - .product("Stabilizer") - .serial_number(serial_number.as_ref().unwrap()) + .strings(&[usb_device::device::StringDescriptors::default() + .manufacturer("ARTIQ/Sinara") + .product("Stabilizer") + .serial_number(serial_number.as_ref().unwrap())]) + .unwrap() .device_class(usbd_serial::USB_CLASS_CDC) .build(); - (usb_device, serial) + (UsbDevice::new(usb_device), serial) }; let usb_serial = { @@ -1078,8 +1081,6 @@ pub fn setup( let serialize_buffer = cortex_m::singleton!(: [u8; 512] = [0u8; 512]).unwrap(); - let usb_interface = SerialInterface::new(usb_device, usb_serial); - let settings_callback = |maybe_settings: Option| { match maybe_settings { @@ -1094,7 +1095,7 @@ pub fn setup( }; serial_settings::SerialSettings::new( - usb_interface, + usb_serial, super::flash::Flash(flash_bank2.unwrap()), settings_callback, input_buffer, @@ -1117,6 +1118,7 @@ pub fn setup( timestamp_timer, digital_inputs, eem_gpio, + usb_device, usb_serial, }; diff --git a/src/hardware/usb.rs b/src/hardware/usb.rs new file mode 100644 index 000000000..030b93cc7 --- /dev/null +++ b/src/hardware/usb.rs @@ -0,0 +1,22 @@ +use super::SerialTerminal; +use super::UsbBus; + +pub struct UsbDevice { + usb_device: usb_device::device::UsbDevice<'static, UsbBus>, +} + +impl UsbDevice { + pub fn new( + usb_device: usb_device::device::UsbDevice<'static, UsbBus>, + ) -> Self { + Self { usb_device } + } + + pub fn usb_is_suspended(&self) -> bool { + self.usb_device.state() == usb_device::device::UsbDeviceState::Suspend + } + + pub fn process(&mut self, terminal: &mut SerialTerminal) { + self.usb_device.poll(&mut [terminal.interface_mut()]); + } +} From 1b3c3a96994e645ab9407732e70467060ca651eb Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Thu, 16 Nov 2023 11:17:19 +0100 Subject: [PATCH 06/20] Removing fixed stack buffers --- serial-settings/src/lib.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 09e18843a..2012e9ae2 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -1,4 +1,4 @@ -#![cfg_attr(not(test), no_std)] +#![no_std] use core::fmt::Write; use embedded_storage::nor_flash::NorFlash; @@ -152,23 +152,19 @@ fn handle_list<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( let mut defaults = context.settings.clone(); defaults.reset(); - let mut buf = [0; 256]; - let mut default_buf = [0; 256]; for path in S::iter_paths::>("/") { let path = path.unwrap(); let current_value = { - let len = context.settings.get_json(&path, &mut buf).unwrap(); - core::str::from_utf8(&buf[..len]).unwrap() + let len = context.settings.get_json(&path, context.buffer).unwrap(); + core::str::from_utf8(&context.buffer[..len]).unwrap() }; + write!(context.interface, "{path}: {current_value}").unwrap(); + let default_value = { - let len = defaults.get_json(&path, &mut default_buf).unwrap(); - core::str::from_utf8(&default_buf[..len]).unwrap() + let len = defaults.get_json(&path, context.buffer).unwrap(); + core::str::from_utf8(&context.buffer[..len]).unwrap() }; - writeln!( - context, - "{path}: {current_value} [default: {default_value}]" - ) - .unwrap(); + writeln!(context.interface, " [default: {default_value}]").unwrap() } } @@ -204,9 +200,8 @@ fn handle_get<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( args: &[&str], context: &mut Context<'a, I, S, Flash>, ) { - let mut buf = [0u8; 256]; let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); - let len = match context.settings.get_json(key, &mut buf) { + let len = match context.settings.get_json(key, context.buffer) { Err(e) => { writeln!(context, "Failed to read {key}: {e}").unwrap(); return; @@ -214,8 +209,8 @@ fn handle_get<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( Ok(len) => len, }; - let stringified = core::str::from_utf8(&buf[..len]).unwrap(); - writeln!(context, "{key}: {stringified}").unwrap(); + let stringified = core::str::from_utf8(&context.buffer[..len]).unwrap(); + writeln!(context.interface, "{key}: {stringified}").unwrap(); } fn handle_set<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( From 214d29fce5cf46e5cbbd317e5ac0e0051034937b Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Thu, 16 Nov 2023 11:50:18 +0100 Subject: [PATCH 07/20] Adding platform object for dfu and service menu options --- serial-settings/src/lib.rs | 178 ++++++++++++++++++++++++++++++------- src/hardware/mod.rs | 1 + src/hardware/setup.rs | 1 + 3 files changed, 148 insertions(+), 32 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 2012e9ae2..8742b051e 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -5,6 +5,8 @@ use embedded_storage::nor_flash::NorFlash; use miniconf::JsonCoreSlash; use serde::{Deserialize, Serialize}; +pub use menu; + pub trait Settings: for<'a> JsonCoreSlash<'a> + Serialize @@ -31,8 +33,34 @@ impl Interface for T where { } -struct Context<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> { +pub trait Platform { + fn reset(&mut self, _interface: impl embedded_io::Write) {} + + fn dfu(&mut self, mut interface: impl embedded_io::Write) { + self.reset(&mut interface); + writeln!( + &mut interface, + "Reset to DFU is not supported on this device" + ) + .ok(); + } + + fn service(&mut self, mut interface: impl embedded_io::Write) { + writeln!(&mut interface, "Service data not available").ok(); + } +} + +impl Platform for T {} + +pub struct Context< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +> { flash: Flash, + platform: P, settings: S, interface: I, buffer: &'a mut [u8], @@ -50,8 +78,8 @@ impl From for Error { } } -impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> - Context<'a, I, S, Flash> +impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> + Context<'a, I, S, P, Flash> { fn save(&mut self) -> Result<(), Error> { let serialized = postcard::to_slice(&self.settings, self.buffer)?; @@ -63,11 +91,32 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> } } -fn make_menu<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( -) -> menu::Menu<'a, Context<'a, I, S, Flash>> { +fn settings_menu< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +>() -> menu::Menu<'a, Context<'a, I, S, P, Flash>> { menu::Menu { - label: "root", + label: "settings", items: &[ + &menu::Item { + command: "dfu", + help: Some("Reboot the device to DFU mode"), + item_type: menu::ItemType::Callback { + function: handle_dfu_reboot, + parameters: &[] + }, + }, + &menu::Item { + command: "service", + help: Some("Read device service information"), + item_type: menu::ItemType::Callback { + function: handle_service, + parameters: &[] + }, + }, &menu::Item { command: "reboot", help: Some("Reboot the device to force new settings to take effect."), @@ -126,8 +175,8 @@ fn make_menu<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( } } -impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> core::fmt::Write - for Context<'a, I, S, Flash> +impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> + core::fmt::Write for Context<'a, I, S, P, Flash> { /// Write data to the serial terminal. /// @@ -141,11 +190,17 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> core::fmt::Write } } -fn handle_list<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_list< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context<'a, I, S, Flash>, + context: &mut Context<'a, I, S, P, Flash>, ) { writeln!(context, "Available items:").unwrap(); @@ -168,20 +223,63 @@ fn handle_list<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( } } -fn handle_reboot<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_reboot< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - _context: &mut Context<'a, I, S, Flash>, + context: &mut Context<'a, I, S, P, Flash>, ) { + context.platform.reset(&mut context.interface); cortex_m::peripheral::SCB::sys_reset(); } -fn handle_reset<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_dfu_reboot< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +>( + _menu: &menu::Menu>, + _item: &menu::Item>, + _args: &[&str], + context: &mut Context<'a, I, S, P, Flash>, +) { + context.platform.dfu(&mut context.interface); +} + +fn handle_service< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +>( + _menu: &menu::Menu>, + _item: &menu::Item>, + _args: &[&str], + context: &mut Context<'a, I, S, P, Flash>, +) { + context.platform.service(&mut context.interface); +} + +fn handle_reset< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context<'a, I, S, Flash>, + context: &mut Context<'a, I, S, P, Flash>, ) { context.settings.reset(); match context.save() { @@ -194,11 +292,17 @@ fn handle_reset<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( } } -fn handle_get<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( - _menu: &menu::Menu>, - item: &menu::Item>, +fn handle_get< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +>( + _menu: &menu::Menu>, + item: &menu::Item>, args: &[&str], - context: &mut Context<'a, I, S, Flash>, + context: &mut Context<'a, I, S, P, Flash>, ) { let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); let len = match context.settings.get_json(key, context.buffer) { @@ -213,11 +317,17 @@ fn handle_get<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( writeln!(context.interface, "{key}: {stringified}").unwrap(); } -fn handle_set<'a, I: Interface, S: Settings, Flash: NorFlash + 'static>( - _menu: &menu::Menu>, - item: &menu::Item>, +fn handle_set< + 'a, + I: Interface, + S: Settings, + P: Platform, + Flash: NorFlash + 'static, +>( + _menu: &menu::Menu>, + item: &menu::Item>, args: &[&str], - context: &mut Context<'a, I, S, Flash>, + context: &mut Context<'a, I, S, P, Flash>, ) { let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); let value = menu::argument_finder(item, args, "value").unwrap().unwrap(); @@ -248,17 +358,19 @@ pub struct SerialSettings< 'a, I: Interface, S: Settings, + P: Platform, Flash: NorFlash + 'static, > { - menu: menu::Runner<'a, Context<'a, I, S, Flash>>, + menu: menu::Runner<'a, Context<'a, I, S, P, Flash>>, } -impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> - SerialSettings<'a, I, S, Flash> +impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> + SerialSettings<'a, I, S, P, Flash> { pub fn new( interface: I, mut flash: Flash, + platform: P, settings_callback: impl FnOnce(Option) -> S, line_buf: &'a mut [u8], serialize_buf: &'a mut [u8], @@ -272,9 +384,11 @@ impl<'a, I: Interface, S: Settings, Flash: NorFlash + 'static> settings, interface, flash, + platform, buffer: serialize_buf, }; - let menu = menu::Runner::new(make_menu(), line_buf, context); + + let menu = menu::Runner::new(settings_menu(), line_buf, context); Ok(Self { menu }) } diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index c7aa1c106..6dd0ca3a9 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -85,6 +85,7 @@ pub type SerialTerminal = serial_settings::SerialSettings< 'static, usbd_serial::SerialPort<'static, UsbBus>, flash::Settings, + (), flash::Flash, >; diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index 53dfa7616..bf1f2a197 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -1097,6 +1097,7 @@ pub fn setup( serial_settings::SerialSettings::new( usb_serial, super::flash::Flash(flash_bank2.unwrap()), + (), settings_callback, input_buffer, serialize_buffer, From 41e97d1f8c790ffffe604715a957bd41c042569f Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Thu, 16 Nov 2023 13:01:10 +0100 Subject: [PATCH 08/20] Refactoring after review --- Cargo.lock | 2 +- Cargo.toml | 2 +- serial-settings/Cargo.toml | 2 +- serial-settings/src/lib.rs | 190 ++++++++++++++----------------------- src/bin/dual-iir.rs | 2 + src/bin/lockin.rs | 2 + src/hardware/mod.rs | 18 ++-- src/hardware/setup.rs | 2 +- 8 files changed, 89 insertions(+), 131 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c25ab9d1b..904cbc0ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1139,7 +1139,7 @@ dependencies = [ [[package]] name = "usbd-serial" version = "0.2.0" -source = "git+https://github.com/rust-embedded-community/usbd-serial?branch=feature/embedded-io#1b2d0fc7049640761f55a71b765e63a835637151" +source = "git+https://github.com/rust-embedded-community/usbd-serial?branch=rs/bugfix#4b5c090a064c30f5e95ccc19e4e54ec7ab929b29" dependencies = [ "embedded-hal", "embedded-io 0.6.1", diff --git a/Cargo.toml b/Cargo.toml index ac3b333d3..c0a7ff65c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,7 +78,7 @@ features = ["stm32h743v", "rt", "ethernet", "xspi", "usb_hs"] [patch.crates-io.usbd-serial] git = "https://github.com/rust-embedded-community/usbd-serial" -branch = "feature/embedded-io" +branch = "rs/bugfix" [patch.crates-io.synopsys-usb-otg] git = "https://github.com/quartiq/synopsys-usb-otg" diff --git a/serial-settings/Cargo.toml b/serial-settings/Cargo.toml index b54779678..491a63b6d 100644 --- a/serial-settings/Cargo.toml +++ b/serial-settings/Cargo.toml @@ -8,7 +8,7 @@ edition = "2021" [dependencies] embedded-storage = "0.3" miniconf = "0.9" -menu = "0.4" +menu = {version = "0.4", features = ["echo"]} cortex-m = "0.7" heapless = "0.8" postcard = "1" diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 8742b051e..6845253bf 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -1,8 +1,9 @@ #![no_std] use core::fmt::Write; -use embedded_storage::nor_flash::NorFlash; -use miniconf::JsonCoreSlash; +use embedded_io::{Read, ReadReady, Write as EioWrite, WriteReady}; +use embedded_storage::nor_flash::{NorFlash, ReadNorFlash}; +use miniconf::{JsonCoreSlash, TreeKey}; use serde::{Deserialize, Serialize}; pub use menu; @@ -34,6 +35,12 @@ impl Interface for T where } pub trait Platform { + type Interface: Interface; + + type Settings: Settings; + + type Storage: NorFlash; + fn reset(&mut self, _interface: impl embedded_io::Write) {} fn dfu(&mut self, mut interface: impl embedded_io::Write) { @@ -50,19 +57,11 @@ pub trait Platform { } } -impl Platform for T {} - -pub struct Context< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, -> { - flash: Flash, +pub struct Context<'a, P: Platform> { + flash: P::Storage, platform: P, - settings: S, - interface: I, + settings: P::Settings, + interface: P::Interface, buffer: &'a mut [u8], } @@ -78,10 +77,13 @@ impl From for Error { } } -impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> - Context<'a, I, S, P, Flash> -{ - fn save(&mut self) -> Result<(), Error> { +impl<'a, P: Platform> Context<'a, P> { + fn save( + &mut self, + ) -> Result< + (), + Error<::Error>, + > { let serialized = postcard::to_slice(&self.settings, self.buffer)?; self.flash .erase(0, serialized.len() as u32) @@ -91,13 +93,7 @@ impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> } } -fn settings_menu< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, ->() -> menu::Menu<'a, Context<'a, I, S, P, Flash>> { +fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { menu::Menu { label: "settings", items: &[ @@ -175,9 +171,7 @@ fn settings_menu< } } -impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> - core::fmt::Write for Context<'a, I, S, P, Flash> -{ +impl<'a, P: Platform> core::fmt::Write for Context<'a, P> { /// Write data to the serial terminal. /// /// # Note @@ -190,24 +184,18 @@ impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> } } -fn handle_list< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, ->( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_list<'a, P: Platform>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context<'a, I, S, P, Flash>, + context: &mut Context<'a, P>, ) { writeln!(context, "Available items:").unwrap(); let mut defaults = context.settings.clone(); defaults.reset(); - for path in S::iter_paths::>("/") { + for path in P::Settings::iter_paths::>("/") { let path = path.unwrap(); let current_value = { let len = context.settings.get_json(&path, context.buffer).unwrap(); @@ -223,63 +211,39 @@ fn handle_list< } } -fn handle_reboot< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, ->( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_reboot<'a, P: Platform>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context<'a, I, S, P, Flash>, + context: &mut Context<'a, P>, ) { context.platform.reset(&mut context.interface); cortex_m::peripheral::SCB::sys_reset(); } -fn handle_dfu_reboot< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, ->( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_dfu_reboot<'a, P: Platform>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context<'a, I, S, P, Flash>, + context: &mut Context<'a, P>, ) { context.platform.dfu(&mut context.interface); } -fn handle_service< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, ->( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_service<'a, P: Platform>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context<'a, I, S, P, Flash>, + context: &mut Context<'a, P>, ) { context.platform.service(&mut context.interface); } -fn handle_reset< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, ->( - _menu: &menu::Menu>, - _item: &menu::Item>, +fn handle_reset<'a, P: Platform>( + _menu: &menu::Menu>, + _item: &menu::Item>, _args: &[&str], - context: &mut Context<'a, I, S, P, Flash>, + context: &mut Context<'a, P>, ) { context.settings.reset(); match context.save() { @@ -292,17 +256,11 @@ fn handle_reset< } } -fn handle_get< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, ->( - _menu: &menu::Menu>, - item: &menu::Item>, +fn handle_get<'a, P: Platform>( + _menu: &menu::Menu>, + item: &menu::Item>, args: &[&str], - context: &mut Context<'a, I, S, P, Flash>, + context: &mut Context<'a, P>, ) { let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); let len = match context.settings.get_json(key, context.buffer) { @@ -317,17 +275,11 @@ fn handle_get< writeln!(context.interface, "{key}: {stringified}").unwrap(); } -fn handle_set< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, ->( - _menu: &menu::Menu>, - item: &menu::Item>, +fn handle_set<'a, P: Platform>( + _menu: &menu::Menu>, + item: &menu::Item>, args: &[&str], - context: &mut Context<'a, I, S, P, Flash>, + context: &mut Context<'a, P>, ) { let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); let value = menu::argument_finder(item, args, "value").unwrap().unwrap(); @@ -354,31 +306,27 @@ Reset device to apply settings." } } -pub struct SerialSettings< - 'a, - I: Interface, - S: Settings, - P: Platform, - Flash: NorFlash + 'static, -> { - menu: menu::Runner<'a, Context<'a, I, S, P, Flash>>, +pub struct SerialSettings<'a, P: Platform> { + menu: menu::Runner<'a, Context<'a, P>>, } -impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> - SerialSettings<'a, I, S, P, Flash> -{ +impl<'a, P: Platform> SerialSettings<'a, P> { pub fn new( - interface: I, - mut flash: Flash, platform: P, - settings_callback: impl FnOnce(Option) -> S, + interface: P::Interface, + mut flash: P::Storage, + settings_callback: impl FnOnce(Option) -> P::Settings, line_buf: &'a mut [u8], serialize_buf: &'a mut [u8], - ) -> Result { + ) -> Result< + Self, + ::Error, + > { flash.read(0, &mut serialize_buf[..])?; - let settings = - settings_callback(postcard::from_bytes::(serialize_buf).ok()); + let settings = settings_callback( + postcard::from_bytes::(serialize_buf).ok(), + ); let context = Context { settings, @@ -392,19 +340,21 @@ impl<'a, I: Interface, S: Settings, P: Platform, Flash: NorFlash + 'static> Ok(Self { menu }) } - pub fn settings(&self) -> &S { + pub fn settings(&self) -> &P::Settings { &self.menu.context.settings } - pub fn interface_mut(&mut self) -> &mut I { + pub fn interface_mut(&mut self) -> &mut P::Interface { &mut self.menu.context.interface } - pub fn interface(&self) -> &I { + pub fn interface(&self) -> &P::Interface { &self.menu.context.interface } - pub fn process(&mut self) -> Result<(), I::Error> { + pub fn process( + &mut self, + ) -> Result<(), ::Error> { while self.menu.context.interface.read_ready()? { let mut buffer = [0u8; 64]; let count = self.menu.context.interface.read(&mut buffer)?; diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index 8b3418f64..c4fae7a89 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -472,6 +472,8 @@ mod app { usb.process(c.local.usb_terminal); }); + c.local.usb_terminal.process().unwrap(); + // Schedule to run this task every 10 milliseconds. usb::spawn_after(10u64.millis()).unwrap(); } diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 381de9c1f..8a220d200 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -523,6 +523,8 @@ mod app { usb.process(c.local.usb_terminal); }); + c.local.usb_terminal.process().unwrap(); + // Schedule to run this task every 10 milliseconds. usb::spawn_after(10u64.millis()).unwrap(); } diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index 6dd0ca3a9..4430a385c 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -81,13 +81,17 @@ pub type I2c1 = hal::i2c::I2c; pub type I2c1Proxy = shared_bus::I2cProxy<'static, shared_bus::AtomicCheckMutex>; -pub type SerialTerminal = serial_settings::SerialSettings< - 'static, - usbd_serial::SerialPort<'static, UsbBus>, - flash::Settings, - (), - flash::Flash, ->; +#[derive(Default)] +pub struct SerialSettingsPlatform; + +impl serial_settings::Platform for SerialSettingsPlatform { + type Interface = usbd_serial::SerialPort<'static, UsbBus>; + type Storage = flash::Flash; + type Settings = flash::Settings; +} + +pub type SerialTerminal = + serial_settings::SerialSettings<'static, SerialSettingsPlatform>; #[inline(never)] #[panic_handler] diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index bf1f2a197..0b111c51f 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -1095,9 +1095,9 @@ pub fn setup( }; serial_settings::SerialSettings::new( + super::SerialSettingsPlatform, usb_serial, super::flash::Flash(flash_bank2.unwrap()), - (), settings_callback, input_buffer, serialize_buffer, From 1eaa4b4665ffea5dd8c989846f91767b339d7f8e Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Thu, 16 Nov 2023 13:05:44 +0100 Subject: [PATCH 09/20] Simplifying traits --- serial-settings/src/lib.rs | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 6845253bf..1aeb32572 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -6,36 +6,17 @@ use embedded_storage::nor_flash::{NorFlash, ReadNorFlash}; use miniconf::{JsonCoreSlash, TreeKey}; use serde::{Deserialize, Serialize}; -pub use menu; - pub trait Settings: - for<'a> JsonCoreSlash<'a> - + Serialize - + Clone - + 'static - + for<'a> Deserialize<'a> + for<'a> JsonCoreSlash<'a> + Serialize + Clone + for<'a> Deserialize<'a> { fn reset(&mut self); } -pub trait Interface: - embedded_io::Read - + embedded_io::ReadReady - + embedded_io::Write - + embedded_io::WriteReady -{ -} - -impl Interface for T where - T: embedded_io::Read +pub trait Platform { + type Interface: embedded_io::Read + embedded_io::ReadReady + embedded_io::Write - + embedded_io::WriteReady -{ -} - -pub trait Platform { - type Interface: Interface; + + embedded_io::WriteReady; type Settings: Settings; From a8f2f4d045bd5ab59f07255d5a40fd9aa1c563f6 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Thu, 16 Nov 2023 13:22:16 +0100 Subject: [PATCH 10/20] Adding docs --- serial-settings/src/lib.rs | 97 +++++++++++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 2 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 1aeb32572..52e94e1f8 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -1,3 +1,53 @@ +//! Persistent Settings Management Serial Interface +//! +//! # Description +//! This crate provides a simple means to load, configure, and save 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. +//! +//! # Example +//! Let's assume that your settings structure looks as follows: +//! ```rust +//! #[derive(miniconf::Tree, ...)] +//! struct Settings { +//! broker: String, +//! id: String, +//! } +//! ``` +//! +//! A user would be displayed the following terminal interface: +//! ``` +//!> help +//! AVAILABLE ITEMS: +//! dfu +//! service +//! reboot +//! factory-reset +//! list +//! get +//! set +//! help [ ] +//! +//! > dfu +//! Reset to DFU is not supported on this device +//! +//! > service +//! Service data not available +//! +//! > list +//! Available items: +//! /broker: "test" [default: "mqtt"] +//! /id: "04-91-62-d2-a8-6f" [default: "04-91-62-d2-a8-6f"] +//! ``` +//! +//! # Design +//! Settings are specified in a [`Miniconf::Tree`] settings tree and are transferred over the +//! serial interface using JSON encoding. This means that things like strings must be encased in +//! qutoes. +//! +//! # Limitations +//! Currently, there is a hardcoded limit of 32-bytes on the settings path. This is arbitrary and +//! can be changed if needed. #![no_std] use core::fmt::Write; @@ -6,24 +56,43 @@ use embedded_storage::nor_flash::{NorFlash, ReadNorFlash}; use miniconf::{JsonCoreSlash, TreeKey}; use serde::{Deserialize, Serialize}; +/// 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> { - fn reset(&mut self); + /// Reset the settings to their default values. + fn reset(&mut self) {} +} + +impl Settings for T where + T: for<'a> JsonCoreSlash<'a> + Serialize + Clone + for<'a> Deserialize<'a> +{ } 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 + embedded_io::WriteReady; + /// 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; + /// This function is called immediately before a device reset is initiated. + /// + /// # Args + /// * `interface` - An object to write device informational messages to. fn reset(&mut self, _interface: impl embedded_io::Write) {} + /// This device is called when a reboot to DFU (device firmware update) mode is requested. + /// + /// # Args + /// * `interface` - An object to write device informational messages to. fn dfu(&mut self, mut interface: impl embedded_io::Write) { self.reset(&mut interface); writeln!( @@ -33,12 +102,17 @@ pub trait Platform { .ok(); } + /// This function is called to provide information about the operating device state to the + /// user. + /// + /// # Args + /// * `interface` - An object to write device status to. fn service(&mut self, mut interface: impl embedded_io::Write) { writeln!(&mut interface, "Service data not available").ok(); } } -pub struct Context<'a, P: Platform> { +struct Context<'a, P: Platform> { flash: P::Storage, platform: P, settings: P::Settings, @@ -287,11 +361,26 @@ Reset device to apply settings." } } +/// The serial settings management object. pub struct SerialSettings<'a, P: Platform> { menu: menu::Runner<'a, Context<'a, P>>, } impl<'a, P: Platform> SerialSettings<'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, interface: P::Interface, @@ -321,18 +410,22 @@ impl<'a, P: Platform> SerialSettings<'a, P> { Ok(Self { menu }) } + /// Get the current device settings. pub fn settings(&self) -> &P::Settings { &self.menu.context.settings } + /// Get the device communication interface pub fn interface_mut(&mut self) -> &mut P::Interface { &mut self.menu.context.interface } + /// Get the device communication interface pub fn interface(&self) -> &P::Interface { &self.menu.context.interface } + /// Must be called periodically to process user input. pub fn process( &mut self, ) -> Result<(), ::Error> { From 05847bad3f5e261066eb0671f7562796ed49e195 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Thu, 16 Nov 2023 13:54:32 +0100 Subject: [PATCH 11/20] Removing dead code --- serial-settings/src/lib.rs | 5 ----- src/hardware/flash.rs | 42 +------------------------------------- 2 files changed, 1 insertion(+), 46 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 52e94e1f8..5e1256358 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -65,11 +65,6 @@ pub trait Settings: fn reset(&mut self) {} } -impl Settings for T where - T: for<'a> JsonCoreSlash<'a> + Serialize + Clone + for<'a> Deserialize<'a> -{ -} - pub trait Platform { /// This type specifies the interface to the user, for example, a USB CDC-ACM serial port. type Interface: embedded_io::Read diff --git a/src/hardware/flash.rs b/src/hardware/flash.rs index 12232a9ac..674e588c9 100644 --- a/src/hardware/flash.rs +++ b/src/hardware/flash.rs @@ -1,6 +1,5 @@ use core::fmt::Write; -use embedded_storage::nor_flash::{NorFlash, ReadNorFlash}; -use stm32h7xx_hal::flash::{LockedFlashBank, UnlockedFlashBank}; +use stm32h7xx_hal::flash::LockedFlashBank; #[derive(Clone, serde::Serialize, serde::Deserialize, miniconf::Tree)] pub struct Settings { @@ -69,42 +68,3 @@ impl embedded_storage::nor_flash::NorFlash for Flash { bank.write(offset, bytes) } } - -pub struct FlashSettings { - flash: LockedFlashBank, - pub settings: Settings, -} - -impl FlashSettings { - pub fn new( - mut flash: LockedFlashBank, - mac: smoltcp_nal::smoltcp::wire::EthernetAddress, - ) -> Self { - let mut buffer = [0u8; 512]; - flash.read(0, &mut buffer[..]).unwrap(); - - let settings = match postcard::from_bytes::(&buffer) { - Ok(mut settings) => { - settings.mac = mac; - settings - } - Err(_) => { - log::warn!( - "Failed to load settings from flash. Using defaults" - ); - Settings::new(mac) - } - }; - - Self { flash, settings } - } - - pub fn save(&mut self) { - let mut bank = self.flash.unlocked(); - - let mut data = [0; 512]; - let serialized = postcard::to_slice(&self.settings, &mut data).unwrap(); - bank.erase(0, UnlockedFlashBank::ERASE_SIZE as u32).unwrap(); - bank.write(0, serialized).unwrap(); - } -} From df20519f78010c7d30df130f91ba874e1e6404c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Thu, 16 Nov 2023 14:30:18 +0100 Subject: [PATCH 12/20] move handlers into Context --- serial-settings/src/lib.rs | 269 +++++++++++++++++++------------------ 1 file changed, 136 insertions(+), 133 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 5e1256358..5f74ca822 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -141,17 +141,141 @@ impl<'a, P: Platform> Context<'a, P> { self.flash.write(0, serialized).map_err(Error::Flash)?; Ok(()) } -} -fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { - menu::Menu { + fn handle_reboot( + _menu: &menu::Menu, + _item: &menu::Item, + _args: &[&str], + context: &mut Self, + ) { + context.platform.reset(&mut context.interface); + cortex_m::peripheral::SCB::sys_reset(); + } + + fn handle_list( + _menu: &menu::Menu, + _item: &menu::Item, + _args: &[&str], + context: &mut Self, + ) { + writeln!(context, "Available items:").unwrap(); + + let mut defaults = context.settings.clone(); + defaults.reset(); + + for path in P::Settings::iter_paths::>("/") { + let path = path.unwrap(); + let current_value = { + let len = + context.settings.get_json(&path, context.buffer).unwrap(); + core::str::from_utf8(&context.buffer[..len]).unwrap() + }; + write!(context.interface, "{path}: {current_value}").unwrap(); + + let default_value = { + let len = defaults.get_json(&path, context.buffer).unwrap(); + core::str::from_utf8(&context.buffer[..len]).unwrap() + }; + writeln!(context.interface, " [default: {default_value}]").unwrap() + } + } + + fn handle_dfu( + _menu: &menu::Menu, + _item: &menu::Item, + _args: &[&str], + context: &mut Self, + ) { + context.platform.dfu(&mut context.interface); + } + + fn handle_service( + _menu: &menu::Menu, + _item: &menu::Item, + _args: &[&str], + context: &mut Self, + ) { + context.platform.service(&mut context.interface); + } + + fn handle_factory_reset( + _menu: &menu::Menu, + _item: &menu::Item, + _args: &[&str], + context: &mut Self, + ) { + context.settings.reset(); + match context.save() { + Ok(_) => { + writeln!(context, "Settings reset to default").unwrap(); + } + Err(e) => { + writeln!(context, "Failed to reset settings: {e:?}").unwrap(); + } + } + } + + fn handle_get( + _menu: &menu::Menu, + item: &menu::Item, + args: &[&str], + context: &mut Self, + ) { + let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); + let len = match context.settings.get_json(key, context.buffer) { + Err(e) => { + writeln!(context, "Failed to read {key}: {e}").unwrap(); + return; + } + Ok(len) => len, + }; + + let stringified = core::str::from_utf8(&context.buffer[..len]).unwrap(); + writeln!(context.interface, "{key}: {stringified}").unwrap(); + } + + fn handle_set( + _menu: &menu::Menu, + item: &menu::Item, + args: &[&str], + context: &mut Self, + ) { + let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); + let value = + menu::argument_finder(item, args, "value").unwrap().unwrap(); + + // Now, write the new value into memory. + // TODO: Validate it first? + match context.settings.set_json(key, value.as_bytes()) { + Ok(_) => match context.save() { + Ok(_) => { + writeln!( + context, + "Settings in memory may differ from currently operating settings. \ + Reset device to apply settings." + ) + .unwrap(); + } + Err(e) => { + writeln!(context, "Failed to save settings: {e:?}") + .unwrap(); + } + }, + Err(e) => { + writeln!(context, "Failed to update {key}: {e:?}").unwrap(); + } + } + } + + fn menu() -> menu::Menu<'a, Self> { + menu::Menu { label: "settings", items: &[ &menu::Item { command: "dfu", help: Some("Reboot the device to DFU mode"), item_type: menu::ItemType::Callback { - function: handle_dfu_reboot, + function: Self::handle_dfu, parameters: &[] }, }, @@ -159,7 +283,7 @@ fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { command: "service", help: Some("Read device service information"), item_type: menu::ItemType::Callback { - function: handle_service, + function: Self::handle_service, parameters: &[] }, }, @@ -167,7 +291,7 @@ fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { command: "reboot", help: Some("Reboot the device to force new settings to take effect."), item_type: menu::ItemType::Callback { - function: handle_reboot, + function: Self::handle_reboot, parameters: &[] }, }, @@ -175,7 +299,7 @@ fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { command: "factory-reset", help: Some("Reset the device settings to default values."), item_type: menu::ItemType::Callback { - function: handle_reset, + function: Self::handle_factory_reset, parameters: &[] }, }, @@ -183,7 +307,7 @@ fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { command: "list", help: Some("List all available settings and their current values."), item_type: menu::ItemType::Callback { - function: handle_list, + function: Self::handle_list, parameters: &[], }, }, @@ -191,7 +315,7 @@ fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { command: "get", help: Some("Read a setting_from the device."), item_type: menu::ItemType::Callback { - function: handle_get, + function: Self::handle_get, parameters: &[menu::Parameter::Mandatory { parameter_name: "item", help: Some("The name of the setting to read."), @@ -202,7 +326,7 @@ fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { command: "set", help: Some("Update a a setting in the device."), item_type: menu::ItemType::Callback { - function: handle_set, + function: Self::handle_set, parameters: &[ menu::Parameter::Mandatory { parameter_name: "item", @@ -219,6 +343,7 @@ fn settings_menu<'a, P: Platform>() -> menu::Menu<'a, Context<'a, P>> { entry: None, exit: None, } + } } impl<'a, P: Platform> core::fmt::Write for Context<'a, P> { @@ -234,128 +359,6 @@ impl<'a, P: Platform> core::fmt::Write for Context<'a, P> { } } -fn handle_list<'a, P: Platform>( - _menu: &menu::Menu>, - _item: &menu::Item>, - _args: &[&str], - context: &mut Context<'a, P>, -) { - writeln!(context, "Available items:").unwrap(); - - let mut defaults = context.settings.clone(); - defaults.reset(); - - for path in P::Settings::iter_paths::>("/") { - let path = path.unwrap(); - let current_value = { - let len = context.settings.get_json(&path, context.buffer).unwrap(); - core::str::from_utf8(&context.buffer[..len]).unwrap() - }; - write!(context.interface, "{path}: {current_value}").unwrap(); - - let default_value = { - let len = defaults.get_json(&path, context.buffer).unwrap(); - core::str::from_utf8(&context.buffer[..len]).unwrap() - }; - writeln!(context.interface, " [default: {default_value}]").unwrap() - } -} - -fn handle_reboot<'a, P: Platform>( - _menu: &menu::Menu>, - _item: &menu::Item>, - _args: &[&str], - context: &mut Context<'a, P>, -) { - context.platform.reset(&mut context.interface); - cortex_m::peripheral::SCB::sys_reset(); -} - -fn handle_dfu_reboot<'a, P: Platform>( - _menu: &menu::Menu>, - _item: &menu::Item>, - _args: &[&str], - context: &mut Context<'a, P>, -) { - context.platform.dfu(&mut context.interface); -} - -fn handle_service<'a, P: Platform>( - _menu: &menu::Menu>, - _item: &menu::Item>, - _args: &[&str], - context: &mut Context<'a, P>, -) { - context.platform.service(&mut context.interface); -} - -fn handle_reset<'a, P: Platform>( - _menu: &menu::Menu>, - _item: &menu::Item>, - _args: &[&str], - context: &mut Context<'a, P>, -) { - context.settings.reset(); - match context.save() { - Ok(_) => { - writeln!(context, "Settings reset to default").unwrap(); - } - Err(e) => { - writeln!(context, "Failed to reset settings: {e:?}").unwrap(); - } - } -} - -fn handle_get<'a, P: Platform>( - _menu: &menu::Menu>, - item: &menu::Item>, - args: &[&str], - context: &mut Context<'a, P>, -) { - let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); - let len = match context.settings.get_json(key, context.buffer) { - Err(e) => { - writeln!(context, "Failed to read {key}: {e}").unwrap(); - return; - } - Ok(len) => len, - }; - - let stringified = core::str::from_utf8(&context.buffer[..len]).unwrap(); - writeln!(context.interface, "{key}: {stringified}").unwrap(); -} - -fn handle_set<'a, P: Platform>( - _menu: &menu::Menu>, - item: &menu::Item>, - args: &[&str], - context: &mut Context<'a, P>, -) { - let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); - let value = menu::argument_finder(item, args, "value").unwrap().unwrap(); - - // Now, write the new value into memory. - // TODO: Validate it first? - match context.settings.set_json(key, value.as_bytes()) { - Ok(_) => match context.save() { - Ok(_) => { - writeln!( - context, - "Settings in memory may differ from currently operating settings. \ -Reset device to apply settings." - ) - .unwrap(); - } - Err(e) => { - writeln!(context, "Failed to save settings: {e:?}").unwrap(); - } - }, - Err(e) => { - writeln!(context, "Failed to update {key}: {e:?}").unwrap(); - } - } -} - /// The serial settings management object. pub struct SerialSettings<'a, P: Platform> { menu: menu::Runner<'a, Context<'a, P>>, @@ -401,7 +404,7 @@ impl<'a, P: Platform> SerialSettings<'a, P> { buffer: serialize_buf, }; - let menu = menu::Runner::new(settings_menu(), line_buf, context); + let menu = menu::Runner::new(Context::menu(), line_buf, context); Ok(Self { menu }) } From 3679c30c289733d3bde025bf47f630927005d159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Thu, 16 Nov 2023 16:59:57 +0100 Subject: [PATCH 13/20] own Storage,Settings,Interface in platform --- serial-settings/src/lib.rs | 231 ++++++++++++++++--------------------- src/hardware/mod.rs | 20 +++- src/hardware/setup.rs | 33 +++--- 3 files changed, 133 insertions(+), 151 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 5f74ca822..c6f817c1b 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -19,23 +19,21 @@ //! ``` //!> help //! AVAILABLE ITEMS: -//! dfu -//! service -//! reboot +//! platform //! factory-reset //! list //! get //! set //! help [ ] //! -//! > dfu -//! Reset to DFU is not supported on this device +//! > plaform dfu +//! Reset to DFU is not supported //! -//! > service +//! > plaform service //! Service data not available //! //! > list -//! Available items: +//! Available settings: //! /broker: "test" [default: "mqtt"] //! /id: "04-91-62-d2-a8-6f" [default: "04-91-62-d2-a8-6f"] //! ``` @@ -52,10 +50,22 @@ use core::fmt::Write; use embedded_io::{Read, ReadReady, Write as EioWrite, WriteReady}; -use embedded_storage::nor_flash::{NorFlash, ReadNorFlash}; +use embedded_storage::nor_flash::NorFlash; use miniconf::{JsonCoreSlash, TreeKey}; use serde::{Deserialize, Serialize}; +#[derive(Debug)] +enum Error { + Postcard(postcard::Error), + Flash(F), +} + +impl From for Error { + 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: @@ -65,7 +75,7 @@ pub trait Settings: fn reset(&mut self) {} } -pub trait Platform { +pub trait Platform: Sized { /// This type specifies the interface to the user, for example, a USB CDC-ACM serial port. type Interface: embedded_io::Read + embedded_io::ReadReady @@ -78,55 +88,39 @@ pub trait Platform { /// Specifies the type of storage used for persisting settings between device boots. type Storage: NorFlash; - /// This function is called immediately before a device reset is initiated. - /// - /// # Args - /// * `interface` - An object to write device informational messages to. - fn reset(&mut self, _interface: impl embedded_io::Write) {} - - /// This device is called when a reboot to DFU (device firmware update) mode is requested. - /// - /// # Args - /// * `interface` - An object to write device informational messages to. - fn dfu(&mut self, mut interface: impl embedded_io::Write) { - self.reset(&mut interface); - writeln!( - &mut interface, - "Reset to DFU is not supported on this device" - ) + /// 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(); } - /// This function is called to provide information about the operating device state to the - /// user. - /// - /// # Args - /// * `interface` - An object to write device status to. - fn service(&mut self, mut interface: impl embedded_io::Write) { - writeln!(&mut interface, "Service data not available").ok(); - } + /// 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; } struct Context<'a, P: Platform> { - flash: P::Storage, platform: P, - settings: P::Settings, - interface: P::Interface, buffer: &'a mut [u8], } -#[derive(Debug)] -enum Error { - Postcard(postcard::Error), - Flash(F), -} - -impl From for Error { - fn from(e: postcard::Error) -> Self { - Self::Postcard(e) - } -} - impl<'a, P: Platform> Context<'a, P> { fn save( &mut self, @@ -134,22 +128,27 @@ impl<'a, P: Platform> Context<'a, P> { (), Error<::Error>, > { - let serialized = postcard::to_slice(&self.settings, self.buffer)?; - self.flash + 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.flash.write(0, serialized).map_err(Error::Flash)?; + self.platform + .storage_mut() + .write(0, serialized) + .map_err(Error::Flash)?; Ok(()) } - fn handle_reboot( + fn handle_platform( _menu: &menu::Menu, - _item: &menu::Item, - _args: &[&str], + item: &menu::Item, + args: &[&str], context: &mut Self, ) { - context.platform.reset(&mut context.interface); - cortex_m::peripheral::SCB::sys_reset(); + let key = menu::argument_finder(item, args, "cmd").unwrap().unwrap(); + context.platform.cmd(key); } fn handle_list( @@ -158,53 +157,43 @@ impl<'a, P: Platform> Context<'a, P> { _args: &[&str], context: &mut Self, ) { - writeln!(context, "Available items:").unwrap(); + writeln!(context, "Available settings:").unwrap(); - let mut defaults = context.settings.clone(); + let mut defaults = context.platform.settings().clone(); defaults.reset(); for path in P::Settings::iter_paths::>("/") { let path = path.unwrap(); let current_value = { - let len = - context.settings.get_json(&path, context.buffer).unwrap(); + let len = context + .platform + .settings() + .get_json(&path, context.buffer) + .unwrap(); core::str::from_utf8(&context.buffer[..len]).unwrap() }; - write!(context.interface, "{path}: {current_value}").unwrap(); + write!(context.platform.interface_mut(), "{path}: {current_value}") + .unwrap(); let default_value = { let len = defaults.get_json(&path, context.buffer).unwrap(); core::str::from_utf8(&context.buffer[..len]).unwrap() }; - writeln!(context.interface, " [default: {default_value}]").unwrap() + writeln!( + context.platform.interface_mut(), + " [default: {default_value}]" + ) + .unwrap() } } - fn handle_dfu( - _menu: &menu::Menu, - _item: &menu::Item, - _args: &[&str], - context: &mut Self, - ) { - context.platform.dfu(&mut context.interface); - } - - fn handle_service( - _menu: &menu::Menu, - _item: &menu::Item, - _args: &[&str], - context: &mut Self, - ) { - context.platform.service(&mut context.interface); - } - fn handle_factory_reset( _menu: &menu::Menu, _item: &menu::Item, _args: &[&str], context: &mut Self, ) { - context.settings.reset(); + context.platform.settings_mut().reset(); match context.save() { Ok(_) => { writeln!(context, "Settings reset to default").unwrap(); @@ -222,16 +211,18 @@ impl<'a, P: Platform> Context<'a, P> { context: &mut Self, ) { let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); - let len = match context.settings.get_json(key, context.buffer) { - Err(e) => { - writeln!(context, "Failed to read {key}: {e}").unwrap(); - return; - } - Ok(len) => len, - }; + let len = + match context.platform.settings().get_json(key, context.buffer) { + Err(e) => { + writeln!(context, "Failed to read {key}: {e}").unwrap(); + return; + } + Ok(len) => len, + }; let stringified = core::str::from_utf8(&context.buffer[..len]).unwrap(); - writeln!(context.interface, "{key}: {stringified}").unwrap(); + writeln!(context.platform.interface_mut(), "{key}: {stringified}") + .unwrap(); } fn handle_set( @@ -246,7 +237,11 @@ impl<'a, P: Platform> Context<'a, P> { // Now, write the new value into memory. // TODO: Validate it first? - match context.settings.set_json(key, value.as_bytes()) { + match context + .platform + .settings_mut() + .set_json(key, value.as_bytes()) + { Ok(_) => match context.save() { Ok(_) => { writeln!( @@ -272,27 +267,14 @@ impl<'a, P: Platform> Context<'a, P> { label: "settings", items: &[ &menu::Item { - command: "dfu", - help: Some("Reboot the device to DFU mode"), - item_type: menu::ItemType::Callback { - function: Self::handle_dfu, - parameters: &[] - }, - }, - &menu::Item { - command: "service", - help: Some("Read device service information"), + command: "platform", + help: Some("Platform specific command"), item_type: menu::ItemType::Callback { - function: Self::handle_service, - parameters: &[] - }, - }, - &menu::Item { - command: "reboot", - help: Some("Reboot the device to force new settings to take effect."), - item_type: menu::ItemType::Callback { - function: Self::handle_reboot, - parameters: &[] + function: Self::handle_platform, + parameters: &[menu::Parameter::Mandatory { + parameter_name: "cmd", + help: Some("The name of the command (e.g. `reboot`, `service`, `dfu`)."), + }] }, }, &menu::Item { @@ -352,8 +334,8 @@ impl<'a, P: Platform> core::fmt::Write for Context<'a, P> { /// # Note /// The terminal uses an internal buffer. Overflows of the output buffer are silently ignored. fn write_str(&mut self, s: &str) -> core::fmt::Result { - if let Ok(true) = self.interface.write_ready() { - self.interface.write_all(s.as_bytes()).ok(); + if let Ok(true) = self.platform.interface_mut().write_ready() { + self.platform.interface_mut().write_all(s.as_bytes()).ok(); } Ok(()) } @@ -381,25 +363,13 @@ impl<'a, P: Platform> SerialSettings<'a, P> { /// needs to be at least as big as the entire serialized settings structure. pub fn new( platform: P, - interface: P::Interface, - mut flash: P::Storage, - settings_callback: impl FnOnce(Option) -> P::Settings, line_buf: &'a mut [u8], serialize_buf: &'a mut [u8], ) -> Result< Self, ::Error, > { - flash.read(0, &mut serialize_buf[..])?; - - let settings = settings_callback( - postcard::from_bytes::(serialize_buf).ok(), - ); - let context = Context { - settings, - interface, - flash, platform, buffer: serialize_buf, }; @@ -410,26 +380,21 @@ impl<'a, P: Platform> SerialSettings<'a, P> { /// Get the current device settings. pub fn settings(&self) -> &P::Settings { - &self.menu.context.settings + self.menu.context.platform.settings() } /// Get the device communication interface pub fn interface_mut(&mut self) -> &mut P::Interface { - &mut self.menu.context.interface - } - - /// Get the device communication interface - pub fn interface(&self) -> &P::Interface { - &self.menu.context.interface + self.menu.context.platform.interface_mut() } /// Must be called periodically to process user input. pub fn process( &mut self, ) -> Result<(), ::Error> { - while self.menu.context.interface.read_ready()? { + while self.interface_mut().read_ready()? { let mut buffer = [0u8; 64]; - let count = self.menu.context.interface.read(&mut buffer)?; + let count = self.interface_mut().read(&mut buffer)?; for &value in &buffer[..count] { self.menu.input_byte(value); } diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index 4430a385c..743cc7694 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -81,13 +81,29 @@ pub type I2c1 = hal::i2c::I2c; pub type I2c1Proxy = shared_bus::I2cProxy<'static, shared_bus::AtomicCheckMutex>; -#[derive(Default)] -pub struct SerialSettingsPlatform; +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 = diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index 0b111c51f..9720e3e7d 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -3,6 +3,7 @@ //! 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}, @@ -1075,30 +1076,30 @@ 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(); - let settings_callback = - |maybe_settings: Option| { - match maybe_settings { - Some(mut settings) => { - settings.mac = network_devices.mac_address; - settings - } - None => { - super::flash::Settings::new(network_devices.mac_address) - } - } - }; + flash_bank2.read(0, &mut serialize_buffer[..]).unwrap(); + let settings = match postcard::from_bytes::( + 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, - usb_serial, - super::flash::Flash(flash_bank2.unwrap()), - settings_callback, + super::SerialSettingsPlatform { + interface: usb_serial, + settings, + storage: super::flash::Flash(flash_bank2), + }, input_buffer, serialize_buffer, ) From d5dc94bdea03d0f661c8873e1ed37d22d987792e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Thu, 16 Nov 2023 17:46:07 +0100 Subject: [PATCH 14/20] platform commands, make serial-settings lighter * remove hardware/platform specific logic into Platform * reduce dependencies and hardware stuff from serial-settings --- Cargo.lock | 4 -- serial-settings/Cargo.toml | 4 -- serial-settings/src/lib.rs | 108 +++++++++---------------------------- src/hardware/flash.rs | 76 ++++++++++++++++++++++++++ src/hardware/mod.rs | 27 +--------- src/hardware/setup.rs | 23 +++----- 6 files changed, 108 insertions(+), 134 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 904cbc0ee..f0f1cf028 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -892,14 +892,10 @@ dependencies = [ name = "serial-settings" version = "0.1.0" dependencies = [ - "cortex-m 0.7.7", "embedded-io 0.6.1", - "embedded-storage", "heapless 0.8.0", "menu", "miniconf", - "postcard", - "serde", ] [[package]] diff --git a/serial-settings/Cargo.toml b/serial-settings/Cargo.toml index 491a63b6d..a88ac82a3 100644 --- a/serial-settings/Cargo.toml +++ b/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 } diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index c6f817c1b..20496391c 100644 --- a/serial-settings/src/lib.rs +++ b/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 { - Postcard(postcard::Error), - Flash(F), -} - -impl From for Error { - 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<::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, item: &menu::Item, @@ -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, - ::Error, - > { - let context = Context { - platform, - buffer: serialize_buf, - }; - - let menu = menu::Runner::new(Context::menu(), line_buf, context); - Ok(Self { menu }) + ) -> Result { + 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); } } diff --git a/src/hardware/flash.rs b/src/hardware/flash.rs index 674e588c9..c8d1b6c7a 100644 --- a/src/hardware/flash.rs +++ b/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 { + 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: 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<::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::(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 + } +} diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index 743cc7694..a6634f762 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -81,33 +81,8 @@ pub type I2c1 = hal::i2c::I2c; pub type I2c1Proxy = shared_bus::I2cProxy<'static, shared_bus::AtomicCheckMutex>; -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] diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index 9720e3e7d..d58ac5169 100644 --- a/src/hardware/setup.rs +++ b/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::( - 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, From 46f3a9cabfa98597319f88c113d1c8521085933c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Thu, 16 Nov 2023 17:54:54 +0100 Subject: [PATCH 15/20] factory-reset -> clear, reorder --- serial-settings/src/lib.rs | 66 ++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 20496391c..380b713b4 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -19,11 +19,11 @@ //! ``` //!> help //! AVAILABLE ITEMS: -//! platform -//! factory-reset //! list //! get //! set +//! clear +//! platform //! help [ ] //! //! > plaform dfu @@ -69,20 +69,26 @@ pub trait Platform: Sized { /// Specifies the settings that are used on the device. type Settings: Settings; + /// `load()`/`save()` Error type type Error: core::fmt::Debug; + /// 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>; + /// Execute a platform specific command. 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; - /// 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> { @@ -137,7 +143,7 @@ impl<'a, P: Platform> Context<'a, P> { } } - fn handle_factory_reset( + fn handle_clear( _menu: &menu::Menu, _item: &menu::Item, _args: &[&str], @@ -146,10 +152,10 @@ impl<'a, P: Platform> Context<'a, P> { context.platform.settings_mut().reset(); match context.platform.save(context.buffer) { Ok(_) => { - writeln!(context, "Settings reset to default").unwrap(); + writeln!(context, "Settings cleared to defaults").unwrap(); } Err(e) => { - writeln!(context, "Failed to reset settings: {e:?}").unwrap(); + writeln!(context, "Failed to clear settings: {e:?}").unwrap(); } } } @@ -196,8 +202,7 @@ impl<'a, P: Platform> Context<'a, P> { Ok(_) => { writeln!( context, - "Settings in memory may differ from currently operating settings. \ - Reset device to apply settings." + "Settings saved. Reboot device (`platform reboot`) to apply." ) .unwrap(); } @@ -216,25 +221,6 @@ impl<'a, P: Platform> Context<'a, P> { menu::Menu { label: "settings", items: &[ - &menu::Item { - command: "platform", - help: Some("Platform specific command"), - item_type: menu::ItemType::Callback { - function: Self::handle_platform, - parameters: &[menu::Parameter::Mandatory { - parameter_name: "cmd", - help: Some("The name of the command (e.g. `reboot`, `service`, `dfu`)."), - }] - }, - }, - &menu::Item { - command: "factory-reset", - help: Some("Reset the device settings to default values."), - item_type: menu::ItemType::Callback { - function: Self::handle_factory_reset, - parameters: &[] - }, - }, &menu::Item { command: "list", help: Some("List all available settings and their current values."), @@ -271,6 +257,25 @@ impl<'a, P: Platform> Context<'a, P> { ] }, }, + &menu::Item { + command: "clear", + help: Some("Clear the device settings to default values."), + item_type: menu::ItemType::Callback { + function: Self::handle_clear, + parameters: &[] + }, + }, + &menu::Item { + command: "platform", + help: Some("Platform specific commands"), + item_type: menu::ItemType::Callback { + function: Self::handle_platform, + parameters: &[menu::Parameter::Mandatory { + parameter_name: "cmd", + help: Some("The name of the command (e.g. `reboot`, `service`, `dfu`)."), + }] + }, + }, ], entry: None, exit: None, @@ -341,7 +346,6 @@ impl<'a, P: Platform> Runner<'a, P> { self.0.input_byte(value); } } - Ok(()) } } From 1bdf22a02cdbc6fef97e8c26ba636752ab8129d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 17 Nov 2023 13:53:19 +0100 Subject: [PATCH 16/20] reduce unwraps --- serial-settings/src/lib.rs | 103 +++++++++++++++++++++++-------------- 1 file changed, 63 insertions(+), 40 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 380b713b4..bb8636bf5 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -104,7 +104,7 @@ impl<'a, P: Platform> Context<'a, P> { context: &mut Self, ) { let key = menu::argument_finder(item, args, "cmd").unwrap().unwrap(); - context.platform.cmd(key); + context.platform.cmd(key) } fn handle_list( @@ -113,32 +113,52 @@ impl<'a, P: Platform> Context<'a, P> { _args: &[&str], context: &mut Self, ) { - writeln!(context, "Available settings:").unwrap(); - let mut defaults = context.platform.settings().clone(); defaults.reset(); for path in P::Settings::iter_paths::>("/") { - let path = path.unwrap(); - let current_value = { - let len = context - .platform - .settings() - .get_json(&path, context.buffer) - .unwrap(); - core::str::from_utf8(&context.buffer[..len]).unwrap() - }; - write!(context.platform.interface_mut(), "{path}: {current_value}") - .unwrap(); + match path { + Err(e) => writeln!( + context.platform.interface_mut(), + "Failed to get path: {e}" + ), + Ok(path) => { + match context + .platform + .settings() + .get_json(&path, context.buffer) + { + Err(e) => { + writeln!( + context.platform.interface_mut(), + "Failed to read {path}: {e}" + ) + .unwrap(); + continue; + } + Ok(len) => write!( + context.platform.interface_mut(), + "{path}: {}", + core::str::from_utf8(&context.buffer[..len]) + .unwrap() + ) + .unwrap(), + } - let default_value = { - let len = defaults.get_json(&path, context.buffer).unwrap(); - core::str::from_utf8(&context.buffer[..len]).unwrap() - }; - writeln!( - context.platform.interface_mut(), - " [default: {default_value}]" - ) + match defaults.get_json(&path, context.buffer) { + Err(e) => writeln!( + context.platform.interface_mut(), + "[default serialization error: {e}]" + ), + Ok(len) => writeln!( + context.platform.interface_mut(), + " [default: {}]", + core::str::from_utf8(&context.buffer[..len]) + .unwrap() + ), + } + } + } .unwrap() } } @@ -152,12 +172,13 @@ impl<'a, P: Platform> Context<'a, P> { context.platform.settings_mut().reset(); match context.platform.save(context.buffer) { Ok(_) => { - writeln!(context, "Settings cleared to defaults").unwrap(); + writeln!(context, "Settings cleared to defaults and saved.") } Err(e) => { - writeln!(context, "Failed to clear settings: {e:?}").unwrap(); + writeln!(context, "Failed to clear settings: {e:?}") } } + .unwrap(); } fn handle_get( @@ -167,18 +188,22 @@ impl<'a, P: Platform> Context<'a, P> { context: &mut Self, ) { let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); - let len = - match context.platform.settings().get_json(key, context.buffer) { - Err(e) => { - writeln!(context, "Failed to read {key}: {e}").unwrap(); - return; - } - Ok(len) => len, - }; - - let stringified = core::str::from_utf8(&context.buffer[..len]).unwrap(); - writeln!(context.platform.interface_mut(), "{key}: {stringified}") - .unwrap(); + match context.platform.settings().get_json(key, context.buffer) { + Err(e) => { + writeln!( + context.platform.interface_mut(), + "Failed to read {key}: {e}" + ) + } + Ok(len) => { + writeln!( + context.platform.interface_mut(), + "{key}: {}", + core::str::from_utf8(&context.buffer[..len]).unwrap() + ) + } + } + .unwrap(); } fn handle_set( @@ -204,17 +229,15 @@ impl<'a, P: Platform> Context<'a, P> { context, "Settings saved. Reboot device (`platform reboot`) to apply." ) - .unwrap(); } Err(e) => { writeln!(context, "Failed to save settings: {e:?}") - .unwrap(); } }, Err(e) => { - writeln!(context, "Failed to update {key}: {e:?}").unwrap(); + writeln!(context, "Failed to update {key}: {e:?}") } - } + }.unwrap(); } fn menu() -> menu::Menu<'a, Self> { From 2f41b1d80f38391fb8831eceedc216e2a55a89b8 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 20 Nov 2023 11:00:19 +0100 Subject: [PATCH 17/20] Simplifying code --- Cargo.lock | 2 +- Cargo.toml | 1 - src/bin/dual-iir.rs | 8 ++++---- src/bin/lockin.rs | 8 ++++---- src/hardware/mod.rs | 4 +++- src/hardware/setup.rs | 8 ++++---- src/hardware/usb.rs | 22 ---------------------- 7 files changed, 16 insertions(+), 37 deletions(-) delete mode 100644 src/hardware/usb.rs diff --git a/Cargo.lock b/Cargo.lock index f0f1cf028..4333a3106 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1135,7 +1135,7 @@ dependencies = [ [[package]] name = "usbd-serial" version = "0.2.0" -source = "git+https://github.com/rust-embedded-community/usbd-serial?branch=rs/bugfix#4b5c090a064c30f5e95ccc19e4e54ec7ab929b29" +source = "git+https://github.com/rust-embedded-community/usbd-serial#6611dc092822c60b4482e28aab2fe084c1756410" dependencies = [ "embedded-hal", "embedded-io 0.6.1", diff --git a/Cargo.toml b/Cargo.toml index c0a7ff65c..b3188a600 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,7 +78,6 @@ features = ["stm32h743v", "rt", "ethernet", "xspi", "usb_hs"] [patch.crates-io.usbd-serial] git = "https://github.com/rust-embedded-community/usbd-serial" -branch = "rs/bugfix" [patch.crates-io.synopsys-usb-otg] git = "https://github.com/quartiq/synopsys-usb-otg" diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index c4fae7a89..0ad14fd12 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -46,7 +46,7 @@ use stabilizer::{ hal, signal_generator::{self, SignalGenerator}, timers::SamplingTimer, - usb::UsbDevice, + UsbDevice, DigitalInput0, DigitalInput1, SerialTerminal, SystemTimer, Systick, AFE0, AFE1, }, @@ -233,7 +233,7 @@ mod app { let settings = Settings::default(); let shared = Shared { - usb: stabilizer.usb_device, + usb: stabilizer.usb, network, settings, telemetry: TelemetryBuffer::default(), @@ -406,7 +406,7 @@ mod app { NetworkState::Updated => {} NetworkState::NoChange => { // We can't sleep if USB is not in suspend. - if c.shared.usb.lock(|usb| usb.usb_is_suspended()) { + if c.shared.usb.lock(|usb| usb.state() == usb_device::device::UsbDeviceState::Suspend) { cortex_m::asm::wfi(); } } @@ -469,7 +469,7 @@ mod app { fn usb(mut c: usb::Context) { // Handle the USB serial terminal. c.shared.usb.lock(|usb| { - usb.process(c.local.usb_terminal); + usb.poll(&mut [c.local.usb_terminal.interface_mut()]); }); c.local.usb_terminal.process().unwrap(); diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 8a220d200..575ecb669 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -49,7 +49,7 @@ use stabilizer::{ input_stamper::InputStamper, signal_generator, timers::SamplingTimer, - usb::UsbDevice, + UsbDevice, DigitalInput0, DigitalInput1, SerialTerminal, SystemTimer, Systick, AFE0, AFE1, }, @@ -272,7 +272,7 @@ mod app { let shared = Shared { network, - usb: stabilizer.usb_device, + usb: stabilizer.usb, telemetry: TelemetryBuffer::default(), settings: Settings::default(), }; @@ -468,7 +468,7 @@ mod app { NetworkState::Updated => {} NetworkState::NoChange => { // We can't sleep if USB is not in suspend. - if c.shared.usb.lock(|usb| usb.usb_is_suspended()) { + if c.shared.usb.lock(|usb| usb.state() == usb_device::device::UsbDeviceState::Suspend) { cortex_m::asm::wfi(); } } @@ -520,7 +520,7 @@ mod app { fn usb(mut c: usb::Context) { // Handle the USB serial terminal. c.shared.usb.lock(|usb| { - usb.process(c.local.usb_terminal); + usb.poll(&mut [c.local.usb_terminal.interface_mut()]); }); c.local.usb_terminal.process().unwrap(); diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index a6634f762..bc51a9fef 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -16,7 +16,6 @@ pub mod setup; pub mod shared_adc; pub mod signal_generator; pub mod timers; -pub mod usb; mod eeprom; @@ -34,6 +33,9 @@ pub type AFE1 = afe::ProgrammableGainAmplifier< pub type UsbBus = stm32h7xx_hal::usb_hs::UsbBus; +// Type alias for the USB device. +pub type UsbDevice = usb_device::device::UsbDevice<'static, UsbBus>; + // Type alias for digital input 0 (DI0). pub type DigitalInput0 = hal::gpio::gpiog::PG9; diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index d58ac5169..66fe931a0 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -16,7 +16,7 @@ use super::{ adc, afe, cpu_temp_sensor::CpuTempSensor, dac, delay, design_parameters, eeprom, input_stamper::InputStamper, pounder, pounder::dds_output::DdsOutput, shared_adc::SharedAdc, timers, - usb::UsbDevice, DigitalInput0, DigitalInput1, EemDigitalInput0, + UsbDevice, DigitalInput0, DigitalInput1, EemDigitalInput0, EemDigitalInput1, EemDigitalOutput0, EemDigitalOutput1, EthernetPhy, NetworkStack, SerialTerminal, SystemTimer, Systick, UsbBus, AFE0, AFE1, }; @@ -118,7 +118,7 @@ pub struct StabilizerDevices { pub digital_inputs: (DigitalInput0, DigitalInput1), pub eem_gpio: EemGpioDevices, pub usb_serial: SerialTerminal, - pub usb_device: UsbDevice, + pub usb: UsbDevice, } /// The available Pounder-specific hardware interfaces. @@ -1070,7 +1070,7 @@ pub fn setup( .device_class(usbd_serial::USB_CLASS_CDC) .build(); - (UsbDevice::new(usb_device), serial) + (usb_device, serial) }; let usb_serial = { @@ -1109,7 +1109,7 @@ pub fn setup( timestamp_timer, digital_inputs, eem_gpio, - usb_device, + usb: usb_device, usb_serial, }; diff --git a/src/hardware/usb.rs b/src/hardware/usb.rs deleted file mode 100644 index 030b93cc7..000000000 --- a/src/hardware/usb.rs +++ /dev/null @@ -1,22 +0,0 @@ -use super::SerialTerminal; -use super::UsbBus; - -pub struct UsbDevice { - usb_device: usb_device::device::UsbDevice<'static, UsbBus>, -} - -impl UsbDevice { - pub fn new( - usb_device: usb_device::device::UsbDevice<'static, UsbBus>, - ) -> Self { - Self { usb_device } - } - - pub fn usb_is_suspended(&self) -> bool { - self.usb_device.state() == usb_device::device::UsbDeviceState::Suspend - } - - pub fn process(&mut self, terminal: &mut SerialTerminal) { - self.usb_device.poll(&mut [terminal.interface_mut()]); - } -} From 61466820b3db0474a16f2cc61f56a65a2d6b49ac Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 20 Nov 2023 11:52:35 +0100 Subject: [PATCH 18/20] Removing trait API in favor of initializing with default state --- Cargo.lock | 8 ++++---- Cargo.toml | 7 ++----- serial-settings/src/lib.rs | 12 ++++-------- src/bin/dual-iir.rs | 8 +++++--- src/bin/lockin.rs | 8 +++++--- src/hardware/flash.rs | 23 +++++++++++------------ src/hardware/setup.rs | 17 ++++++++++------- 7 files changed, 41 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4333a3106..41be6c62a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1041,8 +1041,7 @@ dependencies = [ [[package]] name = "stm32h7xx-hal" version = "0.15.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e08bcfbdbe4458133f2fd55994a5c4f1b4bf28084f0218e93cdbc19d7c70219f" +source = "git+https://github.com/quartiq/stm32h7xx-hal?branch=feature/usb-updates#4d424ff9bb67e7df112ee57b11c488c699979e90" dependencies = [ "bare-metal 1.0.0", "cast", @@ -1083,8 +1082,9 @@ dependencies = [ [[package]] name = "synopsys-usb-otg" -version = "0.3.2" -source = "git+https://github.com/quartiq/synopsys-usb-otg?branch=feature/usb-device-bump#06bd4b0827f4ae7907ce3a2f9db1444c4ad10d1c" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e948d523b316939545d8b21a48c27aef150ce25321b9f95ff7978647a806a6fe" dependencies = [ "cortex-m 0.7.7", "embedded-hal", diff --git a/Cargo.toml b/Cargo.toml index b3188a600..1641b96ad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,7 +63,6 @@ enum-iterator = "1.4.1" rand_xorshift = "0.3.0" rand_core = "0.6.4" minimq = "0.8.0" -# patch with https://github.com/rust-embedded-community/usb-device/pull/129 usb-device = "0.3.0" usbd-serial = "0.2" # Keep this synced with the miniconf version in py/setup.py @@ -74,15 +73,13 @@ postcard = "1" [dependencies.stm32h7xx-hal] version = "0.15.1" +git = "https://github.com/quartiq/stm32h7xx-hal" +branch = "feature/usb-updates" features = ["stm32h743v", "rt", "ethernet", "xspi", "usb_hs"] [patch.crates-io.usbd-serial] git = "https://github.com/rust-embedded-community/usbd-serial" -[patch.crates-io.synopsys-usb-otg] -git = "https://github.com/quartiq/synopsys-usb-otg" -branch = "feature/usb-device-bump" - [features] nightly = [ ] pounder_v1_0 = [ ] diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index bb8636bf5..c3d9d3936 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -69,12 +69,9 @@ pub trait Platform: Sized { /// Specifies the settings that are used on the device. type Settings: Settings; - /// `load()`/`save()` Error type + /// `save()` Error type type Error: core::fmt::Debug; - /// 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>; @@ -313,7 +310,7 @@ impl<'a, P: Platform> core::fmt::Write for Context<'a, P> { /// The terminal uses an internal buffer. Overflows of the output buffer are silently ignored. fn write_str(&mut self, s: &str) -> core::fmt::Result { if let Ok(true) = self.platform.interface_mut().write_ready() { - self.platform.interface_mut().write_all(s.as_bytes()).ok(); + self.platform.interface_mut().write(s.as_bytes()).ok(); } Ok(()) } @@ -331,13 +328,12 @@ impl<'a, P: Platform> Runner<'a, P> { /// * `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 the entire serialized settings member. pub fn new( - mut platform: P, + platform: P, line_buf: &'a mut [u8], serialize_buf: &'a mut [u8], ) -> Result { - platform.load(serialize_buf)?; Ok(Self(menu::Runner::new( Context::menu(), line_buf, diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index 0ad14fd12..bdd44215d 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -46,9 +46,8 @@ use stabilizer::{ hal, signal_generator::{self, SignalGenerator}, timers::SamplingTimer, - UsbDevice, DigitalInput0, DigitalInput1, SerialTerminal, SystemTimer, Systick, - AFE0, AFE1, + UsbDevice, AFE0, AFE1, }, net::{ data_stream::{FrameGenerator, StreamFormat, StreamTarget}, @@ -406,7 +405,10 @@ mod app { NetworkState::Updated => {} NetworkState::NoChange => { // We can't sleep if USB is not in suspend. - if c.shared.usb.lock(|usb| usb.state() == usb_device::device::UsbDeviceState::Suspend) { + if c.shared.usb.lock(|usb| { + usb.state() + == usb_device::device::UsbDeviceState::Suspend + }) { cortex_m::asm::wfi(); } } diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 575ecb669..d052d8936 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -49,9 +49,8 @@ use stabilizer::{ input_stamper::InputStamper, signal_generator, timers::SamplingTimer, - UsbDevice, DigitalInput0, DigitalInput1, SerialTerminal, SystemTimer, Systick, - AFE0, AFE1, + UsbDevice, AFE0, AFE1, }, net::{ data_stream::{FrameGenerator, StreamFormat, StreamTarget}, @@ -468,7 +467,10 @@ mod app { NetworkState::Updated => {} NetworkState::NoChange => { // We can't sleep if USB is not in suspend. - if c.shared.usb.lock(|usb| usb.state() == usb_device::device::UsbDeviceState::Suspend) { + if c.shared.usb.lock(|usb| { + usb.state() + == usb_device::device::UsbDeviceState::Suspend + }) { cortex_m::asm::wfi(); } } diff --git a/src/hardware/flash.rs b/src/hardware/flash.rs index c8d1b6c7a..c780c7250 100644 --- a/src/hardware/flash.rs +++ b/src/hardware/flash.rs @@ -19,6 +19,17 @@ impl serial_settings::Settings for Settings { } impl Settings { + pub fn reload(&mut self, storage: &mut Flash) { + let mut buffer = [0u8; 512]; + storage.read(0, &mut buffer).unwrap(); + let Ok(mut settings) = postcard::from_bytes::(&buffer) else { + return; + }; + + settings.mac = self.mac; + *self = settings; + } + pub fn new(mac: smoltcp_nal::smoltcp::wire::EthernetAddress) -> Self { let mut id = heapless::String::new(); write!(&mut id, "{mac}").unwrap(); @@ -98,18 +109,6 @@ impl serial_settings::Platform for SerialSettingsPlatform { type Error = Error<::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::(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 diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index 66fe931a0..5b0147eec 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -16,9 +16,9 @@ use super::{ adc, afe, cpu_temp_sensor::CpuTempSensor, dac, delay, design_parameters, eeprom, input_stamper::InputStamper, pounder, pounder::dds_output::DdsOutput, shared_adc::SharedAdc, timers, - UsbDevice, DigitalInput0, DigitalInput1, EemDigitalInput0, - EemDigitalInput1, EemDigitalOutput0, EemDigitalOutput1, EthernetPhy, - NetworkStack, SerialTerminal, SystemTimer, Systick, UsbBus, AFE0, AFE1, + DigitalInput0, DigitalInput1, EemDigitalInput0, EemDigitalInput1, + EemDigitalOutput0, EemDigitalOutput1, EthernetPhy, NetworkStack, + SerialTerminal, SystemTimer, Systick, UsbBus, UsbDevice, AFE0, AFE1, }; const NUM_TCP_SOCKETS: usize = 4; @@ -1081,13 +1081,16 @@ pub fn setup( let serialize_buffer = cortex_m::singleton!(: [u8; 512] = [0u8; 512]).unwrap(); + let mut storage = super::flash::Flash(flash_bank2.unwrap()); + let mut settings = + super::flash::Settings::new(network_devices.mac_address); + settings.reload(&mut storage); + serial_settings::Runner::new( super::flash::SerialSettingsPlatform { interface: usb_serial, - storage: super::flash::Flash(flash_bank2.unwrap()), - settings: super::flash::Settings::new( - network_devices.mac_address, - ), + storage, + settings, }, input_buffer, serialize_buffer, From 2c391b669991052b5c450110bebcc7083ec989d4 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 20 Nov 2023 12:06:48 +0100 Subject: [PATCH 19/20] Fixing writer --- serial-settings/src/lib.rs | 42 ++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index c3d9d3936..7039a4c61 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -115,10 +115,7 @@ impl<'a, P: Platform> Context<'a, P> { for path in P::Settings::iter_paths::>("/") { match path { - Err(e) => writeln!( - context.platform.interface_mut(), - "Failed to get path: {e}" - ), + Err(e) => writeln!(context, "Failed to get path: {e}"), Ok(path) => { match context .platform @@ -126,15 +123,12 @@ impl<'a, P: Platform> Context<'a, P> { .get_json(&path, context.buffer) { Err(e) => { - writeln!( - context.platform.interface_mut(), - "Failed to read {path}: {e}" - ) - .unwrap(); + writeln!(context, "Failed to read {path}: {e}") + .unwrap(); continue; } Ok(len) => write!( - context.platform.interface_mut(), + Writer(&mut context.platform.interface_mut()), "{path}: {}", core::str::from_utf8(&context.buffer[..len]) .unwrap() @@ -144,11 +138,11 @@ impl<'a, P: Platform> Context<'a, P> { match defaults.get_json(&path, context.buffer) { Err(e) => writeln!( - context.platform.interface_mut(), + context, "[default serialization error: {e}]" ), Ok(len) => writeln!( - context.platform.interface_mut(), + Writer(&mut context.platform.interface_mut()), " [default: {}]", core::str::from_utf8(&context.buffer[..len]) .unwrap() @@ -187,14 +181,11 @@ impl<'a, P: Platform> Context<'a, P> { let key = menu::argument_finder(item, args, "item").unwrap().unwrap(); match context.platform.settings().get_json(key, context.buffer) { Err(e) => { - writeln!( - context.platform.interface_mut(), - "Failed to read {key}: {e}" - ) + writeln!(context, "Failed to read {key}: {e}") } Ok(len) => { writeln!( - context.platform.interface_mut(), + Writer(&mut context.platform.interface_mut()), "{key}: {}", core::str::from_utf8(&context.buffer[..len]).unwrap() ) @@ -309,8 +300,19 @@ impl<'a, P: Platform> core::fmt::Write for Context<'a, P> { /// # Note /// The terminal uses an internal buffer. Overflows of the output buffer are silently ignored. fn write_str(&mut self, s: &str) -> core::fmt::Result { - if let Ok(true) = self.platform.interface_mut().write_ready() { - self.platform.interface_mut().write(s.as_bytes()).ok(); + let mut writer = Writer(self.platform.interface_mut()); + writer.write_str(s) + } +} + +struct Writer<'a, T: WriteReady + EioWrite>(&'a mut T); + +impl<'a, T: embedded_io::WriteReady + embedded_io::Write> core::fmt::Write + for Writer<'a, T> +{ + fn write_str(&mut self, s: &str) -> core::fmt::Result { + if let Ok(true) = self.0.write_ready() { + self.0.write(s.as_bytes()).ok(); } Ok(()) } @@ -328,7 +330,7 @@ impl<'a, P: Platform> Runner<'a, P> { /// * `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 member. + /// needs to be at least as big as the entire serialized settings structure. pub fn new( platform: P, line_buf: &'a mut [u8], From 3ab4fffac271e5c84fd343eac2a7c5b27ec96491 Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 20 Nov 2023 15:32:03 +0100 Subject: [PATCH 20/20] Adding support for a best effort interface wrapper --- serial-settings/src/interface.rs | 66 ++++++++++++++++++++++++++++++++ serial-settings/src/lib.rs | 35 +++++------------ src/bin/dual-iir.rs | 2 +- src/bin/lockin.rs | 2 +- src/hardware/flash.rs | 9 +++-- src/hardware/setup.rs | 4 +- 6 files changed, 87 insertions(+), 31 deletions(-) create mode 100644 serial-settings/src/interface.rs diff --git a/serial-settings/src/interface.rs b/serial-settings/src/interface.rs new file mode 100644 index 000000000..167833203 --- /dev/null +++ b/serial-settings/src/interface.rs @@ -0,0 +1,66 @@ +/// Wrapper type for a "best effort" serial interface. +/// +/// # Note +/// Overflows of the output are silently ignored. +pub struct BestEffortInterface(T); + +impl BestEffortInterface +where + T: embedded_io::Write + + embedded_io::WriteReady + + embedded_io::Read + + embedded_io::ReadReady, +{ + /// Construct an interface where overflows and errors when writing on the output are silently + /// ignored. + pub fn new(interface: T) -> Self { + Self(interface) + } + + /// Get access to the inner (wrapped) interface + pub fn inner(&self) -> &T { + &self.0 + } + + /// Get mutable access to the inner (wrapped) interface + pub fn inner_mut(&mut self) -> &mut T { + &mut self.0 + } +} + +impl core::fmt::Write for BestEffortInterface +where + T: embedded_io::Write + embedded_io::WriteReady, +{ + fn write_str(&mut self, s: &str) -> core::fmt::Result { + if let Ok(true) = self.0.write_ready() { + self.0.write(s.as_bytes()).ok(); + } + Ok(()) + } +} + +impl embedded_io::ErrorType for BestEffortInterface +where + T: embedded_io::ErrorType, +{ + type Error = ::Error; +} + +impl embedded_io::Read for BestEffortInterface +where + T: embedded_io::Read, +{ + fn read(&mut self, buf: &mut [u8]) -> Result { + self.0.read(buf) + } +} + +impl embedded_io::ReadReady for BestEffortInterface +where + T: embedded_io::ReadReady, +{ + fn read_ready(&mut self) -> Result { + self.0.read_ready() + } +} diff --git a/serial-settings/src/lib.rs b/serial-settings/src/lib.rs index 7039a4c61..cb188feaf 100644 --- a/serial-settings/src/lib.rs +++ b/serial-settings/src/lib.rs @@ -49,9 +49,13 @@ #![no_std] use core::fmt::Write; -use embedded_io::{Read, ReadReady, Write as EioWrite, WriteReady}; +use embedded_io::{Read, ReadReady}; use miniconf::{JsonCoreSlash, 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> + Clone { @@ -63,8 +67,7 @@ pub trait Platform: Sized { /// 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 - + embedded_io::WriteReady; + + core::fmt::Write; /// Specifies the settings that are used on the device. type Settings: Settings; @@ -128,7 +131,7 @@ impl<'a, P: Platform> Context<'a, P> { continue; } Ok(len) => write!( - Writer(&mut context.platform.interface_mut()), + &mut context.platform.interface_mut(), "{path}: {}", core::str::from_utf8(&context.buffer[..len]) .unwrap() @@ -142,7 +145,7 @@ impl<'a, P: Platform> Context<'a, P> { "[default serialization error: {e}]" ), Ok(len) => writeln!( - Writer(&mut context.platform.interface_mut()), + &mut context.platform.interface_mut(), " [default: {}]", core::str::from_utf8(&context.buffer[..len]) .unwrap() @@ -185,7 +188,7 @@ impl<'a, P: Platform> Context<'a, P> { } Ok(len) => { writeln!( - Writer(&mut context.platform.interface_mut()), + &mut context.platform.interface_mut(), "{key}: {}", core::str::from_utf8(&context.buffer[..len]).unwrap() ) @@ -295,26 +298,8 @@ impl<'a, P: Platform> Context<'a, P> { } impl<'a, P: Platform> core::fmt::Write for Context<'a, P> { - /// Write data to the serial terminal. - /// - /// # Note - /// The terminal uses an internal buffer. Overflows of the output buffer are silently ignored. - fn write_str(&mut self, s: &str) -> core::fmt::Result { - let mut writer = Writer(self.platform.interface_mut()); - writer.write_str(s) - } -} - -struct Writer<'a, T: WriteReady + EioWrite>(&'a mut T); - -impl<'a, T: embedded_io::WriteReady + embedded_io::Write> core::fmt::Write - for Writer<'a, T> -{ fn write_str(&mut self, s: &str) -> core::fmt::Result { - if let Ok(true) = self.0.write_ready() { - self.0.write(s.as_bytes()).ok(); - } - Ok(()) + self.platform.interface_mut().write_str(s) } } diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index bdd44215d..92a973f82 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -471,7 +471,7 @@ mod app { fn usb(mut c: usb::Context) { // Handle the USB serial terminal. c.shared.usb.lock(|usb| { - usb.poll(&mut [c.local.usb_terminal.interface_mut()]); + usb.poll(&mut [c.local.usb_terminal.interface_mut().inner_mut()]); }); c.local.usb_terminal.process().unwrap(); diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index d052d8936..7959c5af9 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -522,7 +522,7 @@ mod app { fn usb(mut c: usb::Context) { // Handle the USB serial terminal. c.shared.usb.lock(|usb| { - usb.poll(&mut [c.local.usb_terminal.interface_mut()]); + usb.poll(&mut [c.local.usb_terminal.interface_mut().inner_mut()]); }); c.local.usb_terminal.process().unwrap(); diff --git a/src/hardware/flash.rs b/src/hardware/flash.rs index c780c7250..0e69b312f 100644 --- a/src/hardware/flash.rs +++ b/src/hardware/flash.rs @@ -1,7 +1,6 @@ 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 { @@ -96,7 +95,9 @@ impl From for Error { 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>, + pub interface: serial_settings::BestEffortInterface< + usbd_serial::SerialPort<'static, super::UsbBus>, + >, /// The Settings structure. pub settings: Settings, /// The storage mechanism used to persist settings to between boots. @@ -104,7 +105,9 @@ pub struct SerialSettingsPlatform { } impl serial_settings::Platform for SerialSettingsPlatform { - type Interface = usbd_serial::SerialPort<'static, super::UsbBus>; + type Interface = serial_settings::BestEffortInterface< + usbd_serial::SerialPort<'static, super::UsbBus>, + >; type Settings = Settings; type Error = Error<::Error>; diff --git a/src/hardware/setup.rs b/src/hardware/setup.rs index 5b0147eec..72b09a4b9 100644 --- a/src/hardware/setup.rs +++ b/src/hardware/setup.rs @@ -1088,7 +1088,9 @@ pub fn setup( serial_settings::Runner::new( super::flash::SerialSettingsPlatform { - interface: usb_serial, + interface: serial_settings::BestEffortInterface::new( + usb_serial, + ), storage, settings, },