Refactor get_serial_port function to return SerialPortInfo instead of port name

jessebraham · jessebraham · commit 85c8eb84db7f · 2022-01-26T08:35:51.000-08:00
diff --git a/espflash/src/cli/mod.rs b/espflash/src/cli/mod.rs
@@ -1,31 +1,38 @@
-use self::clap::ConnectArgs;
 /// CLI utilities shared between espflash and cargo-espflash
 ///
-/// No stability guaranties applies
+/// No stability guaranties apply
 use config::Config;
 use miette::{Result, WrapErr};
-use serialport::FlowControl;
+use serialport::{FlowControl, SerialPortType};
 
-use crate::cli::serial::get_serial_port;
-use crate::{error::Error, Flasher};
+use self::clap::ConnectArgs;
+use crate::{cli::serial::get_serial_port_info, error::Error, Flasher};
 
 pub mod clap;
 pub mod config;
-mod line_endings;
 pub mod monitor;
+
+mod line_endings;
 mod serial;
 
 pub fn connect(matches: &ConnectArgs, config: &Config) -> Result<Flasher> {
-    let port = get_serial_port(matches, config)?;
+    let port_info = get_serial_port_info(matches, config)?;
 
     // Attempt to open the serial port and set its initial baud rate.
-    println!("Serial port: {}", port);
+    println!("Serial port: {}", port_info.port_name);
     println!("Connecting...\n");
-    let serial = serialport::new(&port, 115_200)
+    let serial = serialport::new(&port_info.port_name, 115_200)
         .flow_control(FlowControl::None)
         .open()
         .map_err(Error::from)
-        .wrap_err_with(|| format!("Failed to open serial port {}", port))?;
+        .wrap_err_with(|| format!("Failed to open serial port {}", port_info.port_name))?;
+
+    // NOTE: since `get_serial_port_info` filters out all non-USB serial ports, we
+    //       can just pretend the remaining types don't exist here.
+    let port_info = match port_info.port_type {
+        SerialPortType::UsbPort(info) => info,
+        _ => unreachable!(),
+    };
 
-    Ok(Flasher::connect(serial, matches.speed)?)
+    Ok(Flasher::connect(serial, port_info, matches.speed)?)
 }
diff --git a/espflash/src/cli/serial.rs b/espflash/src/cli/serial.rs
@@ -1,30 +1,33 @@
-use super::config::Config;
 use crossterm::style::Stylize;
 use dialoguer::{theme::ColorfulTheme, Confirm, Select};
 use miette::{IntoDiagnostic, Result};
 use serialport::{available_ports, SerialPortInfo, SerialPortType};
 
-use super::clap::ConnectArgs;
-use crate::cli::config::UsbDevice;
-use crate::error::Error;
+use super::{clap::ConnectArgs, config::Config};
+use crate::{cli::config::UsbDevice, error::Error};
 
