Use Context trait

src/boot/mod.rs

@@ -1,11 +1,10 @@
 use crate::{
     hardware::processor::Processor,
     hardware::{Bank, Config},
-    state::{Exchange, ExchangeError, ExchangeProgress, ExchangeStep, State, Update, UpdateError},
+    state::{Exchange, ExchangeProgress, ExchangeStep, State, Update, UpdateError},
+    Context,
 };
 
-use embedded_storage::Storage;
-
 use crate::log;
 
 #[cfg(feature = "defmt")]
@@ -13,36 +12,23 @@ use defmt::Format;
 
 /// Use this from your bootloader application and call boot() to do the magic, reading the current
 /// state via the State type and then jumping to the new image using the Jumper specified
-pub struct MoonbootBoot<
+pub struct MoonbootBoot<CONTEXT: Context, const INTERNAL_PAGE_SIZE: usize> {
-    STORAGE: Storage,
-    STATE: State,
-    PROCESSOR: Processor, // TODO: Wrap these into a context struct like rubble?
-    EXCHANGE: Exchange,
-    const INTERNAL_PAGE_SIZE: usize,
-> {
     config: Config,
-    storage: STORAGE,
+    storage: CONTEXT::Storage,
-    state: STATE,
+    state: CONTEXT::State,
-    processor: PROCESSOR,
+    processor: CONTEXT::Processor,
-    exchange: EXCHANGE,
+    exchange: CONTEXT::Exchange,
 }
 
-impl<
+impl<CONTEXT: Context, const INTERNAL_PAGE_SIZE: usize> MoonbootBoot<CONTEXT, INTERNAL_PAGE_SIZE> {
-        STORAGE: Storage,
-        STATE: State,
-        PROCESSOR: Processor,
-        EXCHANGE: Exchange,
-        const INTERNAL_PAGE_SIZE: usize,
-    > MoonbootBoot<STORAGE, STATE, PROCESSOR, EXCHANGE, INTERNAL_PAGE_SIZE>
-{
     /// create a new instance of the bootloader
     pub fn new(
         config: Config,
-        storage: STORAGE,
+        storage: CONTEXT::Storage,
-        state: STATE,
+        state: CONTEXT::State,
-        processor: PROCESSOR,
+        processor: CONTEXT::Processor,
-        exchange: EXCHANGE,
+        exchange: CONTEXT::Exchange,
-    ) -> MoonbootBoot<STORAGE, STATE, PROCESSOR, EXCHANGE, INTERNAL_PAGE_SIZE> {
+    ) -> Self {
         Self {
             config,
             storage,
@@ -53,12 +39,12 @@ impl<
     }
 
     /// Destroy this instance of the bootloader and return access to the hardware peripheral
-    pub fn destroy(self) -> (STORAGE, STATE, PROCESSOR) {
+    pub fn destroy(self) -> (CONTEXT::Storage, CONTEXT::State, CONTEXT::Processor) {
         (self.storage, self.state, self.processor)
     }
 
     /// Execute the update and boot logic of the bootloader
-    pub fn boot(&mut self) -> Result<void::Void, STATE::Error> {
+    pub fn boot(&mut self) -> Result<void::Void, <CONTEXT::State as State>::Error> {
         // TODO: consider error handling
         log::info!("Booting with moonboot!");
 
@@ -116,19 +102,20 @@ impl<
 
     // Handle a case of power interruption or similar, which lead to a exchange_banks being
     // interrupted.
-    fn handle_exchanging(&mut self, progress: ExchangeProgress) -> Result<Update, STATE::Error> {
+    fn handle_exchanging(
+        &mut self,
+        progress: ExchangeProgress,
+    ) -> Result<Update, <CONTEXT::State as State>::Error> {
         log::error!(
             "Firmware Update was interrupted! Trying to recover with exchange operation: {:?}",
             progress
         );
 
-        let exchange_result = self
+        let exchange_result = self.exchange.exchange::<INTERNAL_PAGE_SIZE>(
-            .exchange
+            &mut self.storage,
-            .exchange::<STORAGE, STATE, INTERNAL_PAGE_SIZE>(
+            &mut self.state,
-                &mut self.storage,
+            progress,
-                &mut self.state,
+        );
-                progress,
-            );
 
         Ok(if exchange_result.is_ok() {
             let state = self.state.read()?.update;
@@ -168,7 +155,17 @@ impl<
             );
 
             // Try to exchange the firmware images
-            let exchange_result = self.exchange_banks(new, old);
+            let exchange_result = self.exchange.exchange::<INTERNAL_PAGE_SIZE>(
+                &mut self.storage,
+                &mut self.state,
+                ExchangeProgress {
+                    a: new,
+                    b: old,
+                    page_index: 0,
+                    recovering: false,
+                    step: ExchangeStep::AToScratch,
+                },
+            );
             if exchange_result.is_ok() {
                 if with_failsafe_revert {
                     // Update Firmware Update State to revert. The Application will set this to
@@ -193,25 +190,6 @@ impl<
         }
     }
 
-    fn exchange_banks(
-        &mut self,
-        a: Bank,
-        b: Bank,
-    ) -> Result<(), ExchangeError<STORAGE::Error, STATE::Error, EXCHANGE::OtherError>> {
-        self.exchange
-            .exchange::<STORAGE, STATE, INTERNAL_PAGE_SIZE>(
-                &mut self.storage,
-                &mut self.state,
-                ExchangeProgress {
-                    a,
-                    b,
-                    page_index: 0,
-                    recovering: false,
-                    step: ExchangeStep::AToScratch,
-                },
-            )
-    }
-
     // Jump to the firmware image marked as bootable
     fn jump_to_firmware(&mut self) -> ! {
         let app_exec_image = self.config.boot_bank;

src/lib.rs

@@ -45,6 +45,13 @@ pub(crate) use defmt as log;
 #[cfg(feature = "use-log")]
 pub(crate) use logger_crate as log;
 
+pub trait Context {
+    type Storage: embedded_storage::Storage;
+    type State: state::State;
+    type Processor: hardware::processor::Processor;
+    type Exchange: state::Exchange<Self::Storage, Self::State>;
+}
+
 #[cfg(not(any(feature = "use-log", feature = "use-defmt")))]
 pub(crate) mod log {
     macro_rules! info {

src/manager/mod.rs

@@ -1,24 +1,18 @@
 use crate::{
     hardware::{processor::Processor, Config},
     state::{State, Update},
+    Context,
 };
 
-use embedded_storage::Storage;
-
 use crate::log;
 
 /// Instantiate this in your application to enable mutation of the State specified in this and jump
 /// to the bootloader to apply any updates.
-pub struct MoonbootManager<
+pub struct MoonbootManager<CONTEXT: Context, const INTERNAL_PAGE_SIZE: usize> {
-    STORAGE: Storage,
-    STATE: State,
-    PROCESSOR: Processor,
-    const INTERNAL_PAGE_SIZE: usize,
-> {
     config: Config,
-    storage: STORAGE,
+    storage: CONTEXT::Storage,
-    state: STATE,
+    state: CONTEXT::State,
-    processor: PROCESSOR,
+    processor: CONTEXT::Processor,
 }
 
 pub struct InitError;
@@ -28,24 +22,29 @@ pub enum MarkError<E> {
     State(E),
 }
 
-impl<STORAGE: Storage, STATE: State, PROCESSOR: Processor, const INTERNAL_PAGE_SIZE: usize>
+impl<CONTEXT: Context, const INTERNAL_PAGE_SIZE: usize>
-    MoonbootManager<STORAGE, STATE, PROCESSOR, INTERNAL_PAGE_SIZE>
+    MoonbootManager<CONTEXT, INTERNAL_PAGE_SIZE>
 {
     pub fn new(
         config: Config,
-        storage: STORAGE,
+        storage: CONTEXT::Storage,
-        state: STATE,
+        state: CONTEXT::State,
-        processor: PROCESSOR,
+        processor: CONTEXT::Processor,
-    ) -> Result<MoonbootManager<STORAGE, STATE, PROCESSOR, INTERNAL_PAGE_SIZE>, InitError> {
+    ) -> Result<Self, InitError> {
         if config.update_bank.size > config.boot_bank.size {
             log::error!(
                 "Requested update bank {:?} is larger than boot bank {:?}",
-                bank,
+                config.update_bank,
-                self.config.boot_bank
+                config.boot_bank
             );
             return Err(InitError);
         }
 
+        if config.update_bank.size == 0 || config.boot_bank.size == 0 {
+            log::error!("Requested banks are of zero size");
+            return Err(InitError);
+        }
+
         Ok(Self {
             config,
             storage,
@@ -55,14 +54,16 @@ impl<STORAGE: Storage, STATE: State, PROCESSOR: Processor, const INTERNAL_PAGE_S
     }
 
     /// Destroy this instance of the boot manager and return access to the hardware peripheral
-    pub fn destroy(self) -> (STORAGE, STATE, PROCESSOR) {
+    pub fn destroy(self) -> (CONTEXT::Storage, CONTEXT::State, CONTEXT::Processor) {
         (self.storage, self.state, self.processor)
     }
 
     /// Run this immediately after booting your new image successfully to mark the boot as
     /// succesful. If you do not do this, any reset will cause the bootloader to restore to the
     /// previous firmware image.
-    pub fn mark_boot_successful(&mut self) -> Result<(), MarkError<STATE::Error>> {
+    pub fn mark_boot_successful(
+        &mut self,
+    ) -> Result<(), MarkError<<CONTEXT::State as State>::Error>> {
         let mut current_state = self.state.read().map_err(MarkError::State)?;
 
         log::info!(
@@ -92,7 +93,7 @@ impl<STORAGE: Storage, STATE: State, PROCESSOR: Processor, const INTERNAL_PAGE_S
 
     // Upgrade firmware verifiying the given signature over the size of size.
     // Can only return an error or diverge (!, represented by Void while ! is not a type yet)
-    pub fn update(&mut self) -> Result<void::Void, STATE::Error> {
+    pub fn update(&mut self) -> Result<void::Void, <CONTEXT::State as State>::Error> {
         // Apply the update stored in the update bank
         let bank = self.config.update_bank;
 

src/state/mod.rs

@@ -4,28 +4,11 @@ pub mod ram;
 #[cfg(feature = "scratch-state")]
 pub mod scratch;
 
-use embedded_storage::Storage;
-
-pub enum ExchangeError<STORAGE, STATE, OTHER> {
-    Storage(STORAGE),
-    State(STATE),
-    Other(OTHER),
-}
-
-/// Abstraction for the exchange operation of the current state.
-pub trait Exchange {
-    type OtherError;
-
-    fn exchange<STORAGE: Storage, STATE: State, const INTERNAL_PAGE_SIZE: usize>(
-        &mut self,
-        internal_memory: &mut STORAGE,
-        state: &mut STATE,
-        progress: ExchangeProgress,
-    ) -> Result<(), ExchangeError<STORAGE::Error, STATE::Error, Self::OtherError>>;
-}
-
 use crate::hardware::Bank;
 
+use core::fmt::Debug;
+use embedded_storage::Storage;
+
 use crc::{Crc, CRC_32_CKSUM};
 #[cfg(feature = "defmt")]
 use defmt::Format;
@@ -34,6 +17,18 @@ use desse::{Desse, DesseSized};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
+/// Abstraction for the exchange operation of the current state.
+pub trait Exchange<STORAGE: Storage, STATE: State> {
+    type Error: Debug;
+
+    fn exchange<const INTERNAL_PAGE_SIZE: usize>(
+        &mut self,
+        storage: &mut STORAGE,
+        state: &mut STATE,
+        progress: ExchangeProgress,
+    ) -> Result<(), Self::Error>;
+}
+
 /// Decision making states for the bootloader
 // TODO: Hash + Signature? Should be done on download I think! This way, the algorithms can be
 // exchanged via software updates easily

src/state/ram.rs

@@ -58,15 +58,37 @@ impl State for RamState {
     }
 }
 
-impl Exchange for RamState {
+pub enum ExchangeError<STORAGE, STATE> {
-    type OtherError = void::Void;
+    Storage(STORAGE),
+    State(STATE),
+}
 
-    fn exchange<STORAGE: Storage, STATE: State, const INTERNAL_PAGE_SIZE: usize>(
+impl<STORAGE, STATE> Debug for ExchangeError<STORAGE, STATE>
+where
+    STORAGE: Debug,
+    STATE: Debug,
+{
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            Self::Storage(arg0) => f.debug_tuple("Storage").field(arg0).finish(),
+            Self::State(arg0) => f.debug_tuple("State").field(arg0).finish(),
+        }
+    }
+}
+
+impl<STORAGE: Storage, STATE: State> Exchange<STORAGE, STATE> for RamState
+where
+    STORAGE::Error: Debug,
+    STATE::Error: Debug,
+{
+    type Error = ExchangeError<STORAGE::Error, STATE::Error>;
+
+    fn exchange<const INTERNAL_PAGE_SIZE: usize>(
         &mut self,
         storage: &mut STORAGE,
         state: &mut STATE,
         progress: ExchangeProgress,
-    ) -> Result<(), ExchangeError<STORAGE::Error, STATE::Error, Self::OtherError>> {
+    ) -> Result<(), Self::Error> {
         let ExchangeProgress {
             a,
             b,
@@ -75,6 +97,10 @@ impl Exchange for RamState {
             ..
         } = progress;
 
+        assert_eq!(a.size, b.size);
+        assert_ne!(a.size, 0);
+        assert_ne!(b.size, 0);
+
         let size = a.size; // Both are equal
 
         let full_pages = size / INTERNAL_PAGE_SIZE as Address;

src/state/scratch.rs

@@ -1,11 +1,10 @@
-use core::ops::Range;
+use core::{fmt::Debug, ops::Range};
 
 use embedded_storage::Storage;
 
 use crate::{
     log,
-    state::{ExchangeProgress, ExchangeStep, State, Update},
+    state::{Exchange, ExchangeProgress, ExchangeStep, State, Update},
-    swap::{MemoryError, Swap},
     Address,
 };
 
@@ -13,29 +12,48 @@ pub struct Scratch<'a> {
     pub pages: &'a [Range<Address>],
 }
 
-impl<'a> Swap for Scratch<'a> {
+pub enum ExchangeError<STORAGE, STATE> {
-    fn exchange<InternalMemory: Storage, HardwareState: State, const INTERNAL_PAGE_SIZE: usize>(
+    Storage(STORAGE),
+    State(STATE),
+}
+
+impl<STORAGE, STATE> Debug for ExchangeError<STORAGE, STATE>
+where
+    STORAGE: Debug,
+    STATE: Debug,
+{
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            Self::Storage(arg0) => f.debug_tuple("Storage").field(arg0).finish(),
+            Self::State(arg0) => f.debug_tuple("State").field(arg0).finish(),
+        }
+    }
+}
+
+impl<'a, STORAGE: Storage, STATE: State> Exchange<STORAGE, STATE> for Scratch<'a>
+where
+    STORAGE::Error: Debug,
+    STATE::Error: Debug,
+{
+    type Error = ExchangeError<STORAGE::Error, STATE::Error>;
+
+    fn exchange<const INTERNAL_PAGE_SIZE: usize>(
         &mut self,
-        internal_memory: &mut InternalMemory,
+        storage: &mut STORAGE,
-        state: &mut HardwareState,
+        state: &mut STATE,
-        exchange: ExchangeProgress,
+        progress: ExchangeProgress,
-    ) -> Result<(), MemoryError> {
+    ) -> Result<(), Self::Error> {
         let ExchangeProgress {
             a,
             b,
             page_index,
             mut step,
             ..
-        } = exchange;
+        } = progress;
 
-        // TODO: Sanity Check start_index
+        assert_eq!(a.size, b.size);
-        if a.size != b.size {
+        assert_ne!(a.size, 0);
-            return Err(MemoryError::BankSizeNotEqual);
+        assert_ne!(b.size, 0);
-        }
-
-        if a.size == 0 || b.size == 0 {
-            return Err(MemoryError::BankSizeZero);
-        }
 
         let size = a.size; // Both are equal
 
@@ -46,7 +64,7 @@ impl<'a> Swap for Scratch<'a> {
 
         let mut ram_buf = [0_u8; INTERNAL_PAGE_SIZE];
 
-        let mut last_state = state.read();
+        let mut last_state = state.read().map_err(ExchangeError::State)?;
 
         // Set this in the exchanging part to know whether we are in a recovery process from a
         // failed update or on the initial update
@@ -85,37 +103,35 @@ impl<'a> Swap for Scratch<'a> {
                         page_index,
                         step,
                     });
-                    state
+                    state.write(&last_state).map_err(ExchangeError::State)?;
-                        .write(&last_state)
-                        .map_err(|_| MemoryError::WriteFailure)?;
                 }
 
                 match step {
                     ExchangeStep::AToScratch => {
-                        internal_memory
+                        storage
                             .read(a_location, &mut ram_buf)
-                            .map_err(|_| MemoryError::ReadFailure)?;
+                            .map_err(ExchangeError::Storage)?;
-                        internal_memory
+                        storage
                             .write(scratch_location, &ram_buf)
-                            .map_err(|_| MemoryError::WriteFailure)?;
+                            .map_err(ExchangeError::Storage)?;
                         step = ExchangeStep::BToA;
                     }
                     ExchangeStep::BToA => {
-                        internal_memory
+                        storage
                             .read(b_location, &mut ram_buf)
-                            .map_err(|_| MemoryError::ReadFailure)?;
+                            .map_err(ExchangeError::Storage)?;
-                        internal_memory
+                        storage
                             .write(a_location, &ram_buf)
-                            .map_err(|_| MemoryError::WriteFailure)?;
+                            .map_err(ExchangeError::Storage)?;
                         step = ExchangeStep::ScratchToB;
                     }
                     ExchangeStep::ScratchToB => {
-                        internal_memory
+                        storage
                             .read(scratch_location, &mut ram_buf)
-                            .map_err(|_| MemoryError::ReadFailure)?;
+                            .map_err(ExchangeError::Storage)?;
-                        internal_memory
+                        storage
                             .write(b_location, &ram_buf)
-                            .map_err(|_| MemoryError::WriteFailure)?;
+                            .map_err(ExchangeError::Storage)?;
                         step = ExchangeStep::AToScratch;
                         break;
                     }