Address reviews

4 files changed, 279 insertions, 24 deletions

diff --git a/embassy-usb-dfu/src/application.rs b/embassy-usb-dfu/src/application.rs
index 2e7bda121..0b7b53af8 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<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);
diff --git a/embassy-usb-dfu/src/bootloader.rs b/embassy-usb-dfu/src/bootloader.rs
index 215058932..99384d961 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 000000000..78e583c1c
--- /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<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)
+    }
+}
diff --git a/embassy-usb-dfu/src/lib.rs b/embassy-usb-dfu/src/lib.rs
index 81ef041f8..ae0fbbd4c 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;