Merge pull request #97 from jessebraham/fixes/partition-table

Fix bug where partition table offset is not used

Author: icewind1991

espflash/src/error.rs

@@ -1,14 +1,20 @@
-use crate::command::CommandType;
-use crate::image_format::ImageFormatId;
-use crate::partition_table::{SubType, Type};
-use crate::Chip;
+use std::{
+    fmt::{Display, Formatter},
+    io,
+};
+
 use miette::{Diagnostic, SourceOffset, SourceSpan};
 use slip_codec::SlipError;
-use std::fmt::{Display, Formatter};
-use std::io;
 use strum::VariantNames;
 use thiserror::Error;
 
+use crate::{
+    command::CommandType,
+    image_format::ImageFormatId,
+    partition_table::{SubType, Type},
+    Chip,
+};
+
 #[derive(Error, Debug, Diagnostic)]
 #[non_exhaustive]
 pub enum Error {
@@ -308,6 +314,9 @@ pub enum PartitionTableError {
     InvalidSubType(#[from] InvalidSubTypeError),
     #[error(transparent)]
     #[diagnostic(transparent)]
+    NoFactoryApp(#[from] NoFactoryAppError),
+    #[error(transparent)]
+    #[diagnostic(transparent)]
     UnalignedPartitionError(#[from] UnalignedPartitionError),
 }
 
@@ -459,6 +468,25 @@ impl InvalidSubTypeError {
     }
 }
 
+#[derive(Debug, Error, Diagnostic)]
+#[error("No factory app partition was found")]
+#[diagnostic(
+    code(espflash::partition_table::no_factory_app),
+    help("Partition table must contain a factory app partition")
+)]
+pub struct NoFactoryAppError {
+    #[source_code]
+    source_code: String,
+}
+
+impl NoFactoryAppError {
+    pub fn new(source: &str) -> Self {
+        NoFactoryAppError {
+            source_code: source.into(),
+        }
+    }
+}
+
 #[derive(Debug, Error, Diagnostic)]
 #[error("Unaligned partition")]
 #[diagnostic(code(espflash::partition_table::unaligned))]

espflash/src/image_format/esp32bootloader.rs

@@ -121,10 +121,16 @@ impl<'a> Esp32BootloaderFormat<'a> {
         let hash = hasher.finalize();
         data.write_all(&hash)?;
 
+        // The default partition table contains the "factory" partition, and if a user
+        // provides a partition table via command-line then the validation step confirms
+        // this is present, so it's safe to unwrap.
+        let factory_partition = partition_table.find("factory").unwrap();
+
         let flash_segment = RomSegment {
-            addr: params.app_addr,
+            addr: factory_partition.offset(),
             data: Cow::Owned(data),
         };
+
         Ok(Self {
             params,
             bootloader,

espflash/src/partition_table.rs

@@ -1,15 +1,18 @@
-use crate::error::{
-    CSVError, DuplicatePartitionsError, InvalidSubTypeError, OverlappingPartitionsError,
-    PartitionTableError, UnalignedPartitionError,
+use std::{
+    cmp::{max, min},
+    fmt::{Display, Formatter, Write as _},
+    io::Write,
+    ops::Rem,
 };
+
 use md5::{Context, Digest};
 use regex::Regex;
 use serde::{Deserialize, Deserializer, Serialize};
-use std::cmp::{max, min};
-use std::fmt::Write as _;
-use std::fmt::{Display, Formatter};
-use std::io::Write;
-use std::ops::Rem;
+
+use crate::error::{
+    CSVError, DuplicatePartitionsError, InvalidSubTypeError, NoFactoryAppError,
+    OverlappingPartitionsError, PartitionTableError, UnalignedPartitionError,
+};
 
 const MAX_PARTITION_LENGTH: usize = 0xC00;
 const PARTITION_TABLE_SIZE: usize = 0x1000;
@@ -28,16 +31,9 @@ impl Type {
         match self {
             Type::App => "'factory', 'ota_0' through 'ota_15' and 'test'".into(),
             Type::Data => {
+                use DataType::*;
                 let types = [
-                    DataType::Ota,
-                    DataType::Phy,
-                    DataType::Nvs,
-                    DataType::CoreDump,
-                    DataType::NvsKeys,
-                    DataType::EFuse,
-                    DataType::EspHttpd,
-                    DataType::Fat,
-                    DataType::Spiffs,
+                    Ota, Phy, Nvs, CoreDump, NvsKeys, EFuse, EspHttpd, Fat, Spiffs,
                 ];
 
                 let mut out = format!("'{}'", serde_plain::to_string(&types[0]).unwrap());
@@ -57,8 +53,7 @@ impl Type {
 
 impl Display for Type {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        let ser = serde_plain::to_string(self).unwrap();
-        write!(f, "{}", ser)
+        write!(f, "{}", serde_plain::to_string(self).unwrap())
     }
 }
 
@@ -246,13 +241,18 @@ impl PartitionTable {
         Ok(())
     }
 
+    pub fn find(&self, name: &str) -> Option<&Partition> {
+        self.partitions.iter().find(|&p| p.name == name)
+    }
+
     fn validate(&self, source: &str) -> Result<(), PartitionTableError> {
         for partition in &self.partitions {
             if let Some(line) = &partition.line {
                 let expected_type = match partition.sub_type {
                     SubType::App(_) => Type::App,
                     SubType::Data(_) => Type::Data,
                 };
+
                 if expected_type != partition.ty {
                     return Err(InvalidSubTypeError::new(
                         source,
@@ -262,6 +262,7 @@ impl PartitionTable {
                     )
                     .into());
                 }
+
                 if partition.ty == Type::App && partition.offset.rem(0x10000) != 0 {
                     return Err(UnalignedPartitionError::new(source, *line).into());
                 }
@@ -277,12 +278,14 @@ impl PartitionTable {
                                 OverlappingPartitionsError::new(source, *line1, *line2).into()
                             );
                         }
+
                         if partition1.name == partition2.name {
                             return Err(DuplicatePartitionsError::new(
                                 source, *line1, *line2, "name",
                             )
                             .into());
                         }
+
                         if partition1.sub_type == partition2.sub_type {
                             return Err(DuplicatePartitionsError::new(
                                 source, *line1, *line2, "sub-type",
@@ -294,14 +297,20 @@ impl PartitionTable {
             }
         }
 
+        if self.find("factory").is_none() {
+            return Err(PartitionTableError::NoFactoryApp(NoFactoryAppError::new(
+                source,
+            )));
+        }
+
         Ok(())
     }
 }
 
 const PARTITION_SIZE: usize = 32;
 
 #[derive(Debug, Deserialize)]
-struct Partition {
+pub struct Partition {
     #[serde(deserialize_with = "deserialize_partition_name")]
     name: String,
     ty: Type,
@@ -358,6 +367,10 @@ impl Partition {
         Ok(())
     }
 
+    pub fn offset(&self) -> u32 {
+        self.offset
+    }
+
     fn overlaps(&self, other: &Partition) -> bool {
         max(self.offset, other.offset) < min(self.offset + self.size, other.offset + other.size)
     }