Check monitor args (#842)

* feat: Check monitor args

* docs: Udpate changelog

* style: Fix format

Author: SergioGasquez

CHANGELOG.md

@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `espflash` no longer allows flashing a too-big partition table (#830)
 - Allow specifying a partition label for `write-bin`, add `--partition-table`. (#828)
 - `--mmu-page-size` parameter for `flash` and `save-image` (#835)
+- Run some arguments checks for monitoring flags. (#842)
 
 ### Changed
 

cargo-espflash/src/main.rs

@@ -7,7 +7,12 @@ use std::{
 use cargo_metadata::{Message, MetadataCommand};
 use clap::{Args, CommandFactory, Parser, Subcommand};
 use espflash::{
-    cli::{self, config::Config, monitor::monitor, *},
+    cli::{
+        self,
+        config::Config,
+        monitor::{check_monitor_args, monitor},
+        *,
+    },
     flasher::FlashSize,
     logging::initialize_logger,
     targets::{Chip, XtalFrequency},
@@ -313,6 +318,11 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
     // Read the ELF data from the build path and load it to the target.
     let elf_data = fs::read(build_ctx.artifact_path.clone()).into_diagnostic()?;
 
+    let mut monitor_args = args.flash_args.monitor_args;
+    monitor_args.elf = Some(build_ctx.artifact_path.clone());
+
+    check_monitor_args(&args.flash_args.monitor, &monitor_args)?;
+
     print_board_info(&mut flasher)?;
     ensure_chip_compatibility(chip, Some(elf_data.as_slice()))?;
 
@@ -348,7 +358,6 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
 
     if args.flash_args.monitor {
         let pid = flasher.usb_pid();
-        let mut monitor_args = args.flash_args.monitor_args;
 
         // The 26MHz ESP32-C2's need to be treated as a special case.
         if chip == Chip::Esp32c2

espflash/src/bin/espflash.rs

@@ -2,7 +2,12 @@ use std::{fs, path::PathBuf};
 
 use clap::{Args, CommandFactory, Parser, Subcommand};
 use espflash::{
-    cli::{self, config::Config, monitor::monitor, *},
+    cli::{
+        self,
+        config::Config,
+        monitor::{check_monitor_args, monitor},
+        *,
+    },
     flasher::FlashSize,
     logging::initialize_logger,
     targets::{Chip, XtalFrequency},
@@ -203,6 +208,10 @@ pub fn erase_parts(args: ErasePartsArgs, config: &Config) -> Result<()> {
 }
 
 fn flash(args: FlashArgs, config: &Config) -> Result<()> {
+    let mut monitor_args = args.flash_args.monitor_args;
+    monitor_args.elf = Some(args.image.clone());
+    check_monitor_args(&args.flash_args.monitor, &monitor_args)?;
+
     let mut flasher = connect(
         &args.connect_args,
         config,
@@ -255,7 +264,6 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
 
     if args.flash_args.monitor {
         let pid = flasher.usb_pid();
-        let mut monitor_args = args.flash_args.monitor_args;
 
         // The 26MHz ESP32-C2's need to be treated as a special case.
         if chip == Chip::Esp32c2

espflash/src/cli/mod.rs

@@ -29,7 +29,7 @@ use serialport::{FlowControl, SerialPortInfo, SerialPortType, UsbPortInfo};
 
 use self::{
     config::Config,
-    monitor::{monitor, LogFormat},
+    monitor::{check_monitor_args, monitor, LogFormat},
 };
 use crate::{
     connection::reset::{ResetAfterOperation, ResetBeforeOperation},
@@ -1040,6 +1040,9 @@ pub fn make_flash_data(
 
 /// Write a binary to the flash memory of a target device
 pub fn write_bin(args: WriteBinArgs, config: &Config) -> Result<()> {
+    // Check monitor arguments
+    check_monitor_args(&args.monitor, &args.monitor_args)?;
+
     // Load the file to be flashed
     let mut f = File::open(&args.file).into_diagnostic()?;
 

espflash/src/cli/monitor/mod.rs

@@ -259,3 +259,34 @@ fn handle_key_event(key_event: KeyEvent) -> Option<Vec<u8>> {
 
     key_str.map(|slice| slice.into())
 }
+
+pub fn check_monitor_args(monitor: &bool, monitor_args: &MonitorConfigArgs) -> Result<()> {
+    // Check if any monitor args are provided but monitor flag isn't set
+    if !monitor
+        && (monitor_args.elf.is_some()
+            || monitor_args.log_format.is_some()
+            || monitor_args.output_format.is_some()
+            || monitor_args.processors.is_some()
+            || monitor_args.non_interactive
+            || monitor_args.no_reset
+            || monitor_args.monitor_baud != 115_200)
+    {
+        warn!("Monitor options were provided, but `--monitor/-M` flag isn't set. These options will be ignored.");
+    }
+
+    // Check if log-format is used with serial but output-format is specified
+    if let Some(LogFormat::Serial) = monitor_args.log_format {
+        if monitor_args.output_format.is_some() {
+            warn!("Output format specified but log format is serial. The output format option will be ignored.");
+        }
+    }
+
+    // Check if log-format is defmt but no ELF file is provided
+    if let Some(LogFormat::Defmt) = monitor_args.log_format {
+        if monitor_args.elf.is_none() {
+            warn!("Log format `defmt` requires an ELF file. Please provide one with the `--elf` option.");
+        }
+    }
+
+    Ok(())
+}