Move non-interactive flag to connect args (#906)

* fix: Move non-interactive flag to connect args

* feat: Update changelog

* fix: Update cargo-espflash

* feat: Filter "dev/tty.*" ports in macos

Author: SergioGasquez

CHANGELOG.md

@@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Update checks can now be skipped by setting the the `ESPFLASH_SKIP_UPDATE_CHECK` environment variable (#900)
 - `flash_write_size` and `max_ram_block_size` functions no longer take a connection parameter and return a Result type (#903)
 - `DefaultProgressCallback` which implements `ProgressCallbacks` but all methods are no-ops (#904)
+- Update checks can now be skipped by setting the `ESPFLASH_SKIP_UPDATE_CHECK` environment variable (#900)
 
 ### Changed
 
@@ -82,6 +83,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `ResetBeforeOperation` & `ResetAfterOperation` are now public, to allow the creation of a `Connection` (#895)
 - `Flasher` now respects its internal `verify` and `skip` flags for all methods. (#901)
 - Progress is now reported on skipped segments and verification (#904)
+- Moved the `non-interactive` flag to `ConnectArgs` so we also avoid asking the user to select a port (#906)
 
 ### Removed
 

cargo-espflash/src/main.rs

@@ -348,7 +348,11 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
     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)?;
+    check_monitor_args(
+        &args.flash_args.monitor,
+        &monitor_args,
+        args.connect_args.non_interactive,
+    )?;
     check_idf_args(
         args.format,
         &args.flash_args.erase_parts,
@@ -419,6 +423,7 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
             Some(&elf_data),
             pid,
             monitor_args,
+            args.connect_args.non_interactive,
         )
     } else {
         Ok(())

espflash/src/bin/espflash.rs

@@ -234,7 +234,11 @@ 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)?;
+    check_monitor_args(
+        &args.flash_args.monitor,
+        &monitor_args,
+        args.connect_args.non_interactive,
+    )?;
     check_idf_args(
         args.format,
         &args.flash_args.erase_parts,
@@ -332,6 +336,7 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
             Some(&elf_data),
             pid,
             monitor_args,
+            args.connect_args.non_interactive,
         )
     } else {
         Ok(())

espflash/src/cli/mod.rs

@@ -90,6 +90,10 @@ pub struct ConnectArgs {
     /// Serial port connected to target device
     #[arg(short = 'p', long, env = "ESPFLASH_PORT")]
     pub port: Option<String>,
+    /// Avoids asking the user for interactions like selecting/resetting the
+    /// device
+    #[arg(long)]
+    pub non_interactive: bool,
 }
 
 /// Generate completions for the given shell
@@ -302,11 +306,10 @@ pub struct MonitorConfigArgs {
     /// ELF image to load the symbols from
     #[arg(long, value_name = "FILE")]
     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")]
+    ///
+    /// Only valid when `non_interactive` is also set.
+    #[arg(long)]
     no_reset: bool,
     /// The encoding of the target's serial output.
     #[arg(long, short = 'L')]
@@ -421,7 +424,7 @@ pub fn connect(
         .map_err(Error::from)
         .wrap_err_with(|| format!("Failed to open serial port {}", port_info.port_name))?;
 
-    // NOTE: since `get_serial_port_info` filters out all PCI Port and Bluetooth
+    // NOTE: since `serial_port_info` filters out all PCI Port and Bluetooth
     //       serial ports, we can just pretend these types don't exist here.
     let port_info = match port_info.port_type {
         SerialPortType::UsbPort(info) => info,
@@ -659,6 +662,7 @@ pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {
         elf.as_deref(),
         pid,
         monitor_args,
+        args.connect_args.non_interactive,
     )
 }
 
@@ -1095,7 +1099,11 @@ 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)?;
+    check_monitor_args(
+        &args.monitor,
+        &args.monitor_args,
+        args.connect_args.non_interactive,
+    )?;
 
     // Load the file to be flashed
     let mut f = File::open(&args.file).into_diagnostic()?;
@@ -1132,6 +1140,7 @@ pub fn write_bin(args: WriteBinArgs, config: &Config) -> Result<()> {
             None,
             pid,
             monitor_args,
+            args.connect_args.non_interactive,
         )?;
     }
 

espflash/src/cli/monitor/mod.rs

@@ -79,13 +79,14 @@ pub fn monitor(
     elf: Option<&[u8]>,
     pid: u16,
     monitor_args: MonitorConfigArgs,
+    non_interactive: bool,
 ) -> miette::Result<()> {
-    println!("Commands:");
-    println!("    CTRL+R    Reset chip");
-    println!("    CTRL+C    Exit");
-    println!();
-
-    if monitor_args.non_interactive && !monitor_args.no_reset {
+    if !non_interactive {
+        println!("Commands:");
+        println!("    CTRL+R    Reset chip");
+        println!("    CTRL+C    Exit");
+        println!();
+    } else if !monitor_args.no_reset {
         reset_after_flash(&mut serial, pid).into_diagnostic()?;
     }
 
@@ -139,7 +140,7 @@ pub fn monitor(
         // Don't forget to flush the writer!
         stdout.flush().ok();
 
-        if !handle_user_input(&mut serial, pid, monitor_args.non_interactive)? {
+        if !handle_user_input(&mut serial, pid, non_interactive)? {
             break;
         }
     }
@@ -263,14 +264,18 @@ fn handle_key_event(key_event: KeyEvent) -> Option<Vec<u8>> {
 }
 
 /// Checks the monitor arguments and emits warnings if they are invalid.
-pub fn check_monitor_args(monitor: &bool, monitor_args: &MonitorConfigArgs) -> Result<()> {
+pub fn check_monitor_args(
+    monitor: &bool,
+    monitor_args: &MonitorConfigArgs,
+    non_interactive: bool,
+) -> 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
+            || non_interactive
             || monitor_args.no_reset
             || monitor_args.monitor_baud != 115_200)
     {
@@ -279,6 +284,12 @@ pub fn check_monitor_args(monitor: &bool, monitor_args: &MonitorConfigArgs) -> R
         );
     }
 
+    if !non_interactive && monitor_args.no_reset {
+        warn!(
+            "The `--no-reset` flag only applies when using the `--non-interactive` flag. Ignoring it."
+        );
+    }
+
     // 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() {

espflash/src/cli/serial.rs

@@ -41,6 +41,9 @@ pub fn serial_port_info(matches: &ConnectArgs, config: &Config) -> Result<Serial
         find_serial_port(&ports, serial)
     } else {
         let ports = detect_usb_serial_ports(matches.list_all_ports).unwrap_or_default();
+        if ports.len() > 1 && matches.non_interactive {
+            return Err(Error::SerialNotSelected);
+        }
         let (port, matches) = select_serial_port(ports, &config.port_config, matches.confirm_port)?;
         match &port.port_type {
             SerialPortType::UsbPort(usb_info) if !matches => {
@@ -110,6 +113,18 @@ pub(super) fn detect_usb_serial_ports(list_all_ports: bool) -> Result<Vec<Serial
     let ports = ports
         .into_iter()
         .filter(|port_info| {
+            // On macOS, each USB serial device is exposed twice: once as a
+            // "/dev/tty.*" callin device and once as a "/dev/cu.*" callout
+            // device. The latter is what users typically want because opening
+            // a tty.* device will block until a DCD signal is asserted.  To
+            // avoid confusing users with duplicate entries we hide the tty.*
+            // variants unless the caller explicitly asked to list *all* ports.
+            // See https://web.archive.org/web/20120602152224/http://lists.berlios.de/pipermail/gpsd-dev/2005-April/001288.html
+            #[cfg(target_os = "macos")]
+            if !list_all_ports && port_info.port_name.starts_with("/dev/tty.") {
+                return false;
+            }
+
             if list_all_ports {
                 matches!(
                     &port_info.port_type,

espflash/src/error.rs

@@ -154,6 +154,15 @@ pub enum Error {
     )]
     SerialNotFound(String),
 
+    #[error("No serial port was provided")]
+    #[diagnostic(
+        code(espflash::serial_not_selected),
+        help(
+            "Make sure to provide a serial port via argument, environment variable or configuration file when using the `--non-interactive` flag"
+        )
+    )]
+    SerialNotSelected,
+
     #[error("The {chip} does not support {feature}")]
     #[diagnostic(code(espflash::unsupported_feature))]
     UnsupportedFeature { chip: Chip, feature: String },