Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve monitoring #737

Merged
merged 8 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions cargo-espflash/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
27 changes: 11 additions & 16 deletions cargo-espflash/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
6 changes: 3 additions & 3 deletions espflash/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
27 changes: 11 additions & 16 deletions espflash/src/bin/espflash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
58 changes: 29 additions & 29 deletions espflash/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment on lines +149 to +150
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to make this require monitor (I coulndt get requires to work as it flatten), but it doesnt make sense that an user could set the monitor_args and not set monitor. I can only think of a check in the flash method that checks this and throws an error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah not sure how much we can really do about this. There are unfortunately some issues with using flatten, but it's a bit of a necessity with how things are currently structured. For now it's probably fine, we can merge as-is and open an issue to investigate this further.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue created: #764

/// Load the application to RAM instead of Flash
#[arg(long)]
pub ram: bool,
Expand All @@ -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
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -437,7 +445,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()?;

Expand All @@ -449,26 +457,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
Expand Down
28 changes: 15 additions & 13 deletions espflash/src/cli/monitor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

use std::{
io::{stdout, ErrorKind, Read, Write},
path::PathBuf,
time::Duration,
};

Expand All @@ -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},
};

Expand Down Expand Up @@ -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()?;
Expand All @@ -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 {
Expand All @@ -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) {
Expand Down
7 changes: 6 additions & 1 deletion espflash/tests/scripts/flash.sh
Original file line number Diff line number Diff line change
@@ -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
Loading