From 4757257ac00f77f306d5177265ac24eaf614f7c0 Mon Sep 17 00:00:00 2001 From: Kaitlyn Kenwell Date: Thu, 14 Dec 2023 09:36:22 -0500 Subject: [PATCH] Address reviews --- .../boot/src/firmware_updater/asynch.rs | 10 + .../boot/src/firmware_updater/blocking.rs | 10 + embassy-boot/boot/src/lib.rs | 2 +- embassy-usb-dfu/src/application.rs | 36 ++- embassy-usb-dfu/src/bootloader.rs | 8 +- embassy-usb-dfu/src/fmt.rs | 258 ++++++++++++++++++ embassy-usb-dfu/src/lib.rs | 1 + examples/boot-usb-dfu/.cargo/config.toml | 9 - .../stm32wb-dfu}/.cargo/config.toml | 0 .../application/stm32wb-dfu}/Cargo.toml | 1 + .../application/stm32wb-dfu}/README.md | 0 .../application/stm32wb-dfu}/build.rs | 0 .../application/stm32wb-dfu}/memory.x | 0 .../application/stm32wb-dfu}/src/main.rs | 0 .../bootloader/stm32wb-dfu}/Cargo.toml | 0 .../bootloader/stm32wb-dfu}/README.md | 0 .../bootloader/stm32wb-dfu}/build.rs | 0 .../bootloader/stm32wb-dfu}/memory.x | 0 .../bootloader/stm32wb-dfu}/src/main.rs | 13 +- 19 files changed, 307 insertions(+), 41 deletions(-) create mode 100644 embassy-usb-dfu/src/fmt.rs delete mode 100644 examples/boot-usb-dfu/.cargo/config.toml rename examples/{boot-usb-dfu/application/stm32wb => boot/application/stm32wb-dfu}/.cargo/config.toml (100%) rename examples/{boot-usb-dfu/application/stm32wb => boot/application/stm32wb-dfu}/Cargo.toml (98%) rename examples/{boot-usb-dfu/application/stm32wb => boot/application/stm32wb-dfu}/README.md (100%) rename examples/{boot-usb-dfu/application/stm32wb => boot/application/stm32wb-dfu}/build.rs (100%) rename examples/{boot-usb-dfu/application/stm32wb => boot/application/stm32wb-dfu}/memory.x (100%) rename examples/{boot-usb-dfu/application/stm32wb => boot/application/stm32wb-dfu}/src/main.rs (100%) rename examples/{boot-usb-dfu/bootloader/stm32wb => boot/bootloader/stm32wb-dfu}/Cargo.toml (100%) rename examples/{boot-usb-dfu/bootloader/stm32wb => boot/bootloader/stm32wb-dfu}/README.md (100%) rename examples/{boot-usb-dfu/bootloader/stm32wb => boot/bootloader/stm32wb-dfu}/build.rs (100%) rename examples/{boot-usb-dfu/bootloader/stm32wb => boot/bootloader/stm32wb-dfu}/memory.x (100%) rename examples/{boot-usb-dfu/bootloader/stm32wb => boot/bootloader/stm32wb-dfu}/src/main.rs (91%) diff --git a/embassy-boot/boot/src/firmware_updater/asynch.rs b/embassy-boot/boot/src/firmware_updater/asynch.rs index 82e99965..d8d85c3d 100644 --- a/embassy-boot/boot/src/firmware_updater/asynch.rs +++ b/embassy-boot/boot/src/firmware_updater/asynch.rs @@ -213,6 +213,16 @@ pub struct FirmwareState<'d, STATE> { } impl<'d, STATE: NorFlash> FirmwareState<'d, STATE> { + /// Create a firmware state instance from a FirmwareUpdaterConfig with a buffer for magic content and state partition. + /// + /// # Safety + /// + /// The `aligned` buffer must have a size of STATE::WRITE_SIZE, and follow the alignment rules for the flash being read from + /// and written to. + pub fn from_config(config: FirmwareUpdaterConfig, aligned: &'d mut [u8]) -> Self { + Self::new(config.state, aligned) + } + /// Create a firmware state instance with a buffer for magic content and state partition. /// /// # Safety diff --git a/embassy-boot/boot/src/firmware_updater/blocking.rs b/embassy-boot/boot/src/firmware_updater/blocking.rs index ae4179ba..4f56f152 100644 --- a/embassy-boot/boot/src/firmware_updater/blocking.rs +++ b/embassy-boot/boot/src/firmware_updater/blocking.rs @@ -219,6 +219,16 @@ pub struct BlockingFirmwareState<'d, STATE> { } impl<'d, STATE: NorFlash> BlockingFirmwareState<'d, STATE> { + /// Creates a firmware state instance from a FirmwareUpdaterConfig, with a buffer for magic content and state partition. + /// + /// # Safety + /// + /// The `aligned` buffer must have a size of STATE::WRITE_SIZE, and follow the alignment rules for the flash being read from + /// and written to. + pub fn from_config(config: FirmwareUpdaterConfig, aligned: &'d mut [u8]) -> Self { + Self::new(config.state, aligned) + } + /// Create a firmware state instance with a buffer for magic content and state partition. /// /// # Safety diff --git a/embassy-boot/boot/src/lib.rs b/embassy-boot/boot/src/lib.rs index 45199294..15b69f69 100644 --- a/embassy-boot/boot/src/lib.rs +++ b/embassy-boot/boot/src/lib.rs @@ -33,7 +33,7 @@ pub enum State { Boot, /// Bootloader has swapped the active partition with the dfu partition and will attempt boot. Swap, - /// Application has received a DFU_DETACH request over USB, and is rebooting into the bootloader to apply a DFU. + /// Application has received a request to reboot into DFU mode to apply an update. DfuDetach, } diff --git a/embassy-usb-dfu/src/application.rs b/embassy-usb-dfu/src/application.rs index 2e7bda12..0b7b53af 100644 --- a/embassy-usb-dfu/src/application.rs +++ b/embassy-usb-dfu/src/application.rs @@ -1,4 +1,4 @@ -use embassy_boot::BlockingFirmwareUpdater; +use embassy_boot::BlockingFirmwareState; use embassy_time::{Duration, Instant}; use embassy_usb::control::{InResponse, OutResponse, Recipient, RequestType}; use embassy_usb::driver::Driver; @@ -11,18 +11,18 @@ use crate::consts::{ }; /// Internal state for the DFU class -pub struct Control<'d, DFU: NorFlash, STATE: NorFlash> { - updater: BlockingFirmwareUpdater<'d, DFU, STATE>, +pub struct Control<'d, STATE: NorFlash> { + firmware_state: BlockingFirmwareState<'d, STATE>, attrs: DfuAttributes, state: State, timeout: Option, detach_start: Option, } -impl<'d, DFU: NorFlash, STATE: NorFlash> Control<'d, DFU, STATE> { - pub fn new(updater: BlockingFirmwareUpdater<'d, DFU, STATE>, attrs: DfuAttributes) -> Self { +impl<'d, STATE: NorFlash> Control<'d, STATE> { + pub fn new(firmware_state: BlockingFirmwareState<'d, STATE>, attrs: DfuAttributes) -> Self { Control { - updater, + firmware_state, attrs, state: State::AppIdle, detach_start: None, @@ -31,19 +31,20 @@ impl<'d, DFU: NorFlash, STATE: NorFlash> Control<'d, DFU, STATE> { } } -impl<'d, DFU: NorFlash, STATE: NorFlash> Handler for Control<'d, DFU, STATE> { +impl<'d, STATE: NorFlash> Handler for Control<'d, STATE> { fn reset(&mut self) { if let Some(start) = self.detach_start { let delta = Instant::now() - start; let timeout = self.timeout.unwrap(); - #[cfg(feature = "defmt")] - defmt::info!( + trace!( "Received RESET with delta = {}, timeout = {}", delta.as_millis(), timeout.as_millis() ); if delta < timeout { - self.updater.mark_dfu().expect("Failed to mark DFU mode in bootloader"); + self.firmware_state + .mark_dfu() + .expect("Failed to mark DFU mode in bootloader"); cortex_m::asm::dsb(); cortex_m::peripheral::SCB::sys_reset(); } @@ -59,13 +60,11 @@ impl<'d, DFU: NorFlash, STATE: NorFlash> Handler for Control<'d, DFU, STATE> { return None; } - #[cfg(feature = "defmt")] - defmt::info!("Received request {}", req); + trace!("Received request {}", req); match Request::try_from(req.request) { Ok(Request::Detach) => { - #[cfg(feature = "defmt")] - defmt::info!("Received DETACH, awaiting USB reset"); + trace!("Received DETACH, awaiting USB reset"); self.detach_start = Some(Instant::now()); self.timeout = Some(Duration::from_millis(req.value as u64)); self.state = State::AppDetach; @@ -84,8 +83,7 @@ impl<'d, DFU: NorFlash, STATE: NorFlash> Handler for Control<'d, DFU, STATE> { return None; } - #[cfg(feature = "defmt")] - defmt::info!("Received request {}", req); + trace!("Received request {}", req); match Request::try_from(req.request) { Ok(Request::GetStatus) => { @@ -106,13 +104,11 @@ impl<'d, DFU: NorFlash, STATE: NorFlash> Handler for Control<'d, DFU, STATE> { /// it should expose a DFU device, and a software reset will be issued. /// /// To apply USB DFU updates, the bootloader must be capable of recognizing the DFU magic and exposing a device to handle the full DFU transaction with the host. -pub fn usb_dfu<'d, D: Driver<'d>, DFU: NorFlash, STATE: NorFlash>( +pub fn usb_dfu<'d, D: Driver<'d>, STATE: NorFlash>( builder: &mut Builder<'d, D>, - handler: &'d mut Control<'d, DFU, STATE>, + handler: &'d mut Control<'d, STATE>, timeout: Duration, ) { - #[cfg(feature = "defmt")] - defmt::info!("Application USB DFU initializing"); let mut func = builder.function(0x00, 0x00, 0x00); let mut iface = func.interface(); let mut alt = iface.alt_setting(USB_CLASS_APPN_SPEC, APPN_SPEC_SUBCLASS_DFU, DFU_PROTOCOL_RT, None); diff --git a/embassy-usb-dfu/src/bootloader.rs b/embassy-usb-dfu/src/bootloader.rs index 21505893..99384d96 100644 --- a/embassy-usb-dfu/src/bootloader.rs +++ b/embassy-usb-dfu/src/bootloader.rs @@ -1,4 +1,4 @@ -use embassy_boot::BlockingFirmwareUpdater; +use embassy_boot::{AlignedBuffer, BlockingFirmwareUpdater}; use embassy_usb::control::{InResponse, OutResponse, Recipient, RequestType}; use embassy_usb::driver::Driver; use embassy_usb::{Builder, Handler}; @@ -56,8 +56,8 @@ impl<'d, DFU: NorFlash, STATE: NorFlash, const BLOCK_SIZE: usize> Handler for Co self.offset = 0; } - let mut buf = [0; BLOCK_SIZE]; - buf[..data.len()].copy_from_slice(data); + let mut buf = AlignedBuffer([0; BLOCK_SIZE]); + buf.as_mut()[..data.len()].copy_from_slice(data); if req.length == 0 { match self.updater.mark_updated() { @@ -85,7 +85,7 @@ impl<'d, DFU: NorFlash, STATE: NorFlash, const BLOCK_SIZE: usize> Handler for Co self.state = State::Error; return Some(OutResponse::Rejected); } - match self.updater.write_firmware(self.offset, &buf[..]) { + match self.updater.write_firmware(self.offset, buf.as_ref()) { Ok(_) => { self.status = Status::Ok; self.state = State::DlSync; diff --git a/embassy-usb-dfu/src/fmt.rs b/embassy-usb-dfu/src/fmt.rs new file mode 100644 index 00000000..78e583c1 --- /dev/null +++ b/embassy-usb-dfu/src/fmt.rs @@ -0,0 +1,258 @@ +#![macro_use] +#![allow(unused_macros)] + +use core::fmt::{Debug, Display, LowerHex}; + +#[cfg(all(feature = "defmt", feature = "log"))] +compile_error!("You may not enable both `defmt` and `log` features."); + +macro_rules! assert { + ($($x:tt)*) => { + { + #[cfg(not(feature = "defmt"))] + ::core::assert!($($x)*); + #[cfg(feature = "defmt")] + ::defmt::assert!($($x)*); + } + }; +} + +macro_rules! assert_eq { + ($($x:tt)*) => { + { + #[cfg(not(feature = "defmt"))] + ::core::assert_eq!($($x)*); + #[cfg(feature = "defmt")] + ::defmt::assert_eq!($($x)*); + } + }; +} + +macro_rules! assert_ne { + ($($x:tt)*) => { + { + #[cfg(not(feature = "defmt"))] + ::core::assert_ne!($($x)*); + #[cfg(feature = "defmt")] + ::defmt::assert_ne!($($x)*); + } + }; +} + +macro_rules! debug_assert { + ($($x:tt)*) => { + { + #[cfg(not(feature = "defmt"))] + ::core::debug_assert!($($x)*); + #[cfg(feature = "defmt")] + ::defmt::debug_assert!($($x)*); + } + }; +} + +macro_rules! debug_assert_eq { + ($($x:tt)*) => { + { + #[cfg(not(feature = "defmt"))] + ::core::debug_assert_eq!($($x)*); + #[cfg(feature = "defmt")] + ::defmt::debug_assert_eq!($($x)*); + } + }; +} + +macro_rules! debug_assert_ne { + ($($x:tt)*) => { + { + #[cfg(not(feature = "defmt"))] + ::core::debug_assert_ne!($($x)*); + #[cfg(feature = "defmt")] + ::defmt::debug_assert_ne!($($x)*); + } + }; +} + +macro_rules! todo { + ($($x:tt)*) => { + { + #[cfg(not(feature = "defmt"))] + ::core::todo!($($x)*); + #[cfg(feature = "defmt")] + ::defmt::todo!($($x)*); + } + }; +} + +#[cfg(not(feature = "defmt"))] +macro_rules! unreachable { + ($($x:tt)*) => { + ::core::unreachable!($($x)*) + }; +} + +#[cfg(feature = "defmt")] +macro_rules! unreachable { + ($($x:tt)*) => { + ::defmt::unreachable!($($x)*) + }; +} + +macro_rules! panic { + ($($x:tt)*) => { + { + #[cfg(not(feature = "defmt"))] + ::core::panic!($($x)*); + #[cfg(feature = "defmt")] + ::defmt::panic!($($x)*); + } + }; +} + +macro_rules! trace { + ($s:literal $(, $x:expr)* $(,)?) => { + { + #[cfg(feature = "log")] + ::log::trace!($s $(, $x)*); + #[cfg(feature = "defmt")] + ::defmt::trace!($s $(, $x)*); + #[cfg(not(any(feature = "log", feature="defmt")))] + let _ = ($( & $x ),*); + } + }; +} + +macro_rules! debug { + ($s:literal $(, $x:expr)* $(,)?) => { + { + #[cfg(feature = "log")] + ::log::debug!($s $(, $x)*); + #[cfg(feature = "defmt")] + ::defmt::debug!($s $(, $x)*); + #[cfg(not(any(feature = "log", feature="defmt")))] + let _ = ($( & $x ),*); + } + }; +} + +macro_rules! info { + ($s:literal $(, $x:expr)* $(,)?) => { + { + #[cfg(feature = "log")] + ::log::info!($s $(, $x)*); + #[cfg(feature = "defmt")] + ::defmt::info!($s $(, $x)*); + #[cfg(not(any(feature = "log", feature="defmt")))] + let _ = ($( & $x ),*); + } + }; +} + +macro_rules! warn { + ($s:literal $(, $x:expr)* $(,)?) => { + { + #[cfg(feature = "log")] + ::log::warn!($s $(, $x)*); + #[cfg(feature = "defmt")] + ::defmt::warn!($s $(, $x)*); + #[cfg(not(any(feature = "log", feature="defmt")))] + let _ = ($( & $x ),*); + } + }; +} + +macro_rules! error { + ($s:literal $(, $x:expr)* $(,)?) => { + { + #[cfg(feature = "log")] + ::log::error!($s $(, $x)*); + #[cfg(feature = "defmt")] + ::defmt::error!($s $(, $x)*); + #[cfg(not(any(feature = "log", feature="defmt")))] + let _ = ($( & $x ),*); + } + }; +} + +#[cfg(feature = "defmt")] +macro_rules! unwrap { + ($($x:tt)*) => { + ::defmt::unwrap!($($x)*) + }; +} + +#[cfg(not(feature = "defmt"))] +macro_rules! unwrap { + ($arg:expr) => { + match $crate::fmt::Try::into_result($arg) { + ::core::result::Result::Ok(t) => t, + ::core::result::Result::Err(e) => { + ::core::panic!("unwrap of `{}` failed: {:?}", ::core::stringify!($arg), e); + } + } + }; + ($arg:expr, $($msg:expr),+ $(,)? ) => { + match $crate::fmt::Try::into_result($arg) { + ::core::result::Result::Ok(t) => t, + ::core::result::Result::Err(e) => { + ::core::panic!("unwrap of `{}` failed: {}: {:?}", ::core::stringify!($arg), ::core::format_args!($($msg,)*), e); + } + } + } +} + +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +pub struct NoneError; + +pub trait Try { + type Ok; + type Error; + fn into_result(self) -> Result; +} + +impl Try for Option { + type Ok = T; + type Error = NoneError; + + #[inline] + fn into_result(self) -> Result { + self.ok_or(NoneError) + } +} + +impl Try for Result { + type Ok = T; + type Error = E; + + #[inline] + fn into_result(self) -> Self { + self + } +} + +#[allow(unused)] +pub(crate) struct Bytes<'a>(pub &'a [u8]); + +impl<'a> Debug for Bytes<'a> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "{:#02x?}", self.0) + } +} + +impl<'a> Display for Bytes<'a> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "{:#02x?}", self.0) + } +} + +impl<'a> LowerHex for Bytes<'a> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "{:#02x?}", self.0) + } +} + +#[cfg(feature = "defmt")] +impl<'a> defmt::Format for Bytes<'a> { + fn format(&self, fmt: defmt::Formatter) { + defmt::write!(fmt, "{:02x}", self.0) + } +} diff --git a/embassy-usb-dfu/src/lib.rs b/embassy-usb-dfu/src/lib.rs index 81ef041f..ae0fbbd4 100644 --- a/embassy-usb-dfu/src/lib.rs +++ b/embassy-usb-dfu/src/lib.rs @@ -1,4 +1,5 @@ #![no_std] +mod fmt; pub mod consts; diff --git a/examples/boot-usb-dfu/.cargo/config.toml b/examples/boot-usb-dfu/.cargo/config.toml deleted file mode 100644 index de3a814f..00000000 --- a/examples/boot-usb-dfu/.cargo/config.toml +++ /dev/null @@ -1,9 +0,0 @@ -[unstable] -build-std = ["core"] -build-std-features = ["panic_immediate_abort"] - -[build] -target = "thumbv7em-none-eabi" - -[env] -DEFMT_LOG = "trace" diff --git a/examples/boot-usb-dfu/application/stm32wb/.cargo/config.toml b/examples/boot/application/stm32wb-dfu/.cargo/config.toml similarity index 100% rename from examples/boot-usb-dfu/application/stm32wb/.cargo/config.toml rename to examples/boot/application/stm32wb-dfu/.cargo/config.toml diff --git a/examples/boot-usb-dfu/application/stm32wb/Cargo.toml b/examples/boot/application/stm32wb-dfu/Cargo.toml similarity index 98% rename from examples/boot-usb-dfu/application/stm32wb/Cargo.toml rename to examples/boot/application/stm32wb-dfu/Cargo.toml index 0a41c064..e67224ce 100644 --- a/examples/boot-usb-dfu/application/stm32wb/Cargo.toml +++ b/examples/boot/application/stm32wb-dfu/Cargo.toml @@ -25,6 +25,7 @@ cortex-m-rt = "0.7.0" [features] defmt = [ "dep:defmt", + "dep:defmt-rtt", "embassy-stm32/defmt", "embassy-boot-stm32/defmt", "embassy-sync/defmt", diff --git a/examples/boot-usb-dfu/application/stm32wb/README.md b/examples/boot/application/stm32wb-dfu/README.md similarity index 100% rename from examples/boot-usb-dfu/application/stm32wb/README.md rename to examples/boot/application/stm32wb-dfu/README.md diff --git a/examples/boot-usb-dfu/application/stm32wb/build.rs b/examples/boot/application/stm32wb-dfu/build.rs similarity index 100% rename from examples/boot-usb-dfu/application/stm32wb/build.rs rename to examples/boot/application/stm32wb-dfu/build.rs diff --git a/examples/boot-usb-dfu/application/stm32wb/memory.x b/examples/boot/application/stm32wb-dfu/memory.x similarity index 100% rename from examples/boot-usb-dfu/application/stm32wb/memory.x rename to examples/boot/application/stm32wb-dfu/memory.x diff --git a/examples/boot-usb-dfu/application/stm32wb/src/main.rs b/examples/boot/application/stm32wb-dfu/src/main.rs similarity index 100% rename from examples/boot-usb-dfu/application/stm32wb/src/main.rs rename to examples/boot/application/stm32wb-dfu/src/main.rs diff --git a/examples/boot-usb-dfu/bootloader/stm32wb/Cargo.toml b/examples/boot/bootloader/stm32wb-dfu/Cargo.toml similarity index 100% rename from examples/boot-usb-dfu/bootloader/stm32wb/Cargo.toml rename to examples/boot/bootloader/stm32wb-dfu/Cargo.toml diff --git a/examples/boot-usb-dfu/bootloader/stm32wb/README.md b/examples/boot/bootloader/stm32wb-dfu/README.md similarity index 100% rename from examples/boot-usb-dfu/bootloader/stm32wb/README.md rename to examples/boot/bootloader/stm32wb-dfu/README.md diff --git a/examples/boot-usb-dfu/bootloader/stm32wb/build.rs b/examples/boot/bootloader/stm32wb-dfu/build.rs similarity index 100% rename from examples/boot-usb-dfu/bootloader/stm32wb/build.rs rename to examples/boot/bootloader/stm32wb-dfu/build.rs diff --git a/examples/boot-usb-dfu/bootloader/stm32wb/memory.x b/examples/boot/bootloader/stm32wb-dfu/memory.x similarity index 100% rename from examples/boot-usb-dfu/bootloader/stm32wb/memory.x rename to examples/boot/bootloader/stm32wb-dfu/memory.x diff --git a/examples/boot-usb-dfu/bootloader/stm32wb/src/main.rs b/examples/boot/bootloader/stm32wb-dfu/src/main.rs similarity index 91% rename from examples/boot-usb-dfu/bootloader/stm32wb/src/main.rs rename to examples/boot/bootloader/stm32wb-dfu/src/main.rs index 00a535d3..a2b88477 100644 --- a/examples/boot-usb-dfu/bootloader/stm32wb/src/main.rs +++ b/examples/boot/bootloader/stm32wb-dfu/src/main.rs @@ -26,13 +26,12 @@ fn main() -> ! { config.rcc = WPAN_DEFAULT; let p = embassy_stm32::init(config); - // Uncomment this if you are debugging the bootloader with debugger/RTT attached, - // as it prevents a hard fault when accessing flash 'too early' after boot. - /* - for i in 0..10000000 { - cortex_m::asm::nop(); - } - */ + // Prevent a hard fault when accessing flash 'too early' after boot. + #[cfg(feature = "defmt")] + for _ in 0..10000000 { + cortex_m::asm::nop(); + } + let layout = Flash::new_blocking(p.FLASH).into_blocking_regions(); let flash = Mutex::new(RefCell::new(layout.bank1_region));