Skip to content

Commit

Permalink
API improvements (#512)
Browse files Browse the repository at this point in the history 
* refactor: Use structs to simplify some functions args

* refactor: Refactor FlashData so it can be reused in other functions

* style: Remove unnecesary comments

* docs: Udpate changelog

* fix: Fix rebase errors

* fix: Rebase errors
  • Loading branch information
@SergioGasquez
SergioGasquez authored Jan 9, 2024
1 parent b52303e commit 3d91a9b
Show file tree
Hide file tree
Showing 16 changed files with 276 additions and 401 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add `Serialize` and `Deserialize` to `FlashFrequency`, `FlashMode` and `FlashSize`. (#528)
- Add `checksum-md5` command (#536)
- Add verify and skipping of unchanged flash regions - add `--no-verify` and `--no-skip` (#538)
- Add --min-chip-rev argument to specify minimum chip revision (#252)

### Fixed

Expand All @@ -27,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed a missed `flush` call that may be causing communication errors (#521)

### Changed
- Created `FlashData` and `FlashSettings` structs to reduce number of input arguments in some functions (#512)

- espflash will now exit with an error if `defmt` is selected but not usable (#524)

Expand Down
62 changes: 35 additions & 27 deletions cargo-espflash/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use espflash::{
FlashConfigArgs, MonitorArgs, PartitionTableArgs,
},
error::Error as EspflashError,
flasher::{FlashData, FlashSettings},
image_format::ImageFormatKind,
logging::initialize_logger,
targets::Chip,
Expand Down Expand Up @@ -313,33 +314,32 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
println!("Partition table: {}", path.display());
}

let partition_table = match partition_table {
Some(path) => Some(parse_partition_table(path)?),
None => None,
};
let flash_settings = FlashSettings::new(
args.build_args.flash_config_args.flash_mode,
args.build_args.flash_config_args.flash_size,
args.build_args.flash_config_args.flash_freq,
);

let flash_data = FlashData::new(
bootloader,
partition_table,
args.flash_args.partition_table_offset,
args.flash_args.format.or(metadata.format),
args.flash_args.target_app_partition,
flash_settings,
args.flash_args.min_chip_rev,
)?;

if args.flash_args.erase_parts.is_some() || args.flash_args.erase_data_parts.is_some() {
erase_partitions(
&mut flasher,
partition_table.clone(),
flash_data.partition_table.clone(),
args.flash_args.erase_parts,
args.flash_args.erase_data_parts,
)?;
}

flash_elf_image(
&mut flasher,
&elf_data,
bootloader,
partition_table,
args.flash_args.target_app_partition,
args.flash_args.format.or(metadata.format),
args.build_args.flash_config_args.flash_mode,
args.build_args.flash_config_args.flash_size,
args.build_args.flash_config_args.flash_freq,
args.flash_args.partition_table_offset,
args.flash_args.min_chip_rev,
)?;
flash_elf_image(&mut flasher, &elf_data, flash_data)?;
}

if args.flash_args.monitor {
Expand Down Expand Up @@ -564,20 +564,28 @@ fn save_image(args: SaveImageArgs) -> Result<()> {
println!("Partition table: {}", path.display());
}

save_elf_as_image(
args.save_image_args.chip,
args.save_image_args.min_chip_rev,
&elf_data,
args.save_image_args.file,
args.format.or(metadata.format),
let flash_settings = FlashSettings::new(
args.build_args.flash_config_args.flash_mode,
args.build_args.flash_config_args.flash_size,
args.build_args.flash_config_args.flash_freq,
);

let flash_data = FlashData::new(
bootloader.as_deref(),
partition_table.as_deref(),
args.save_image_args.partition_table_offset,
args.save_image_args.merge,
bootloader,
partition_table,
args.format.or(metadata.format),
args.save_image_args.target_app_partition,
flash_settings,
args.save_image_args.min_chip_rev,
)?;

save_elf_as_image(
&elf_data,
args.save_image_args.chip,
args.save_image_args.file,
flash_data,
args.save_image_args.merge,
args.save_image_args.skip_padding,
)?;

Expand Down
62 changes: 35 additions & 27 deletions espflash/src/bin/espflash.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use espflash::{
EspflashProgress, FlashConfigArgs, MonitorArgs, PartitionTableArgs,
},
error::Error,
flasher::{FlashData, FlashSettings},
image_format::ImageFormatKind,
logging::initialize_logger,
targets::Chip,
Expand Down Expand Up @@ -237,33 +238,32 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
println!("Partition table: {}", path.display());
}

let partition_table = match partition_table {
Some(path) => Some(parse_partition_table(path)?),
None => None,
};
let flash_settings = FlashSettings::new(
args.flash_config_args.flash_mode,
args.flash_config_args.flash_size,
args.flash_config_args.flash_freq,
);

let flash_data = FlashData::new(
bootloader,
partition_table,
args.flash_args.partition_table_offset,
args.flash_args.format,
args.flash_args.target_app_partition,
flash_settings,
args.flash_args.min_chip_rev,
)?;

if args.flash_args.erase_parts.is_some() || args.flash_args.erase_data_parts.is_some() {
erase_partitions(
&mut flasher,
partition_table.clone(),
flash_data.partition_table.clone(),
args.flash_args.erase_parts,
args.flash_args.erase_data_parts,
)?;
}

flash_elf_image(
&mut flasher,
&elf_data,
bootloader,
partition_table,
args.flash_args.target_app_partition,
args.flash_args.format,
args.flash_config_args.flash_mode,
args.flash_config_args.flash_size,
args.flash_config_args.flash_freq,
args.flash_args.partition_table_offset,
args.flash_args.min_chip_rev,
)?;
flash_elf_image(&mut flasher, &elf_data, flash_data)?;
}

if args.flash_args.monitor {
Expand Down Expand Up @@ -309,20 +309,28 @@ fn save_image(args: SaveImageArgs) -> Result<()> {
println!("Partition table: {}", path.display());
}

save_elf_as_image(
args.save_image_args.chip,
args.save_image_args.min_chip_rev,
&elf_data,
args.save_image_args.file,
args.format,
let flash_settings = FlashSettings::new(
args.flash_config_args.flash_mode,
args.flash_config_args.flash_size,
args.flash_config_args.flash_freq,
);

let flash_data = FlashData::new(
args.save_image_args.bootloader.as_deref(),
args.save_image_args.partition_table.as_deref(),
args.save_image_args.partition_table_offset,
args.save_image_args.merge,
args.save_image_args.bootloader,
args.save_image_args.partition_table,
args.format,
args.save_image_args.target_app_partition,
flash_settings,
args.save_image_args.min_chip_rev,
)?;

save_elf_as_image(
&elf_data,
args.save_image_args.chip,
args.save_image_args.file,
flash_data,
args.save_image_args.merge,
args.save_image_args.skip_padding,
)?;

Expand Down
114 changes: 13 additions & 101 deletions espflash/src/cli/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use self::{
use crate::{
elf::ElfFirmwareImage,
error::{Error, MissingPartition, MissingPartitionTable},
flasher::{FlashFrequency, FlashMode, FlashSize, Flasher, ProgressCallbacks},
flasher::{FlashData, FlashFrequency, FlashMode, FlashSize, Flasher, ProgressCallbacks},
image_format::ImageFormatKind,
interface::Interface,
targets::Chip,
Expand Down Expand Up @@ -418,69 +418,21 @@ pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {

/// Convert the provided firmware image from ELF to binary
pub fn save_elf_as_image(
chip: Chip,
min_rev_full: u16,
elf_data: &[u8],
chip: Chip,
image_path: PathBuf,
image_format: Option<ImageFormatKind>,
flash_mode: Option<FlashMode>,
flash_size: Option<FlashSize>,
flash_freq: Option<FlashFrequency>,
partition_table_offset: Option<u32>,
flash_data: FlashData,
merge: bool,
bootloader_path: Option<PathBuf>,
partition_table_path: Option<PathBuf>,
target_app_partition: Option<String>,
skip_padding: bool,
) -> Result<()> {
let image = ElfFirmwareImage::try_from(elf_data)?;

if merge {
// merge_bin is TRUE
// merge bootloader, partition table and app binaries
// basic functionality, only merge 3 binaries

// If the '-B' option is provided, load the bootloader binary file at the
// specified path.
let bootloader = if let Some(bootloader_path) = bootloader_path {
let path = fs::canonicalize(bootloader_path).into_diagnostic()?;
let data = fs::read(path).into_diagnostic()?;

Some(data)
} else {
None
};

// If the '-T' option is provided, load the partition table from
// the CSV or binary file at the specified path.
let partition_table = if let Some(partition_table_path) = partition_table_path {
let path = fs::canonicalize(partition_table_path).into_diagnostic()?;
let data = fs::read(path)
.into_diagnostic()
.wrap_err("Failed to open partition table")?;

let table = PartitionTable::try_from(data).into_diagnostic()?;

Some(table)
} else {
None
};

// To get a chip revision, the connection is needed
// For simplicity, the revision None is used
let image = chip.into_target().get_flash_image(
&image,
bootloader,
partition_table,
target_app_partition,
image_format,
None,
min_rev_full,
flash_mode,
flash_size,
flash_freq,
partition_table_offset,
)?;
let image = chip
.into_target()
.get_flash_image(&image, flash_data.clone(), None)?;

display_image_size(image.app_size(), image.part_size());

Expand All @@ -505,25 +457,16 @@ pub fn save_elf_as_image(
// Take flash_size as input parameter, if None, use default value of 4Mb
let padding_bytes = vec![
0xffu8;
flash_size.unwrap_or_default().size() as usize
flash_data.flash_settings.size.unwrap_or_default().size()
as usize
- file.metadata().into_diagnostic()?.len() as usize
];
file.write_all(&padding_bytes).into_diagnostic()?;
}
} else {
let image = chip.into_target().get_flash_image(
&image,
None,
None,
None,
image_format,
None,
min_rev_full,
flash_mode,
flash_size,
flash_freq,
partition_table_offset,
)?;
let image = chip
.into_target()
.get_flash_image(&image, flash_data, None)?;

display_image_size(image.app_size(), image.part_size());

Expand Down Expand Up @@ -629,42 +572,11 @@ pub fn erase_region(args: EraseRegionArgs, config: &Config) -> Result<()> {
pub fn flash_elf_image(
flasher: &mut Flasher,
elf_data: &[u8],
bootloader: Option<&Path>,
partition_table: Option<PartitionTable>,
target_app_partition: Option<String>,
image_format: Option<ImageFormatKind>,
flash_mode: Option<FlashMode>,
flash_size: Option<FlashSize>,
flash_freq: Option<FlashFrequency>,
partition_table_offset: Option<u32>,
min_rev_full: u16,
flash_data: FlashData,
) -> Result<()> {
// If the '--bootloader' option is provided, load the binary file at the
// specified path.
let bootloader = if let Some(path) = bootloader {
let path = fs::canonicalize(path).into_diagnostic()?;
let data = fs::read(path).into_diagnostic()?;

Some(data)
} else {
None
};

// Load the ELF data, optionally using the provider bootloader/partition
// table/image format, to the device's flash memory.
flasher.load_elf_to_flash_with_format(
elf_data,
bootloader,
partition_table,
target_app_partition,
image_format,
flash_mode,
flash_size,
flash_freq,
partition_table_offset,
min_rev_full,
Some(&mut EspflashProgress::default()),
)?;
flasher.load_elf_to_flash(elf_data, flash_data, Some(&mut EspflashProgress::default()))?;
info!("Flashing has completed!");

Ok(())
Expand Down
Loading

0 comments on commit 3d91a9b

Please sign in to comment.