changed read_status_response_deadline to `read_status_response_time…

…out`,

moved all timing functionality to the `SerialPort` trait and added `set_timeout`
re wrapped `serial2::SerialPort` to allow to timeout field,
updated CHANGELOG

CHANGELOG

@@ -1,10 +1,13 @@
 # Unreleased
-- [major][add] Added `System` and `Transport` traits.
-- [major][add] Added `StdSystem` & `SerialPort` structs.
+- [major][add] Added `Transport` trait. Used to abstract the serial port.
+- [major][add] Added `Serial2Port` struct, implementing the `Transport` trait for the `serial2` crate.
 - [major][add] Added `std` and `serial2` as default features
-- [major][change] Changed `bus` to be generic over `System/Transport`
+- [major][add] Added `alloc` as a feature.
+- [major][add] Added feature gates for `std`, `alloc` and `serial2`.
+- [major][change] Changed `Bus` to be generic over `Transport`
 - [major][change] Changed errors types to be generic over `Transport::Error`
-- [major][change] Moved `crate::serial2` re-export to `crate::system::serial_port::serial2`.
+- [major][change] Moved `crate::serial2` re-export to `crate::transport::serial2::serial2`.
+- [minor][change] Changed `Bus::read_status_response_deadline()` to `Bus::read_status_response_timeout()`, which takes a `Duration` instead of an `Instant`.
 
 # Version 0.9.1 - 2024-07-31
 - [minor][add] Add missing `Error` impl for `InitializeError`.

dynamixel2-cli/src/bin/dynamixel2/main.rs

@@ -1,6 +1,7 @@
-use serial2::SerialPort;
 use std::path::Path;
 use std::time::{Duration, Instant};
+use dynamixel2::transport::serial2::Serial2Port;
+
 mod logging;
 mod options;
 
@@ -151,7 +152,7 @@ fn do_main(options: Options) -> Result<(), ()> {
 	Ok(())
 }
 
