CLI improvements and dependency updates (#334)

* Update dependencies to their latest versions

* Derive `clap::ValueEnum` instead of using a macro

* Simplify flash config enums, make them more consistent

* Fix a bunch of clippy warnings

Author: jessebraham

Cargo.lock

@@ -31,13 +31,12 @@ pkg-fmt = "zip"
 [dependencies]
 cargo = { version = "0.66.0", features = ["vendored-openssl"] }
 cargo_metadata = "0.15.2"
-clap = { version = "4.0.29", features = ["derive"] }
+clap = { version = "4.0.32", features = ["derive"] }
 env_logger = "0.10.0"
 esp-idf-part = "0.1.2"
 espflash = { version = "=2.0.0-rc.2", path = "../espflash" }
 log = "0.4.17"
 miette = { version = "5.5.0", features = ["fancy"] }
-serde = { version = "1.0.148", features = ["derive"] }
-strum = "0.24.1"
-thiserror = "1.0.37"
-toml = "0.5.9"
+serde = { version = "1.0.152", features = ["derive"] }
+thiserror = "1.0.38"
+toml = "0.5.10"

cargo-espflash/Cargo.toml

@@ -8,10 +8,10 @@ use cargo_metadata::Message;
 use clap::{Args, Parser, Subcommand};
 use espflash::{
     cli::{
-        self, board_info, clap_enum_variants, config::Config, connect, erase_partitions,
-        flash_elf_image, monitor::monitor, parse_partition_table, partition_table,
-        print_board_info, save_elf_as_image, serial_monitor, ConnectArgs, FlashConfigArgs,
-        MonitorArgs, PartitionTableArgs,
+        self, board_info, config::Config, connect, erase_partitions, flash_elf_image,
+        monitor::monitor, parse_partition_table, partition_table, print_board_info,
+        save_elf_as_image, serial_monitor, ConnectArgs, FlashConfigArgs, MonitorArgs,
+        PartitionTableArgs,
     },
     image_format::ImageFormatKind,
     logging::initialize_logger,
@@ -20,7 +20,6 @@ use espflash::{
 };
 use log::{debug, LevelFilter};
 use miette::{IntoDiagnostic, Result, WrapErr};
-use strum::VariantNames;
 
 use crate::{
     cargo_config::CargoConfig,
@@ -110,7 +109,7 @@ struct FlashArgs {
 #[derive(Debug, Args)]
 struct SaveImageArgs {
     /// Image format to flash
-    #[arg(long, value_parser = clap_enum_variants!(ImageFormatKind))]
+    #[arg(long, value_enum)]
     pub format: Option<ImageFormatKind>,
 
     #[clap(flatten)]
@@ -163,7 +162,7 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
 
     let chip = flasher.chip();
     let target = chip.into_target();
-    let target_xtal_freq = target.crystal_freq(&mut flasher.connection())?;
+    let target_xtal_freq = target.crystal_freq(flasher.connection())?;
 
     let build_ctx =
         build(&args.build_args, &cargo_config, chip).wrap_err("Failed to build project")?;
@@ -321,7 +320,7 @@ fn build(
     let output = Command::new("cargo")
         .arg("build")
         .args(args)
-        .args(&["--message-format", "json-diagnostic-rendered-ansi"])
+        .args(["--message-format", "json-diagnostic-rendered-ansi"])
         .stdout(Stdio::piped())
         .stderr(Stdio::inherit())
         .spawn()

cargo-espflash/src/main.rs

@@ -36,11 +36,11 @@ required-features = ["cli"]
 
 [dependencies]
 addr2line = { version = "0.19.0", optional = true }
-base64 = "0.13.1"
+base64 = "0.21.0"
 binread = "2.2.0"
 bytemuck = { version = "1.12.3", features = ["derive"] }
-clap = { version = "4.0.29", features = ["derive", "env"], optional = true }
-comfy-table = { version = "6.1.3", optional = true }
+clap = { version = "4.0.32", features = ["derive", "env"], optional = true }
+comfy-table = { version = "6.1.4", optional = true }
 crossterm = { version = "0.25.0", optional = true }
 dialoguer = { version = "0.10.2", optional = true }
 directories-next = { version = "2.0.0", optional = true }
@@ -52,18 +52,18 @@ lazy_static = { version = "1.4.0", optional = true }
 log = "0.4.17"
 miette = { version = "5.5.0", features = ["fancy"] }
 parse_int = { version = "0.6.0", optional = true }
-regex = { version = "1.7.0", optional = true }
+regex = { version = "1.7.1", optional = true }
 rppal = { version = "0.14.1", optional = true }
-serde = { version = "1.0.148", features = ["derive"] }
+serde = { version = "1.0.152", features = ["derive"] }
 serde-hex = { version = "0.1.0", optional = true }
 serialport = "4.2.0"
 sha2 = "0.10.6"
 slip-codec = "0.3.3"
 strum = { version = "0.24.1", features = ["derive"] }
-thiserror = "1.0.37"
-toml = "0.5.9"
-update-informer = { version = "0.5.0", optional = true }
-xmas-elf = "0.8.0"
+thiserror = "1.0.38"
+toml = "0.5.10"
+update-informer = { version = "0.6.0", optional = true }
+xmas-elf = "0.9.0"
 
 [features]
 default = ["cli"]

espflash/Cargo.toml

@@ -8,10 +8,10 @@ use std::{
 use clap::{Args, Parser, Subcommand};
 use espflash::{
     cli::{
-        self, board_info, build_progress_bar_callback, clap_enum_variants, config::Config, connect,
-        erase_partitions, flash_elf_image, monitor::monitor, parse_partition_table,
-        partition_table, print_board_info, progress_bar, save_elf_as_image, serial_monitor,
-        ConnectArgs, FlashConfigArgs, MonitorArgs, PartitionTableArgs,
+        self, board_info, build_progress_bar_callback, config::Config, connect, erase_partitions,
+        flash_elf_image, monitor::monitor, parse_partition_table, partition_table,
+        print_board_info, progress_bar, save_elf_as_image, serial_monitor, ConnectArgs,
+        FlashConfigArgs, MonitorArgs, PartitionTableArgs,
     },
     image_format::ImageFormatKind,
     logging::initialize_logger,
@@ -20,7 +20,6 @@ use espflash::{
 };
 use log::{debug, LevelFilter};
 use miette::{IntoDiagnostic, Result, WrapErr};
-use strum::VariantNames;
 
 #[derive(Debug, Parser)]
 #[clap(about, version, propagate_version = true)]
@@ -57,7 +56,7 @@ struct FlashArgs {
 #[derive(Debug, Args)]
 struct SaveImageArgs {
     /// Image format to flash
-    #[arg(long, value_parser = clap_enum_variants!(ImageFormatKind))]
+    #[arg(long, value_enum)]
     format: Option<ImageFormatKind>,
 
     #[clap(flatten)]
@@ -121,7 +120,7 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
 
     let chip = flasher.chip();
     let target = chip.into_target();
-    let target_xtal_freq = target.crystal_freq(&mut flasher.connection())?;
+    let target_xtal_freq = target.crystal_freq(flasher.connection())?;
 
     // Read the ELF data from the build path and load it to the target.
     let elf_data = fs::read(&args.image).into_diagnostic()?;

espflash/src/bin/espflash.rs

@@ -22,7 +22,6 @@ use indicatif::{style::ProgressStyle, HumanCount, ProgressBar, ProgressDrawTarge
 use log::{debug, info};
 use miette::{IntoDiagnostic, Result, WrapErr};
 use serialport::{SerialPortType, UsbPortInfo};
-use strum::VariantNames;
 
 use self::{config::Config, monitor::monitor, serial::get_serial_port_info};
 use crate::{
@@ -39,26 +38,6 @@ pub mod monitor;
 
 mod serial;
 
-// Since as of `[email protected]` the `possible_values` attribute is no longer
-// present, we must use the more convoluted `value_parser` attribute instead.
-// Since this is a bit tedious, we'll use a helper macro to abstract away all
-// the cruft. It's important to note that this macro assumes the
-// `strum::EnumVariantNames` trait has been implemented for the provided type,
-// and that the provided type is in scope when calling this macro.
-//
-// See this comment for details:
-// https://github.com/clap-rs/clap/discussions/4264#discussioncomment-3737696
-#[doc(hidden)]
-#[macro_export]
-macro_rules! clap_enum_variants {
-    ($e: ty) => {{
-        use clap::builder::TypedValueParser;
-        clap::builder::PossibleValuesParser::new(<$e>::VARIANTS).map(|s| s.parse::<$e>().unwrap())
-    }};
-}
-
-pub use clap_enum_variants;
-
 /// Establish a connection with a target device
 #[derive(Debug, Args)]
 pub struct ConnectArgs {
@@ -85,13 +64,13 @@ pub struct ConnectArgs {
 #[derive(Debug, Args)]
 pub struct FlashConfigArgs {
     /// Flash frequency
-    #[arg(short = 'f', long, value_name = "FREQ", value_parser = clap_enum_variants!(FlashFrequency))]
+    #[arg(short = 'f', long, value_name = "FREQ", value_enum)]
     pub flash_freq: Option<FlashFrequency>,
     /// Flash mode to use
-    #[arg(short = 'm', long, value_name = "MODE", value_parser = clap_enum_variants!(FlashMode))]
+    #[arg(short = 'm', long, value_name = "MODE", value_enum)]
     pub flash_mode: Option<FlashMode>,
     /// Flash size of the target
-    #[arg(short = 's', long, value_name = "SIZE", value_parser = clap_enum_variants!(FlashSize))]
+    #[arg(short = 's', long, value_name = "SIZE", value_enum)]
     pub flash_size: Option<FlashSize>,
 }
 
@@ -111,10 +90,16 @@ pub struct FlashArgs {
     )]
     pub erase_parts: Option<Vec<String>>,
     /// Erase specified data partitions
-    #[arg(long, requires = "partition_table", value_name = "PARTS", value_parser = clap_enum_variants!(DataType), value_delimiter = ',')]
+    #[arg(
+        long,
+        requires = "partition_table",
+        value_name = "PARTS",
+        value_enum,
+        value_delimiter = ','
+    )]
     pub erase_data_parts: Option<Vec<DataType>>,
     /// Image format to flash
-    #[arg(long, value_parser = clap_enum_variants!(ImageFormatKind))]
+    #[arg(long, value_enum)]
     pub format: Option<ImageFormatKind>,
     /// Open a serial monitor after flashing
     #[arg(short = 'M', long)]
@@ -155,7 +140,7 @@ pub struct SaveImageArgs {
     #[arg(long, value_name = "FILE")]
     pub bootloader: Option<PathBuf>,
     /// Chip to create an image for
-    #[arg(long, value_parser = clap_enum_variants!(Chip))]
+    #[arg(long, value_enum)]
     pub chip: Chip,
     /// File name to save the generated image to
     pub file: PathBuf,
@@ -193,16 +178,14 @@ where
         _ => ProgressDrawTarget::hidden(),
     };
 
-    let progress = ProgressBar::with_draw_target(len, draw_target)
+    ProgressBar::with_draw_target(len, draw_target)
         .with_message(msg)
         .with_style(
             ProgressStyle::default_bar()
                 .template("[{elapsed_precise}] [{bar:40}] {pos:>7}/{len:7} {msg}")
                 .unwrap()
                 .progress_chars("=> "),
-        );
-
-    progress
+        )
 }
 
 /// Create a callback function for the provided [ProgressBar]
@@ -277,7 +260,7 @@ pub fn connect(args: &ConnectArgs, config: &Config) -> Result<Flasher> {
 
 /// Connect to a target device and print information about its chip
 pub fn board_info(args: &ConnectArgs, config: &Config) -> Result<()> {
-    let mut flasher = connect(&args, config)?;
+    let mut flasher = connect(args, config)?;
     print_board_info(&mut flasher)?;
 
     Ok(())
@@ -291,7 +274,7 @@ pub fn print_board_info(flasher: &mut Flasher) -> Result<()> {
     if let Some((major, minor)) = info.revision {
         println!(" (revision v{major}.{minor})");
     } else {
-        println!("");
+        println!();
     }
     println!("Crystal frequency: {}MHz", info.crystal_frequency);
     println!("Flash size:        {}", info.flash_size);
@@ -321,7 +304,7 @@ pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {
     // The 26MHz ESP32-C2's need to be treated as a special case.
     let default_baud = if chip == Chip::Esp32c2
         && !args.connect_args.use_stub
-        && target.crystal_freq(&mut flasher.connection())? == 26
+        && target.crystal_freq(flasher.connection())? == 26
     {
         74_880
     } else {
@@ -537,7 +520,7 @@ pub fn erase_partitions(
         for label in part_labels {
             let part = partition_table
                 .find(label.as_str())
-                .ok_or(MissingPartition::from(label))?;
+                .ok_or_else(|| MissingPartition::from(label))?;
 
             parts_to_erase
                 .get_or_insert(HashMap::new())
@@ -648,12 +631,12 @@ fn pretty_print(table: PartitionTable) {
 
     for p in table.partitions() {
         pretty.add_row(vec![
-            Cell::new(&p.name()).fg(Color::Green),
-            Cell::new(&p.ty().to_string()).fg(Color::Cyan),
-            Cell::new(&p.subtype().to_string()).fg(Color::Magenta),
-            Cell::new(&format!("{:#x}", p.offset())).fg(Color::Red),
-            Cell::new(&format!("{:#x} ({}KiB)", p.size(), p.size() / 1024)).fg(Color::Yellow),
-            Cell::new(&p.encrypted()).fg(Color::DarkCyan),
+            Cell::new(p.name()).fg(Color::Green),
+            Cell::new(p.ty().to_string()).fg(Color::Cyan),
+            Cell::new(p.subtype().to_string()).fg(Color::Magenta),
+            Cell::new(format!("{:#x}", p.offset())).fg(Color::Red),
+            Cell::new(format!("{:#x} ({}KiB)", p.size(), p.size() / 1024)).fg(Color::Yellow),
+            Cell::new(p.encrypted()).fg(Color::DarkCyan),
         ]);
     }
 

espflash/src/cli/mod.rs

@@ -33,9 +33,9 @@ pub fn get_serial_port_info(
     let ports = detect_usb_serial_ports().unwrap_or_default();
 
     if let Some(serial) = &matches.port {
-        find_serial_port(&ports, &serial)
+        find_serial_port(&ports, serial)
     } else if let Some(serial) = &config.connection.serial {
-        find_serial_port(&ports, &serial)
+        find_serial_port(&ports, serial)
     } else {
         let (port, matches) = select_serial_port(ports, config)?;
 

espflash/src/cli/serial.rs

@@ -205,7 +205,7 @@ impl<'a> Command<'a> {
                 data_command(writer, data, pad_to, pad_byte, sequence)?;
             }
             Command::FlashEnd { reboot } => {
-                write_basic(writer, &[if reboot { 0 } else { 1 }], 0)?;
+                write_basic(writer, &[u8::from(!reboot)], 0)?;
             }
             Command::MemBegin {
                 size,
@@ -242,7 +242,7 @@ impl<'a> Command<'a> {
                     entry: u32,
                 }
                 let params = EntryParams {
-                    no_entry: if reboot { 1 } else { 0 },
+                    no_entry: u32::from(reboot),
                     entry,
                 };
                 write_basic(writer, bytes_of(&params), 0)?;
@@ -325,7 +325,7 @@ impl<'a> Command<'a> {
                 data_command(writer, data, pad_to, pad_byte, sequence)?;
             }
             Command::FlashDeflateEnd { reboot } => {
-                write_basic(writer, &[if reboot { 0 } else { 1 }], 0)?;
+                write_basic(writer, &[u8::from(!reboot)], 0)?;
             }
             Command::FlashDetect => {
                 write_basic(writer, &[], 0)?;