Skip to content

Commit 56a23a7

Browse files
SergioGasquezjessebraham
SergioGasquez
and
jessebraham
authored
Refactor image format (#877)
* Begin making it possible to add additional image formats in the future * Add a method to return app image metadata, fix(?) size printing code * Fix clippy warning * feat: Remove Esp32Params * feat: Esp-idf only stuff * feat: Initial CLI changes * feat: Avoid code duplication * feat: Remove the option to pass partition name to write bin * feat: Allow image format args in configuration * feat: Reduce IdfBootloaderFormat arguments * feat: Remove flash_image and rely on ImageFormat * tests: Remove write-bin to a partition name test * style: Rename methods * feat: Check erase-parts arguments * docs: Remove todo * feat: Simplify parse_partition_table * feat: Improve docstrings, rename bootloader method * feat: Update visivility and create a getter for partition-table * docs: Add changelog entries * docs: Remove esp-idf only annotations * feat: Implement Chip::default_crystal_frequency * docs: Remove unnecesary todo * docs: Add docstrings for new Chip methods * docs: Add missing docstrings * style: Fix rustfmt --------- Co-authored-by: Jesse Braham <[email protected]>
1 parent ce0ad9d commit 56a23a7

File tree

19 files changed

+641
-834
lines changed

19 files changed

+641
-834
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4747
- Updated to Rust 2024 edition (#843)
4848
- Complete rework of reading eFuse field values (#847)
4949
- Updated bootloaders with `release/v5.4` ones from IDF (#857)
50+
- Refactor image formatting to allow supporting more image formats in a backward compatible way (#877)
51+
- Avoid having ESP-IDF format assumptions in the codebase (#877)
5052

5153
### Fixed
5254

‎cargo-espflash/src/main.rs‎

Lines changed: 68 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ use espflash::{
1515
*,
1616
},
1717
flasher::FlashSize,
18-
image_format::check_idf_bootloader,
18+
image_format::{
19+
ImageFormat,
20+
ImageFormatKind,
21+
check_idf_bootloader,
22+
esp_idf::parse_partition_table,
23+
},
1924
logging::initialize_logger,
2025
targets::{Chip, XtalFrequency},
2126
update::check_for_update,
@@ -198,6 +203,12 @@ struct FlashArgs {
198203
connect_args: ConnectArgs,
199204
#[clap(flatten)]
200205
flash_args: cli::FlashArgs,
206+
/// Application image format to use
207+
#[clap(long, default_value = "esp-idf")]
208+
format: ImageFormatKind,
209+
/// ESP-IDF format arguments
210+
#[clap(flatten)]
211+
esp_idf_format_args: cli::EspIdfFormatArgs,
201212
}
202213

203214
#[derive(Debug, Args)]
@@ -207,6 +218,12 @@ struct SaveImageArgs {
207218
build_args: BuildArgs,
208219
#[clap(flatten)]
209220
save_image_args: cli::SaveImageArgs,
221+
/// Application image format to use
222+
#[clap(long, default_value = "esp-idf")]
223+
format: ImageFormatKind,
224+
/// ESP-IDF format arguments
225+
#[clap(flatten)]
226+
esp_idf_format_args: cli::EspIdfFormatArgs,
210227
}
211228

212229
fn main() -> Result<()> {
@@ -265,11 +282,11 @@ pub fn erase_parts(args: ErasePartsArgs, config: &Config) -> Result<()> {
265282
return Err(EspflashError::StubRequired).into_diagnostic();
266283
}
267284

268-
let partition_table = args
285+
let partition_table = args.partition_table.as_deref().or(config
286+
.project_config
287+
.esp_idf_format_args
269288
.partition_table
270-
.as_deref()
271-
.or(config.project_config.partition_table.as_deref());
272-
289+
.as_deref());
273290
let mut flasher = connect(&args.connect_args, config, false, false)?;
274291
let chip = flasher.chip();
275292
let partition_table = match partition_table {
@@ -328,6 +345,11 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
328345
monitor_args.elf = Some(build_ctx.artifact_path.clone());
329346

330347
check_monitor_args(&args.flash_args.monitor, &monitor_args)?;
348+
check_esp_idf_args(
349+
args.format,
350+
&args.flash_args.erase_parts,
351+
&args.flash_args.erase_data_parts,
352+
)?;
331353

332354
print_board_info(&mut flasher)?;
333355
ensure_chip_compatibility(chip, Some(elf_data.as_slice()))?;
@@ -346,20 +368,32 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
346368
args.flash_args.image,
347369
&flash_config,
348370
config,
349-
build_ctx.bootloader_path.as_deref(),
350-
build_ctx.partition_table_path.as_deref(),
371+
chip,
372+
target_xtal_freq,
373+
);
374+
let image_format = make_image_format(
375+
&elf_data,
376+
&flash_data,
377+
args.format,
378+
config,
379+
Some(args.esp_idf_format_args),
380+
build_ctx.bootloader_path,
381+
build_ctx.partition_table_path,
351382
)?;
352383

353-
if args.flash_args.erase_parts.is_some() || args.flash_args.erase_data_parts.is_some() {
354-
erase_partitions(
355-
&mut flasher,
356-
flash_data.partition_table.clone(),
357-
args.flash_args.erase_parts,
358-
args.flash_args.erase_data_parts,
359-
)?;
384+
// If using ESP-IDF image format, check if we need to erase partitions.
385+
if let ImageFormat::EspIdf(idf_format) = &image_format {
386+
if args.flash_args.erase_parts.is_some() || args.flash_args.erase_data_parts.is_some() {
387+
erase_partitions(
388+
&mut flasher,
389+
Some(idf_format.partition_table()),
390+
args.flash_args.erase_parts,
391+
args.flash_args.erase_data_parts,
392+
)?;
393+
}
360394
}
361395

362-
flash_elf_image(&mut flasher, &elf_data, flash_data, target_xtal_freq)?;
396+
flash_image(&mut flasher, image_format)?;
363397
}
364398

365399
if args.flash_args.monitor {
@@ -571,7 +605,7 @@ fn save_image(args: SaveImageArgs, config: &Config) -> Result<()> {
571605
let cargo_config = CargoConfig::load(&metadata.workspace_root, &metadata.package_root);
572606

573607
let build_ctx = build(&args.build_args, &cargo_config, args.save_image_args.chip)?;
574-
let elf_data = fs::read(build_ctx.artifact_path).into_diagnostic()?;
608+
let elf_data = fs::read(&build_ctx.artifact_path).into_diagnostic()?;
575609

576610
// Since we have no `Flasher` instance and as such cannot print the board
577611
// information, we will print whatever information we _do_ have.
@@ -585,27 +619,34 @@ fn save_image(args: SaveImageArgs, config: &Config) -> Result<()> {
585619
.or(config.project_config.flash.size) // If no CLI argument, try the config file
586620
.or_else(|| Some(FlashSize::default())); // Otherwise, use a reasonable default value
587621

622+
let xtal_freq = args
623+
.save_image_args
624+
.xtal_freq
625+
.unwrap_or(args.save_image_args.chip.default_crystal_frequency());
626+
588627
let flash_data = make_flash_data(
589628
args.save_image_args.image,
590629
&flash_config,
591630
config,
592-
build_ctx.bootloader_path.as_deref(),
593-
build_ctx.partition_table_path.as_deref(),
631+
args.save_image_args.chip,
632+
xtal_freq,
633+
);
634+
let image_format = make_image_format(
635+
&elf_data,
636+
&flash_data,
637+
args.format,
638+
config,
639+
Some(args.esp_idf_format_args),
640+
build_ctx.bootloader_path,
641+
build_ctx.partition_table_path,
594642
)?;
595643

596-
let xtal_freq = args
597-
.save_image_args
598-
.xtal_freq
599-
.unwrap_or(XtalFrequency::default(args.save_image_args.chip));
600-
601644
save_elf_as_image(
602-
&elf_data,
603-
args.save_image_args.chip,
604645
args.save_image_args.file,
605-
flash_data,
646+
flash_data.flash_settings.size,
606647
args.save_image_args.merge,
607648
args.save_image_args.skip_padding,
608-
xtal_freq,
649+
image_format,
609650
)?;
610651

611652
Ok(())

‎espflash/src/bin/espflash.rs‎

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ use espflash::{
1010
*,
1111
},
1212
flasher::FlashSize,
13-
image_format::check_idf_bootloader,
13+
image_format::{
14+
ImageFormat,
15+
ImageFormatKind,
16+
check_idf_bootloader,
17+
esp_idf::parse_partition_table,
18+
},
1419
logging::initialize_logger,
1520
targets::{Chip, XtalFrequency},
1621
update::check_for_update,
@@ -128,6 +133,12 @@ struct FlashArgs {
128133
flash_args: cli::FlashArgs,
129134
/// ELF image to flash
130135
image: PathBuf,
136+
/// Application image format to use
137+
#[clap(long, default_value = "esp-idf")]
138+
format: ImageFormatKind,
139+
/// ESP-IDF arguments
140+
#[clap(flatten)]
141+
esp_idf_format_args: cli::EspIdfFormatArgs,
131142
}
132143

133144
#[derive(Debug, Args)]
@@ -141,13 +152,19 @@ struct SaveImageArgs {
141152
/// Sage image arguments
142153
#[clap(flatten)]
143154
save_image_args: cli::SaveImageArgs,
155+
/// Application image format to use
156+
#[clap(long, default_value = "esp-idf")]
157+
format: ImageFormatKind,
158+
// ESP-IDF arguments
159+
#[clap(flatten)]
160+
esp_idf_format_args: cli::EspIdfFormatArgs,
144161
}
145162

146163
fn main() -> Result<()> {
147164
miette::set_panic_hook();
148165
initialize_logger(LevelFilter::Info);
149166

150-
// Attempt to parse any provided comand-line arguments, or print the help
167+
// Attempt to parse any provided command-line arguments, or print the help
151168
// message and terminate if the invocation is not correct.
152169
let cli = Cli::parse();
153170
let args = cli.subcommand;
@@ -212,6 +229,11 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
212229
let mut monitor_args = args.flash_args.monitor_args;
213230
monitor_args.elf = Some(args.image.clone());
214231
check_monitor_args(&args.flash_args.monitor, &monitor_args)?;
232+
check_esp_idf_args(
233+
args.format,
234+
&args.flash_args.erase_parts,
235+
&args.flash_args.erase_data_parts,
236+
)?;
215237

216238
let mut flasher = connect(
217239
&args.connect_args,
@@ -254,18 +276,36 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
254276
if args.flash_args.ram {
255277
flasher.load_elf_to_ram(&elf_data, Some(&mut EspflashProgress::default()))?;
256278
} else {
257-
let flash_data = make_flash_data(args.flash_args.image, &flash_config, config, None, None)?;
258-
259-
if args.flash_args.erase_parts.is_some() || args.flash_args.erase_data_parts.is_some() {
260-
erase_partitions(
261-
&mut flasher,
262-
flash_data.partition_table.clone(),
263-
args.flash_args.erase_parts,
264-
args.flash_args.erase_data_parts,
265-
)?;
279+
let flash_data = make_flash_data(
280+
args.flash_args.image,
281+
&flash_config,
282+
config,
283+
chip,
284+
target_xtal_freq,
285+
);
286+
let image_format = make_image_format(
287+
&elf_data,
288+
&flash_data,
289+
args.format,
290+
config,
291+
Some(args.esp_idf_format_args),
292+
None,
293+
None,
294+
)?;
295+
296+
// If using ESP-IDF image format, check if we need to erase partitions.
297+
if let ImageFormat::EspIdf(idf_format) = &image_format {
298+
if args.flash_args.erase_parts.is_some() || args.flash_args.erase_data_parts.is_some() {
299+
erase_partitions(
300+
&mut flasher,
301+
Some(idf_format.partition_table()),
302+
args.flash_args.erase_parts,
303+
args.flash_args.erase_data_parts,
304+
)?;
305+
}
266306
}
267307

268-
flash_elf_image(&mut flasher, &elf_data, flash_data, target_xtal_freq)?;
308+
flash_image(&mut flasher, image_format)?;
269309
}
270310

271311
if args.flash_args.monitor {
@@ -305,27 +345,34 @@ fn save_image(args: SaveImageArgs, config: &Config) -> Result<()> {
305345
.or(config.project_config.flash.size) // If no CLI argument, try the config file
306346
.or_else(|| Some(FlashSize::default())); // Otherwise, use a reasonable default value
307347

348+
let xtal_freq = args
349+
.save_image_args
350+
.xtal_freq
351+
.unwrap_or(args.save_image_args.chip.default_crystal_frequency());
352+
308353
let flash_data = make_flash_data(
309354
args.save_image_args.image,
310355
&flash_config,
311356
config,
357+
args.save_image_args.chip,
358+
xtal_freq,
359+
);
360+
let image_format = make_image_format(
361+
&elf_data,
362+
&flash_data,
363+
args.format,
364+
config,
365+
Some(args.esp_idf_format_args),
312366
None,
313367
None,
314368
)?;
315369

316-
let xtal_freq = args
317-
.save_image_args
318-
.xtal_freq
319-
.unwrap_or(XtalFrequency::default(args.save_image_args.chip));
320-
321370
save_elf_as_image(
322-
&elf_data,
323-
args.save_image_args.chip,
324371
args.save_image_args.file,
325-
flash_data,
372+
flash_data.flash_settings.size,
326373
args.save_image_args.merge,
327374
args.save_image_args.skip_padding,
328-
xtal_freq,
375+
image_format,
329376
)?;
330377

331378
Ok(())

‎espflash/src/cli/config.rs‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use miette::{IntoDiagnostic, Result, WrapErr};
1919
use serde::{Deserialize, Serialize};
2020
use serialport::UsbPortInfo;
2121

22-
use crate::{Error, flasher::FlashSettings};
22+
use crate::{Error, cli, flasher::FlashSettings, image_format::ImageFormatKind};
2323

2424
/// A configured, known serial connection
2525
#[derive(Debug, Deserialize, Serialize, Default, Clone)]
@@ -87,15 +87,12 @@ pub struct ProjectConfig {
8787
/// Baudrate
8888
#[serde(default)]
8989
pub baudrate: Option<u32>,
90-
/// Bootloader path
90+
/// Image format
9191
#[serde(default)]
92-
pub bootloader: Option<PathBuf>,
93-
/// Partition table path
92+
pub format: ImageFormatKind,
93+
/// ESP-IDF format arguments
9494
#[serde(default)]
95-
pub partition_table: Option<PathBuf>,
96-
/// Partition table offset
97-
#[serde(default)]
98-
pub partition_table_offset: Option<u32>,
95+
pub esp_idf_format_args: cli::EspIdfFormatArgs,
9996
/// Flash settings
10097
#[serde(default)]
10198
pub flash: FlashSettings,
@@ -127,14 +124,14 @@ impl Config {
127124
ProjectConfig::default()
128125
};
129126

130-
if let Some(table) = &project_config.partition_table {
127+
if let Some(table) = &project_config.esp_idf_format_args.partition_table {
131128
match table.extension() {
132129
Some(ext) if ext == "bin" || ext == "csv" => {}
133130
_ => return Err(Error::InvalidPartitionTablePath.into()),
134131
}
135132
}
136133

137-
if let Some(bootloader) = &project_config.bootloader {
134+
if let Some(bootloader) = &project_config.esp_idf_format_args.bootloader {
138135
if bootloader.extension() != Some(OsStr::new("bin")) {
139136
return Err(Error::InvalidBootloaderPath.into());
140137
}

0 commit comments

Comments
 (0)