Minor error handling and function naming improvements (#795)

* Remove unnecessary `Result` from return type, don't leak constants from `connection` module

* Improve error handling when receiving an invalid command response type

* Update `CHANGELOG.md`
This commit is contained in:
Jesse Braham
2025-02-28 11:06:08 +01:00
committed by GitHub
parent acf2034f80
commit 11a5b7bb83
8 changed files with 39 additions and 25 deletions

View File

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `board-info` now prints `Security information`. (#758)
- The `command`, `elf` and `error` modules are no longer public (#772)
- `write-bin` now works for files whose lengths are not divisible by 4 (#780, #788)
- `get_usb_pid` is now `usb_pid` and no longer needlessly returns a `Result` (#795)
### Fixed

View File

@@ -354,7 +354,7 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
}
if args.flash_args.monitor {
let pid = flasher.get_usb_pid()?;
let pid = flasher.usb_pid();
let mut monitor_args = args.flash_args.monitor_args;
// The 26MHz ESP32-C2's need to be treated as a special case.

View File

@@ -267,7 +267,7 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
}
if args.flash_args.monitor {
let pid = flasher.get_usb_pid()?;
let pid = flasher.usb_pid();
let mut monitor_args = args.flash_args.monitor_args;
// The 26MHz ESP32-C2's need to be treated as a special case.

View File

@@ -557,7 +557,7 @@ pub fn print_board_info(flasher: &mut Flasher) -> Result<()> {
/// Open a serial monitor
pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {
let mut flasher = connect(&args.connect_args, config, true, true)?;
let pid = flasher.get_usb_pid()?;
let pid = flasher.usb_pid();
let elf = if let Some(elf_path) = args.monitor_args.elf.clone() {
let path = fs::canonicalize(elf_path).into_diagnostic()?;

View File

@@ -40,7 +40,7 @@ pub(crate) mod reset;
const MAX_CONNECT_ATTEMPTS: usize = 7;
const MAX_SYNC_ATTEMPTS: usize = 5;
pub(crate) const USB_SERIAL_JTAG_PID: u16 = 0x1001;
const USB_SERIAL_JTAG_PID: u16 = 0x1001;
#[cfg(unix)]
pub type Port = serialport::TTYPort;
@@ -60,8 +60,12 @@ impl TryInto<u32> for CommandResponseValue {
fn try_into(self) -> Result<u32, Self::Error> {
match self {
CommandResponseValue::ValueU32(value) => Ok(value),
CommandResponseValue::ValueU128(_) => Err(Error::InternalError),
CommandResponseValue::Vector(_) => Err(Error::InternalError),
CommandResponseValue::ValueU128(_) => Err(Error::InvalidResponse(
"expected `u32` but found `u128`".into(),
)),
CommandResponseValue::Vector(_) => Err(Error::InvalidResponse(
"expected `u32` but found `Vec`".into(),
)),
}
}
}
@@ -71,9 +75,13 @@ impl TryInto<u128> for CommandResponseValue {
fn try_into(self) -> Result<u128, Self::Error> {
match self {
CommandResponseValue::ValueU32(_) => Err(Error::InternalError),
CommandResponseValue::ValueU32(_) => Err(Error::InvalidResponse(
"expected `u128` but found `u32`".into(),
)),
CommandResponseValue::ValueU128(value) => Ok(value),
CommandResponseValue::Vector(_) => Err(Error::InternalError),
CommandResponseValue::Vector(_) => Err(Error::InvalidResponse(
"expected `u128` but found `Vec`".into(),
)),
}
}
}
@@ -83,8 +91,12 @@ impl TryInto<Vec<u8>> for CommandResponseValue {
fn try_into(self) -> Result<Vec<u8>, Self::Error> {
match self {
CommandResponseValue::ValueU32(_) => Err(Error::InternalError),
CommandResponseValue::ValueU128(_) => Err(Error::InternalError),
CommandResponseValue::ValueU32(_) => Err(Error::InvalidResponse(
"expected `Vec` but found `u32`".into(),
)),
CommandResponseValue::ValueU128(_) => Err(Error::InvalidResponse(
"expected `Vec` but found `u128`".into(),
)),
CommandResponseValue::Vector(value) => Ok(value),
}
}
@@ -263,7 +275,7 @@ 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()?;
let pid = self.usb_pid();
match self.after_operation {
ResetAfterOperation::HardReset => hard_reset(&mut self.serial, pid),
@@ -461,10 +473,11 @@ impl Connection {
/// Read a register command with a timeout
pub fn read_reg(&mut self, reg: u32) -> Result<u32, Error> {
self.with_timeout(CommandType::ReadReg.timeout(), |connection| {
let resp = self.with_timeout(CommandType::ReadReg.timeout(), |connection| {
connection.command(Command::ReadReg { address: reg })
})
.map(|v| v.try_into().unwrap())
})?;
resp.try_into()
}
/// Write a register command with a timeout
@@ -502,8 +515,12 @@ impl Connection {
}
/// Get the USB PID of the serial port
pub fn get_usb_pid(&self) -> Result<u16, Error> {
Ok(self.port_info.pid)
pub fn usb_pid(&self) -> u16 {
self.port_info.pid
}
pub(crate) fn is_using_usb_serial_jtag(&self) -> bool {
self.port_info.pid == USB_SERIAL_JTAG_PID
}
}

View File

@@ -205,9 +205,6 @@ pub enum Error {
#[error(transparent)]
TryFromSlice(#[from] TryFromSliceError),
#[error("Internal Error")]
InternalError,
#[error("Failed to open file: {0}")]
FileOpenError(String, #[source] io::Error),

View File

@@ -1188,8 +1188,8 @@ impl Flasher {
self.connection.into_serial()
}
pub fn get_usb_pid(&self) -> Result<u16, Error> {
self.connection.get_usb_pid()
pub fn usb_pid(&self) -> u16 {
self.connection.usb_pid()
}
pub fn erase_region(&mut self, offset: u32, size: u32) -> Result<(), Error> {
@@ -1248,7 +1248,7 @@ impl Flasher {
while data.len() < size as usize {
let response = self.connection.read_response()?;
let chunk: Vec<u8> = if let Some(response) = response {
response.value.try_into().unwrap()
response.value.try_into()?
} else {
return Err(Error::IncorrectResponse);
};
@@ -1268,7 +1268,7 @@ impl Flasher {
let response = self.connection.read_response()?;
let digest: Vec<u8> = if let Some(response) = response {
response.value.try_into().unwrap()
response.value.try_into()?
} else {
return Err(Error::IncorrectResponse);
};

View File

@@ -12,7 +12,6 @@ use crate::{
connection::{
command::{Command, CommandType},
Connection,
USB_SERIAL_JTAG_PID,
},
flasher::ProgressCallbacks,
targets::FlashTarget,
@@ -76,7 +75,7 @@ impl FlashTarget for Esp32Target {
//
// TODO: the stub doesn't appear to disable the watchdog on ESP32-S3, so we
// explicitly disable the watchdog here.
if connection.get_usb_pid()? == USB_SERIAL_JTAG_PID {
if connection.is_using_usb_serial_jtag() {
match self.chip {
Chip::Esp32c3 => {
connection.command(Command::WriteReg {