-pub fn get_serial_port(matches: &ConnectArgs, config: &Config) -> Result<String, Error> {
+pub fn get_serial_port_info(
+    matches: &ConnectArgs,
+    config: &Config,
+) -> Result<SerialPortInfo, Error> {
     // A serial port should be specified either as a command-line argument or in a
     // configuration file. In the case that both have been provided the command-line
     // argument takes precedence.
     //
     // Users may optionally specify the device's VID and PID in the configuration
-    // file. If no VID/PID have been provided, the user will always be prompted to
-    // select a serial device. If some VID/PID have been provided the user will be
-    // prompted to select a serial device, unless there is only one found and its
-    // VID/PID matches the configured values.
-    if let Some(serial) = &matches.serial {
-        Ok(serial.to_owned())
+    // file. If no VID/PID has been provided, the user will always be prompted to
+    // select a serial port. If some VID and PID were provided then the user will
+    // also be prompted to select a port, unless there is only one found and its VID
+    // and PID match the configured values.
+    let ports = detect_usb_serial_ports().unwrap_or_default();
+
+    let maybe_port = if let Some(serial) = &matches.serial {
+        find_serial_port(&ports, serial.to_owned())
     } else if let Some(serial) = &config.connection.serial {
-        Ok(serial.to_owned())
-    } else if let Ok(ports) = detect_usb_serial_ports() {
+        find_serial_port(&ports, serial.to_owned())
+    } else if ports.len() > 0 {
         let (port, matches) = select_serial_port(ports, config)?;
-        match port.port_type {
+        match &port.port_type {
             SerialPortType::UsbPort(usb_info) if !matches => {
                 if Confirm::with_theme(&ColorfulTheme::default())
                     .with_prompt("Remember this serial port for future use?")
@@ -43,20 +46,39 @@ pub fn get_serial_port(matches: &ConnectArgs, config: &Config) -> Result<String,
             }
             _ => {}
         }
-        Ok(port.port_name)
+
+        Some(port)
+    } else {
+        None
+    };
+
+    if let Some(port_info) = maybe_port {
+        Ok(port_info)
     } else {
         Err(Error::NoSerial)
     }
 }
 
-/// serialport's autodetect doesn't provide any port information when using musl linux
-/// we can do some manual parsing of sysfs to get the relevant bits without udev
+/// Given a vector of `SerialPortInfo` structs, attempt to find and return one
+/// whose `port_name` field matches the provided `name` argument.
+fn find_serial_port(ports: &Vec<SerialPortInfo>, name: String) -> Option<SerialPortInfo> {
+    ports
+        .iter()
+        .find(|port| port.port_name == name)
+        .map(|port| port.to_owned())
+}
+
+/// serialport's autodetect doesn't provide any port information when using musl
+/// linux we can do some manual parsing of sysfs to get the relevant bits
+/// without udev
 #[cfg(all(target_os = "linux", target_env = "musl"))]
 fn detect_usb_serial_ports() -> Result<Vec<SerialPortInfo>> {
+    use std::{
+        fs::{read_link, read_to_string},
+        path::PathBuf,
+    };
+
     use serialport::UsbPortInfo;
-    use std::fs::read_link;
-    use std::fs::read_to_string;
-    use std::path::PathBuf;
 
     let ports = available_ports().into_diagnostic()?;
     let ports = ports
diff --git a/espflash/src/connection.rs b/espflash/src/connection.rs
@@ -6,7 +6,7 @@ use std::{
 
 use binread::{io::Cursor, BinRead, BinReaderExt};
 use bytemuck::{Pod, Zeroable};
-use serialport::{available_ports, SerialPort, SerialPortType};
+use serialport::{SerialPort, UsbPortInfo};
 use slip_codec::SlipDecoder;
 
 use crate::{
@@ -29,6 +29,7 @@ pub struct CommandResponse {
 
 pub struct Connection {
     serial: Box<dyn SerialPort>,
+    port_info: UsbPortInfo,
     decoder: SlipDecoder,
 }
 
@@ -42,9 +43,10 @@ struct WriteRegParams {
 }
 
 impl Connection {
-    pub fn new(serial: Box<dyn SerialPort>) -> Self {
+    pub fn new(serial: Box<dyn SerialPort>, port_info: UsbPortInfo) -> Self {
         Connection {
             serial,
+            port_info,
             decoder: SlipDecoder::new(),
         }
     }
@@ -62,33 +64,8 @@ impl Connection {
         Ok(())
     }
 
-    // This function is probably redundant there should be another way how to get
-    // PID in src/cli/mod.rs
-    pub fn get_pid(&self) -> u16 {
-        match available_ports() {
-            Ok(ports) => {
-                for p in ports {
-                    match p.port_type {
-                        SerialPortType::UsbPort(info) => {
-                            if self.serial.name().unwrap() == p.port_name {
-                                return info.pid;
-                            }
-                        }
-                        _ => {}
-                    }
-                }
-            }
-            Err(e) => {
-                eprintln!("{:?}", e);
-                eprintln!("Error listing serial ports");
-            }
-        }
-
-        0
-    }
-
     pub fn reset_to_flash(&mut self, extra_delay: bool) -> Result<(), Error> {
-        if Connection::get_pid(&self) == USB_SERIAL_JTAG_PID {
+        if self.port_info.pid == USB_SERIAL_JTAG_PID {
             self.serial.write_data_terminal_ready(false)?;
             self.serial.write_request_to_send(false)?;
 
diff --git a/espflash/src/flasher.rs b/espflash/src/flasher.rs
@@ -1,7 +1,7 @@
 use std::{borrow::Cow, thread::sleep};
 
 use bytemuck::{Pod, Zeroable, __core::time::Duration};
-use serialport::SerialPort;
+use serialport::{SerialPort, UsbPortInfo};
 use strum_macros::Display;
 
 use crate::{
@@ -151,10 +151,14 @@ pub struct Flasher {
 }
 
 impl Flasher {
-    pub fn connect(serial: Box<dyn SerialPort>, speed: Option<u32>) -> Result<Self, Error> {
+    pub fn connect(
+        serial: Box<dyn SerialPort>,
+        port_info: UsbPortInfo,
+        speed: Option<u32>,
+    ) -> Result<Self, Error> {
         let mut flasher = Flasher {
-            connection: Connection::new(serial), // default baud is always 115200
-            chip: Chip::Esp8266,                 // dummy, set properly later
+            connection: Connection::new(serial, port_info), // default baud is always 115200
+            chip: Chip::Esp8266,                            // dummy, set properly later
             flash_size: FlashSize::Flash4Mb,
             spi_params: SpiAttachParams::default(), // may be set when trying to attach to flash
         };
@@ -165,7 +169,7 @@ impl Flasher {
 
         if let Some(b) = speed {
             match flasher.chip {
-                Chip::Esp8266 => (), /* Not available */
+                Chip::Esp8266 => (), // Not available
                 _ => {
                     if b > 115_200 {
                         println!("WARN setting baud rate higher than 115200 can cause issues.");
