Normalize similar arguments (#759)

* feat: Normalize similar arguments

* feat: Remove unnecesary value_names

* feat: Remove some short version of arguments

* docs: Update changelog

* tests: Fix arguments

* feat: Use elf for MonitorConfigArgs to avoid duplication

* feat: Rename monitor_baud arg

* fix: Allow using log-format with custom runner

* feat: Add help for NoElf error

* clippy: Remove too_many_args expection

Author: SergioGasquez

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 - Split the baudrate for connecting and monitorinig in `flash` subcommand (#737)
+- Normalized arguments of the CLI commands (#759)
 
 - `board-info` now prints `Security information`. (#758)
 

cargo-espflash/src/main.rs

@@ -351,10 +351,10 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
         // The 26MHz ESP32-C2's need to be treated as a special case.
         if chip == Chip::Esp32c2
             && target_xtal_freq == XtalFrequency::_26Mhz
-            && monitor_args.baud_rate == 115_200
+            && monitor_args.monitor_baud == 115_200
         {
             // 115_200 * 26 MHz / 40 MHz = 74_880
-            monitor_args.baud_rate = 74_880;
+            monitor_args.monitor_baud = 74_880;
         }
 
         monitor_args.elf = Some(build_ctx.artifact_path);

espflash/src/bin/espflash.rs

@@ -125,7 +125,7 @@ struct FlashArgs {
 #[derive(Debug, Args)]
 #[non_exhaustive]
 struct SaveImageArgs {
-    /// ELF image to flash
+    /// ELF image
     image: PathBuf,
     /// Flashing configuration
     #[clap(flatten)]
@@ -141,9 +141,9 @@ struct SaveImageArgs {
 struct WriteBinArgs {
     /// Address at which to write the binary file
     #[arg(value_parser = parse_u32)]
-    pub addr: u32,
+    pub address: u32,
     /// File containing the binary data to write
-    pub bin_file: String,
+    pub file: String,
     /// Connection configuration
     #[clap(flatten)]
     connect_args: ConnectArgs,
@@ -285,10 +285,10 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
         // The 26MHz ESP32-C2's need to be treated as a special case.
         if chip == Chip::Esp32c2
             && target_xtal_freq == XtalFrequency::_26Mhz
-            && monitor_args.baud_rate == 115_200
+            && monitor_args.monitor_baud == 115_200
         {
             // 115_200 * 26 MHz / 40 MHz = 74_880
-            monitor_args.baud_rate = 74_880;
+            monitor_args.monitor_baud = 74_880;
         }
 
         monitor_args.elf = Some(args.image);
@@ -340,12 +340,16 @@ fn write_bin(args: WriteBinArgs, config: &Config) -> Result<()> {
     let mut flasher = connect(&args.connect_args, config, false, false)?;
     print_board_info(&mut flasher)?;
 
-    let mut f = File::open(&args.bin_file).into_diagnostic()?;
+    let mut f = File::open(&args.file).into_diagnostic()?;
     let size = f.metadata().into_diagnostic()?.len();
     let mut buffer = Vec::with_capacity(size.try_into().into_diagnostic()?);
     f.read_to_end(&mut buffer).into_diagnostic()?;
 
-    flasher.write_bin_to_flash(args.addr, &buffer, Some(&mut EspflashProgress::default()))?;
+    flasher.write_bin_to_flash(
+        args.address,
+        &buffer,
+        Some(&mut EspflashProgress::default()),
+    )?;
 
     Ok(())
 }

espflash/src/cli/mod.rs

@@ -71,7 +71,7 @@ pub struct ConnectArgs {
     #[arg(short = 'c', long)]
     pub chip: Option<Chip>,
     /// Require confirmation before auto-connecting to a recognized device.
-    #[arg(short = 'C', long)]
+    #[arg(long)]
     pub confirm_port: bool,
     /// List all available ports.
     #[arg(long)]
@@ -108,11 +108,15 @@ pub struct EraseRegionArgs {
     /// Connection configuration
     #[clap(flatten)]
     pub connect_args: ConnectArgs,
-    /// Offset to start erasing from
-    #[arg(value_name = "OFFSET", value_parser = parse_u32)]
-    pub addr: u32,
+    /// Start address
+    ///
+    /// Must be multiple of 4096(0x1000)
+    #[arg(value_parser = parse_u32)]
+    pub address: u32,
     /// Size of the region to erase
-    #[arg(value_name = "SIZE", value_parser = parse_u32)]
+    ///
+    /// Must be multiple of 4096(0x1000)
+    #[arg(value_parser = parse_u32)]
     pub size: u32,
 }
 
@@ -183,9 +187,9 @@ pub struct PartitionTableArgs {
 #[derive(Debug, Args)]
 #[non_exhaustive]
 pub struct ReadFlashArgs {
-    /// Offset to start reading from
-    #[arg(value_name = "OFFSET", value_parser = parse_u32)]
-    pub addr: u32,
+    /// Address to start reading from
+    #[arg(value_parser = parse_u32)]
+    pub address: u32,
     /// Size of each individual packet of data
     ///
     /// Defaults to 0x1000 (FLASH_SECTOR_SIZE)
@@ -195,10 +199,9 @@ pub struct ReadFlashArgs {
     #[clap(flatten)]
     connect_args: ConnectArgs,
     /// Size of the region to read
-    #[arg(value_name = "SIZE", value_parser = parse_u32)]
+    #[arg(value_parser = parse_u32)]
     pub size: u32,
-    /// Name of binary dump
-    #[arg(value_name = "FILE")]
+    /// File name to save the read data to
     pub file: PathBuf,
     /// Maximum number of un-acked packets
     #[arg(long, default_value = "64", value_parser = parse_u32)]
@@ -219,7 +222,7 @@ pub struct SaveImageArgs {
     #[arg(long)]
     pub merge: bool,
     /// Don't pad the image to the flash size
-    #[arg(long, short = 'P', requires = "merge")]
+    #[arg(long, requires = "merge")]
     pub skip_padding: bool,
     /// Cristal frequency of the target
     #[arg(long, short = 'x')]
@@ -236,7 +239,7 @@ pub struct ImageArgs {
     #[arg(long, value_name = "FILE")]
     pub bootloader: Option<PathBuf>,
     /// Path to a CSV file containing partition table
-    #[arg(long, short = 'T', value_name = "FILE")]
+    #[arg(long, value_name = "FILE")]
     pub partition_table: Option<PathBuf>,
     /// Partition table offset
     #[arg(long, value_name = "OFFSET", value_parser = parse_u32)]
@@ -264,11 +267,11 @@ pub struct MonitorArgs {
 #[derive(Debug, Args)]
 #[non_exhaustive]
 pub struct MonitorConfigArgs {
-    /// Baud rate at which to communicate with target device
+    /// Baud rate at which to monitor the target device
     #[arg(short = 'r', long, env = "MONITOR_BAUD", default_value = "115_200", value_parser = parse_u32)]
-    pub baud_rate: u32,
-    /// File name of the ELF image to load the symbols from
-    #[arg(short = 'e', long, value_name = "FILE")]
+    pub monitor_baud: u32,
+    /// ELF image to load the symbols from
+    #[arg(long, value_name = "FILE")]
     pub elf: Option<PathBuf>,
     /// Avoids asking the user for interactions like resetting the device
     #[arg(long)]
@@ -277,7 +280,7 @@ pub struct MonitorConfigArgs {
     #[arg(long, requires = "non_interactive")]
     no_reset: bool,
     /// Logging format.
-    #[arg(long, short = 'L', default_value = "serial", requires = "elf")]
+    #[arg(long, short = 'L', default_value = "serial")]
     log_format: LogFormat,
     /// External log processors to use (comma separated executables)
     #[arg(long)]
@@ -288,11 +291,11 @@ pub struct MonitorConfigArgs {
 #[non_exhaustive]
 pub struct ChecksumMd5Args {
     /// Start address
-    #[clap(long, value_parser=parse_u32)]
+    #[clap(value_parser=parse_u32)]
     address: u32,
-    /// Length
-    #[clap(short, long, value_parser=parse_u32)]
-    length: u32,
+    /// Size of the region to check
+    #[clap(value_parser=parse_u32)]
+    size: u32,
     /// Connection configuration
     #[clap(flatten)]
     connect_args: ConnectArgs,
@@ -387,7 +390,7 @@ pub fn board_info(args: &ConnectArgs, config: &Config) -> Result<()> {
 pub fn checksum_md5(args: &ChecksumMd5Args, config: &Config) -> Result<()> {
     let mut flasher = connect(&args.connect_args, config, true, true)?;
 
-    let checksum = flasher.checksum_md5(args.address, args.length)?;
+    let checksum = flasher.checksum_md5(args.address, args.size)?;
     println!("0x{:x}", checksum);
 
     Ok(())
@@ -470,10 +473,10 @@ pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {
     // The 26MHz ESP32-C2's need to be treated as a special case.
     if chip == Chip::Esp32c2
         && target.crystal_freq(flasher.connection())? == XtalFrequency::_26Mhz
-        && monitor_args.baud_rate == 115_200
+        && monitor_args.monitor_baud == 115_200
     {
         // 115_200 * 26 MHz / 40 MHz = 74_880
-        monitor_args.baud_rate = 74_880;
+        monitor_args.monitor_baud = 74_880;
     }
 
     monitor(flasher.into_serial(), elf.as_deref(), pid, monitor_args)
@@ -629,10 +632,10 @@ pub fn erase_region(args: EraseRegionArgs, config: &Config) -> Result<()> {
 
     info!(
         "Erasing region at 0x{:08x} ({} bytes)",
-        args.addr, args.size
+        args.address, args.size
     );
 
-    flasher.erase_region(args.addr, args.size)?;
+    flasher.erase_region(args.address, args.size)?;
     flasher
         .connection()
         .reset_after(!args.connect_args.no_stub)?;
@@ -733,7 +736,7 @@ pub fn read_flash(args: ReadFlashArgs, config: &Config) -> Result<()> {
     let mut flasher = connect(&args.connect_args, config, false, false)?;
     print_board_info(&mut flasher)?;
     flasher.read_flash(
-        args.addr,
+        args.address,
         args.size,
         args.block_size,
         args.max_in_flight,

espflash/src/cli/monitor/mod.rs

@@ -71,7 +71,6 @@ impl Drop for RawModeGuard {
 
 /// Open a serial monitor on the given serial port, using the given input
 /// parser.
-#[allow(clippy::too_many_arguments)]
 pub fn monitor(
     mut serial: Port,
     elf: Option<&[u8]>,
@@ -87,7 +86,7 @@ pub fn monitor(
         reset_after_flash(&mut serial, pid).into_diagnostic()?;
     }
 
-    let baud = monitor_args.baud_rate;
+    let baud = monitor_args.monitor_baud;
     debug!("Opening serial monitor with baudrate: {}", baud);
 
     // Explicitly set the baud rate when starting the serial monitor, to allow using

espflash/src/cli/monitor/parser/esp_defmt.rs

@@ -11,7 +11,10 @@ use crate::cli::monitor::parser::InputParser;
 #[error("Could not set up defmt logger")]
 pub enum DefmtError {
     #[error("No elf data available")]
-    #[diagnostic(code(espflash::monitor::defmt::no_elf))]
+    #[diagnostic(
+        code(espflash::monitor::defmt::no_elf),
+        help("Please provide an ELF file with the `--elf` argument")
+    )]
     NoElf,
 
     #[error("No defmt data was found in the elf file")]

espflash/src/command.rs

@@ -312,13 +312,13 @@ impl Command<'_> {
                 #[derive(Zeroable, Pod, Copy, Clone, Debug)]
                 #[repr(C)]
                 struct WriteRegParams {
-                    addr: u32,
+                    address: u32,
                     value: u32,
                     mask: u32,
                     delay_us: u32,
                 }
                 let params = WriteRegParams {
-                    addr: address,
+                    address,
                     value,
                     mask: mask.unwrap_or(0xFFFFFFFF),
                     delay_us: 0,

espflash/tests/scripts/checksum-md5.sh

@@ -5,7 +5,7 @@ echo "$result"
 if [[ ! $result =~ "Flash has been erased!" ]]; then
     exit 1
 fi
-result=$(espflash checksum-md5 --address 0x1000 --length 0x100 2>&1)
+result=$(espflash checksum-md5 0x1000 0x100 2>&1)
 echo "$result"
 if [[ ! $result =~ "0x827f263ef9fb63d05499d14fcef32f60" ]]; then
     exit 1