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

Add log-format argument #493

Merged
merged 6 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
43 changes: 9 additions & 34 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions cargo-espflash/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,3 @@ cargo = { version = "0.73.1", features = ["vendored-openssl"] }

[target.'cfg(windows)'.dependencies]
cargo = "0.73.1"

[features]
defmt = ["espflash/defmt"]
13 changes: 7 additions & 6 deletions cargo-espflash/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ use std::{

use cargo_metadata::Message;
use clap::{Args, CommandFactory, Parser, Subcommand};
use espflash::cli::{erase_flash, erase_region, EraseFlashArgs, EraseRegionArgs};
use espflash::{
cli::{
self, board_info, completions, config::Config, connect, erase_partitions, flash_elf_image,
monitor::monitor, parse_partition_table, partition_table, print_board_info,
save_elf_as_image, serial_monitor, CompletionsArgs, ConnectArgs, EspflashProgress,
FlashConfigArgs, MonitorArgs, PartitionTableArgs,
self, board_info, completions, config::Config, connect, erase_flash, erase_partitions,
erase_region, flash_elf_image, monitor::monitor_with, parse_partition_table,
partition_table, print_board_info, save_elf_as_image, serial_monitor, CompletionsArgs,
ConnectArgs, EraseFlashArgs, EraseRegionArgs, EspflashProgress, FlashConfigArgs,
MonitorArgs, PartitionTableArgs,
},
error::Error as EspflashError,
image_format::ImageFormatKind,
Expand Down Expand Up @@ -341,11 +341,12 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
115_200
};

monitor(
monitor_with(
flasher.into_interface(),
Some(&elf_data),
pid,
args.flash_args.monitor_baud.unwrap_or(default_baud),
args.flash_args.log_format,
)
.into_diagnostic()?;
}
Expand Down
11 changes: 5 additions & 6 deletions espflash/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ clap_complete = { version = "4.4.3", optional = true }
comfy-table = { version = "7.0.1", optional = true }
crossterm = { version = "0.25.0", optional = true } # 0.26.x causes issues on Windows
ctrlc = { version = "3.4.0", optional = true }
defmt-decoder = { version = "=0.3.8", features = ["unstable"], optional = true }
defmt-parser = { version = "=0.3.3", features = ["unstable"], optional = true }
defmt-decoder = { version = "0.3.9", features = ["unstable"], optional = true }
defmt-parser = { version = "0.3.3", features = ["unstable"], optional = true }
SergioGasquez marked this conversation as resolved.
Show resolved Hide resolved
dialoguer = { version = "0.10.4", optional = true }
directories = { version = "5.0.1", optional = true }
env_logger = { version = "0.10.0", optional = true }
Expand Down Expand Up @@ -74,6 +74,8 @@ cli = [
"dep:comfy-table",
"dep:crossterm",
"dep:ctrlc",
"dep:defmt-decoder",
"dep:defmt-parser",
"dep:dialoguer",
"dep:directories",
"dep:env_logger",
Expand All @@ -84,8 +86,5 @@ cli = [
"dep:regex",
"dep:update-informer",
]
defmt = [
"dep:defmt-decoder",
"dep:defmt-parser",
]

raspberry = ["dep:rppal"]
5 changes: 3 additions & 2 deletions espflash/src/bin/espflash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use clap::{Args, CommandFactory, Parser, Subcommand};
use espflash::{
cli::{
self, board_info, completions, config::Config, connect, erase_flash, erase_partitions,
erase_region, flash_elf_image, monitor::monitor, parse_partition_table, parse_uint32,
erase_region, flash_elf_image, monitor::monitor_with, parse_partition_table, parse_uint32,
partition_table, print_board_info, save_elf_as_image, serial_monitor, CompletionsArgs,
ConnectArgs, EraseFlashArgs, EraseRegionArgs, EspflashProgress, FlashConfigArgs,
MonitorArgs, PartitionTableArgs,
Expand Down Expand Up @@ -263,11 +263,12 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
115_200
};

monitor(
monitor_with(
flasher.into_interface(),
Some(&elf_data),
pid,
args.flash_args.monitor_baud.unwrap_or(default_baud),
args.flash_args.log_format,
)
.into_diagnostic()?;
}
Expand Down
15 changes: 13 additions & 2 deletions espflash/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ use log::{debug, info};
use miette::{IntoDiagnostic, Result, WrapErr};
use serialport::{SerialPortType, UsbPortInfo};

use self::{config::Config, monitor::monitor, serial::get_serial_port_info};
use self::{
config::Config,
monitor::{monitor_with, LogFormat},
serial::get_serial_port_info,
};
use crate::{
elf::ElfFirmwareImage,
error::{Error, MissingPartition, MissingPartitionTable},
Expand Down Expand Up @@ -134,6 +138,9 @@ pub struct FlashArgs {
/// Load the application to RAM instead of Flash
#[arg(long)]
pub ram: bool,
/// Logging format.
#[arg(long, short = 'f', default_value = "serial", requires = "monitor")]
pub log_format: LogFormat,
SergioGasquez marked this conversation as resolved.
Show resolved Hide resolved
}

/// Operations for partitions tables
Expand Down Expand Up @@ -185,6 +192,9 @@ pub struct MonitorArgs {
/// Connection configuration
#[clap(flatten)]
connect_args: ConnectArgs,
/// Logging format.
#[arg(long, short = 'f', default_value = "serial")]
pub log_format: LogFormat,
}

/// Select a serial port and establish a connection with a target device
Expand Down Expand Up @@ -291,11 +301,12 @@ pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {
115_200
};

monitor(
monitor_with(
flasher.into_interface(),
elf.as_deref(),
pid,
args.connect_args.baud.unwrap_or(default_baud),
args.log_format,
)
.into_diagnostic()?;

Expand Down
35 changes: 25 additions & 10 deletions espflash/src/cli/monitor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crossterm::{
};
use log::error;
use miette::{IntoDiagnostic, Result};
use strum::{Display, EnumIter, EnumString, EnumVariantNames};

use crate::{
cli::monitor::parser::{InputParser, ResolvingPrinter},
Expand All @@ -33,6 +34,17 @@ pub mod parser;
mod line_endings;
mod symbols;

#[cfg_attr(feature = "cli", derive(clap::ValueEnum))]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Display, EnumIter, EnumString, EnumVariantNames)]
#[non_exhaustive]
#[strum(serialize_all = "lowercase")]
pub enum LogFormat {
/// defmt
Defmt,
/// serial
Serial,
SergioGasquez marked this conversation as resolved.
Show resolved Hide resolved
}

/// Type that ensures that raw mode is disabled when dropped.
struct RawModeGuard;

Expand All @@ -58,22 +70,16 @@ pub fn monitor(
pid: u16,
baud: u32,
) -> serialport::Result<()> {
#[cfg(feature = "defmt")]
let parser = parser::esp_defmt::EspDefmt::new(elf);

#[cfg(not(feature = "defmt"))]
let parser = parser::serial::Serial;

monitor_with(serial, elf, pid, baud, parser)
monitor_with(serial, elf, pid, baud, LogFormat::Serial)
}

/// Open a serial monitor on the given interface, using the given input parser.
pub fn monitor_with<L: InputParser>(
pub fn monitor_with(
mut serial: Interface,
elf: Option<&[u8]>,
pid: u16,
baud: u32,
mut parser: L,
log_format: LogFormat,
) -> serialport::Result<()> {
println!("Commands:");
println!(" CTRL+R Reset chip");
Expand Down Expand Up @@ -102,7 +108,16 @@ pub fn monitor_with<L: InputParser>(
err => err,
}?;

parser.feed(&buff[0..read_count], &mut stdout);
match log_format {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why I didn't notice this but this isn't correct. The defmt parser is stateful, we can't just create a new one every time we have read some bytes.

LogFormat::Defmt => {
let mut parser = parser::esp_defmt::EspDefmt::new(elf);
parser.feed(&buff[0..read_count], &mut stdout);
}
LogFormat::Serial => {
let mut parser = parser::serial::Serial;
parser.feed(&buff[0..read_count], &mut stdout);
}
}

// Don't forget to flush the writer!
stdout.flush().ok();
Expand Down
1 change: 0 additions & 1 deletion espflash/src/cli/monitor/parser/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#[cfg(feature = "defmt")]
pub mod esp_defmt;
pub mod serial;

Expand Down