Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unix tight reset #387

Closed
wants to merge 7 commits into from
+118 −38
Closed

Add unix tight reset #387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
de24719
#349 Add UnixTight reset strategy.
AVee Apr 16, 2023
191a212
#349 Add UnixTight reset strategy.
AVee Apr 16, 2023
cab0db5
#349 Add UnixTight reset strategy.
AVee Apr 19, 2023
b60dae6
#349 Add UnixTight reset strategy.
AVee May 11, 2023
d35df50
Merge branch 'esp-rs:main' into add_unix_tight_reset
AVee May 30, 2023
5db203e
#349 Add UnixTight reset strategy.
AVee Jun 1, 2023
453fd16
#349 Add UnixTight reset strategy.
AVee Jun 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions espflash/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ thiserror = "1.0.38"
toml = "0.7.3"
update-informer = { version = "0.6.0", optional = true }
xmas-elf = "0.9.0"
libc = "0.2"

[features]
default = ["cli"]
Expand Down
104 changes: 72 additions & 32 deletions espflash/src/connection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ impl Connection {
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(());
}
Expand All @@ -69,8 +64,19 @@ impl Connection {

/// Try to connect to a device
fn connect_attempt(&mut self, extra_delay: bool) -> Result<(), Error> {
self.reset_to_flash(extra_delay)?;
if self.port_info.pid == USB_SERIAL_JTAG_PID {
self.usb_jtag_reset()
} else {
#[cfg(unix)]
if self.unix_tight_reset(extra_delay).is_ok() {
return Ok(());
}

self.classic_reset(extra_delay)
}
}

fn wait_for_connection(&mut self) -> Result<(), Error> {
for _ in 0..5 {
self.flush()?;
if self.sync().is_ok() {
Expand Down Expand Up @@ -119,43 +125,77 @@ impl Connection {
Ok(reset_after_flash(&mut self.serial, pid)?)
}

// 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)?;
fn classic_reset(&mut self, extra_delay: bool) -> Result<(), Error> {
info!(
"Attempting Classic reset with {} delay...",
if extra_delay { "extra" } else { "default" }
);

sleep(Duration::from_millis(100));
self.serial.write_data_terminal_ready(false)?;
self.serial.write_request_to_send(false)?;

self.serial.write_data_terminal_ready(true)?;
self.serial.write_request_to_send(false)?;
self.serial.write_data_terminal_ready(true)?;
self.serial.write_request_to_send(true)?;
Comment on lines +134 to +138
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this come from? esptool doesn't do this. (?)

Also, since in unix_tight_reset we have the IO0/EN comments, it would be good to have them here too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only difference I can discern between this branch and the branch I have trying to solve this issue. If these 4 lines are all it takes to get my branch working that would be great, but I would like to be certain this does not have detrimental effects on other OSs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit of code essentially turns classic reset into unix tight reset. This seems to work because of the way the retrying is working currently:

https://github.com/esp-rs/espflash/pull/387/files#diff-9b8216d416248667f4612fb114829cb89c5754ca818d5784b3b0d879ec0d4ee6R70-R75

Here, we should be retrying the UNIX reset for X attempts, and then moving on to the classic mode.


sleep(Duration::from_millis(100));
self.serial.write_data_terminal_ready(false)?;
self.serial.write_request_to_send(true)?;

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));

sleep(Duration::from_millis(100));
self.serial.write_data_terminal_ready(true)?;
self.serial.write_request_to_send(false)?;

self.serial.write_data_terminal_ready(false)?;
self.serial.write_request_to_send(false)?;
} else {
self.serial.write_data_terminal_ready(false)?;
self.serial.write_request_to_send(true)?;
let millis = if extra_delay { 500 } else { 50 };
sleep(Duration::from_millis(millis));

sleep(Duration::from_millis(100));
self.serial.write_data_terminal_ready(false)?;
self.wait_for_connection()
}

self.serial.write_data_terminal_ready(true)?;
self.serial.write_request_to_send(false)?;
#[cfg(unix)]
fn unix_tight_reset(&mut self, extra_delay: bool) -> Result<(), Error> {
SergioGasquez marked this conversation as resolved.
Show resolved Hide resolved
info!(
"Attempting UnixTight reset with {} delay...",
if extra_delay { "extra" } else { "default" }
);

let millis = if extra_delay { 500 } else { 50 };
sleep(Duration::from_millis(millis));
self.serial.write_dtr_rts(false, false)?;
self.serial.write_dtr_rts(true, true)?;
self.serial.write_dtr_rts(false, true)?; // IO0 = HIGH, EN = LOW, chip in reset

self.serial.write_data_terminal_ready(false)?;
}
sleep(Duration::from_millis(100));

Ok(())
self.serial.write_dtr_rts(true, false)?; // IO0 = LOW, EN = HIGH, chip out of reset

let millis = if extra_delay { 500 } else { 50 };
sleep(Duration::from_millis(millis));

self.serial.write_dtr_rts(false, false)?; // IO0 = HIGH, done
self.serial.write_data_terminal_ready(false)?; // Needed in some environments to ensure IO0 = HIGH
self.wait_for_connection()
}

fn usb_jtag_reset(&mut self) -> Result<(), Error> {
SergioGasquez marked this conversation as resolved.
Show resolved Hide resolved
info!("Attempting USB-JTAG reset...");
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)?;
self.wait_for_connection()
}

