Merge #1322

1322: Remove FirmwareWriter r=Dirbaio a=rmja

FirmwareWriter currently has a "max-write-size" parameter, but this is a limitation that should be handled by chunking inside the NorFlash driver, and not "up here" in user code. In case that the driver (e.g. qspi driver) is unaware of any max-write limitations, one could simply add an intermediate NorFlash adapter providing the chunk'ing capability.

Co-authored-by: Rasmus Melchior Jacobsen <rmja@laesoe.org>

embassy-boot/boot/src/firmware_updater.rs

@@ -1,7 +1,7 @@
 use embedded_storage::nor_flash::{NorFlash, NorFlashError, NorFlashErrorKind};
 use embedded_storage_async::nor_flash::NorFlash as AsyncNorFlash;
 
-use crate::{FirmwareWriter, Partition, State, BOOT_MAGIC, SWAP_MAGIC};
+use crate::{Partition, State, BOOT_MAGIC, SWAP_MAGIC};
 
 /// Errors returned by FirmwareUpdater
 #[derive(Debug)]
@@ -243,7 +243,6 @@ impl FirmwareUpdater {
         offset: usize,
         data: &[u8],
         dfu_flash: &mut F,
-        block_size: usize,
     ) -> Result<(), FirmwareUpdaterError> {
         assert!(data.len() >= F::ERASE_SIZE);
 
@@ -251,25 +250,23 @@ impl FirmwareUpdater {
             .erase(dfu_flash, offset as u32, (offset + data.len()) as u32)
             .await?;
 
-        FirmwareWriter(self.dfu)
+        self.dfu.write(dfu_flash, offset as u32, data).await?;
-            .write_block(offset, data, dfu_flash, block_size)
-            .await?;
 
         Ok(())
     }
 
     /// Prepare for an incoming DFU update by erasing the entire DFU area and
-    /// returning a `FirmwareWriter`.
+    /// returning its `Partition`.
     ///
     /// Using this instead of `write_firmware` allows for an optimized API in
     /// exchange for added complexity.
     pub async fn prepare_update<F: AsyncNorFlash>(
         &mut self,
         dfu_flash: &mut F,
-    ) -> Result<FirmwareWriter, FirmwareUpdaterError> {
+    ) -> Result<Partition, FirmwareUpdaterError> {
         self.dfu.wipe(dfu_flash).await?;
 
-        Ok(FirmwareWriter(self.dfu))
+        Ok(self.dfu)
     }
 
     //
@@ -443,29 +440,25 @@ impl FirmwareUpdater {
         offset: usize,
         data: &[u8],
         dfu_flash: &mut F,
-        block_size: usize,
     ) -> Result<(), FirmwareUpdaterError> {
         assert!(data.len() >= F::ERASE_SIZE);
 
         self.dfu
             .erase_blocking(dfu_flash, offset as u32, (offset + data.len()) as u32)?;
 
-        FirmwareWriter(self.dfu).write_block_blocking(offset, data, dfu_flash, block_size)?;
+        self.dfu.write_blocking(dfu_flash, offset as u32, data)?;
 
         Ok(())
     }
 
     /// Prepare for an incoming DFU update by erasing the entire DFU area and
-    /// returning a `FirmwareWriter`.
+    /// returning its `Partition`.
     ///
     /// Using this instead of `write_firmware_blocking` allows for an optimized
     /// API in exchange for added complexity.
-    pub fn prepare_update_blocking<F: NorFlash>(
+    pub fn prepare_update_blocking<F: NorFlash>(&mut self, flash: &mut F) -> Result<Partition, FirmwareUpdaterError> {
-        &mut self,
-        flash: &mut F,
-    ) -> Result<FirmwareWriter, FirmwareUpdaterError> {
         self.dfu.wipe_blocking(flash)?;
 
-        Ok(FirmwareWriter(self.dfu))
+        Ok(self.dfu)
     }
 }

embassy-boot/boot/src/firmware_writer.rs

@@ -1,55 +0,0 @@
-use embedded_storage::nor_flash::NorFlash;
-use embedded_storage_async::nor_flash::NorFlash as AsyncNorFlash;
-
-use crate::Partition;
-
-/// FirmwareWriter allows writing blocks to an already erased flash.
-pub struct FirmwareWriter(pub(crate) Partition);
-
-impl FirmwareWriter {
-    /// Write data to a flash page.
-    ///
-    /// The buffer must follow alignment requirements of the target flash and a multiple of page size big.
-    ///
-    /// # Safety
-    ///
-    /// Failing to meet alignment and size requirements may result in a panic.
-    pub async fn write_block<F: AsyncNorFlash>(
-        &mut self,
-        offset: usize,
-        data: &[u8],
-        flash: &mut F,
-        block_size: usize,
-    ) -> Result<(), F::Error> {
-        let mut offset = offset as u32;
-        for chunk in data.chunks(block_size) {
-            self.0.write(flash, offset, chunk).await?;
-            offset += chunk.len() as u32;
-        }
-
-        Ok(())
-    }
-
-    /// Write data to a flash page.
-    ///
-    /// The buffer must follow alignment requirements of the target flash and a multiple of page size big.
-    ///
-    /// # Safety
-    ///
-    /// Failing to meet alignment and size requirements may result in a panic.
-    pub fn write_block_blocking<F: NorFlash>(
-        &mut self,
-        offset: usize,
-        data: &[u8],
-        flash: &mut F,
-        block_size: usize,
-    ) -> Result<(), F::Error> {
-        let mut offset = offset as u32;
-        for chunk in data.chunks(block_size) {
-            self.0.write_blocking(flash, offset, chunk)?;
-            offset += chunk.len() as u32;
-        }
-
-        Ok(())
-    }
-}

embassy-boot/boot/src/lib.rs

@@ -7,12 +7,10 @@ mod fmt;
 
 mod boot_loader;
 mod firmware_updater;
-mod firmware_writer;
 mod partition;
 
 pub use boot_loader::{BootError, BootFlash, BootLoader, Flash, FlashConfig, MultiFlashConfig, SingleFlashConfig};
 pub use firmware_updater::{FirmwareUpdater, FirmwareUpdaterError};
-pub use firmware_writer::FirmwareWriter;
 pub use partition::Partition;
 
 pub(crate) const BOOT_MAGIC: u8 = 0xD0;
@@ -109,7 +107,7 @@ mod tests {
         let mut updater = FirmwareUpdater::new(DFU, STATE);
         let mut offset = 0;
         for chunk in update.chunks(4096) {
-            block_on(updater.write_firmware(offset, chunk, &mut flash, 4096)).unwrap();
+            block_on(updater.write_firmware(offset, chunk, &mut flash)).unwrap();
             offset += chunk.len();
         }
         block_on(updater.mark_updated(&mut flash, &mut aligned)).unwrap();
@@ -182,7 +180,7 @@ mod tests {
 
         let mut offset = 0;
         for chunk in update.chunks(2048) {
-            block_on(updater.write_firmware(offset, chunk, &mut dfu, chunk.len())).unwrap();
+            block_on(updater.write_firmware(offset, chunk, &mut dfu)).unwrap();
             offset += chunk.len();
         }
         block_on(updater.mark_updated(&mut state, &mut aligned)).unwrap();
@@ -235,7 +233,7 @@ mod tests {
 
         let mut offset = 0;
         for chunk in update.chunks(4096) {
-            block_on(updater.write_firmware(offset, chunk, &mut dfu, chunk.len())).unwrap();
+            block_on(updater.write_firmware(offset, chunk, &mut dfu)).unwrap();
             offset += chunk.len();
         }
         block_on(updater.mark_updated(&mut state, &mut aligned)).unwrap();