From da99a6df5c32e5950b577a2ba60773a8ce167d52 Mon Sep 17 00:00:00 2001 From: Sergio Gasquez Arcos Date: Wed, 12 Feb 2025 12:43:10 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 4 +++ cargo-espflash/README.md | 6 ++-- cargo-espflash/src/main.rs | 25 ++++++------- espflash/README.md | 6 ++-- espflash/src/bin/espflash.rs | 25 ++++++------- espflash/src/cli/mod.rs | 62 ++++++++++++++++++--------------- espflash/src/cli/monitor/mod.rs | 28 ++++++++------- espflash/tests/scripts/flash.sh | 7 +++- 8 files changed, 84 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f835a52..8583a77 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cargo-espflash/README.md b/cargo-espflash/README.md index 308dabf..b582ab4 100644 --- a/cargo-espflash/README.md +++ b/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 diff --git a/cargo-espflash/src/main.rs b/cargo-espflash/src/main.rs index 4fcfb6b..55e4371 100644 --- a/cargo-espflash/src/main.rs +++ b/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_args.baud_rate = 74_880; + } - 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.elf = Some(build_ctx.artifact_path); + + monitor(flasher.into_serial(), Some(&elf_data), pid, monitor_args) } else { Ok(()) } diff --git a/espflash/README.md b/espflash/README.md index ead03e6..a3baf34 100644 --- a/espflash/README.md +++ b/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 diff --git a/espflash/src/bin/espflash.rs b/espflash/src/bin/espflash.rs index 7fa5342..50edada 100644 --- a/espflash/src/bin/espflash.rs +++ b/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_args.baud_rate = 74_880; + } - 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.elf = Some(args.image); + + monitor(flasher.into_serial(), Some(&elf_data), pid, monitor_args) } else { Ok(()) } diff --git a/espflash/src/cli/mod.rs b/espflash/src/cli/mod.rs index 762663b..f0e1151 100644 --- a/espflash/src/cli/mod.rs +++ b/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>, - /// 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, + /// 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, } /// 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, + pub elf: Option, /// 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, @@ -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 { + 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()); diff --git a/espflash/src/cli/monitor/mod.rs b/espflash/src/cli/monitor/mod.rs index f7bfb7b..c846e50 100644 --- a/espflash/src/cli/monitor/mod.rs +++ b/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, - elf_file: Option, + 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 = match log_format { + let mut parser: Box = 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) { diff --git a/espflash/tests/scripts/flash.sh b/espflash/tests/scripts/flash.sh index 973ffdd..389de48 100644 --- a/espflash/tests/scripts/flash.sh +++ b/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