/// Set timeout for the serial port
Expand Down
50 changes: 44 additions & 6 deletions espflash/src/interface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,21 @@

use std::io::Read;

#[cfg(unix)]
use {libc, std::io, std::os::unix::io::AsRawFd};

use miette::{Context, Result};
#[cfg(feature = "raspberry")]
use rppal::gpio::{Gpio, OutputPin};
use serialport::{FlowControl, SerialPort, SerialPortInfo};

use crate::error::Error;

#[cfg(unix)]
type Port = serialport::TTYPort;
#[cfg(windows)]
type Port = serialport::COMPort;

/// Errors relating to the configuration of a serial port
#[derive(thiserror::Error, Debug)]
#[non_exhaustive]
Expand All @@ -29,7 +37,7 @@ pub enum SerialConfigError {
/// implemented.
pub struct Interface {
/// Hardware serial port used for communication
pub serial_port: Box<dyn SerialPort>,
pub serial_port: Port,
/// Data Transmit Ready pin
#[cfg(feature = "raspberry")]
pub dtr: Option<OutputPin>,
Expand All @@ -49,10 +57,10 @@ fn write_gpio(gpio: &mut OutputPin, level: bool) {
}

/// Open a serial port
fn open_port(port_info: &SerialPortInfo) -> Result<Box<dyn SerialPort>> {
fn open_port(port_info: &SerialPortInfo) -> Result<Port> {
serialport::new(&port_info.port_name, 115_200)
.flow_control(FlowControl::None)
.open()
.open_native()
.map_err(Error::from)
.wrap_err_with(|| format!("Failed to open serial port {}", port_info.port_name))
}
Expand Down Expand Up @@ -112,6 +120,36 @@ impl Interface {
self.serial_port.write_data_terminal_ready(pin_state)
}

#[cfg(unix)]
pub fn write_dtr_rts(&self, dtr: bool, rts: bool) -> serialport::Result<()> {
let fd = self.serial_port.as_raw_fd();
unsafe {
let mut status: i32 = 0;
let res = libc::ioctl(fd, libc::TIOCMGET, &status);
if res != 0 {
return Err(io::Error::last_os_error().into());
}

if dtr {
status |= libc::TIOCM_DTR
} else {
status &= !libc::TIOCM_DTR
}

if rts {
status |= libc::TIOCM_RTS
} else {
status &= !libc::TIOCM_RTS
}

let res = libc::ioctl(fd, libc::TIOCMSET, &status);
if res != 0 {
return Err(io::Error::last_os_error().into());
}
}
Ok(())
}

/// Set the level of the RTS pin
pub fn write_request_to_send(&mut self, pin_state: bool) -> serialport::Result<()> {
#[cfg(feature = "raspberry")]
Expand All @@ -125,17 +163,17 @@ impl Interface {

/// Turn an [Interface] into a [SerialPort]
pub fn into_serial(self) -> Box<dyn SerialPort> {
self.serial_port
Box::new(self.serial_port)
}

/// Turn an [Interface] into a `&`[SerialPort]
pub fn serial_port(&self) -> &dyn SerialPort {
self.serial_port.as_ref()
&self.serial_port
}

/// Turn an [Interface] into a `&mut `[SerialPort]
pub fn serial_port_mut(&mut self) -> &mut dyn SerialPort {
self.serial_port.as_mut()
&mut self.serial_port
}
}

Expand Down