Address reviews

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<DFU: NorFlash>(config: FirmwareUpdaterConfig<DFU, STATE>, 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

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<DFU: NorFlash>(config: FirmwareUpdaterConfig<DFU, STATE>, 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

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,
 }
 

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<Duration>,
     detach_start: Option<Instant>,
 }
 
-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);

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;

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<Self::Ok, Self::Error>;
+}
+
+impl<T> Try for Option<T> {
+    type Ok = T;
+    type Error = NoneError;
+
+    #[inline]
+    fn into_result(self) -> Result<T, NoneError> {
+        self.ok_or(NoneError)
+    }
+}
+
+impl<T, E> Try for Result<T, E> {
+    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)
+    }
+}

embassy-usb-dfu/src/lib.rs

@@ -1,4 +1,5 @@
 #![no_std]
+mod fmt;
 
 pub mod consts;
 

examples/boot-usb-dfu/.cargo/config.toml

@@ -1,9 +0,0 @@
-[unstable]
-build-std = ["core"]
-build-std-features = ["panic_immediate_abort"]
-
-[build]
-target = "thumbv7em-none-eabi"
-
-[env]
-DEFMT_LOG = "trace"

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",

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 {
+    // 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));