From eb15c6c58d0fdaef9b1e4a4bb3426ea2886ed7df Mon Sep 17 00:00:00 2001 From: Sergio Gasquez Date: Wed, 21 Feb 2024 10:40:35 +0100 Subject: [PATCH 1/5] fix: Hard reset for USB Serial JTAG --- espflash/src/cli/mod.rs | 9 +++++---- espflash/src/connection/mod.rs | 12 +++++++----- espflash/src/connection/reset.rs | 25 ++++++++++++++++++------- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/espflash/src/cli/mod.rs b/espflash/src/cli/mod.rs index 790bb4ae..b514d8a1 100644 --- a/espflash/src/cli/mod.rs +++ b/espflash/src/cli/mod.rs @@ -568,12 +568,13 @@ pub fn erase_flash(args: EraseFlashArgs, config: &Config) -> Result<()> { return Err(Error::StubRequired.into()); } - let mut flash = connect(&args.connect_args, config, true, true)?; - + let mut flasher = connect(&args.connect_args, config, true, true)?; info!("Erasing Flash..."); - flash.erase_flash()?; - flash.connection().reset_after(!args.connect_args.no_stub)?; + flasher.erase_flash()?; + flasher + .connection() + .reset_after(!args.connect_args.no_stub)?; Ok(()) } diff --git a/espflash/src/connection/mod.rs b/espflash/src/connection/mod.rs index f0899eb0..2e2b94c2 100644 --- a/espflash/src/connection/mod.rs +++ b/espflash/src/connection/mod.rs @@ -158,7 +158,7 @@ impl Connection { let mut buff: Vec; if self.before_operation != ResetBeforeOperation::NoReset { // Reset the chip to bootloader (download mode) - reset_strategy.reset(&mut self.serial)?; + reset_strategy.reset(&mut self.serial, None)?; let available_bytes = self.serial.bytes_to_read()?; buff = vec![0; available_bytes as usize]; @@ -256,8 +256,10 @@ impl Connection { // Reset the device taking into account the reset after argument pub fn reset_after(&mut self, is_stub: bool) -> Result<(), Error> { + let pid = self.get_usb_pid()?; + match self.after_operation { - ResetAfterOperation::HardReset => HardReset.reset(&mut self.serial), + ResetAfterOperation::HardReset => HardReset.reset(&mut self.serial, Some(pid)), ResetAfterOperation::NoReset => { info!("Staying in bootloader"); soft_reset(self, true, is_stub)?; @@ -274,17 +276,17 @@ impl Connection { // 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 { - UsbJtagSerialReset.reset(&mut self.serial) + UsbJtagSerialReset.reset(&mut self.serial, None) } else { #[cfg(unix)] if UnixTightReset::new(extra_delay) - .reset(&mut self.serial) + .reset(&mut self.serial, None) .is_ok() { return Ok(()); } - ClassicReset::new(extra_delay).reset(&mut self.serial) + ClassicReset::new(extra_delay).reset(&mut self.serial, None) } } diff --git a/espflash/src/connection/reset.rs b/espflash/src/connection/reset.rs index 77330b02..2605ce13 100644 --- a/espflash/src/connection/reset.rs +++ b/espflash/src/connection/reset.rs @@ -25,7 +25,7 @@ const EXTRA_RESET_DELAY: u64 = 500; // ms /// Some strategy for resting a target device pub trait ResetStrategy { - fn reset(&self, serial_port: &mut Port) -> Result<(), Error>; + fn reset(&self, serial_port: &mut Port, pid: Option) -> Result<(), Error>; fn set_dtr(&self, serial_port: &mut Port, level: bool) -> Result<(), Error> { serial_port.write_data_terminal_ready(level)?; @@ -92,7 +92,7 @@ impl ClassicReset { } impl ResetStrategy for ClassicReset { - fn reset(&self, serial_port: &mut Port) -> Result<(), Error> { + fn reset(&self, serial_port: &mut Port, _pid: Option) -> Result<(), Error> { debug!( "Using Classic reset strategy with delay of {}ms", self.delay @@ -143,7 +143,7 @@ impl UnixTightReset { #[cfg(unix)] impl ResetStrategy for UnixTightReset { - fn reset(&self, serial_port: &mut Port) -> Result<(), Error> { + fn reset(&self, serial_port: &mut Port, _pid: Option) -> Result<(), Error> { debug!( "Using UnixTight reset strategy with delay of {}ms", self.delay @@ -172,7 +172,7 @@ impl ResetStrategy for UnixTightReset { pub struct UsbJtagSerialReset; impl ResetStrategy for UsbJtagSerialReset { - fn reset(&self, serial_port: &mut Port) -> Result<(), Error> { + fn reset(&self, serial_port: &mut Port, _pid: Option) -> Result<(), Error> { debug!("Using UsbJtagSerial reset strategy"); self.set_rts(serial_port, false)?; @@ -205,12 +205,23 @@ impl ResetStrategy for UsbJtagSerialReset { pub struct HardReset; impl ResetStrategy for HardReset { - fn reset(&self, serial_port: &mut Port) -> Result<(), Error> { + fn reset(&self, serial_port: &mut Port, pid: Option) -> Result<(), Error> { debug!("Using HardReset reset strategy"); self.set_rts(serial_port, true)?; - sleep(Duration::from_millis(100)); - self.set_rts(serial_port, false)?; + if let Some(pid) = pid { + if pid == USB_SERIAL_JTAG_PID { + sleep(Duration::from_millis(200)); + self.set_rts(serial_port, false)?; + sleep(Duration::from_millis(200)); + } else { + sleep(Duration::from_millis(100)); + self.set_rts(serial_port, false)?; + } + } else { + sleep(Duration::from_millis(100)); + self.set_rts(serial_port, false)?; + } Ok(()) } From 4ee06c33e4ee22362e7303dc37277c8ce8b6b930 Mon Sep 17 00:00:00 2001 From: Sergio Gasquez Date: Wed, 21 Feb 2024 11:14:58 +0100 Subject: [PATCH 2/5] feat: Reuse reset_after_flash --- espflash/src/connection/reset.rs | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/espflash/src/connection/reset.rs b/espflash/src/connection/reset.rs index 2605ce13..0bbaae6c 100644 --- a/espflash/src/connection/reset.rs +++ b/espflash/src/connection/reset.rs @@ -13,7 +13,7 @@ use libc::ioctl; use crate::{ command::{Command, CommandType}, - connection::{Connection, Port, USB_SERIAL_JTAG_PID}, + connection::{reset_after_flash, Connection, Port, USB_SERIAL_JTAG_PID}, error::Error, flasher, }; @@ -208,20 +208,9 @@ impl ResetStrategy for HardReset { fn reset(&self, serial_port: &mut Port, pid: Option) -> Result<(), Error> { debug!("Using HardReset reset strategy"); - self.set_rts(serial_port, true)?; - if let Some(pid) = pid { - if pid == USB_SERIAL_JTAG_PID { - sleep(Duration::from_millis(200)); - self.set_rts(serial_port, false)?; - sleep(Duration::from_millis(200)); - } else { - sleep(Duration::from_millis(100)); - self.set_rts(serial_port, false)?; - } - } else { - sleep(Duration::from_millis(100)); - self.set_rts(serial_port, false)?; - } + // Using esptool HardReset strategy (https://github.com/espressif/esptool/blob/3301d0ff4638d4db1760a22540dbd9d07c55ec37/esptool/reset.py#L132-L153) + // leads to https://github.com/esp-rs/espflash/issues/592 in Windows. + reset_after_flash(serial_port, pid.unwrap_or_default())?; Ok(()) } From 28395cb55cdb7628b67fc6fd5940e04515946e1a Mon Sep 17 00:00:00 2001 From: Sergio Gasquez Date: Wed, 21 Feb 2024 11:19:19 +0100 Subject: [PATCH 3/5] refactor: Avoid implementing ResetStrategy for hard_reset --- espflash/src/connection/mod.rs | 12 ++++++------ espflash/src/connection/reset.rs | 28 +++++++++++----------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/espflash/src/connection/mod.rs b/espflash/src/connection/mod.rs index 2e2b94c2..79bd9527 100644 --- a/espflash/src/connection/mod.rs +++ b/espflash/src/connection/mod.rs @@ -21,7 +21,7 @@ use self::reset::UnixTightReset; use self::{ encoder::SlipEncoder, reset::{ - construct_reset_strategy_sequence, ClassicReset, HardReset, ResetAfterOperation, + construct_reset_strategy_sequence, hard_reset, ClassicReset, ResetAfterOperation, ResetBeforeOperation, ResetStrategy, UsbJtagSerialReset, }, }; @@ -158,7 +158,7 @@ impl Connection { let mut buff: Vec; if self.before_operation != ResetBeforeOperation::NoReset { // Reset the chip to bootloader (download mode) - reset_strategy.reset(&mut self.serial, None)?; + reset_strategy.reset(&mut self.serial)?; let available_bytes = self.serial.bytes_to_read()?; buff = vec![0; available_bytes as usize]; @@ -259,7 +259,7 @@ impl Connection { let pid = self.get_usb_pid()?; match self.after_operation { - ResetAfterOperation::HardReset => HardReset.reset(&mut self.serial, Some(pid)), + ResetAfterOperation::HardReset => hard_reset(&mut self.serial, pid), ResetAfterOperation::NoReset => { info!("Staying in bootloader"); soft_reset(self, true, is_stub)?; @@ -276,17 +276,17 @@ impl Connection { // 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 { - UsbJtagSerialReset.reset(&mut self.serial, None) + UsbJtagSerialReset.reset(&mut self.serial) } else { #[cfg(unix)] if UnixTightReset::new(extra_delay) - .reset(&mut self.serial, None) + .reset(&mut self.serial) .is_ok() { return Ok(()); } - ClassicReset::new(extra_delay).reset(&mut self.serial, None) + ClassicReset::new(extra_delay).reset(&mut self.serial) } } diff --git a/espflash/src/connection/reset.rs b/espflash/src/connection/reset.rs index 0bbaae6c..1db62c43 100644 --- a/espflash/src/connection/reset.rs +++ b/espflash/src/connection/reset.rs @@ -25,7 +25,7 @@ const EXTRA_RESET_DELAY: u64 = 500; // ms /// Some strategy for resting a target device pub trait ResetStrategy { - fn reset(&self, serial_port: &mut Port, pid: Option) -> Result<(), Error>; + fn reset(&self, serial_port: &mut Port) -> Result<(), Error>; fn set_dtr(&self, serial_port: &mut Port, level: bool) -> Result<(), Error> { serial_port.write_data_terminal_ready(level)?; @@ -92,7 +92,7 @@ impl ClassicReset { } impl ResetStrategy for ClassicReset { - fn reset(&self, serial_port: &mut Port, _pid: Option) -> Result<(), Error> { + fn reset(&self, serial_port: &mut Port) -> Result<(), Error> { debug!( "Using Classic reset strategy with delay of {}ms", self.delay @@ -143,7 +143,7 @@ impl UnixTightReset { #[cfg(unix)] impl ResetStrategy for UnixTightReset { - fn reset(&self, serial_port: &mut Port, _pid: Option) -> Result<(), Error> { + fn reset(&self, serial_port: &mut Port) -> Result<(), Error> { debug!( "Using UnixTight reset strategy with delay of {}ms", self.delay @@ -172,7 +172,7 @@ impl ResetStrategy for UnixTightReset { pub struct UsbJtagSerialReset; impl ResetStrategy for UsbJtagSerialReset { - fn reset(&self, serial_port: &mut Port, _pid: Option) -> Result<(), Error> { + fn reset(&self, serial_port: &mut Port) -> Result<(), Error> { debug!("Using UsbJtagSerial reset strategy"); self.set_rts(serial_port, false)?; @@ -199,21 +199,15 @@ impl ResetStrategy for UsbJtagSerialReset { } /// Reset sequence for hard resetting the chip. -/// -/// Can be used to reset out of the bootloader or to restart a running app. -#[derive(Debug, Clone, Copy)] -pub struct HardReset; - -impl ResetStrategy for HardReset { - fn reset(&self, serial_port: &mut Port, pid: Option) -> Result<(), Error> { - debug!("Using HardReset reset strategy"); +pub fn hard_reset(serial_port: &mut Port, pid: u16) -> Result<(), Error> { + debug!("Using HardReset reset strategy"); - // Using esptool HardReset strategy (https://github.com/espressif/esptool/blob/3301d0ff4638d4db1760a22540dbd9d07c55ec37/esptool/reset.py#L132-L153) - // leads to https://github.com/esp-rs/espflash/issues/592 in Windows. - reset_after_flash(serial_port, pid.unwrap_or_default())?; + // Using esptool HardReset strategy (https://github.com/espressif/esptool/blob/3301d0ff4638d4db1760a22540dbd9d07c55ec37/esptool/reset.py#L132-L153) + // leads to https://github.com/esp-rs/espflash/issues/592 in Windows, using `reset_after_flash` instead works fine for all platforms. + // We had similar issues in the past: https://github.com/esp-rs/espflash/pull/157 + reset_after_flash(serial_port, pid)?; - Ok(()) - } + Ok(()) } /// Perform a soft reset of the device. From ca4967a342e0fbab3071e7cdde1a494c9675ac64 Mon Sep 17 00:00:00 2001 From: Sergio Gasquez Date: Wed, 21 Feb 2024 11:33:19 +0100 Subject: [PATCH 4/5] refactor: Move reset_after_flash method to reset mod --- espflash/src/cli/monitor/mod.rs | 2 +- espflash/src/connection/mod.rs | 31 ++----------------------------- espflash/src/connection/reset.rs | 31 +++++++++++++++++++++++++++++-- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/espflash/src/cli/monitor/mod.rs b/espflash/src/cli/monitor/mod.rs index 41a80cbf..f59cc11a 100644 --- a/espflash/src/cli/monitor/mod.rs +++ b/espflash/src/cli/monitor/mod.rs @@ -27,7 +27,7 @@ use strum::{Display, EnumIter, EnumString, VariantNames}; use crate::{ cli::monitor::parser::{InputParser, ResolvingPrinter}, - connection::{reset_after_flash, Port}, + connection::{reset::reset_after_flash, Port}, }; pub mod parser; diff --git a/espflash/src/connection/mod.rs b/espflash/src/connection/mod.rs index 79bd9527..83d4debf 100644 --- a/espflash/src/connection/mod.rs +++ b/espflash/src/connection/mod.rs @@ -21,8 +21,8 @@ use self::reset::UnixTightReset; use self::{ encoder::SlipEncoder, reset::{ - construct_reset_strategy_sequence, hard_reset, ClassicReset, ResetAfterOperation, - ResetBeforeOperation, ResetStrategy, UsbJtagSerialReset, + construct_reset_strategy_sequence, hard_reset, reset_after_flash, ClassicReset, + ResetAfterOperation, ResetBeforeOperation, ResetStrategy, UsbJtagSerialReset, }, }; use crate::{ @@ -488,33 +488,6 @@ impl Connection { } } -/// Reset the target device when flashing has completed -pub fn reset_after_flash(serial: &mut Port, pid: u16) -> Result<(), serialport::Error> { - sleep(Duration::from_millis(100)); - - if pid == USB_SERIAL_JTAG_PID { - serial.write_data_terminal_ready(false)?; - - sleep(Duration::from_millis(100)); - - serial.write_request_to_send(true)?; - serial.write_data_terminal_ready(false)?; - serial.write_request_to_send(true)?; - - sleep(Duration::from_millis(100)); - - serial.write_request_to_send(false)?; - } else { - serial.write_request_to_send(true)?; - - sleep(Duration::from_millis(100)); - - serial.write_request_to_send(false)?; - } - - Ok(()) -} - mod encoder { use std::io::Write; diff --git a/espflash/src/connection/reset.rs b/espflash/src/connection/reset.rs index 1db62c43..f8c6aa9f 100644 --- a/espflash/src/connection/reset.rs +++ b/espflash/src/connection/reset.rs @@ -1,4 +1,4 @@ -//! This entire module is copied from `esptool.py` (https://github.com/espressif/esptool/blob/a8586d02b1305ebc687d31783437a7f4d4dbb70f/esptool/reset.py) +//! Most of this module is copied from `esptool.py` (https://github.com/espressif/esptool/blob/a8586d02b1305ebc687d31783437a7f4d4dbb70f/esptool/reset.py) #[cfg(unix)] use std::{io, os::fd::AsRawFd}; @@ -13,7 +13,7 @@ use libc::ioctl; use crate::{ command::{Command, CommandType}, - connection::{reset_after_flash, Connection, Port, USB_SERIAL_JTAG_PID}, + connection::{Connection, Port, USB_SERIAL_JTAG_PID}, error::Error, flasher, }; @@ -198,6 +198,33 @@ impl ResetStrategy for UsbJtagSerialReset { } } +/// Reset the target device +pub fn reset_after_flash(serial: &mut Port, pid: u16) -> Result<(), serialport::Error> { + sleep(Duration::from_millis(100)); + + if pid == USB_SERIAL_JTAG_PID { + serial.write_data_terminal_ready(false)?; + + sleep(Duration::from_millis(100)); + + serial.write_request_to_send(true)?; + serial.write_data_terminal_ready(false)?; + serial.write_request_to_send(true)?; + + sleep(Duration::from_millis(100)); + + serial.write_request_to_send(false)?; + } else { + serial.write_request_to_send(true)?; + + sleep(Duration::from_millis(100)); + + serial.write_request_to_send(false)?; + } + + Ok(()) +} + /// Reset sequence for hard resetting the chip. pub fn hard_reset(serial_port: &mut Port, pid: u16) -> Result<(), Error> { debug!("Using HardReset reset strategy"); From eb92bcd0768dd06ec70d1fea199a622d4db66b0b Mon Sep 17 00:00:00 2001 From: Sergio Gasquez Date: Wed, 21 Feb 2024 11:35:45 +0100 Subject: [PATCH 5/5] docs: Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38fabc5e..c6794064 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,10 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added ### Fixed +- Change the `hard_reset` sequence to fix Windows issues (#594) ### Changed - - `FlashData::new` now returns `crate::Error` (#591) +- Moved `reset_after_flash` method to `reset` module (#594) ### Removed