Apply relevant Rust API guidelines (#915)

* Derive more traits on public types in `image_format` module

* Don't leak `PartitionTable` type in public API

* Derive more traits on public types in `command` module

* fixups

* impl Hash for things

* fmt

* wording consistency

* feat: Udpate docstrings and derives

* feat: Implement C-CONV

* Derive more traits on public types in `flasher` module

* docs: Add missing module documentation

* docs: Udpate changelog

* docs: Fix changelog

---------

Co-authored-by: Scott Mabin <[email protected]>
Co-authored-by: Sergio Gasquez <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -31,11 +31,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add a `check-app-descriptor` bool option to `ImageArgs` and add the flag to `flash` command (#872)
 - `Connection::into_serial` to get the underlying port from the connection (#882)
 - All methods on the now removed `Target` & `ReadEFuse`, `UsbOtg` and `RtcWdtReset` traits have been implemented directly on (#891)
-- Update checks can now be skipped by setting the the `ESPFLASH_SKIP_UPDATE_CHECK` environment variable (#900)
+- Update checks can now be skipped by setting the `ESPFLASH_SKIP_UPDATE_CHECK` environment variable (#900)
 - `flash_write_size` and `max_ram_block_size` functions no longer take a connection parameter and return a Result type (#903)
 - `DefaultProgressCallback` which implements `ProgressCallbacks` but all methods are no-ops (#904)
-- Update checks can now be skipped by setting the `ESPFLASH_SKIP_UPDATE_CHECK` environment variable (#900)
 - `ProgressCallbacks` now has a `verifying` method to notify when post-flash checksum checking has begun (#908)
+- Implement `From<Connection> for Port` and both `From<Flasher> for Connection` and `Port` conversions (#915)
 
 ### Changed
 
@@ -79,7 +79,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed typos in error variant names (#782)
 - Fix `read-flash` which didn't work with some lengths (#804)
 - espflash can now flash an ESP32-S2 in download mode over USB (#813)
-- Fixed a case where esplash transformed the firmware elf in a way that made it unbootable (#831)
+- Fixed a case where espflash transformed the firmware ELF in a way that made it unbootable (#831)
 - The app descriptor is now correctly placed in the front of the binary (#835)
 - espflash now extracts the MMU page size from the app descriptor (#835)
 - `ResetBeforeOperation` & `ResetAfterOperation` are now public, to allow the creation of a `Connection` (#895)

cargo-espflash/src/main.rs

@@ -15,11 +15,7 @@ use espflash::{
         *,
     },
     flasher::FlashSize,
-    image_format::{
-        ImageFormat,
-        ImageFormatKind,
-        idf::{check_idf_bootloader, parse_partition_table},
-    },
+    image_format::{ImageFormat, ImageFormatKind, idf::check_idf_bootloader},
     logging::initialize_logger,
     target::{Chip, XtalFrequency},
     update::check_for_update,
@@ -419,7 +415,7 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
         monitor_args.elf = Some(build_ctx.artifact_path);
 
         monitor(
-            flasher.into_connection().into_serial(),
+            flasher.into(),
             Some(&elf_data),
             pid,
             monitor_args,

espflash/src/bin/espflash.rs

@@ -10,11 +10,7 @@ use espflash::{
         *,
     },
     flasher::FlashSize,
-    image_format::{
-        ImageFormat,
-        ImageFormatKind,
-        idf::{check_idf_bootloader, parse_partition_table},
-    },
+    image_format::{ImageFormat, ImageFormatKind, idf::check_idf_bootloader},
     logging::initialize_logger,
     target::{Chip, XtalFrequency},
     update::check_for_update,
@@ -332,7 +328,7 @@ fn flash(args: FlashArgs, config: &Config) -> Result<()> {
         monitor_args.elf = Some(args.image);
 
         monitor(
-            flasher.into_connection().into_serial(),
+            flasher.into(),
             Some(&elf_data),
             pid,
             monitor_args,

espflash/src/cli/mod.rs

@@ -50,12 +50,7 @@ use crate::{
         FlashSize,
         Flasher,
     },
-    image_format::{
-        ImageFormat,
-        ImageFormatKind,
-        Metadata,
-        idf::{IdfBootloaderFormat, parse_partition_table},
-    },
+    image_format::{ImageFormat, ImageFormatKind, Metadata, idf::IdfBootloaderFormat},
     target::{Chip, ProgressCallbacks, XtalFrequency},
 };
 
@@ -660,7 +655,7 @@ pub fn serial_monitor(args: MonitorArgs, config: &Config) -> Result<()> {
     }
 
     monitor(
-        flasher.into_connection().into_serial(),
+        flasher.into(),
         elf.as_deref(),
         pid,
         monitor_args,
@@ -996,6 +991,13 @@ pub fn partition_table(args: PartitionTableArgs) -> Result<()> {
     Ok(())
 }
 
+/// Parse a [PartitionTable] from the provided path
+pub fn parse_partition_table(path: &Path) -> Result<PartitionTable, Error> {
+    let data = fs::read(path).map_err(|e| Error::FileOpenError(path.display().to_string(), e))?;
+
+    Ok(PartitionTable::try_from(data)?)
+}
+
 /// Pretty print a partition table
 fn pretty_print(table: PartitionTable) {
     let mut pretty = Table::new();
@@ -1162,7 +1164,7 @@ pub fn write_bin(args: WriteBinArgs, config: &Config) -> Result<()> {
             monitor_args.monitor_baud = 74_880;
         }
         monitor(
-            flasher.into_connection().into_serial(),
+            flasher.into(),
             None,
             pid,
             monitor_args,

espflash/src/command.rs

@@ -3,6 +3,7 @@
 use std::{io::Write, mem::size_of, time::Duration};
 
 use bytemuck::{Pod, Zeroable, bytes_of};
+use serde::{Deserialize, Serialize};
 use strum::Display;
 
 use crate::{
@@ -30,7 +31,7 @@ const SYNC_FRAME: [u8; 36] = [
 /// Types of commands that can be sent to a target device
 ///
 /// <https://docs.espressif.com/projects/esptool/en/latest/esp32c3/advanced-topics/serial-protocol.html#supported-by-stub-loader-and-rom-loader>
-#[derive(Copy, Clone, Debug, Display)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Display, Deserialize, Serialize)]
 #[non_exhaustive]
 #[repr(u8)]
 pub enum CommandType {
@@ -95,7 +96,7 @@ pub enum CommandType {
 }
 
 /// The value of a command response.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)]
 pub enum CommandResponseValue {
     /// A 32-bit value.
     ValueU32(u32),
@@ -154,7 +155,7 @@ impl TryInto<Vec<u8>> for CommandResponseValue {
 }
 
 /// A response from a target device following a command.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)]
 pub struct CommandResponse {
     /// The response byte.
     pub resp: u8,
@@ -215,7 +216,7 @@ impl CommandType {
 /// Available commands
 ///
 /// See <https://docs.espressif.com/projects/esptool/en/latest/esp32c6/advanced-topics/serial-protocol.html#commands>
-#[derive(Copy, Clone, Debug)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Deserialize, Serialize)]
 #[non_exhaustive]
 pub enum Command<'a> {
     /// Begin flash download

espflash/src/connection/mod.rs

@@ -65,7 +65,7 @@ pub struct Connection {
 }
 
 impl Connection {
-    /// Create a new connection with a target device.
+    /// Creates a new connection with a target device.
     pub fn new(
         serial: Port,
         port_info: UsbPortInfo,
@@ -84,7 +84,7 @@ impl Connection {
         }
     }
 
-    /// Initialize a connection with a device.
+    /// Initializes a connection with a device.
     pub fn begin(&mut self) -> Result<(), Error> {
         let port_name = self.serial.name().unwrap_or_default();
         let reset_sequence = construct_reset_strategy_sequence(
@@ -109,7 +109,7 @@ impl Connection {
         )))
     }
 
-    /// Try to connect to a device.
+    /// Connects to a device.
     fn connect_attempt(&mut self, reset_strategy: &dyn ResetStrategy) -> Result<(), Error> {
         // If we're doing no_sync, we're likely communicating as a pass through
         // with an intermediate device to the ESP32
@@ -187,7 +187,7 @@ impl Connection {
         )))
     }
 
-    /// Try to sync with the device for a given timeout.
+    /// Syncs with a device.
     pub(crate) fn sync(&mut self) -> Result<(), Error> {
         self.with_timeout(CommandType::Sync.timeout(), |connection| {
             connection.command(Command::Sync)?;
@@ -221,14 +221,14 @@ impl Connection {
         Ok(())
     }
 
-    /// Reset the device.
+    /// Resets the device.
     pub fn reset(&mut self) -> Result<(), Error> {
         reset_after_flash(&mut self.serial, self.port_info.pid)?;
 
         Ok(())
     }
 
-    /// Reset the device taking into account the reset after argument.
+    /// Resets the device taking into account the reset after argument.
     pub fn reset_after(&mut self, is_stub: bool, chip: Chip) -> Result<(), Error> {
         let pid = self.usb_pid();
 
@@ -291,7 +291,7 @@ impl Connection {
         }
     }
 
-    /// Reset the device to flash mode.
+    /// Resets the device to flash mode.
     pub fn reset_to_flash(&mut self, extra_delay: bool) -> Result<(), Error> {
         if self.is_using_usb_serial_jtag() {
             UsbJtagSerialReset.reset(&mut self.serial)
@@ -308,25 +308,25 @@ impl Connection {
         }
     }
 
-    /// Set timeout for the serial port.
+    /// Sets the timeout for the serial port.
     pub fn set_timeout(&mut self, timeout: Duration) -> Result<(), Error> {
         self.serial.set_timeout(timeout)?;
         Ok(())
     }
 
-    /// Set baud rate for the serial port.
+    /// Sets the baud rate for the serial port.
     pub fn set_baud(&mut self, baud: u32) -> Result<(), Error> {
         self.serial.set_baud_rate(baud)?;
         self.baud = baud;
         Ok(())
     }
 
-    /// Get the current baud rate of the serial port.
+    /// Returns the current baud rate of the serial port.
     pub fn baud(&self) -> Result<u32, Error> {
         Ok(self.serial.baud_rate()?)
     }
 
-    /// Run a command with a timeout defined by the command type.
+    /// Runs a command with a timeout defined by the command type.
     pub fn with_timeout<T, F>(&mut self, timeout: Duration, mut f: F) -> Result<T, Error>
     where
         F: FnMut(&mut Connection) -> Result<T, Error>,
@@ -346,7 +346,7 @@ impl Connection {
         result
     }
 
-    /// Read the response from a serial port.
+    /// Reads the response from a serial port.
     pub fn read_flash_response(&mut self) -> Result<Option<CommandResponse>, Error> {
         let mut response = Vec::new();
 
@@ -369,7 +369,7 @@ impl Connection {
         Ok(Some(header))
     }
 
-    /// Read the response from a serial port.
+    /// Reads the response from a serial port.
     pub fn read_response(&mut self) -> Result<Option<CommandResponse>, Error> {
         match self.read(10)? {
             None => Ok(None),
@@ -432,7 +432,7 @@ impl Connection {
         }
     }
 
-    /// Write raw data to the serial port.
+    /// Writes raw data to the serial port.
     pub fn write_raw(&mut self, data: u32) -> Result<(), Error> {
         let mut binding = Box::new(&mut self.serial);
         let serial = binding.as_mut();
@@ -445,7 +445,7 @@ impl Connection {
         Ok(())
     }
 
-    /// Write a command to the serial port.
+    /// Writes a command to the serial port.
     pub fn write_command(&mut self, command: Command<'_>) -> Result<(), Error> {
         debug!("Writing command: {command:02x?}");
         let mut binding = Box::new(&mut self.serial);
@@ -460,7 +460,7 @@ impl Connection {
         Ok(())
     }
 
-    ///  Write a command and reads the response.
+    /// Writes a command and reads the response.
     pub fn command(&mut self, command: Command<'_>) -> Result<CommandResponseValue, Error> {
         let ty = command.command_type();
         self.write_command(command).for_command(ty)?;
@@ -495,7 +495,7 @@ impl Connection {
         )))
     }
 
-    /// Read a register command with a timeout.
+    /// Reads a register command with a timeout.
     pub fn read_reg(&mut self, addr: u32) -> Result<u32, Error> {
         let resp = self.with_timeout(CommandType::ReadReg.timeout(), |connection| {
             connection.command(Command::ReadReg { address: addr })
@@ -504,7 +504,7 @@ impl Connection {
         resp.try_into()
     }
 
-    /// Write a register command with a timeout.
+    /// Writes a register command with a timeout.
     pub fn write_reg(&mut self, addr: u32, value: u32, mask: Option<u32>) -> Result<(), Error> {
         self.with_timeout(CommandType::WriteReg.timeout(), |connection| {
             connection.command(Command::WriteReg {
@@ -517,7 +517,7 @@ impl Connection {
         Ok(())
     }
 
-    /// Read a register command with a timeout.
+    /// Reads a register command with a timeout.
     pub(crate) fn read(&mut self, len: usize) -> Result<Option<Vec<u8>>, Error> {
         let mut tmp = Vec::with_capacity(1024);
         loop {
@@ -528,18 +528,18 @@ impl Connection {
         }
     }
 
-    /// Flush the serial port.
+    /// Flushes  the serial port.
     pub fn flush(&mut self) -> Result<(), Error> {
         self.serial.flush()?;
         Ok(())
     }
 
-    /// Turn a serial port into a [Port].
+    /// Turns a serial port into a [Port].
     pub fn into_serial(self) -> Port {
         self.serial
     }
 
-    /// Get the USB PID of the serial port.
+    /// Returns the USB PID of the serial port.
     pub fn usb_pid(&self) -> u16 {
         self.port_info.pid
     }
@@ -559,7 +559,7 @@ impl Connection {
         self.before_operation
     }
 
-    /// Detect which chip is connected to this connection
+    /// Detects which chip is connected to this connection.
     pub fn detect_chip(
         &mut self,
         use_stub: bool,
@@ -580,15 +580,24 @@ impl Connection {
     }
 }
 
+impl From<Connection> for Port {
+    fn from(conn: Connection) -> Self {
+        conn.into_serial()
+    }
+}
+
 mod encoder {
     use std::io::Write;
 
+    use serde::Serialize;
+
     const END: u8 = 0xC0;
     const ESC: u8 = 0xDB;
     const ESC_END: u8 = 0xDC;
     const ESC_ESC: u8 = 0xDD;
 
     /// Encoder for the SLIP protocol.
+    #[derive(Debug, PartialEq, Eq, Serialize, Hash)]
     pub struct SlipEncoder<'a, W: Write> {
         writer: &'a mut W,
         len: usize,
@@ -601,7 +610,7 @@ mod encoder {
             Ok(Self { writer, len })
         }
 
-        /// Finish the encoding.
+        /// Finishes the encoding.
         pub fn finish(mut self) -> std::io::Result<usize> {
             self.len += self.writer.write(&[END])?;
             Ok(self.len)