Implement reset strategies (#487)

* feat: Implement reset strategies * refactor: Use libc instead of nix * feat: Add debug message when reset fails * fix: Fix clippy lint * docs: Update changelog
esp-rs · Nov 16, 2023 · 2c7ee4c · 2c7ee4c
1 parent 2e73c4f
commit 2c7ee4c
Show file tree

Hide file tree

Showing 7 changed files with 309 additions and 96 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [Unreleased]
 
 ### Added
+- Added reset strategies (#487)
 
 - Read esp-println generated defmt messages (#466)
 - Add --target-app-partition argument to flash command (#461)

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cargo-espflash/Cargo.toml b/cargo-espflash/Cargo.toml
@@ -3,21 +3,11 @@ name = "cargo-espflash"
 version = "3.0.0-dev"
 edition = "2021"
 rust-version = "1.70"
-description = "Cargo subcommand for flashing Espressif devices"
-repository = "https://github.com/esp-rs/espflash"
-license = "MIT OR Apache-2.0"
-keywords = [
-    "cargo",
-    "cli",
-    "embedded",
-    "esp",
-]
-categories = [
-    "command-line-utilities",
-    "development-tools",
-    "development-tools::cargo-plugins",
-    "embedded",
-]
+description  = "Cargo subcommand for flashing Espressif devices"
+repository   = "https://github.com/esp-rs/espflash"
+license      = "MIT OR Apache-2.0"
+keywords     = ["cargo", "cli", "embedded", "esp"]
+categories   = ["command-line-utilities", "development-tools", "development-tools::cargo-plugins", "embedded"]
 
 [package.metadata.binstall]
 pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }.{ archive-format }"

diff --git a/espflash/Cargo.toml b/espflash/Cargo.toml
@@ -3,19 +3,11 @@ name = "espflash"
 version = "3.0.0-dev"
 edition = "2021"
 rust-version = "1.70"
-description = "A command-line tool for flashing Espressif devices"
-repository = "https://github.com/esp-rs/espflash"
-license = "MIT OR Apache-2.0"
-keywords = [
-    "cli",
-    "embedded",
-    "esp",
-]
-categories = [
-    "command-line-utilities",
-    "development-tools",
-    "embedded",
-]
+description  = "A command-line tool for flashing Espressif devices"
+repository   = "https://github.com/esp-rs/espflash"
+license      = "MIT OR Apache-2.0"
+keywords     = ["cli", "embedded", "esp"]
+categories   = ["command-line-utilities", "development-tools", "embedded"]
 
 [package.metadata.binstall]
 pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }.{ archive-format }"
@@ -26,8 +18,8 @@ pkg-fmt = "zip"
 rustdoc-args = ["--cfg", "docsrs"]
 
 [[bin]]
-name = "espflash"
-path = "./src/bin/espflash.rs"
+name              = "espflash"
+path              = "./src/bin/espflash.rs"
 required-features = ["cli"]
 
 [dependencies]
@@ -64,7 +56,10 @@ strum = { version = "0.25.0", features = ["derive"] }
 thiserror = "1.0.49"
 toml = "0.8.2"
 update-informer = { version = "1.1.0", optional = true }
-xmas-elf = "0.9.0"
+xmas-elf        = "0.9.0"
+
+[target.'cfg(unix)'.dependencies]
+libc = "0.2.101"
 
 [features]
 default = ["cli"]

diff --git a/espflash/src/connection.rs → espflash/src/connection/mod.rs b/espflash/src/connection.rs → espflash/src/connection/mod.rs
@@ -4,21 +4,29 @@
 //! sending/decoding of commands, and provides higher-level operations with the
 //! device.
 
-use std::{io::BufWriter, thread::sleep, time::Duration};
+use std::{io::BufWriter, iter::zip, thread::sleep, time::Duration};
 
 use binrw::{io::Cursor, BinRead, BinReaderExt};
-use log::{debug, info};
+use log::debug;
 use serialport::UsbPortInfo;
 use slip_codec::SlipDecoder;
 
-use self::encoder::SlipEncoder;
+#[cfg(unix)]
+use self::reset::UnixTightReset;
+use self::{
+    encoder::SlipEncoder,
+    reset::{construct_reset_strategy_sequence, ClassicReset, ResetStrategy, UsbJtagSerialReset},
+};
 use crate::{
     command::{Command, CommandType},
     error::{ConnectionError, Error, ResultExt, RomError, RomErrorKind},
     interface::Interface,
 };
 
-const DEFAULT_CONNECT_ATTEMPTS: usize = 7;
+mod reset;
+
+const MAX_CONNECT_ATTEMPTS: usize = 7;
+const MAX_SYNC_ATTEMPTS: usize = 5;
 pub(crate) const USB_SERIAL_JTAG_PID: u16 = 0x1001;
 
 /// A response from a target device following a command
@@ -50,29 +58,30 @@ impl Connection {
 
     /// Initialize a connection with a device
     pub fn begin(&mut self) -> Result<(), Error> {
-        let mut extra_delay = false;
-        for _ in 0..DEFAULT_CONNECT_ATTEMPTS {
-            if self.connect_attempt(extra_delay).is_err() {
-                extra_delay = !extra_delay;
-
-                info!(
-                    "Unable to connect, retrying with {} delay...",
-                    if extra_delay { "extra" } else { "default" }
-                );
-            } else {
-                return Ok(());
+        let port_name = self.serial.serial_port().name().unwrap_or_default();
+        let reset_sequence = construct_reset_strategy_sequence(&port_name, self.port_info.pid);
+
+        for (_, reset_strategy) in zip(0..MAX_CONNECT_ATTEMPTS, reset_sequence.iter().cycle()) {
+            match self.connect_attempt(reset_strategy) {
+                Ok(_) => {
+                    return Ok(());
+                }
+                Err(e) => {
+                    debug!("Failed to reset, error {:#?}, retrying", e);
+                }
             }
         }
 
         Err(Error::Connection(ConnectionError::ConnectionFailed))
     }
 
     /// Try to connect to a device
-    fn connect_attempt(&mut self, extra_delay: bool) -> Result<(), Error> {
-        self.reset_to_flash(extra_delay)?;
-
-        for _ in 0..5 {
+    #[allow(clippy::borrowed_box)]
+    fn connect_attempt(&mut self, reset_strategy: &Box<dyn ResetStrategy>) -> Result<(), Error> {
+        reset_strategy.reset(&mut self.serial)?;
+        for _ in 0..MAX_SYNC_ATTEMPTS {
             self.flush()?;
+
             if self.sync().is_ok() {
                 return Ok(());
             }
@@ -86,12 +95,14 @@ impl Connection {
         self.with_timeout(CommandType::Sync.timeout(), |connection| {
             connection.command(Command::Sync)?;
             connection.flush()?;
+
             sleep(Duration::from_millis(10));
-            for _ in 0..7 {
+
+            for _ in 0..MAX_CONNECT_ATTEMPTS {
                 match connection.read_response()? {
                     Some(response) if response.return_op == CommandType::Sync as u8 => {
                         if response.status == 1 {
-                            let _error = connection.flush();
+                            connection.flush().ok();
                             return Err(Error::RomError(RomError::new(
                                 CommandType::Sync,
                                 RomErrorKind::from(response.error),
@@ -115,47 +126,26 @@ impl Connection {
 
     // Reset the device
     pub fn reset(&mut self) -> Result<(), Error> {
-        let pid = self.port_info.pid;
-        Ok(reset_after_flash(&mut self.serial, pid)?)
+        reset_after_flash(&mut self.serial, self.port_info.pid)?;
+
+        Ok(())
     }
 
     // Reset the device to flash mode
     pub fn reset_to_flash(&mut self, extra_delay: bool) -> Result<(), Error> {
         if self.port_info.pid == USB_SERIAL_JTAG_PID {
-            self.serial.write_data_terminal_ready(false)?;
-            self.serial.write_request_to_send(false)?;
-
-            sleep(Duration::from_millis(100));
-
-            self.serial.write_data_terminal_ready(true)?;
-            self.serial.write_request_to_send(false)?;
-
-            sleep(Duration::from_millis(100));
-
-            self.serial.write_request_to_send(true)?;
-            self.serial.write_data_terminal_ready(false)?;
-            self.serial.write_request_to_send(true)?;
-
-            sleep(Duration::from_millis(100));
-
-            self.serial.write_data_terminal_ready(false)?;
-            self.serial.write_request_to_send(false)?;
+            UsbJtagSerialReset.reset(&mut self.serial)
         } else {
-            self.serial.write_data_terminal_ready(false)?;
-            self.serial.write_request_to_send(true)?;
-
-            sleep(Duration::from_millis(100));
-
-            self.serial.write_data_terminal_ready(true)?;
-            self.serial.write_request_to_send(false)?;
-
-            let millis = if extra_delay { 500 } else { 50 };
-            sleep(Duration::from_millis(millis));
+            #[cfg(unix)]
+            if UnixTightReset::new(extra_delay)
+                .reset(&mut self.serial)
+                .is_ok()
+            {
+                return Ok(());
+            }
 
-            self.serial.write_data_terminal_ready(false)?;
+            ClassicReset::new(extra_delay).reset(&mut self.serial)
         }
-
-        Ok(())
     }
 
     /// Set timeout for the serial port
@@ -177,11 +167,10 @@ impl Connection {
     }
 
     /// Run a command with a timeout defined by the command type
-    pub fn with_timeout<T, F: FnMut(&mut Connection) -> Result<T, Error>>(
-        &mut self,
-        timeout: Duration,
-        mut f: F,
-    ) -> Result<T, Error> {
+    pub fn with_timeout<T, F>(&mut self, timeout: Duration, mut f: F) -> Result<T, Error>
+    where
+        F: FnMut(&mut Connection) -> Result<T, Error>,
+    {
         let old_timeout = {
             let serial = self.serial.serial_port_mut();
             let old_timeout = serial.timeout();