-fn open_bus(options: &Options) -> Result<dynamixel2::Bus<Vec<u8>, Vec<u8>, SerialPort>, ()> {
+fn open_bus(options: &Options) -> Result<dynamixel2::Bus<Vec<u8>, Vec<u8>, Serial2Port>, ()> {
 	let bus = dynamixel2::Bus::open(&options.serial_port, options.baud_rate)
 		.map_err(|e| log::error!("Failed to open serial port: {}: {}", options.serial_port.display(), e))?;
 	log::debug!(

src/bus.rs

@@ -1,5 +1,4 @@
-use std::time::{Duration, Instant};
-
+use core::time::Duration;
 use crate::bytestuff;
 use crate::checksum::calculate_checksum;
 use crate::endian::{read_u16_le, read_u32_le, read_u8_le, write_u16_le};
@@ -50,7 +49,7 @@ where
 }
 
 #[cfg(feature = "serial2")]
-impl Bus<Vec<u8>, Vec<u8>, serial2::SerialPort> {
+impl Bus<Vec<u8>, Vec<u8>, crate::transport::serial2::Serial2Port> {
 	/// Open a serial port with the given baud rate.
 	///
 	/// This will allocate a new read and write buffer of 128 bytes each.
@@ -69,12 +68,12 @@ impl Bus<Vec<u8>, Vec<u8>, serial2::SerialPort> {
 	/// This will allocate a new read and write buffer of 128 bytes each.
 	/// Use [`Self::with_buffers()`] if you want to use a custom buffers.
 	pub fn new(serial_port: serial2::SerialPort) -> Result<Self, crate::InitializeError<std::io::Error>> {
-		Self::with_buffers(serial_port, vec![0; 128], vec![0; 128])
+		Self::with_buffers(serial_port.into(), vec![0; 128], vec![0; 128])
 	}
 }
 
 #[cfg(feature = "serial2")]
-impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer, serial2::SerialPort>
+impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer, crate::transport::serial2::Serial2Port>
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
@@ -115,14 +114,14 @@ where
 	}
 
 	/// Create a new bus using pre-allocated buffers.
-	fn with_buffers_and_baud_rate(transport: T, read_buffer: ReadBuffer, mut write_buffer: WriteBuffer, baud_rate: u32) -> Self {
+	fn with_buffers_and_baud_rate(transport: impl Into<T>, read_buffer: ReadBuffer, mut write_buffer: WriteBuffer, baud_rate: u32) -> Self {
 		// Pre-fill write buffer with the header prefix.
 		// TODO: return Err instead of panicking.
 		assert!(write_buffer.as_mut().len() >= HEADER_SIZE + 2);
 		write_buffer.as_mut()[..4].copy_from_slice(&HEADER_PREFIX);
 
 		Self {
-			transport,
+			transport: transport.into(),
 			baud_rate,
 			read_buffer,
 			read_len: 0,
@@ -234,10 +233,12 @@ where
 	}
 
 	/// Read a raw status response from the bus with the given deadline.
-	pub fn read_status_response_deadline(&mut self, deadline: Instant) -> Result<StatusPacket, ReadError<T::Error>> {
+	pub fn read_status_response_timeout(&mut self, timeout: Duration) -> Result<StatusPacket, ReadError<T::Error>> {
 		// Check that the read buffer is large enough to hold atleast a status packet header.
 		crate::error::BufferTooSmallError::check(STATUS_HEADER_SIZE, self.read_buffer.as_mut().len())?;
 
+		self.transport.set_timeout(timeout).map_err(ReadError::Io)?;
+
 		let stuffed_message_len = loop {
 			self.remove_garbage();
 
@@ -257,19 +258,8 @@ where
 				}
 			}
 
-			let timeout = match deadline.checked_duration_since(Instant::now()) {
-				Some(x) => x,
-				None => {
-					trace!(
-						"timeout reading status response, data in buffer: {:02X?}",
-						&self.read_buffer.as_ref()[..self.read_len]
-					);
-					return Err(ReadError::Timeout);
-				},
-			};
-
 			// Try to read more data into the buffer.
-			let new_data = self.transport.read(&mut self.read_buffer.as_mut()[self.read_len..], timeout)?;
+			let new_data = self.transport.read(&mut self.read_buffer.as_mut()[self.read_len..])?;
 			if new_data == 0 {
 				continue;
 			}
@@ -315,7 +305,7 @@ where
 		// Official SDK adds a flat 34 milliseconds, so lets just mimick that.
 		let message_size = STATUS_HEADER_SIZE as u32 + u32::from(expected_parameters) + 2;
 		let timeout = message_transfer_time(message_size, self.baud_rate) + Duration::from_millis(34);
-		self.read_status_response_deadline(Instant::now() + timeout)
+		self.read_status_response_timeout(timeout)
 	}
 }
 

src/instructions/bulk_read.rs

@@ -1,7 +1,7 @@
 use super::{instruction_id, packet_id, BulkReadData};
 use crate::endian::{write_u16_le, write_u8_le};
 use crate::transport::Transport;
-use crate::{Bus, ReadError, Response, TransferError, WriteError};
+use crate::{Bus, ReadError, Response, WriteError};
 
 #[cfg(feature = "alloc")]
 use alloc::{vec::Vec, borrow::ToOwned};

src/instructions/bulk_write.rs

@@ -93,11 +93,13 @@ where
 mod tests {
 	use super::*;
 
+	type Bus = crate::Bus<Vec<u8>, Vec<u8>, crate::transport::serial2::Serial2Port>;
+
 	/// Ensure that `bulk_write` accepts a slice of `BulkWriteData`.
 	///
 	/// This is a compile test. It only tests that the test code compiles.
 	#[allow(dead_code)]
-	fn bulk_write_accepts_slice(bus: &mut Bus<Vec<u8>, Vec<u8>, serial2::SerialPort>) -> Result<(), Box<dyn std::error::Error>> {
+	fn bulk_write_accepts_slice(bus: &mut Bus) -> Result<(), Box<dyn std::error::Error>> {
 		bus.bulk_write(&[
 			BulkWriteData {
 				motor_id: 1,
@@ -117,7 +119,7 @@ mod tests {
 	///
 	/// This is a compile test. It only tests that the test code compiles.
 	#[allow(dead_code)]
-	fn bulk_write_accepts_vec_ref(bus: &mut Bus<Vec<u8>, Vec<u8>, serial2::SerialPort>) -> Result<(), Box<dyn std::error::Error>> {
+	fn bulk_write_accepts_vec_ref(bus: &mut Bus) -> Result<(), Box<dyn std::error::Error>> {
 		bus.bulk_write(&vec![
 			BulkWriteData {
 				motor_id: 1,
@@ -138,7 +140,7 @@ mod tests {
 	/// This is a compile test. It only tests that the test code compiles.
 	#[allow(dead_code)]
 	fn bulk_write_accepts_vec_ref_no_clone(
-		bus: &mut Bus<Vec<u8>, Vec<u8>, serial2::SerialPort>,
+		bus: &mut Bus,
 	) -> Result<(), Box<dyn std::error::Error>> {
 		/// Non-clonable wrapper around `&[u8]` to ensure `bulk_write` doesn't clone data from vec references.
 		struct Data<'a> {

src/instructions/ping.rs

@@ -3,7 +3,7 @@ use core::time::{Duration};
 use super::{instruction_id, packet_id};
 use crate::bus::StatusPacket;
 use crate::transport::Transport;
-use crate::{Bus, ReadError, Response, TransferError, WriteError};
+use crate::{Bus, ReadError, Response, TransferError};
 
 #[cfg(feature = "alloc")]
 use alloc::vec::Vec;
@@ -81,10 +81,9 @@ where
 		self.write_instruction(packet_id::BROADCAST, instruction_id::PING, 0, |_| ())?;
 		let response_time = crate::bus::message_transfer_time(14, self.baud_rate());
 		let timeout = response_time * 253 + Duration::from_millis(34);
-		let deadline = Instant::now() + timeout;
 
 		loop {
-			let response = self.read_status_response_deadline(deadline);
+			let response = self.read_status_response_timeout(timeout);
 			match response {
 				Ok(response) => {
 					let response = response.try_into()?;

src/transport/mod.rs

@@ -17,8 +17,10 @@ pub trait Transport {
 	fn set_baud_rate(&mut self, baud_rate: u32) -> Result<(), Self::Error>;
 	/// Discard the input buffer of the transport. Maybe a no-op on some platforms.
 	fn discard_input_buffer(&mut self) -> Result<(), Self::Error>;
-	/// Returns available bytes to read, blocking until at least one byte is available or the timeout duration elapses.
-	fn read(&mut self, buffer: &mut [u8], timeout: Duration) -> Result<usize, ReadError<Self::Error>>;
+	/// Sets the timeout deadline and starts a timer. After the timeout duration elapses, the `[Self::read`] method will return with a timeout error.
+	fn set_timeout(&mut self, timeout: Duration) -> Result<(), Self::Error>;
+	/// Returns available bytes to read, blocking until at least one byte is available or the timeout duration elapses. The timeout must be set prior to calling with [`Self::set_timeout`].
+	fn read(&mut self, buffer: &mut [u8]) -> Result<usize, ReadError<Self::Error>>;
 	/// Write all bytes in the buffer to the transport.
 	fn write_all(&mut self, buffer: &[u8]) -> Result<(), Self::Error>;
 }

src/transport/serial2.rs

@@ -1,42 +1,97 @@
 //! Serial port transport implementation using the `serial2` crate.
 
 use crate::ReadError;
-use std::time::Duration;
+use std::time::{Duration, Instant};
 
 /// Re-exported `serial2` crate in case you need to modify serial port settings.
 pub use serial2;
 
-impl crate::Transport for serial2::SerialPort {
+/// A wrapper around a `serial2::SerialPort` that implements the `Transport` trait.
+pub struct Serial2Port {
+	/// The serial port.
+	pub port: serial2::SerialPort,
+	/// The deadline for the next read operation.
+	pub deadline: Option<Instant>,
+}
+
+impl Serial2Port {
+	/// Create a new `Serial2Port` from a `serial2::SerialPort`.
+	pub fn new(port: serial2::SerialPort) -> Self {
+		Self {
+			port,
+			deadline: None,
+		}
+	}
+}
+
+impl From<serial2::SerialPort> for Serial2Port {
+	fn from(port: serial2::SerialPort) -> Self {
+		Self::new(port)
+	}
+}
+
+impl core::fmt::Debug for Serial2Port {
+	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> std::fmt::Result {
+		#[derive(Debug)]
+		#[allow(dead_code)] // Dead code analysis ignores derive debug impls, but that is the whole point of this struct.
+		enum Raw {
+			#[cfg(unix)]
+			Fd(std::os::unix::io::RawFd),
+			#[cfg(windows)]
+			Handle(std::os::windows::io::RawHandle),
+		}
+
+		#[cfg(unix)]
+		let raw = {
+			use std::os::unix::io::AsRawFd;
+			Raw::Fd(self.port.as_raw_fd())
+		};
+		#[cfg(windows)]
+		let raw = {
+			use std::os::windows::io::AsRawHandle;
+			Raw::Handle(self.port.as_raw_handle())
+		};
+		write!(f, "SerialPort({:?})", raw)
+	}
+}
+
+impl crate::Transport for Serial2Port {
 	type Error = std::io::Error;
 
 	fn baud_rate(&self) -> Result<u32, crate::InitializeError<Self::Error>> {
-		self.get_configuration()
+		self.port.get_configuration()
 			.map_err(crate::InitializeError::GetConfiguration)?
 			.get_baud_rate()
 			.map_err(crate::InitializeError::GetBaudRate)
 	}
 
 	fn set_baud_rate(&mut self, baud_rate: u32) -> Result<(), Self::Error> {
-		let mut settings = self.get_configuration()?;
+		let mut settings = self.port.get_configuration()?;
 		settings.set_baud_rate(baud_rate)?;
-		self.set_configuration(&settings)?;
+		self.port.set_configuration(&settings)?;
 		Ok(())
 	}
 
 	fn discard_input_buffer(&mut self) -> Result<(), Self::Error> {
-		serial2::SerialPort::discard_input_buffer(self)
+		self.port.discard_input_buffer()
+	}
+
+	fn set_timeout(&mut self, timeout: Duration) -> Result<(), Self::Error> {
+		self.deadline = Some(Instant::now() + timeout);
+		Ok(())
 	}
 
-	fn read(&mut self, buffer: &mut [u8], timeout: Duration) -> Result<usize, ReadError<Self::Error>> {
-		self.set_read_timeout(timeout).map_err(ReadError::Io)?;
-		match serial2::SerialPort::read(self, buffer) {
+	fn read(&mut self, buffer: &mut [u8]) -> Result<usize, ReadError<Self::Error>> {
+		let timeout = self.deadline.ok_or(ReadError::Timeout)?.checked_duration_since(Instant::now()).ok_or(ReadError::Timeout)?;
+		self.port.set_read_timeout(timeout).map_err(ReadError::Io)?;
+		match self.port.read(buffer) {
 			Err(e) if e.kind() == std::io::ErrorKind::TimedOut => Err(ReadError::Timeout),
 			Err(e) => Err(ReadError::Io(e)),
 			Ok(count) => Ok(count),
 		}
 	}
 
 	fn write_all(&mut self, buffer: &[u8]) -> Result<(), Self::Error> {
-		serial2::SerialPort::write_all(self, buffer)
+		self.port.write_all(buffer)
 	}
 }