Improve monitoring (#737)

* feat: Make non-interactive available in other commands

* feat: Use monitor_args in monitor()

* feat: Move elf arg to MonitorConfigArgs

* docs: Udpate changelog

* docs: Update readmes

* feat: Use parse_u32

* tests: Add espflash flash --monitor tests

* feat: Allow underscores in parse_u32

Author: SergioGasquez

CHANGELOG.md

@@ -8,8 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
+- Add `non-interactive` flag to `flash` subcommand (#737)
+- Add `no-reset` flag to `monitor` subcommands (#737)
+- Add an environment variable to set monitoring baudrate (`MONITOR_BAUD`) (#737)
 
 ### Changed
+- Split the baudrate for connecting and monitorinig in `flash` subcommand (#737)
 
 ### Fixed
 

cargo-espflash/README.md

@@ -18,7 +18,7 @@ Supports the **ESP32**, **ESP32-C2/C3/C6**, **ESP32-H2**, **ESP32-P4**, and **ES
   - [Windows Subsystem for Linux](#windows-subsystem-for-linux)
 - [Bootloader and Partition Table](#bootloader-and-partition-table)
 - [Configuration File](#configuration-file)
-  - [Configuration precedence](#configuration-precedence)
+  - [Configuration Precedence](#configuration-precedence)
 - [Logging Format](#logging-format)
 - [License](#license)
   - [Contribution](#contribution)
@@ -143,9 +143,9 @@ You can have a local and/or a global configuration file:
   - macOS: `$HOME/Library/Application Support/rs.esp.espflash/espflash.toml`
   - Windows: `%APPDATA%\esp\espflash\espflash.toml`
 
-### Configuration precedence
+### Configuration Precedence
 
-1. Environment variables: If `ESPFLASH_PORT` or `ESPFLASH_BAUD` are set, the will be used instead of the config file value.
+1. Environment variables: If `ESPFLASH_PORT`, `MONITOR_BAUD` or `ESPFLASH_BAUD` are set, the will be used instead of the config file value.
 2. Local configuration file
 3. Global configuration file
 

cargo-espflash/src/main.rs

@@ -346,25 +346,20 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
 
     if args.flash_args.monitor {
         let pid = flasher.get_usb_pid()?;
+        let mut monitor_args = args.flash_args.monitor_args;
 
         // The 26MHz ESP32-C2's need to be treated as a special case.
-        let default_baud = if chip == Chip::Esp32c2 && target_xtal_freq == XtalFrequency::_26Mhz {
+        if chip == Chip::Esp32c2
+            && target_xtal_freq == XtalFrequency::_26Mhz
+            && monitor_args.baud_rate == 115_200
+        {
             // 115_200 * 26 MHz / 40 MHz = 74_880
-            74_880
-        } else {
-            115_200
-        };
-
-        monitor(
-            flasher.into_serial(),
-            Some(&elf_data),
-            pid,
-            args.flash_args.monitor_baud.unwrap_or(default_baud),
-            args.flash_args.log_format,
-            true,
-            args.flash_args.processors,
-            Some(build_ctx.artifact_path),
-        )
+            monitor_args.baud_rate = 74_880;
+        }
+
+        monitor_args.elf = Some(build_ctx.artifact_path);
+
+        monitor(flasher.into_serial(), Some(&elf_data), pid, monitor_args)
     } else {
         Ok(())
     }

espflash/README.md

@@ -20,7 +20,7 @@ Supports the **ESP32**, **ESP32-C2/C3/C6**, **ESP32-H2**, **ESP32-P4**, and **ES
   - [Cargo Runner](#cargo-runner)
 - [Using `espflash` as a Library](#using-espflash-as-a-library)
 - [Configuration File](#configuration-file)
-  - [Configuration precedence](#configuration-precedence)
+  - [Configuration Precedence](#configuration-precedence)
 - [Logging Format](#logging-format)
 - [License](#license)
   - [Contribution](#contribution)
@@ -157,9 +157,9 @@ You can have a local and/or a global configuration file:
   - macOS: `$HOME/Library/Application Support/rs.esp.espflash/espflash.toml`
   - Windows: `%APPDATA%\esp\espflash\espflash.toml`
 
-### Configuration precedence
+### Configuration Precedence
 
-1. Environment variables: If `ESPFLASH_PORT` or `ESPFLASH_BAUD` are set, the will be used instead of the config file value.
+1. Environment variables: If `ESPFLASH_PORT`, `MONITOR_BAUD` or `ESPFLASH_BAUD` are set, the will be used instead of the config file value.
 2. Local configuration file
 3. Global configuration file
 

espflash/src/bin/espflash.rs

@@ -280,25 +280,20 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
 
     if args.flash_args.monitor {
         let pid = flasher.get_usb_pid()?;
+        let mut monitor_args = args.flash_args.monitor_args;
 
         // The 26MHz ESP32-C2's need to be treated as a special case.
-        let default_baud = if chip == Chip::Esp32c2 && target_xtal_freq == XtalFrequency::_26Mhz {
+        if chip == Chip::Esp32c2
+            && target_xtal_freq == XtalFrequency::_26Mhz
+            && monitor_args.baud_rate == 115_200
+        {
             // 115_200 * 26 MHz / 40 MHz = 74_880
-            74_880
-        } else {
-            115_200
-        };
-
-        monitor(
-            flasher.into_serial(),
-            Some(&elf_data),
-            pid,
-            args.flash_args.monitor_baud.unwrap_or(default_baud),
-            args.flash_args.log_format,
-            true,
-            args.flash_args.processors,
-            Some(args.image),
-        )
+            monitor_args.baud_rate = 74_880;
+        }
+
+        monitor_args.elf = Some(args.image);
+
+        monitor(flasher.into_serial(), Some(&elf_data), pid, monitor_args)
     } else {
         Ok(())
     }

espflash/src/cli/mod.rs

@@ -142,15 +142,12 @@ pub struct FlashArgs {
     /// Erase specified data partitions
     #[arg(long, value_name = "PARTS", value_enum, value_delimiter = ',')]
     pub erase_data_parts: Option<Vec<DataType>>,
-    /// Logging format.
-    #[arg(long, short = 'L', default_value = "serial", requires = "monitor")]
-    pub log_format: LogFormat,
     /// Open a serial monitor after flashing
     #[arg(short = 'M', long)]
     pub monitor: bool,
-    /// Baud rate at which to read console output
-    #[arg(long, requires = "monitor", value_name = "BAUD")]
-    pub monitor_baud: Option<u32>,
+    /// Monitor configuration
+    #[clap(flatten)]
+    pub monitor_args: MonitorConfigArgs,
     /// Load the application to RAM instead of Flash
     #[arg(long)]
     pub ram: bool,
@@ -162,9 +159,6 @@ pub struct FlashArgs {
     pub no_skip: bool,
     #[clap(flatten)]
     pub image: ImageArgs,
-    /// External log processors to use (comma separated executables)
-    #[arg(long, requires = "monitor")]
-    pub processors: Option<String>,
 }
 
 /// Operations for partitions tables
@@ -255,22 +249,36 @@ pub struct ImageArgs {
     pub min_chip_rev: u16,
 }
 
-/// Open the serial monitor without flashing
 #[derive(Debug, Args)]
 #[non_exhaustive]
 pub struct MonitorArgs {
     /// Connection configuration
     #[clap(flatten)]
     connect_args: ConnectArgs,
-    /// Optional file name of the ELF image to load the symbols from
+    /// Monitoring arguments
+    #[clap(flatten)]
+    monitor_args: MonitorConfigArgs,
+}
+
+/// Open the serial monitor without flashing
+#[derive(Debug, Args)]
+#[non_exhaustive]
+pub struct MonitorConfigArgs {
+    /// Baud rate at which to communicate with 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")]
-    elf: Option<PathBuf>,
+    pub elf: Option<PathBuf>,
     /// Avoids asking the user for interactions like resetting the device
     #[arg(long)]
     non_interactive: bool,
+    /// Avoids restarting the device before monitoring
+    #[arg(long, requires = "non_interactive")]
+    no_reset: bool,
     /// Logging format.
     #[arg(long, short = 'L', default_value = "serial", requires = "elf")]
-    pub log_format: LogFormat,
+    log_format: LogFormat,
     /// External log processors to use (comma separated executables)
     #[arg(long)]
     processors: Option<String>,
@@ -292,6 +300,7 @@ pub struct ChecksumMd5Args {
 
 /// Parses an integer, in base-10 or hexadecimal format, into a [u32]
 pub fn parse_u32(input: &str) -> Result<u32, ParseIntError> {
+    let input: &str = &input.replace('_', "");
     let (s, radix) = if input.len() > 2 && matches!(&input[0..2], "0x" | "0X") {
         (&input[2..], 16)
     } else {
@@ -437,7 +446,7 @@ pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {
     let mut flasher = connect(&args.connect_args, config, true, true)?;
     let pid = flasher.get_usb_pid()?;
 
-    let elf = if let Some(elf_path) = args.elf.clone() {
+    let elf = if let Some(elf_path) = args.monitor_args.elf.clone() {
         let path = fs::canonicalize(elf_path).into_diagnostic()?;
         let data = fs::read(path).into_diagnostic()?;
 
@@ -449,26 +458,18 @@ pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {
     let chip = flasher.chip();
     let target = chip.into_target();
 
+    let mut monitor_args = args.monitor_args;
+
     // The 26MHz ESP32-C2's need to be treated as a special case.
-    let default_baud = if chip == Chip::Esp32c2
+    if chip == Chip::Esp32c2
         && target.crystal_freq(flasher.connection())? == XtalFrequency::_26Mhz
+        && monitor_args.baud_rate == 115_200
     {
         // 115_200 * 26 MHz / 40 MHz = 74_880
-        74_880
-    } else {
-        115_200
-    };
+        monitor_args.baud_rate = 74_880;
+    }
 
-    monitor(
-        flasher.into_serial(),
-        elf.as_deref(),
-        pid,
-        args.connect_args.baud.unwrap_or(default_baud),
-        args.log_format,
-        !args.non_interactive,
-        args.processors,
-        args.elf,
-    )
+    monitor(flasher.into_serial(), elf.as_deref(), pid, monitor_args)
 }
 
 /// Convert the provided firmware image from ELF to binary
@@ -892,6 +893,9 @@ mod test {
         // Decimal
         assert_eq!(parse_u32("1234"), Ok(1234));
         assert_eq!(parse_u32("0"), Ok(0));
+        // Underscores
+        assert_eq!(parse_u32("12_34"), Ok(1234));
+        assert_eq!(parse_u32("0X12_34"), Ok(0x1234));
         // Errors
         assert!(parse_u32("").is_err());
         assert!(parse_u32("0x").is_err());

espflash/src/cli/monitor/mod.rs

@@ -12,7 +12,6 @@
 
 use std::{
     io::{stdout, ErrorKind, Read, Write},
-    path::PathBuf,
     time::Duration,
 };
 
@@ -21,14 +20,17 @@ use crossterm::{
     terminal::{disable_raw_mode, enable_raw_mode},
 };
 use external_processors::ExternalProcessors;
-use log::error;
+use log::{debug, error};
 use miette::{IntoDiagnostic, Result};
 #[cfg(feature = "serialport")]
 use serialport::SerialPort;
 use strum::{Display, EnumIter, EnumString, VariantNames};
 
 use crate::{
-    cli::monitor::parser::{InputParser, ResolvingPrinter},
+    cli::{
+        monitor::parser::{InputParser, ResolvingPrinter},
+        MonitorConfigArgs,
+    },
     connection::{reset::reset_after_flash, Port},
 };
 
@@ -74,21 +76,20 @@ pub fn monitor(
     mut serial: Port,
     elf: Option<&[u8]>,
     pid: u16,
-    baud: u32,
-    log_format: LogFormat,
-    interactive_mode: bool,
-    processors: Option<String>,
-    elf_file: Option<PathBuf>,
+    monitor_args: MonitorConfigArgs,
 ) -> miette::Result<()> {
-    if interactive_mode {
+    if !monitor_args.non_interactive {
         println!("Commands:");
         println!("    CTRL+R    Reset chip");
         println!("    CTRL+C    Exit");
         println!();
-    } else {
+    } else if !monitor_args.no_reset {
         reset_after_flash(&mut serial, pid).into_diagnostic()?;
     }
 
+    let baud = monitor_args.baud_rate;
+    debug!("Opening serial monitor with baudrate: {}", baud);
+
     // Explicitly set the baud rate when starting the serial monitor, to allow using
     // different rates for flashing.
     serial.set_baud_rate(baud).into_diagnostic()?;
@@ -102,12 +103,13 @@ pub fn monitor(
     let stdout = stdout();
     let mut stdout = ResolvingPrinter::new(elf, stdout.lock());
 
-    let mut parser: Box<dyn InputParser> = match log_format {
+    let mut parser: Box<dyn InputParser> = match monitor_args.log_format {
         LogFormat::Defmt => Box::new(parser::esp_defmt::EspDefmt::new(elf)?),
         LogFormat::Serial => Box::new(parser::serial::Serial),
     };
 
-    let mut external_processors = ExternalProcessors::new(processors, elf_file)?;
+    let mut external_processors =
+        ExternalProcessors::new(monitor_args.processors, monitor_args.elf)?;
 
     let mut buff = [0; 1024];
     loop {
@@ -124,7 +126,7 @@ pub fn monitor(
         // Don't forget to flush the writer!
         stdout.flush().ok();
 
-        if interactive_mode && poll(Duration::from_secs(0)).into_diagnostic()? {
+        if !monitor_args.non_interactive && poll(Duration::from_secs(0)).into_diagnostic()? {
             if let Event::Key(key) = read().into_diagnostic()? {
                 if key.kind == KeyEventKind::Press {
                     if key.modifiers.contains(KeyModifiers::CONTROL) {

espflash/tests/scripts/flash.sh

@@ -1,7 +1,12 @@
 #!/usr/bin/env bash
 
-result=$(espflash flash --no-skip $1 2>&1)
+result=$(timeout 8s espflash flash --no-skip --monitor --non-interactive $1 2>&1)
 echo "$result"
 if [[ ! $result =~ "Flashing has completed!" ]]; then
+    echo "Flashing failed!"
+    exit 1
+fi
+if ! echo "$result" | grep -q "Hello world!"; then
+    echo "Monitoring failed!"
     exit 1
 fi