From 8e5553269c620b25a14a5cf3036cb1fbf55ad6c7 Mon Sep 17 00:00:00 2001
From: Omelia Iliffe <omelia.iliffe@wetaworkshop.co.nz>
Date: Mon, 11 Dec 2023 10:21:11 +1300
Subject: [PATCH 01/15] changed functions to return a response struct, this
 allows for the reading of the alert bit to check for hardware errors

---
 src/bus.rs                        | 112 ++++++++++++++++++++++---
 src/error.rs                      |   2 +-
 src/instructions/action.rs        |   7 +-
 src/instructions/bulk_read.rs     |  44 +++++-----
 src/instructions/clear.rs         |   7 +-
 src/instructions/factory_reset.rs |   7 +-
 src/instructions/mod.rs           |   1 -
 src/instructions/ping.rs          |  34 +++++---
 src/instructions/read.rs          |  47 +++--------
 src/instructions/reboot.rs        |   7 +-
 src/instructions/reg_write.rs     |  18 ++--
 src/instructions/sync_read.rs     | 131 ++++++++----------------------
 src/instructions/write.rs         |  18 ++--
 13 files changed, 221 insertions(+), 214 deletions(-)

diff --git a/src/bus.rs b/src/bus.rs
index ab50f38..f6a1754 100644
--- a/src/bus.rs
+++ b/src/bus.rs
@@ -4,7 +4,7 @@ use std::time::{Duration, Instant};
 
 use crate::bytestuff;
 use crate::checksum::calculate_checksum;
-use crate::endian::{read_u16_le, write_u16_le};
+use crate::endian::{read_u16_le, read_u32_le, read_u8_le, write_u16_le};
 use crate::{ReadError, TransferError, WriteError};
 
 const HEADER_PREFIX: [u8; 4] = [0xFF, 0xFF, 0xFD, 0x00];
@@ -95,7 +95,7 @@ where
 		instruction_id: u8,
 		parameter_count: usize,
 		encode_parameters: F,
-	) -> Result<Response<ReadBuffer, WriteBuffer>, TransferError>
+	) -> Result<StatusPacket<ReadBuffer, WriteBuffer>, TransferError>
 	where
 		F: FnOnce(&mut [u8]),
 	{
@@ -150,15 +150,13 @@ where
 		// Send message.
 		let stuffed_message = &buffer[..checksum_index + 2];
 		trace!("sending instruction: {:02X?}", stuffed_message);
-		self.serial_port.discard_input_buffer()
-			.map_err(WriteError::DiscardBuffer)?;
-		self.serial_port.write_all(stuffed_message)
-			.map_err(WriteError::Write)?;
+		self.serial_port.discard_input_buffer().map_err(WriteError::DiscardBuffer)?;
+		self.serial_port.write_all(stuffed_message).map_err(WriteError::Write)?;
 		Ok(())
 	}
 
 	/// Read a raw status response from the bus.
-	pub fn read_status_response(&mut self) -> Result<Response<ReadBuffer, WriteBuffer>, ReadError> {
+	pub fn read_status_response(&mut self) -> Result<StatusPacket<ReadBuffer, WriteBuffer>, ReadError> {
 		let deadline = Instant::now() + self.read_timeout;
 		let stuffed_message_len = loop {
 			self.remove_garbage();
@@ -176,7 +174,10 @@ where
 			}
 
 			if Instant::now() > deadline {
-				trace!("timeout reading status response, data in buffer: {:02X?}", &self.read_buffer.as_ref()[..self.read_len]);
+				trace!(
+					"timeout reading status response, data in buffer: {:02X?}",
+					&self.read_buffer.as_ref()[..self.read_len]
+				);
 				return Err(std::io::ErrorKind::TimedOut.into());
 			}
 
@@ -208,7 +209,7 @@ where
 		let parameter_count = bytestuff::unstuff_inplace(&mut buffer[STATUS_HEADER_SIZE..parameters_end]);
 
 		// Creating the response struct here means that the data gets purged from the buffer even if we return early using the try operator.
-		let response = Response {
+		let response = StatusPacket {
 			bus: self,
 			stuffed_message_len,
 			parameter_count,
@@ -246,7 +247,7 @@ where
 /// A status response that is currently in the read buffer of a bus.
 ///
 /// When dropped, the response data is removed from the read buffer.
-pub struct Response<'a, ReadBuffer, WriteBuffer>
+pub struct StatusPacket<'a, ReadBuffer, WriteBuffer>
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
@@ -261,7 +262,7 @@ where
 	parameter_count: usize,
 }
 
-impl<'a, ReadBuffer, WriteBuffer> Response<'a, ReadBuffer, WriteBuffer>
+impl<'a, ReadBuffer, WriteBuffer> StatusPacket<'a, ReadBuffer, WriteBuffer>
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
@@ -289,13 +290,18 @@ where
 		self.as_bytes()[8]
 	}
 
+	// The alert bit from the error feild of the response.
+	pub fn alert(&self) -> bool {
+		self.error() & 0x80 != 0
+	}
+
 	/// The parameters of the response.
 	pub fn parameters(&self) -> &[u8] {
 		&self.as_bytes()[STATUS_HEADER_SIZE..][..self.parameter_count]
 	}
 }
 
-impl<'a, ReadBuffer, WriteBuffer> Drop for Response<'a, ReadBuffer, WriteBuffer>
+impl<'a, ReadBuffer, WriteBuffer> Drop for StatusPacket<'a, ReadBuffer, WriteBuffer>
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
@@ -305,6 +311,88 @@ where
 	}
 }
 
+pub struct Response<T> {
+	pub motor_id: u8,
+	pub alert: bool,
+	pub data: T,
+	pub address: Option<u16>,
+}
+
+impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<()>
+where
+	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
+	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
+{
+	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
+		Self {
+			data: (),
+			motor_id: status_packet.packet_id(),
+			alert: status_packet.alert(),
+			address: None,
+		}
+	}
+}
+
+impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<Vec<u8>>
+where
+	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
+	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
+{
+	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
+		Self {
+			data: status_packet.parameters().to_owned(),
+			motor_id: status_packet.packet_id(),
+			alert: status_packet.alert(),
+			address: None,
+		}
+	}
+}
+
+impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u8>
+where
+	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
+	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
+{
+	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
+		Self {
+			data: read_u8_le(status_packet.parameters()),
+			motor_id: status_packet.packet_id(),
+			alert: status_packet.alert(),
+			address: None,
+		}
+	}
+}
+
+impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u16>
+where
+	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
+	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
+{
+	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
+		Self {
+			data: read_u16_le(status_packet.parameters()),
+			motor_id: status_packet.packet_id(),
+			alert: status_packet.alert(),
+			address: None,
+		}
+	}
+}
+
+impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u32>
+where
+	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
+	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
+{
+	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
+		Self {
+			data: read_u32_le(status_packet.parameters()),
+			motor_id: status_packet.packet_id(),
+			alert: status_packet.alert(),
+			address: None,
+		}
+	}
+}
+
 /// Find the potential starting position of a header.
 ///
 /// This will return the first possible position of the header prefix.
diff --git a/src/error.rs b/src/error.rs
index 7cc14e4..38bad71 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -82,7 +82,7 @@ pub struct InvalidParameterCount {
 
 impl MotorError {
 	pub fn check(raw: u8) -> Result<(), Self> {
-		if raw == 0 {
+		if raw & !0x80 == 0 {
 			Ok(())
 		} else {
 			Err(Self { raw })
diff --git a/src/instructions/action.rs b/src/instructions/action.rs
index cacb3a5..b21161f 100644
--- a/src/instructions/action.rs
+++ b/src/instructions/action.rs
@@ -1,5 +1,5 @@
 use super::{instruction_id, packet_id};
-use crate::{Bus, TransferError, WriteError};
+use crate::{Bus, Response, TransferError, WriteError};
 
 impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer>
 where
@@ -10,14 +10,15 @@ where
 	///
 	/// The `motor_id` parameter may be set to [`packet_id::BROADCAST`],
 	/// although the [`Self::broadcast_action`] is generally easier to use.
-	pub fn action(&mut self, motor_id: u8) -> Result<(), TransferError> {
+	pub fn action(&mut self, motor_id: u8) -> Result<Option<Response<()>>, TransferError> {
 		if motor_id == packet_id::BROADCAST {
 			self.broadcast_action()?;
+			Ok(None)
 		} else {
 			let response = self.transfer_single(motor_id, instruction_id::ACTION, 0, |_| ())?;
 			crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
+			Ok(Some(response.into()))
 		}
-		Ok(())
 	}
 
 	/// Broadcast an action command to all connected motors to trigger a previously registered instruction.
diff --git a/src/instructions/bulk_read.rs b/src/instructions/bulk_read.rs
index 5ce79b5..949d17e 100644
--- a/src/instructions/bulk_read.rs
+++ b/src/instructions/bulk_read.rs
@@ -1,6 +1,6 @@
-use super::{instruction_id, packet_id, BulkData};
-use crate::endian::{write_u8_le, write_u16_le};
-use crate::{Bus, ReadError, WriteError, TransferError};
+use super::{instruction_id, packet_id};
+use crate::endian::{write_u16_le, write_u8_le};
+use crate::{Bus, ReadError, Response, TransferError, WriteError};
 
 pub struct BulkRead {
 	pub motor_id: u8,
@@ -31,19 +31,20 @@ where
 	/// # Panics
 	/// The protocol forbids specifying the same motor ID multiple times.
 	/// This function panics if the same motor ID is used for more than one read.
-	pub fn bulk_read_cb<Read, F>(
-		&mut self,
-		reads: &[Read],
-		mut on_response: F,
-	) -> Result<(), WriteError>
+	pub fn bulk_read_cb<Read, F>(&mut self, reads: &[Read], mut on_response: F) -> Result<(), WriteError>
 	where
 		Read: AsRef<BulkRead>,
-		F: FnMut(Result<BulkData<&[u8]>, ReadError>),
+		F: FnMut(Result<Response<Vec<u8>>, ReadError>),
 	{
 		for i in 0..reads.len() {
 			for j in i + 1..reads.len() {
 				if reads[i].as_ref().motor_id == reads[j].as_ref().motor_id {
-					panic!("bulk_read_cb: motor ID {} used multiple at index {} and {}", reads[i].as_ref().motor_id, i, j)
+					panic!(
+						"bulk_read_cb: motor ID {} used multiple at index {} and {}",
+						reads[i].as_ref().motor_id,
+						i,
+						j
+					)
 				}
 			}
 		}
@@ -66,10 +67,10 @@ where
 			});
 
 			match response {
-				Ok(response) => on_response(Ok(BulkData {
-					motor_id: read.motor_id,
-					address: read.address,
-					data: response.parameters(),
+				Ok(response) => on_response(Ok({
+					let mut response: Response<Vec<u8>> = response.into();
+					response.address = Some(read.address);
+					response
 				})),
 				Err(e) => on_response(Err(e)),
 			}
@@ -87,25 +88,18 @@ where
 	/// # Panics
 	/// The protocol forbids specifying the same motor ID multiple times.
 	/// This function panics if the same motor ID is used for more than one read.
-	pub fn bulk_read<Read>(
-		&mut self,
-		reads: &[Read],
-	) -> Result<Vec<BulkData<Vec<u8>>>, TransferError>
+	pub fn bulk_read<Read>(&mut self, reads: &[Read]) -> Result<Vec<Response<Vec<u8>>>, TransferError>
 	where
 		Read: AsRef<BulkRead>,
 	{
 		let mut responses = Vec::with_capacity(reads.len());
 		let mut read_error = None;
 
-		self.bulk_read_cb(reads, |bulk_data| {
+		self.bulk_read_cb(reads, |data| {
 			if read_error.is_none() {
-				match bulk_data {
+				match data {
 					Err(e) => read_error = Some(e),
-					Ok(bulk_data) => responses.push(BulkData {
-						motor_id: bulk_data.motor_id,
-						address: bulk_data.address,
-						data: bulk_data.data.to_owned(),
-					}),
+					Ok(data) => responses.push(data),
 				}
 			}
 		})?;
diff --git a/src/instructions/clear.rs b/src/instructions/clear.rs
index 3952e41..3f82dac 100644
--- a/src/instructions/clear.rs
+++ b/src/instructions/clear.rs
@@ -1,5 +1,5 @@
 use super::{instruction_id, packet_id};
-use crate::Bus;
+use crate::{Bus, Response};
 
 /// The parameters for the CLEAR command to clear the revolution counter.
 const CLEAR_REVOLUTION_COUNT: [u8; 5] = [0x01, 0x44, 0x58, 0x4C, 0x22];
@@ -17,14 +17,15 @@ where
 	///
 	/// The `motor_id` parameter may be set to [`packet_id::BROADCAST`],
 	/// although the [`Self::broadcast_clear_revolution_counter`] is generally easier to use.
-	pub fn clear_revolution_counter(&mut self, motor_id: u8) -> Result<(), crate::TransferError> {
+	pub fn clear_revolution_counter(&mut self, motor_id: u8) -> Result<Option<Response<()>>, crate::TransferError> {
 		if motor_id == packet_id::BROADCAST {
 			self.broadcast_clear_revolution_counter()?;
+			Ok(None)
 		} else {
 			let response = self.transfer_single(motor_id, instruction_id::CLEAR, CLEAR_REVOLUTION_COUNT.len(), encode_parameters)?;
 			crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
+			Ok(Some(response.into()))
 		}
-		Ok(())
 	}
 
 	/// Clear the revolution counter of all connected motors.
diff --git a/src/instructions/factory_reset.rs b/src/instructions/factory_reset.rs
index ef958dd..85fc4a6 100644
--- a/src/instructions/factory_reset.rs
+++ b/src/instructions/factory_reset.rs
@@ -1,5 +1,5 @@
 use super::{instruction_id, packet_id};
-use crate::Bus;
+use crate::{Bus, Response};
 
 #[repr(u8)]
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
@@ -33,14 +33,15 @@ where
 	/// which would cause multiple motors on the bus to have the same ID.
 	/// At that point, communication with those motors is not possible anymore.
 	/// The only way to restore communication is to physically disconnect all but one motor at a time and re-assign unique IDs.
-	pub fn factory_reset(&mut self, motor_id: u8, kind: FactoryResetKind) -> Result<(), crate::TransferError> {
+	pub fn factory_reset(&mut self, motor_id: u8, kind: FactoryResetKind) -> Result<Option<Response<()>>, crate::TransferError> {
 		if motor_id == packet_id::BROADCAST {
 			self.broadcast_factory_reset(kind)?;
+			Ok(None)
 		} else {
 			let response = self.transfer_single(motor_id, instruction_id::FACTORY_RESET, 1, |buffer| buffer[0] = kind as u8)?;
 			crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
+			Ok(Some(response.into()))
 		}
-		Ok(())
 	}
 
 	/// Reset the settings of all connected motors to the factory defaults.
diff --git a/src/instructions/mod.rs b/src/instructions/mod.rs
index 727a13f..e14a936 100644
--- a/src/instructions/mod.rs
+++ b/src/instructions/mod.rs
@@ -34,7 +34,6 @@ mod write;
 
 pub use factory_reset::FactoryResetKind;
 pub use ping::PingResponse;
-pub use read::ReadResponse;
 
 /// Data from or for a specific motor.
 ///
diff --git a/src/instructions/ping.rs b/src/instructions/ping.rs
index aefd1d2..f9f5623 100644
--- a/src/instructions/ping.rs
+++ b/src/instructions/ping.rs
@@ -1,5 +1,5 @@
 use super::{instruction_id, packet_id};
-use crate::{Bus, ReadError, TransferError, WriteError};
+use crate::{bus::StatusPacket, Bus, ReadError, TransferError, WriteError};
 
 #[derive(Debug, Clone)]
 pub struct PingResponse {
@@ -13,6 +13,24 @@ pub struct PingResponse {
 
 	/// The firmware version of the motor.
 	pub firmware: u8,
+
+	pub alert: bool,
+}
+
+impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for PingResponse
+where
+	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
+	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
+{
+	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
+		let parameters = status_packet.parameters();
+		Self {
+			motor_id: status_packet.packet_id(),
+			model: crate::endian::read_u16_le(&parameters[0..]),
+			firmware: crate::endian::read_u8_le(&parameters[2..]),
+			alert: status_packet.error() & 0x80 != 0,
+		}
+	}
 }
 
 impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer>
@@ -26,7 +44,7 @@ where
 	/// Use [`Self::scan`] or [`Self::scan_cb`] instead.
 	pub fn ping(&mut self, motor_id: u8) -> Result<PingResponse, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::PING, 0, |_| ())?;
-		Ok(parse_ping_response(response.packet_id(), response.parameters()))
+		Ok(response.into())
 	}
 
 	/// Scan a bus for motors with a broadcast ping, returning the responses in a [`Vec`].
@@ -56,12 +74,13 @@ where
 			let response = self.read_status_response();
 			if let Err(ReadError::Io(e)) = &response {
 				if e.kind() == std::io::ErrorKind::TimedOut {
+					trace!("Response timed out.");
 					continue;
 				}
 			}
 			let response = response.and_then(|response| {
 				crate::InvalidParameterCount::check(response.parameters().len(), 3)?;
-				Ok(parse_ping_response(response.packet_id(), response.parameters()))
+				Ok(response.into())
 			});
 			on_response(response);
 		}
@@ -69,12 +88,3 @@ where
 		Ok(())
 	}
 }
-
-/// Parse a ping response from the motor ID and status response parameters.
-fn parse_ping_response(motor_id: u8, parameters: &[u8]) -> PingResponse {
-	PingResponse {
-		motor_id,
-		model: crate::endian::read_u16_le(&parameters[0..]),
-		firmware: crate::endian::read_u8_le(&parameters[2..]),
-	}
-}
diff --git a/src/instructions/read.rs b/src/instructions/read.rs
index 431b0a2..cd935e1 100644
--- a/src/instructions/read.rs
+++ b/src/instructions/read.rs
@@ -1,33 +1,6 @@
 use super::instruction_id;
-use crate::endian::{read_u8_le, read_u16_le, read_u32_le, write_u16_le};
-use crate::{Bus, TransferError};
-
-pub struct ReadResponse<'a, ReadBuffer, WriteBuffer>
-where
-	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
-	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
-{
-	pub(crate) response: crate::Response<'a, ReadBuffer, WriteBuffer>,
-}
-
-impl<'a, ReadBuffer, WriteBuffer> ReadResponse<'a, ReadBuffer, WriteBuffer>
-where
-	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
-	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
-{
-	/// Get the ID of the motor.
-	pub fn motor_id(&self) -> u8 {
-		self.response.packet_id()
-	}
-
-	/// Get the read data as byte slice.
-	///
-	/// The individual registers of the motor are encoded as little-endian.
-	/// Refer to the online manual of your motor for the addresses and sizes of all registers.
-	pub fn data(&self) -> &[u8] {
-		self.response.parameters()
-	}
-}
+use crate::endian::write_u16_le;
+use crate::{bus::StatusPacket, Bus, Response, TransferError};
 
 impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer>
 where
@@ -38,39 +11,39 @@ where
 	///
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
-	pub fn read(&mut self, motor_id: u8, address: u16, count: u16) -> Result<ReadResponse<ReadBuffer, WriteBuffer>, TransferError> {
+	pub fn read(&mut self, motor_id: u8, address: u16, count: u16) -> Result<StatusPacket<'_, ReadBuffer, WriteBuffer>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::READ, 4, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			write_u16_le(&mut buffer[2..], count);
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), count.into()).map_err(crate::ReadError::from)?;
-		Ok(ReadResponse { response })
+		Ok(response)
 	}
 
 	/// Read an 8 bit register from a specific motor.
 	///
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
-	pub fn read_u8(&mut self, motor_id: u8, address: u16) -> Result<u8, TransferError> {
+	pub fn read_u8(&mut self, motor_id: u8, address: u16) -> Result<Response<u8>, TransferError> {
 		let response = self.read(motor_id, address, 1)?;
-		Ok(read_u8_le(response.data()))
+		Ok(response.into())
 	}
 
 	/// Read 16 bit register from a specific motor.
 	///
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
-	pub fn read_u16(&mut self, motor_id: u8, address: u16) -> Result<u16, TransferError> {
+	pub fn read_u16(&mut self, motor_id: u8, address: u16) -> Result<Response<u16>, TransferError> {
 		let response = self.read(motor_id, address, 2)?;
-		Ok(read_u16_le(response.data()))
+		Ok(response.into())
 	}
 
 	/// Read 32 bit register from a specific motor.
 	///
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
-	pub fn read_u32(&mut self, motor_id: u8, address: u16) -> Result<u32, TransferError> {
+	pub fn read_u32(&mut self, motor_id: u8, address: u16) -> Result<Response<u32>, TransferError> {
 		let response = self.read(motor_id, address, 4)?;
-		Ok(read_u32_le(response.data()))
+		Ok(response.into())
 	}
 }
diff --git a/src/instructions/reboot.rs b/src/instructions/reboot.rs
index 0133272..8d8056c 100644
--- a/src/instructions/reboot.rs
+++ b/src/instructions/reboot.rs
@@ -1,5 +1,5 @@
 use super::{instruction_id, packet_id};
-use crate::{Bus, TransferError, WriteError};
+use crate::{Bus, Response, TransferError, WriteError};
 
 impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer>
 where
@@ -14,14 +14,15 @@ where
 	///
 	/// The `motor_id` parameter may be set to [`packet_id::BROADCAST`],
 	/// although the [`Self::broadcast_reboot`] is generally easier to use.
-	pub fn reboot(&mut self, motor_id: u8) -> Result<(), TransferError> {
+	pub fn reboot(&mut self, motor_id: u8) -> Result<Option<Response<()>>, TransferError> {
 		if motor_id == packet_id::BROADCAST {
 			self.broadcast_action()?;
+			Ok(None)
 		} else {
 			let response = self.transfer_single(motor_id, instruction_id::REBOOT, 0, |_| ())?;
 			crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
+			Ok(Some(response.into()))
 		}
-		Ok(())
 	}
 
 	/// Broadcast an reboot command to all connected motors to trigger a previously registered instruction.
diff --git a/src/instructions/reg_write.rs b/src/instructions/reg_write.rs
index 3b2054c..dffaac5 100644
--- a/src/instructions/reg_write.rs
+++ b/src/instructions/reg_write.rs
@@ -1,5 +1,5 @@
 use super::instruction_id;
-use crate::{Bus, TransferError};
+use crate::{Bus, Response, TransferError};
 
 use crate::endian::{write_u16_le, write_u32_le};
 
@@ -14,13 +14,13 @@ where
 	///
 	/// You can have all connected motors execute their registered write using [`Self::broadcast_action`],
 	/// or a single motor using [`Self::action`].
-	pub fn reg_write(&mut self, motor_id: u8, address: u16, data: &[u8]) -> Result<(), TransferError> {
+	pub fn reg_write(&mut self, motor_id: u8, address: u16, data: &[u8]) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::REG_WRITE, 2 + data.len(), |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			buffer[2..].copy_from_slice(data)
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(())
+		Ok(response.into())
 	}
 
 	/// Register a write command for a 8 bit value to a specific motor.
@@ -29,13 +29,13 @@ where
 	///
 	/// You can have all connected motors execute their registered write using [`Self::broadcast_action`],
 	/// or a single motor using [`Self::action`].
-	pub fn reg_write_u8(&mut self, motor_id: u8, address: u16, value: u8) -> Result<(), TransferError> {
+	pub fn reg_write_u8(&mut self, motor_id: u8, address: u16, value: u8) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::REG_WRITE, 2 + 1, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			buffer[2] = value;
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(())
+		Ok(response.into())
 	}
 
 	/// Register a write command for a 16 bit value to a specific motor.
@@ -44,13 +44,13 @@ where
 	///
 	/// You can have all connected motors execute their registered write using [`Self::broadcast_action`],
 	/// or a single motor using [`Self::action`].
-	pub fn reg_write_u16(&mut self, motor_id: u8, address: u16, value: u16) -> Result<(), TransferError> {
+	pub fn reg_write_u16(&mut self, motor_id: u8, address: u16, value: u16) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::REG_WRITE, 2 + 2, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			write_u16_le(&mut buffer[2..], value);
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(())
+		Ok(response.into())
 	}
 
 	/// Register a write command for a 32 bit value to a specific motor.
@@ -59,12 +59,12 @@ where
 	///
 	/// You can have all connected motors execute their registered write using [`Self::broadcast_action`],
 	/// or a single motor using [`Self::action`].
-	pub fn reg_write_u32(&mut self, motor_id: u8, address: u16, value: u32) -> Result<(), TransferError> {
+	pub fn reg_write_u32(&mut self, motor_id: u8, address: u16, value: u32) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::REG_WRITE, 2 + 4, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			write_u32_le(&mut buffer[2..], value);
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(())
+		Ok(response.into())
 	}
 }
diff --git a/src/instructions/sync_read.rs b/src/instructions/sync_read.rs
index 6753328..f716b04 100644
--- a/src/instructions/sync_read.rs
+++ b/src/instructions/sync_read.rs
@@ -1,6 +1,6 @@
-use super::{instruction_id, packet_id, SyncData};
-use crate::endian::{read_u8_le, read_u16_le, read_u32_le, write_u16_le};
-use crate::{Bus, ReadError, WriteError, TransferError};
+use super::{instruction_id, packet_id};
+use crate::endian::write_u16_le;
+use crate::{Bus, ReadError, Response, TransferError, WriteError};
 
 impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer>
 where
@@ -11,15 +11,9 @@ where
 	///
 	/// The `on_response` function is called for the reply from each motor.
 	/// If the function fails to write the instruction, an error is returned and the function is not called.
-	pub fn sync_read_cb<'a, F>(
-		&'a mut self,
-		motor_ids: &'a [u8],
-		address: u16,
-		count: u16,
-		mut on_response: F,
-	) -> Result<(), WriteError>
+	pub fn sync_read_cb<'a, F>(&'a mut self, motor_ids: &'a [u8], address: u16, count: u16, mut on_response: F) -> Result<(), WriteError>
 	where
-		F: FnMut(Result<SyncData<&[u8]>, ReadError>),
+		F: FnMut(Result<Response<Vec<u8>>, ReadError>),
 	{
 		self.write_instruction(packet_id::BROADCAST, instruction_id::SYNC_READ, 4 + motor_ids.len(), |buffer| {
 			write_u16_le(&mut buffer[0..], address);
@@ -34,10 +28,7 @@ where
 			});
 
 			match response {
-				Ok(response) => on_response(Ok(SyncData {
-					motor_id,
-					data: response.parameters(),
-				})),
+				Ok(response) => on_response(Ok(response.into())),
 				Err(e) => on_response(Err(e)),
 			}
 		}
@@ -48,14 +39,9 @@ where
 	///
 	/// The `on_response` function is called for the reply from each motor.
 	/// If the function fails to write the instruction, an error is returned and the function is not called.
-	pub fn sync_read_u8_cb<'a, F>(
-		&'a mut self,
-		motor_ids: &'a [u8],
-		address: u16,
-		mut on_response: F,
-	) -> Result<(), WriteError>
+	pub fn sync_read_u8_cb<'a, F>(&'a mut self, motor_ids: &'a [u8], address: u16, mut on_response: F) -> Result<(), WriteError>
 	where
-		F: FnMut(Result<SyncData<u8>, ReadError>),
+		F: FnMut(Result<Response<u8>, ReadError>),
 	{
 		let count = 1;
 		self.write_instruction(packet_id::BROADCAST, instruction_id::SYNC_READ, 4 + motor_ids.len(), |buffer| {
@@ -67,10 +53,7 @@ where
 			let data = self.read_status_response().and_then(|response| {
 				crate::InvalidPacketId::check(response.packet_id(), motor_id)?;
 				crate::InvalidParameterCount::check(response.parameters().len(), count)?;
-				Ok(SyncData {
-					motor_id,
-					data: read_u8_le(response.parameters()),
-				})
+				Ok(response.into())
 			});
 			on_response(data);
 		}
@@ -81,14 +64,9 @@ where
 	///
 	/// The `on_response` function is called for the reply from each motor.
 	/// If the function fails to write the instruction, an error is returned and the function is not called.
-	pub fn sync_read_u16_cb<'a, F>(
-		&'a mut self,
-		motor_ids: &'a [u8],
-		address: u16,
-		mut on_response: F,
-	) -> Result<(), WriteError>
+	pub fn sync_read_u16_cb<'a, F>(&'a mut self, motor_ids: &'a [u8], address: u16, mut on_response: F) -> Result<(), WriteError>
 	where
-		F: FnMut(Result<SyncData<u16>, ReadError>),
+		F: FnMut(Result<Response<u16>, ReadError>),
 	{
 		let count = 1;
 		self.write_instruction(packet_id::BROADCAST, instruction_id::SYNC_READ, 4 + motor_ids.len(), |buffer| {
@@ -100,10 +78,7 @@ where
 			let data = self.read_status_response().and_then(|response| {
 				crate::InvalidPacketId::check(response.packet_id(), motor_id)?;
 				crate::InvalidParameterCount::check(response.parameters().len(), count)?;
-				Ok(SyncData {
-					motor_id,
-					data: read_u16_le(response.parameters()),
-				})
+				Ok(response.into())
 			});
 			on_response(data);
 		}
@@ -114,14 +89,9 @@ where
 	///
 	/// The `on_response` function is called for the reply from each motor.
 	/// If the function fails to write the instruction, an error is returned and the function is not called.
-	pub fn sync_read_u32_cb<'a, F>(
-		&'a mut self,
-		motor_ids: &'a [u8],
-		address: u16,
-		mut on_response: F,
-	) -> Result<(), WriteError>
+	pub fn sync_read_u32_cb<'a, F>(&'a mut self, motor_ids: &'a [u8], address: u16, mut on_response: F) -> Result<(), WriteError>
 	where
-		F: FnMut(Result<SyncData<u32>, ReadError>),
+		F: FnMut(Result<Response<u32>, ReadError>),
 	{
 		let count = 4;
 		self.write_instruction(packet_id::BROADCAST, instruction_id::SYNC_READ, 4 + motor_ids.len(), |buffer| {
@@ -133,10 +103,7 @@ where
 			let data = self.read_status_response().and_then(|response| {
 				crate::InvalidPacketId::check(response.packet_id(), motor_id)?;
 				crate::InvalidParameterCount::check(response.parameters().len(), count)?;
-				Ok(SyncData {
-					motor_id,
-					data: read_u32_le(response.parameters()),
-				})
+				Ok(response.into())
 			});
 			on_response(data);
 		}
@@ -147,23 +114,13 @@ where
 	///
 	/// If this function fails to get the data from any of the motors, the entire function retrns an error.
 	/// If you need access to the data from other motors, or if you want acces to the error for each motor, see [`Self::sync_read_cb`].
-	pub fn sync_read<'a>(
-		&'a mut self,
-		motor_ids: &'a [u8],
-		address: u16,
-		count: u16,
-	) -> Result<Vec<SyncData<Vec<u8>>>, TransferError> {
+	pub fn sync_read<'a>(&'a mut self, motor_ids: &'a [u8], address: u16, count: u16) -> Result<Vec<Response<Vec<u8>>>, TransferError> {
 		let mut result = Vec::with_capacity(motor_ids.len());
 		let mut read_error = None;
-		self.sync_read_cb(motor_ids, address, count, |data| {
-			match data {
-				Err(e) if read_error.is_none() => read_error = Some(e),
-				Err(_) => (),
-				Ok(response) => result.push(SyncData {
-					motor_id: response.motor_id,
-					data: response.data.to_owned(),
-				}),
-			}
+		self.sync_read_cb(motor_ids, address, count, |data| match data {
+			Err(e) if read_error.is_none() => read_error = Some(e),
+			Err(_) => (),
+			Ok(response) => result.push(response),
 		})?;
 		Ok(result)
 	}
@@ -172,19 +129,13 @@ where
 	///
 	/// If this function fails to get the data from any of the motors, the entire function retrns an error.
 	/// If you need access to the data from other motors, or if you want acces to the error for each motor, see [`Self::sync_read_u8_cb`].
-	pub fn sync_read_u8<'a>(
-		&'a mut self,
-		motor_ids: &'a [u8],
-		address: u16,
-	) -> Result<Vec<SyncData<u8>>, TransferError> {
+	pub fn sync_read_u8<'a>(&'a mut self, motor_ids: &'a [u8], address: u16) -> Result<Vec<Response<u8>>, TransferError> {
 		let mut result = Vec::with_capacity(motor_ids.len());
 		let mut read_error = None;
-		self.sync_read_u8_cb(motor_ids, address, |data| {
-			match data {
-				Err(e) if read_error.is_none() => read_error = Some(e),
-				Err(_) => (),
-				Ok(data) => result.push(data),
-			}
+		self.sync_read_u8_cb(motor_ids, address, |data| match data {
+			Err(e) if read_error.is_none() => read_error = Some(e),
+			Err(_) => (),
+			Ok(data) => result.push(data),
 		})?;
 		Ok(result)
 	}
@@ -193,19 +144,13 @@ where
 	///
 	/// If this function fails to get the data from any of the motors, the entire function retrns an error.
 	/// If you need access to the data from other motors, or if you want acces to the error for each motor, see [`Self::sync_read_u16_cb`].
-	pub fn sync_read_u16<'a>(
-		&'a mut self,
-		motor_ids: &'a [u8],
-		address: u16,
-	) -> Result<Vec<SyncData<u16>>, TransferError> {
+	pub fn sync_read_u16<'a>(&'a mut self, motor_ids: &'a [u8], address: u16) -> Result<Vec<Response<u16>>, TransferError> {
 		let mut result = Vec::with_capacity(motor_ids.len());
 		let mut read_error = None;
-		self.sync_read_u16_cb(motor_ids, address, |data| {
-			match data {
-				Err(e) if read_error.is_none() => read_error = Some(e),
-				Err(_) => (),
-				Ok(data) => result.push(data),
-			}
+		self.sync_read_u16_cb(motor_ids, address, |data| match data {
+			Err(e) if read_error.is_none() => read_error = Some(e),
+			Err(_) => (),
+			Ok(data) => result.push(data),
 		})?;
 		Ok(result)
 	}
@@ -214,19 +159,13 @@ where
 	///
 	/// If this function fails to get the data from any of the motors, the entire function retrns an error.
 	/// If you need access to the data from other motors, or if you want acces to the error for each motor, see [`Self::sync_read_u32_cb`].
-	pub fn sync_read_u32<'a>(
-		&'a mut self,
-		motor_ids: &'a [u8],
-		address: u16,
-	) -> Result<Vec<SyncData<u32>>, TransferError> {
+	pub fn sync_read_u32<'a>(&'a mut self, motor_ids: &'a [u8], address: u16) -> Result<Vec<Response<u32>>, TransferError> {
 		let mut result = Vec::with_capacity(motor_ids.len());
 		let mut read_error = None;
-		self.sync_read_u32_cb(motor_ids, address, |data| {
-			match data {
-				Err(e) if read_error.is_none() => read_error = Some(e),
-				Err(_) => (),
-				Ok(data) => result.push(data),
-			}
+		self.sync_read_u32_cb(motor_ids, address, |data| match data {
+			Err(e) if read_error.is_none() => read_error = Some(e),
+			Err(_) => (),
+			Ok(data) => result.push(data),
 		})?;
 		Ok(result)
 	}
diff --git a/src/instructions/write.rs b/src/instructions/write.rs
index 9f665f8..452e751 100644
--- a/src/instructions/write.rs
+++ b/src/instructions/write.rs
@@ -1,6 +1,6 @@
 use super::instruction_id;
 use crate::endian::{write_u16_le, write_u32_le};
-use crate::{Bus, TransferError};
+use crate::{Bus, Response, TransferError};
 
 impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer>
 where
@@ -8,42 +8,42 @@ where
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
 {
 	/// Write an arbitrary number of bytes to a specific motor.
-	pub fn write(&mut self, motor_id: u8, address: u16, data: &[u8]) -> Result<(), TransferError> {
+	pub fn write(&mut self, motor_id: u8, address: u16, data: &[u8]) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::WRITE, 2 + data.len(), |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			buffer[2..].copy_from_slice(data)
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(())
+		Ok(response.into())
 	}
 
 	/// Write an 8 bit value to a specific motor.
-	pub fn write_u8(&mut self, motor_id: u8, address: u16, value: u8) -> Result<(), TransferError> {
+	pub fn write_u8(&mut self, motor_id: u8, address: u16, value: u8) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::WRITE, 2 + 1, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			buffer[2] = value;
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(())
+		Ok(response.into())
 	}
 
 	/// Write an 16 bit value to a specific motor.
-	pub fn write_u16(&mut self, motor_id: u8, address: u16, value: u16) -> Result<(), TransferError> {
+	pub fn write_u16(&mut self, motor_id: u8, address: u16, value: u16) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::WRITE, 2 + 2, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			write_u16_le(&mut buffer[2..], value);
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(())
+		Ok(response.into())
 	}
 
 	/// Write an 32 bit value to a specific motor.
-	pub fn write_u32(&mut self, motor_id: u8, address: u16, value: u32) -> Result<(), TransferError> {
+	pub fn write_u32(&mut self, motor_id: u8, address: u16, value: u32) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::WRITE, 2 + 4, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			write_u32_le(&mut buffer[2..], value);
 		})?;
 		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(())
+		Ok(response.into())
 	}
 }

From 4b73624e53ef28b501458fc9e00f6bd841819e2c Mon Sep 17 00:00:00 2001
From: Omelia Iliffe <omelia.iliffe@wetaworkshop.co.nz>
Date: Mon, 11 Dec 2023 11:55:40 +1300
Subject: [PATCH 02/15] created private _read function

---
 src/instructions/read.rs | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/instructions/read.rs b/src/instructions/read.rs
index cd935e1..b16ce6a 100644
--- a/src/instructions/read.rs
+++ b/src/instructions/read.rs
@@ -7,11 +7,8 @@ where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
 {
-	/// Read an arbitrary number of bytes from a specific motor.
-	///
-	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
-	/// Use [`Self::sync_read`] to read from multiple motors with one command.
-	pub fn read(&mut self, motor_id: u8, address: u16, count: u16) -> Result<StatusPacket<'_, ReadBuffer, WriteBuffer>, TransferError> {
+	/// Read an arbitrary number of bytes from multiple motors.
+	fn _read(&mut self, motor_id: u8, address: u16, count: u16) -> Result<StatusPacket<'_, ReadBuffer, WriteBuffer>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::READ, 4, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			write_u16_le(&mut buffer[2..], count);
@@ -20,12 +17,21 @@ where
 		Ok(response)
 	}
 
+	/// Read an arbitrary number of bytes from a specific motor.
+	///
+	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
+	/// Use [`Self::sync_read`] to read from multiple motors with one command.
+	pub fn read(&mut self, motor_id: u8, address: u16, count: u16) -> Result<Response<Vec<u8>>, TransferError> {
+		let response = self._read(motor_id, address, count)?;
+		Ok(response.into())
+	}
+
 	/// Read an 8 bit register from a specific motor.
 	///
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u8(&mut self, motor_id: u8, address: u16) -> Result<Response<u8>, TransferError> {
-		let response = self.read(motor_id, address, 1)?;
+		let response = self._read(motor_id, address, 1)?;
 		Ok(response.into())
 	}
 
@@ -34,7 +40,7 @@ where
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u16(&mut self, motor_id: u8, address: u16) -> Result<Response<u16>, TransferError> {
-		let response = self.read(motor_id, address, 2)?;
+		let response = self._read(motor_id, address, 2)?;
 		Ok(response.into())
 	}
 
@@ -43,7 +49,7 @@ where
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u32(&mut self, motor_id: u8, address: u16) -> Result<Response<u32>, TransferError> {
-		let response = self.read(motor_id, address, 4)?;
+		let response = self._read(motor_id, address, 4)?;
 		Ok(response.into())
 	}
 }

From 3e003636e6720472228a9351d84e75d7a31da73f Mon Sep 17 00:00:00 2001
From: Omelia Iliffe <omelia.iliffe@wetaworkshop.co.nz>
Date: Mon, 11 Dec 2023 12:16:00 +1300
Subject: [PATCH 03/15] fixed display issue for responses

---
 dynamixel2-cli/src/bin/dynamixel2/main.rs | 34 ++++++++++++++---------
 src/bus.rs                                |  1 +
 src/instructions/ping.rs                  |  2 +-
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/dynamixel2-cli/src/bin/dynamixel2/main.rs b/dynamixel2-cli/src/bin/dynamixel2/main.rs
index ee5236a..5a78940 100644
--- a/dynamixel2-cli/src/bin/dynamixel2/main.rs
+++ b/dynamixel2-cli/src/bin/dynamixel2/main.rs
@@ -25,7 +25,7 @@ fn do_main(options: Options) -> Result<(), ()> {
 				MotorId::Broadcast => {
 					log::debug!("Scanning bus for connected motors");
 					bus.scan_cb(|response| match response {
-						Ok(response) => log_ping_response(&response),
+						Ok(response) => log::info!("{:?}", response),
 						Err(e) => log::warn!("Communication error: {}", e),
 					})
 					.map_err(|e| log::error!("Command failed: {}", e))?;
@@ -41,26 +41,31 @@ fn do_main(options: Options) -> Result<(), ()> {
 		Command::Read8 { motor_id, address } => {
 			let mut bus = open_bus(&options)?;
 			log::debug!("Reading an 8-bit value from motor {} at address {}", motor_id.raw(), address);
-			let value = bus
+			let response = bus
 				.read_u8(motor_id.assume_unicast()?, *address)
 				.map_err(|e| log::error!("Command failed: {}", e))?;
-			log::info!("Ok: {} (0x{:02X})", value, value);
+			log::info!("{:?} (0x{:02X})", response, response.data);
 		},
 		Command::Read16 { motor_id, address } => {
 			let mut bus = open_bus(&options)?;
 			log::debug!("Reading a 16-bit value from motor {} at address {}", motor_id.raw(), address);
-			let value = bus
+			let response = bus
 				.read_u16(motor_id.assume_unicast()?, *address)
 				.map_err(|e| log::error!("Command failed: {}", e))?;
-			log::info!("Ok: {} (0x{:04X})", value, value);
+			log::info!("{:?} (0x{:04X})", response, response.data);
 		},
 		Command::Read32 { motor_id, address } => {
 			let mut bus = open_bus(&options)?;
 			log::debug!("Reading a 32-bit value from motor {} at address {}", motor_id.raw(), address);
-			let value = bus
+			let response = bus
 				.read_u32(motor_id.assume_unicast()?, *address)
 				.map_err(|e| log::error!("Command failed: {}", e))?;
-			log::info!("Ok: {} (0x{:04X} {:04X})", value, (value >> 16) & 0xFFFF, value & 0xFFFF);
+			log::info!(
+				"{:?} (0x{:04X} {:04X})",
+				response,
+				(response.data >> 16) & 0xFFFF,
+				response.data & 0xFFFF
+			);
 		},
 		Command::Write8 { motor_id, address, value } => {
 			let mut bus = open_bus(&options)?;
@@ -71,9 +76,10 @@ fn do_main(options: Options) -> Result<(), ()> {
 				motor_id.raw(),
 				address
 			);
-			bus.write_u8(motor_id.raw(), *address, *value)
+			let response = bus
+				.write_u8(motor_id.raw(), *address, *value)
 				.map_err(|e| log::error!("Write failed: {}", e))?;
-			log::info!("Ok");
+			log::info!("Ok (Hardware error: {})", response.alert);
 		},
 		Command::Write16 { motor_id, address, value } => {
 			let mut bus = open_bus(&options)?;
@@ -84,9 +90,10 @@ fn do_main(options: Options) -> Result<(), ()> {
 				motor_id.raw(),
 				address
 			);
-			bus.write_u16(motor_id.raw(), *address, *value)
+			let response = bus
+				.write_u16(motor_id.raw(), *address, *value)
 				.map_err(|e| log::error!("Command failed: {}", e))?;
-			log::info!("Ok");
+			log::info!("Ok (Hardware error: {})", response.alert);
 		},
 		Command::Write32 { motor_id, address, value } => {
 			let mut bus = open_bus(&options)?;
@@ -98,9 +105,10 @@ fn do_main(options: Options) -> Result<(), ()> {
 				motor_id.raw(),
 				address
 			);
-			bus.write_u32(motor_id.raw(), *address, *value)
+			let response = bus
+				.write_u32(motor_id.raw(), *address, *value)
 				.map_err(|e| log::error!("Command failed: {}", e))?;
-			log::info!("Ok");
+			log::info!("Ok (Hardware error: {})", response.alert);
 		},
 		Command::ShellCompletion { shell, output } => {
 			write_shell_completion(*shell, output.as_deref())?;
diff --git a/src/bus.rs b/src/bus.rs
index f6a1754..1634398 100644
--- a/src/bus.rs
+++ b/src/bus.rs
@@ -311,6 +311,7 @@ where
 	}
 }
 
+#[derive(Debug)]
 pub struct Response<T> {
 	pub motor_id: u8,
 	pub alert: bool,
diff --git a/src/instructions/ping.rs b/src/instructions/ping.rs
index f9f5623..f60aed6 100644
--- a/src/instructions/ping.rs
+++ b/src/instructions/ping.rs
@@ -1,7 +1,7 @@
 use super::{instruction_id, packet_id};
 use crate::{bus::StatusPacket, Bus, ReadError, TransferError, WriteError};
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct PingResponse {
 	/// The ID of the motor.
 	pub motor_id: u8,

From 933110a701588373883d5155213c2c548e9112bd Mon Sep 17 00:00:00 2001
From: Omelia Iliffe <omelia.iliffe@wetaworkshop.co.nz>
Date: Mon, 11 Dec 2023 12:31:55 +1300
Subject: [PATCH 04/15] Refactor bulk read function to use BulkResponse

---
 src/bus.rs                    |  6 ------
 src/instructions/bulk_read.rs | 13 ++++++-------
 src/instructions/mod.rs       | 11 ++++++++++-
 src/lib.rs                    |  1 +
 4 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/bus.rs b/src/bus.rs
index 1634398..a778cca 100644
--- a/src/bus.rs
+++ b/src/bus.rs
@@ -316,7 +316,6 @@ pub struct Response<T> {
 	pub motor_id: u8,
 	pub alert: bool,
 	pub data: T,
-	pub address: Option<u16>,
 }
 
 impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<()>
@@ -329,7 +328,6 @@ where
 			data: (),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-			address: None,
 		}
 	}
 }
@@ -344,7 +342,6 @@ where
 			data: status_packet.parameters().to_owned(),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-			address: None,
 		}
 	}
 }
@@ -359,7 +356,6 @@ where
 			data: read_u8_le(status_packet.parameters()),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-			address: None,
 		}
 	}
 }
@@ -374,7 +370,6 @@ where
 			data: read_u16_le(status_packet.parameters()),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-			address: None,
 		}
 	}
 }
@@ -389,7 +384,6 @@ where
 			data: read_u32_le(status_packet.parameters()),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-			address: None,
 		}
 	}
 }
diff --git a/src/instructions/bulk_read.rs b/src/instructions/bulk_read.rs
index 949d17e..1f711de 100644
--- a/src/instructions/bulk_read.rs
+++ b/src/instructions/bulk_read.rs
@@ -1,6 +1,6 @@
 use super::{instruction_id, packet_id};
 use crate::endian::{write_u16_le, write_u8_le};
-use crate::{Bus, ReadError, Response, TransferError, WriteError};
+use crate::{BulkResponse, Bus, ReadError, TransferError, WriteError};
 
 pub struct BulkRead {
 	pub motor_id: u8,
@@ -34,7 +34,7 @@ where
 	pub fn bulk_read_cb<Read, F>(&mut self, reads: &[Read], mut on_response: F) -> Result<(), WriteError>
 	where
 		Read: AsRef<BulkRead>,
-		F: FnMut(Result<Response<Vec<u8>>, ReadError>),
+		F: FnMut(Result<BulkResponse<Vec<u8>>, ReadError>),
 	{
 		for i in 0..reads.len() {
 			for j in i + 1..reads.len() {
@@ -67,10 +67,9 @@ where
 			});
 
 			match response {
-				Ok(response) => on_response(Ok({
-					let mut response: Response<Vec<u8>> = response.into();
-					response.address = Some(read.address);
-					response
+				Ok(response) => on_response(Ok(BulkResponse {
+					response: response.into(),
+					address: read.address,
 				})),
 				Err(e) => on_response(Err(e)),
 			}
@@ -88,7 +87,7 @@ where
 	/// # Panics
 	/// The protocol forbids specifying the same motor ID multiple times.
 	/// This function panics if the same motor ID is used for more than one read.
-	pub fn bulk_read<Read>(&mut self, reads: &[Read]) -> Result<Vec<Response<Vec<u8>>>, TransferError>
+	pub fn bulk_read<Read>(&mut self, reads: &[Read]) -> Result<Vec<BulkResponse<Vec<u8>>>, TransferError>
 	where
 		Read: AsRef<BulkRead>,
 	{
diff --git a/src/instructions/mod.rs b/src/instructions/mod.rs
index e14a936..ac05dfc 100644
--- a/src/instructions/mod.rs
+++ b/src/instructions/mod.rs
@@ -35,6 +35,8 @@ mod write;
 pub use factory_reset::FactoryResetKind;
 pub use ping::PingResponse;
 
+use crate::Response;
+
 /// Data from or for a specific motor.
 ///
 /// Used by synchronous read and write commands.
@@ -52,7 +54,7 @@ impl<T> AsRef<SyncData<T>> for SyncData<T> {
 	}
 }
 
-/// Bulk data from or for a specific motor.
+/// Bulk data for a specific motor.
 ///
 /// This struct is very comparable to [`SyncData`],
 /// but it supports reads and writes
@@ -75,3 +77,10 @@ impl<T> AsRef<BulkData<T>> for BulkData<T> {
 		self
 	}
 }
+
+/// The data response giving by a motor in response to a bulk read command.
+#[derive(Debug)]
+pub struct BulkResponse<T> {
+	pub response: Response<T>,
+	pub address: u16,
+}
diff --git a/src/lib.rs b/src/lib.rs
index 1d3442c..fb2bdc8 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -43,3 +43,4 @@ pub use error::WriteError;
 
 pub use bus::Bus;
 pub use bus::Response;
+pub use instructions::BulkResponse;

From a0933891201967b4f9a29daff0d4ff606a4d44f4 Mon Sep 17 00:00:00 2001
From: omelia-wetaworkshop
 <137133300+omelia-wetaworkshop@users.noreply.github.com>
Date: Thu, 14 Dec 2023 10:34:16 +1300
Subject: [PATCH 05/15] added documenting comments

Co-authored-by: Maarten de Vries <maarten@de-vri.es>
---
 src/bus.rs              | 13 +++++++++++--
 src/error.rs            |  2 ++
 src/instructions/mod.rs |  3 +++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/bus.rs b/src/bus.rs
index a778cca..cd04a91 100644
--- a/src/bus.rs
+++ b/src/bus.rs
@@ -208,7 +208,7 @@ where
 		// Remove byte-stuffing from the parameters.
 		let parameter_count = bytestuff::unstuff_inplace(&mut buffer[STATUS_HEADER_SIZE..parameters_end]);
 
-		// Creating the response struct here means that the data gets purged from the buffer even if we return early using the try operator.
+		// Creating the status packet struct here means that the data gets purged from the buffer even if we return early using the try operator.
 		let response = StatusPacket {
 			bus: self,
 			stuffed_message_len,
@@ -290,7 +290,7 @@ where
 		self.as_bytes()[8]
 	}
 
-	// The alert bit from the error feild of the response.
+	// The alert bit from the error field of the response.
 	pub fn alert(&self) -> bool {
 		self.error() & 0x80 != 0
 	}
@@ -311,10 +311,19 @@ where
 	}
 }
 
+/// A response from a motor.
 #[derive(Debug)]
 pub struct Response<T> {
+	/// The motor that sent the response.
 	pub motor_id: u8,
+
+	/// The alert bit from the response message.
+	///
+	/// If this is set, you can normally check the "Hardware Error" register for more details.
+	/// Consult your motor manual for more details.
 	pub alert: bool,
+
+	/// The data from the motor.
 	pub data: T,
 }
 
diff --git a/src/error.rs b/src/error.rs
index 38bad71..3139c5d 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -82,6 +82,8 @@ pub struct InvalidParameterCount {
 
 impl MotorError {
 	pub fn check(raw: u8) -> Result<(), Self> {
+		// Ignore the alert bit for this check.
+		// If the alert bit is set, the motor encountered an error, but the instruction was still executed.
 		if raw & !0x80 == 0 {
 			Ok(())
 		} else {
diff --git a/src/instructions/mod.rs b/src/instructions/mod.rs
index ac05dfc..3950b68 100644
--- a/src/instructions/mod.rs
+++ b/src/instructions/mod.rs
@@ -81,6 +81,9 @@ impl<T> AsRef<BulkData<T>> for BulkData<T> {
 /// The data response giving by a motor in response to a bulk read command.
 #[derive(Debug)]
 pub struct BulkResponse<T> {
+	/// The response message.
 	pub response: Response<T>,
+	
+	/// The address of the bulk read/write.
 	pub address: u16,
 }

From c100635912c6c895f17bdf9457821ebd6cb05720 Mon Sep 17 00:00:00 2001
From: omelia-wetaworkshop
 <137133300+omelia-wetaworkshop@users.noreply.github.com>
Date: Thu, 14 Dec 2023 10:35:06 +1300
Subject: [PATCH 06/15] renamed _read to read_raw

A leading underscore tells the compiler not to complain when the function is unused. We should avoid that.

Co-authored-by: Maarten de Vries <maarten@de-vri.es>
---
 src/instructions/read.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/instructions/read.rs b/src/instructions/read.rs
index b16ce6a..5dfaaf6 100644
--- a/src/instructions/read.rs
+++ b/src/instructions/read.rs
@@ -8,7 +8,7 @@ where
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
 {
 	/// Read an arbitrary number of bytes from multiple motors.
-	fn _read(&mut self, motor_id: u8, address: u16, count: u16) -> Result<StatusPacket<'_, ReadBuffer, WriteBuffer>, TransferError> {
+	fn read_raw(&mut self, motor_id: u8, address: u16, count: u16) -> Result<StatusPacket<'_, ReadBuffer, WriteBuffer>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::READ, 4, |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			write_u16_le(&mut buffer[2..], count);

From 9433bc914ca99f4d25d4cd0626c68129e53b2136 Mon Sep 17 00:00:00 2001
From: Omelia Iliffe <omelia.iliffe@wetaworkshop.co.nz>
Date: Thu, 14 Dec 2023 11:26:48 +1300
Subject: [PATCH 07/15] removed BulkResonse BulkRead now returns Response with
 no address data renamed BulkData to BulkWriteData renamed SyncData to
 SyncWriteData moved and rename BulkRead to BulkReadData

---
 src/instructions/bulk_read.rs  | 29 +++++++-------------------
 src/instructions/bulk_write.rs | 13 ++++++++----
 src/instructions/mod.rs        | 37 +++++++++++++++++++---------------
 src/instructions/sync_write.rs | 10 ++++-----
 src/lib.rs                     |  1 -
 5 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/src/instructions/bulk_read.rs b/src/instructions/bulk_read.rs
index 1f711de..3c80a84 100644
--- a/src/instructions/bulk_read.rs
+++ b/src/instructions/bulk_read.rs
@@ -1,18 +1,6 @@
-use super::{instruction_id, packet_id};
+use super::{instruction_id, packet_id, BulkReadData};
 use crate::endian::{write_u16_le, write_u8_le};
-use crate::{BulkResponse, Bus, ReadError, TransferError, WriteError};
-
-pub struct BulkRead {
-	pub motor_id: u8,
-	pub address: u16,
-	pub count: u16,
-}
-
-impl AsRef<BulkRead> for BulkRead {
-	fn as_ref(&self) -> &Self {
-		self
-	}
-}
+use crate::{Bus, ReadError, Response, TransferError, WriteError};
 
 impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer>
 where
@@ -33,8 +21,8 @@ where
 	/// This function panics if the same motor ID is used for more than one read.
 	pub fn bulk_read_cb<Read, F>(&mut self, reads: &[Read], mut on_response: F) -> Result<(), WriteError>
 	where
-		Read: AsRef<BulkRead>,
-		F: FnMut(Result<BulkResponse<Vec<u8>>, ReadError>),
+		Read: AsRef<BulkReadData>,
+		F: FnMut(Result<Response<Vec<u8>>, ReadError>),
 	{
 		for i in 0..reads.len() {
 			for j in i + 1..reads.len() {
@@ -67,10 +55,7 @@ where
 			});
 
 			match response {
-				Ok(response) => on_response(Ok(BulkResponse {
-					response: response.into(),
-					address: read.address,
-				})),
+				Ok(response) => on_response(Ok(response.into())),
 				Err(e) => on_response(Err(e)),
 			}
 		}
@@ -87,9 +72,9 @@ where
 	/// # Panics
 	/// The protocol forbids specifying the same motor ID multiple times.
 	/// This function panics if the same motor ID is used for more than one read.
-	pub fn bulk_read<Read>(&mut self, reads: &[Read]) -> Result<Vec<BulkResponse<Vec<u8>>>, TransferError>
+	pub fn bulk_read<Read>(&mut self, reads: &[Read]) -> Result<Vec<Response<Vec<u8>>>, TransferError>
 	where
-		Read: AsRef<BulkRead>,
+		Read: AsRef<BulkReadData>,
 	{
 		let mut responses = Vec::with_capacity(reads.len());
 		let mut read_error = None;
diff --git a/src/instructions/bulk_write.rs b/src/instructions/bulk_write.rs
index 0c61faf..c7e33ed 100644
--- a/src/instructions/bulk_write.rs
+++ b/src/instructions/bulk_write.rs
@@ -1,5 +1,5 @@
-use super::{instruction_id, packet_id, BulkData};
-use crate::endian::{write_u8_le, write_u16_le};
+use super::{instruction_id, packet_id, BulkWriteData};
+use crate::endian::{write_u16_le, write_u8_le};
 use crate::{Bus, WriteError};
 
 impl<ReadBuffer, WriteBuffer> Bus<ReadBuffer, WriteBuffer>
@@ -22,7 +22,7 @@ where
 	/// This function also panics if the data length for a motor exceeds the capacity of a `u16`.
 	pub fn bulk_write<Write, T>(&mut self, writes: &[Write]) -> Result<(), WriteError>
 	where
-		Write: std::borrow::Borrow<BulkData<T>>,
+		Write: std::borrow::Borrow<BulkWriteData<T>>,
 		T: AsRef<[u8]>,
 	{
 		let mut parameter_count = 0;
@@ -30,7 +30,12 @@ where
 			let write = write.borrow();
 			let data = write.data.as_ref();
 			if data.len() > u16::MAX.into() {
-				panic!("bulk_write: data length ({}) for motor {} exceeds maximum size of {}", data.len(), write.motor_id, u16::MAX);
+				panic!(
+					"bulk_write: data length ({}) for motor {} exceeds maximum size of {}",
+					data.len(),
+					write.motor_id,
+					u16::MAX
+				);
 			}
 			parameter_count += 5 + data.len();
 		}
diff --git a/src/instructions/mod.rs b/src/instructions/mod.rs
index 3950b68..31010eb 100644
--- a/src/instructions/mod.rs
+++ b/src/instructions/mod.rs
@@ -35,20 +35,18 @@ mod write;
 pub use factory_reset::FactoryResetKind;
 pub use ping::PingResponse;
 
-use crate::Response;
-
 /// Data from or for a specific motor.
 ///
-/// Used by synchronous read and write commands.
-pub struct SyncData<T> {
+/// Used by synchronous write commands.
+pub struct SyncWriteData<T> {
 	/// The ID of the motor.
 	pub motor_id: u8,
 
-	/// The data read from or to be written to the motor.
+	/// The data to be written to the motor.
 	pub data: T,
 }
 
-impl<T> AsRef<SyncData<T>> for SyncData<T> {
+impl<T> AsRef<SyncWriteData<T>> for SyncWriteData<T> {
 	fn as_ref(&self) -> &Self {
 		self
 	}
@@ -60,8 +58,8 @@ impl<T> AsRef<SyncData<T>> for SyncData<T> {
 /// but it supports reads and writes
 /// of different sizes and to different addresses for each motor.
 ///
-/// Used by bulk read and write commands.
-pub struct BulkData<T> {
+/// Used by bulk write commands.
+pub struct BulkWriteData<T> {
 	/// The ID of the motor.
 	pub motor_id: u8,
 
@@ -72,18 +70,25 @@ pub struct BulkData<T> {
 	pub data: T,
 }
 
-impl<T> AsRef<BulkData<T>> for BulkData<T> {
+impl<T> AsRef<BulkWriteData<T>> for BulkWriteData<T> {
 	fn as_ref(&self) -> &Self {
 		self
 	}
 }
 
-/// The data response giving by a motor in response to a bulk read command.
-#[derive(Debug)]
-pub struct BulkResponse<T> {
-	/// The response message.
-	pub response: Response<T>,
-	
-	/// The address of the bulk read/write.
+pub struct BulkReadData {
+	/// The ID of the motor.
+	pub motor_id: u8,
+
+	/// The address for the read or write.
 	pub address: u16,
+
+	// The length of the data to be read.
+	pub count: u16,
+}
+
+impl AsRef<BulkReadData> for BulkReadData {
+	fn as_ref(&self) -> &Self {
+		self
+	}
 }
diff --git a/src/instructions/sync_write.rs b/src/instructions/sync_write.rs
index fc2921e..c0e0c01 100644
--- a/src/instructions/sync_write.rs
+++ b/src/instructions/sync_write.rs
@@ -1,4 +1,4 @@
-use super::{instruction_id, packet_id, SyncData};
+use super::{instruction_id, packet_id, SyncWriteData};
 use crate::endian::{write_u16_le, write_u32_le};
 use crate::{Bus, WriteError};
 
@@ -19,7 +19,7 @@ where
 	where
 		Iter: IntoIterator<Item = Data>,
 		Iter::IntoIter: std::iter::ExactSizeIterator,
-		Data: AsRef<SyncData<&'a [u8]>>,
+		Data: AsRef<SyncWriteData<&'a [u8]>>,
 	{
 		let data = data.into_iter();
 		let motors = data.len();
@@ -46,7 +46,7 @@ where
 	where
 		Iter: IntoIterator<Item = Data>,
 		Iter::IntoIter: std::iter::ExactSizeIterator,
-		Data: AsRef<SyncData<u8>>,
+		Data: AsRef<SyncWriteData<u8>>,
 	{
 		let data = data.into_iter();
 		let count = core::mem::size_of::<u8>();
@@ -73,7 +73,7 @@ where
 	where
 		Iter: IntoIterator<Item = Data>,
 		Iter::IntoIter: std::iter::ExactSizeIterator,
-		Data: AsRef<SyncData<u16>>,
+		Data: AsRef<SyncWriteData<u16>>,
 	{
 		let data = data.into_iter();
 		let count = core::mem::size_of::<u16>();
@@ -100,7 +100,7 @@ where
 	where
 		Iter: IntoIterator<Item = Data>,
 		Iter::IntoIter: std::iter::ExactSizeIterator,
-		Data: AsRef<SyncData<u32>>,
+		Data: AsRef<SyncWriteData<u32>>,
 	{
 		let data = data.into_iter();
 		let count = core::mem::size_of::<u32>();
diff --git a/src/lib.rs b/src/lib.rs
index fb2bdc8..1d3442c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -43,4 +43,3 @@ pub use error::WriteError;
 
 pub use bus::Bus;
 pub use bus::Response;
-pub use instructions::BulkResponse;

From 8f0a8ed9712a9d63d930955805e60ed81230e92d Mon Sep 17 00:00:00 2001
From: Omelia Iliffe <omelia.iliffe@wetaworkshop.co.nz>
Date: Thu, 14 Dec 2023 11:29:35 +1300
Subject: [PATCH 08/15] completed rename of _read to read_raw

---
 src/instructions/read.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/instructions/read.rs b/src/instructions/read.rs
index 5dfaaf6..b1292e6 100644
--- a/src/instructions/read.rs
+++ b/src/instructions/read.rs
@@ -22,7 +22,7 @@ where
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read(&mut self, motor_id: u8, address: u16, count: u16) -> Result<Response<Vec<u8>>, TransferError> {
-		let response = self._read(motor_id, address, count)?;
+		let response = self.read_raw(motor_id, address, count)?;
 		Ok(response.into())
 	}
 
@@ -31,7 +31,7 @@ where
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u8(&mut self, motor_id: u8, address: u16) -> Result<Response<u8>, TransferError> {
-		let response = self._read(motor_id, address, 1)?;
+		let response = self.read_raw(motor_id, address, 1)?;
 		Ok(response.into())
 	}
 
@@ -40,7 +40,7 @@ where
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u16(&mut self, motor_id: u8, address: u16) -> Result<Response<u16>, TransferError> {
-		let response = self._read(motor_id, address, 2)?;
+		let response = self.read_raw(motor_id, address, 2)?;
 		Ok(response.into())
 	}
 
@@ -49,7 +49,7 @@ where
 	/// This function will not work correctly if the motor ID is set to [`packet_id::BROADCAST`][crate::instructions::packet_id::BROADCAST].
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u32(&mut self, motor_id: u8, address: u16) -> Result<Response<u32>, TransferError> {
-		let response = self._read(motor_id, address, 4)?;
+		let response = self.read_raw(motor_id, address, 4)?;
 		Ok(response.into())
 	}
 }

From e935ae8ee4550c6d8a761d3bfd6fe1afdafd3c0a Mon Sep 17 00:00:00 2001
From: Omelia Iliffe <omelia.iliffe@wetaworkshop.co.nz>
Date: Thu, 14 Dec 2023 12:27:51 +1300
Subject: [PATCH 09/15] removed broadcast calls from unicast functions

---
 src/instructions/action.rs        | 17 ++++++-----------
 src/instructions/clear.rs         | 17 ++++++-----------
 src/instructions/factory_reset.rs | 18 +++++++-----------
 src/instructions/reboot.rs        | 17 ++++++-----------
 4 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/src/instructions/action.rs b/src/instructions/action.rs
index b21161f..ce24b8e 100644
--- a/src/instructions/action.rs
+++ b/src/instructions/action.rs
@@ -8,17 +8,12 @@ where
 {
 	/// Send an action command to trigger a previously registered instruction.
 	///
-	/// The `motor_id` parameter may be set to [`packet_id::BROADCAST`],
-	/// although the [`Self::broadcast_action`] is generally easier to use.
-	pub fn action(&mut self, motor_id: u8) -> Result<Option<Response<()>>, TransferError> {
-		if motor_id == packet_id::BROADCAST {
-			self.broadcast_action()?;
-			Ok(None)
-		} else {
-			let response = self.transfer_single(motor_id, instruction_id::ACTION, 0, |_| ())?;
-			crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-			Ok(Some(response.into()))
-		}
+	/// The `motor_id` parameter must not be set to [`packet_id::BROADCAST`],
+	/// Instead use [`Self::broadcast_action`].
+	pub fn action(&mut self, motor_id: u8) -> Result<Response<()>, TransferError> {
+		let response = self.transfer_single(motor_id, instruction_id::ACTION, 0, |_| ())?;
+		crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
+		Ok(response.into())
 	}
 
 	/// Broadcast an action command to all connected motors to trigger a previously registered instruction.
diff --git a/src/instructions/clear.rs b/src/instructions/clear.rs
index 3f82dac..f1008d7 100644
--- a/src/instructions/clear.rs
+++ b/src/instructions/clear.rs
@@ -15,17 +15,12 @@ where
 	/// It is not possible to clear the revolution counter of a motor while it is moving.
 	/// Doing so will cause the motor to return an error, and the revolution counter will not be reset.
 	///
-	/// The `motor_id` parameter may be set to [`packet_id::BROADCAST`],
-	/// although the [`Self::broadcast_clear_revolution_counter`] is generally easier to use.
-	pub fn clear_revolution_counter(&mut self, motor_id: u8) -> Result<Option<Response<()>>, crate::TransferError> {
-		if motor_id == packet_id::BROADCAST {
-			self.broadcast_clear_revolution_counter()?;
-			Ok(None)
-		} else {
-			let response = self.transfer_single(motor_id, instruction_id::CLEAR, CLEAR_REVOLUTION_COUNT.len(), encode_parameters)?;
-			crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-			Ok(Some(response.into()))
-		}
+	/// The `motor_id` parameter must not be set to [`packet_id::BROADCAST`],
+	/// Instead use [`Self::broadcast_clear_revolution_counter`].
+	pub fn clear_revolution_counter(&mut self, motor_id: u8) -> Result<Response<()>, crate::TransferError> {
+		let response = self.transfer_single(motor_id, instruction_id::CLEAR, CLEAR_REVOLUTION_COUNT.len(), encode_parameters)?;
+		crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
+		Ok(response.into())
 	}
 
 	/// Clear the revolution counter of all connected motors.
diff --git a/src/instructions/factory_reset.rs b/src/instructions/factory_reset.rs
index 85fc4a6..ce9157a 100644
--- a/src/instructions/factory_reset.rs
+++ b/src/instructions/factory_reset.rs
@@ -24,8 +24,8 @@ where
 	/// This will reset all registers to the factory default, including the EEPROM registers.
 	/// The only exceptions are the ID and baud rate settings, which may be kept depending on the `kind` argument.
 	///
-	/// The `motor_id` parameter may be set to [`packet_id::BROADCAST`],
-	/// although the [`Self::broadcast_factory_reset`] is generally easier to use.
+	/// The `motor_id` parameter must not be set to [`packet_id::BROADCAST`],
+	/// Instead use [`Self::broadcast_factory_reset`].
 	///
 	/// Starting with version 42 of the firmware for the MX-series and X-series,
 	/// motors ignore a broadcast reset command with `FactoryResetKind::ResetAll`.
@@ -33,15 +33,11 @@ where
 	/// which would cause multiple motors on the bus to have the same ID.
 	/// At that point, communication with those motors is not possible anymore.
 	/// The only way to restore communication is to physically disconnect all but one motor at a time and re-assign unique IDs.
-	pub fn factory_reset(&mut self, motor_id: u8, kind: FactoryResetKind) -> Result<Option<Response<()>>, crate::TransferError> {
-		if motor_id == packet_id::BROADCAST {
-			self.broadcast_factory_reset(kind)?;
-			Ok(None)
-		} else {
-			let response = self.transfer_single(motor_id, instruction_id::FACTORY_RESET, 1, |buffer| buffer[0] = kind as u8)?;
-			crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-			Ok(Some(response.into()))
-		}
+	/// Or use the ID Inspection Tool in the Dynamixel Wizard 2.0
+	pub fn factory_reset(&mut self, motor_id: u8, kind: FactoryResetKind) -> Result<Response<()>, crate::TransferError> {
+		let response = self.transfer_single(motor_id, instruction_id::FACTORY_RESET, 1, |buffer| buffer[0] = kind as u8)?;
+		crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
+		Ok(response.into())
 	}
 
 	/// Reset the settings of all connected motors to the factory defaults.
diff --git a/src/instructions/reboot.rs b/src/instructions/reboot.rs
index 8d8056c..651342a 100644
--- a/src/instructions/reboot.rs
+++ b/src/instructions/reboot.rs
@@ -12,17 +12,12 @@ where
 	/// When a motor reboots, all volatile (non-EEPROM) registers are reset to their initial value.
 	/// This also has the effect of disabling motor torque and resetting the multi-revolution information.
 	///
-	/// The `motor_id` parameter may be set to [`packet_id::BROADCAST`],
-	/// although the [`Self::broadcast_reboot`] is generally easier to use.
-	pub fn reboot(&mut self, motor_id: u8) -> Result<Option<Response<()>>, TransferError> {
-		if motor_id == packet_id::BROADCAST {
-			self.broadcast_action()?;
-			Ok(None)
-		} else {
-			let response = self.transfer_single(motor_id, instruction_id::REBOOT, 0, |_| ())?;
-			crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-			Ok(Some(response.into()))
-		}
+	/// The `motor_id` parameter must not be set to [`packet_id::BROADCAST`],
+	/// Instead use [`Self::broadcast_reboot`].
+	pub fn reboot(&mut self, motor_id: u8) -> Result<Response<()>, TransferError> {
+		let response = self.transfer_single(motor_id, instruction_id::REBOOT, 0, |_| ())?;
+		crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
+		Ok(response.into())
 	}
 
 	/// Broadcast an reboot command to all connected motors to trigger a previously registered instruction.

From a99999c088474184ca31395267bc6f3e6ea752ca Mon Sep 17 00:00:00 2001
From: Omelia Iliffe <omelia.iliffe@wetaworkshop.co.nz>
Date: Fri, 15 Dec 2023 08:28:43 +1300
Subject: [PATCH 10/15] replace use of error with alert

---
 src/instructions/ping.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/instructions/ping.rs b/src/instructions/ping.rs
index f60aed6..824db0f 100644
--- a/src/instructions/ping.rs
+++ b/src/instructions/ping.rs
@@ -28,7 +28,7 @@ where
 			motor_id: status_packet.packet_id(),
 			model: crate::endian::read_u16_le(&parameters[0..]),
 			firmware: crate::endian::read_u8_le(&parameters[2..]),
-			alert: status_packet.error() & 0x80 != 0,
+			alert: status_packet.alert(),
 		}
 	}
 }

From 5dfdb71d56b3bd92c9412f921bea8f0a8060f76f Mon Sep 17 00:00:00 2001
From: Maarten de Vries <maarten@de-vri.es>
Date: Sat, 16 Dec 2023 11:46:47 +0100
Subject: [PATCH 11/15] Fix amount of data read with sync_read_u16.

---
 CHANGELOG.md                  | 3 +++
 src/instructions/sync_read.rs | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4bef2e2..241a635 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,6 @@
+main:
+  * Fix amount of data read in `sync_read_u16` and `sync_read_u16_cb`.
+
 v0.5.0 - 2023-12-02:
   * Update `serial2` to `v0.2`.
 
diff --git a/src/instructions/sync_read.rs b/src/instructions/sync_read.rs
index f716b04..9a82380 100644
--- a/src/instructions/sync_read.rs
+++ b/src/instructions/sync_read.rs
@@ -68,7 +68,7 @@ where
 	where
 		F: FnMut(Result<Response<u16>, ReadError>),
 	{
-		let count = 1;
+		let count = 2;
 		self.write_instruction(packet_id::BROADCAST, instruction_id::SYNC_READ, 4 + motor_ids.len(), |buffer| {
 			write_u16_le(&mut buffer[0..], address);
 			write_u16_le(&mut buffer[2..], count as u16);

From 8a190fdfca7fa4e30a5965cfeaddd0cd228164e4 Mon Sep 17 00:00:00 2001
From: Maarten de Vries <maarten@de-vri.es>
Date: Sat, 16 Dec 2023 11:59:14 +0100
Subject: [PATCH 12/15] Add direct conversions from specific read errors to
 TransferError.

---
 src/error.rs | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/src/error.rs b/src/error.rs
index 3139c5d..eb70e9f 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -183,6 +183,42 @@ impl From<ReadError> for TransferError {
 	}
 }
 
+impl From<InvalidMessage> for TransferError {
+	fn from(other: InvalidMessage) -> Self {
+		Self::ReadError(other.into())
+	}
+}
+
+impl From<InvalidHeaderPrefix> for TransferError {
+	fn from(other: InvalidHeaderPrefix) -> Self {
+		Self::ReadError(other.into())
+	}
+}
+
+impl From<InvalidChecksum> for TransferError {
+	fn from(other: InvalidChecksum) -> Self {
+		Self::ReadError(other.into())
+	}
+}
+
+impl From<InvalidPacketId> for TransferError {
+	fn from(other: InvalidPacketId) -> Self {
+		Self::ReadError(other.into())
+	}
+}
+
+impl From<InvalidInstruction> for TransferError {
+	fn from(other: InvalidInstruction) -> Self {
+		Self::ReadError(other.into())
+	}
+}
+
+impl From<InvalidParameterCount> for TransferError {
+	fn from(other: InvalidParameterCount) -> Self {
+		Self::ReadError(other.into())
+	}
+}
+
 impl From<std::io::Error> for ReadError {
 	fn from(other: std::io::Error) -> Self {
 		Self::Io(other)

From 9399dc3ed41589938896d4f63cd705da473c9f17 Mon Sep 17 00:00:00 2001
From: Maarten de Vries <maarten@de-vri.es>
Date: Sat, 16 Dec 2023 12:00:42 +0100
Subject: [PATCH 13/15] Implement TryFrom instead of From for fallible message
 conversions.

---
 src/bus.rs                        | 76 ++++++++++++++++++-------------
 src/instructions/action.rs        |  3 +-
 src/instructions/clear.rs         |  3 +-
 src/instructions/factory_reset.rs |  3 +-
 src/instructions/ping.rs          | 21 +++++----
 src/instructions/read.rs          |  6 +--
 src/instructions/reboot.rs        |  3 +-
 src/instructions/reg_write.rs     | 12 ++---
 src/instructions/sync_read.rs     |  8 ++--
 src/instructions/write.rs         | 12 ++---
 10 files changed, 73 insertions(+), 74 deletions(-)

diff --git a/src/bus.rs b/src/bus.rs
index cd04a91..94da9d7 100644
--- a/src/bus.rs
+++ b/src/bus.rs
@@ -311,6 +311,22 @@ where
 	}
 }
 
+/// Find the potential starting position of a header.
+///
+/// This will return the first possible position of the header prefix.
+/// Note that if the buffer ends with a partial header prefix,
+/// the start position of the partial header prefix is returned.
+fn find_header(buffer: &[u8]) -> usize {
+	for i in 0..buffer.len() {
+		let possible_prefix = HEADER_PREFIX.len().min(buffer.len() - i);
+		if buffer[i..].starts_with(&HEADER_PREFIX[..possible_prefix]) {
+			return i;
+		}
+	}
+
+	buffer.len()
+}
+
 /// A response from a motor.
 #[derive(Debug)]
 pub struct Response<T> {
@@ -327,17 +343,20 @@ pub struct Response<T> {
 	pub data: T,
 }
 
-impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<()>
+impl<'a, ReadBuffer, WriteBuffer> TryFrom<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<()>
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
 {
-	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
-		Self {
+	type Error = crate::InvalidParameterCount;
+
+	fn try_from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Result<Self, Self::Error> {
+		crate::InvalidParameterCount::check(status_packet.parameter_count, 0)?;
+		Ok(Self {
 			data: (),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-		}
+		})
 	}
 }
 
@@ -355,62 +374,55 @@ where
 	}
 }
 
-impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u8>
+impl<'a, ReadBuffer, WriteBuffer> TryFrom<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u8>
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
 {
-	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
-		Self {
+	type Error = crate::InvalidParameterCount;
+
+	fn try_from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Result<Self, Self::Error> {
+		crate::InvalidParameterCount::check(status_packet.parameter_count, 1)?;
+		Ok(Self {
 			data: read_u8_le(status_packet.parameters()),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-		}
+		})
 	}
 }
 
-impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u16>
+impl<'a, ReadBuffer, WriteBuffer> TryFrom<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u16>
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
 {
-	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
-		Self {
+	type Error = crate::InvalidParameterCount;
+
+	fn try_from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Result<Self, Self::Error> {
+		crate::InvalidParameterCount::check(status_packet.parameter_count, 2)?;
+		Ok(Self {
 			data: read_u16_le(status_packet.parameters()),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-		}
+		})
 	}
 }
 
-impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u32>
+impl<'a, ReadBuffer, WriteBuffer> TryFrom<StatusPacket<'a, ReadBuffer, WriteBuffer>> for Response<u32>
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
 {
-	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
-		Self {
+	type Error = crate::InvalidParameterCount;
+
+	fn try_from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Result<Self, Self::Error> {
+		crate::InvalidParameterCount::check(status_packet.parameter_count, 4)?;
+		Ok(Self {
 			data: read_u32_le(status_packet.parameters()),
 			motor_id: status_packet.packet_id(),
 			alert: status_packet.alert(),
-		}
-	}
-}
-
-/// Find the potential starting position of a header.
-///
-/// This will return the first possible position of the header prefix.
-/// Note that if the buffer ends with a partial header prefix,
-/// the start position of the partial header prefix is returned.
-fn find_header(buffer: &[u8]) -> usize {
-	for i in 0..buffer.len() {
-		let possible_prefix = HEADER_PREFIX.len().min(buffer.len() - i);
-		if buffer[i..].starts_with(&HEADER_PREFIX[..possible_prefix]) {
-			return i;
-		}
+		})
 	}
-
-	buffer.len()
 }
 
 #[cfg(test)]
diff --git a/src/instructions/action.rs b/src/instructions/action.rs
index ce24b8e..b02a89e 100644
--- a/src/instructions/action.rs
+++ b/src/instructions/action.rs
@@ -12,8 +12,7 @@ where
 	/// Instead use [`Self::broadcast_action`].
 	pub fn action(&mut self, motor_id: u8) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::ACTION, 0, |_| ())?;
-		crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Broadcast an action command to all connected motors to trigger a previously registered instruction.
diff --git a/src/instructions/clear.rs b/src/instructions/clear.rs
index f1008d7..b5808e0 100644
--- a/src/instructions/clear.rs
+++ b/src/instructions/clear.rs
@@ -19,8 +19,7 @@ where
 	/// Instead use [`Self::broadcast_clear_revolution_counter`].
 	pub fn clear_revolution_counter(&mut self, motor_id: u8) -> Result<Response<()>, crate::TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::CLEAR, CLEAR_REVOLUTION_COUNT.len(), encode_parameters)?;
-		crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Clear the revolution counter of all connected motors.
diff --git a/src/instructions/factory_reset.rs b/src/instructions/factory_reset.rs
index ce9157a..28b1246 100644
--- a/src/instructions/factory_reset.rs
+++ b/src/instructions/factory_reset.rs
@@ -36,8 +36,7 @@ where
 	/// Or use the ID Inspection Tool in the Dynamixel Wizard 2.0
 	pub fn factory_reset(&mut self, motor_id: u8, kind: FactoryResetKind) -> Result<Response<()>, crate::TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::FACTORY_RESET, 1, |buffer| buffer[0] = kind as u8)?;
-		crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Reset the settings of all connected motors to the factory defaults.
diff --git a/src/instructions/ping.rs b/src/instructions/ping.rs
index 824db0f..472b7c2 100644
--- a/src/instructions/ping.rs
+++ b/src/instructions/ping.rs
@@ -1,5 +1,6 @@
 use super::{instruction_id, packet_id};
-use crate::{bus::StatusPacket, Bus, ReadError, TransferError, WriteError};
+use crate::bus::StatusPacket;
+use crate::{Bus, ReadError, TransferError, WriteError};
 
 #[derive(Debug)]
 pub struct PingResponse {
@@ -17,19 +18,22 @@ pub struct PingResponse {
 	pub alert: bool,
 }
 
-impl<'a, ReadBuffer, WriteBuffer> From<StatusPacket<'a, ReadBuffer, WriteBuffer>> for PingResponse
+impl<'a, ReadBuffer, WriteBuffer> TryFrom<StatusPacket<'a, ReadBuffer, WriteBuffer>> for PingResponse
 where
 	ReadBuffer: AsRef<[u8]> + AsMut<[u8]>,
 	WriteBuffer: AsRef<[u8]> + AsMut<[u8]>,
 {
-	fn from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Self {
+	type Error = crate::InvalidParameterCount;
+
+	fn try_from(status_packet: StatusPacket<'a, ReadBuffer, WriteBuffer>) -> Result<Self, Self::Error> {
 		let parameters = status_packet.parameters();
-		Self {
+		crate::InvalidParameterCount::check(parameters.len(), 3)?;
+		Ok(Self {
 			motor_id: status_packet.packet_id(),
 			model: crate::endian::read_u16_le(&parameters[0..]),
 			firmware: crate::endian::read_u8_le(&parameters[2..]),
 			alert: status_packet.alert(),
-		}
+		})
 	}
 }
 
@@ -44,7 +48,7 @@ where
 	/// Use [`Self::scan`] or [`Self::scan_cb`] instead.
 	pub fn ping(&mut self, motor_id: u8) -> Result<PingResponse, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::PING, 0, |_| ())?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Scan a bus for motors with a broadcast ping, returning the responses in a [`Vec`].
@@ -78,10 +82,7 @@ where
 					continue;
 				}
 			}
-			let response = response.and_then(|response| {
-				crate::InvalidParameterCount::check(response.parameters().len(), 3)?;
-				Ok(response.into())
-			});
+			let response = response.and_then(|response| Ok(response.try_into()?));
 			on_response(response);
 		}
 
diff --git a/src/instructions/read.rs b/src/instructions/read.rs
index b1292e6..0e8379a 100644
--- a/src/instructions/read.rs
+++ b/src/instructions/read.rs
@@ -32,7 +32,7 @@ where
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u8(&mut self, motor_id: u8, address: u16) -> Result<Response<u8>, TransferError> {
 		let response = self.read_raw(motor_id, address, 1)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Read 16 bit register from a specific motor.
@@ -41,7 +41,7 @@ where
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u16(&mut self, motor_id: u8, address: u16) -> Result<Response<u16>, TransferError> {
 		let response = self.read_raw(motor_id, address, 2)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Read 32 bit register from a specific motor.
@@ -50,6 +50,6 @@ where
 	/// Use [`Self::sync_read`] to read from multiple motors with one command.
 	pub fn read_u32(&mut self, motor_id: u8, address: u16) -> Result<Response<u32>, TransferError> {
 		let response = self.read_raw(motor_id, address, 4)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 }
diff --git a/src/instructions/reboot.rs b/src/instructions/reboot.rs
index 651342a..810c5ef 100644
--- a/src/instructions/reboot.rs
+++ b/src/instructions/reboot.rs
@@ -16,8 +16,7 @@ where
 	/// Instead use [`Self::broadcast_reboot`].
 	pub fn reboot(&mut self, motor_id: u8) -> Result<Response<()>, TransferError> {
 		let response = self.transfer_single(motor_id, instruction_id::REBOOT, 0, |_| ())?;
-		crate::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Broadcast an reboot command to all connected motors to trigger a previously registered instruction.
diff --git a/src/instructions/reg_write.rs b/src/instructions/reg_write.rs
index dffaac5..4070fd2 100644
--- a/src/instructions/reg_write.rs
+++ b/src/instructions/reg_write.rs
@@ -19,8 +19,7 @@ where
 			write_u16_le(&mut buffer[0..], address);
 			buffer[2..].copy_from_slice(data)
 		})?;
-		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Register a write command for a 8 bit value to a specific motor.
@@ -34,8 +33,7 @@ where
 			write_u16_le(&mut buffer[0..], address);
 			buffer[2] = value;
 		})?;
-		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Register a write command for a 16 bit value to a specific motor.
@@ -49,8 +47,7 @@ where
 			write_u16_le(&mut buffer[0..], address);
 			write_u16_le(&mut buffer[2..], value);
 		})?;
-		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Register a write command for a 32 bit value to a specific motor.
@@ -64,7 +61,6 @@ where
 			write_u16_le(&mut buffer[0..], address);
 			write_u32_le(&mut buffer[2..], value);
 		})?;
-		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 }
diff --git a/src/instructions/sync_read.rs b/src/instructions/sync_read.rs
index 9a82380..2133dfb 100644
--- a/src/instructions/sync_read.rs
+++ b/src/instructions/sync_read.rs
@@ -52,8 +52,7 @@ where
 		for &motor_id in motor_ids {
 			let data = self.read_status_response().and_then(|response| {
 				crate::InvalidPacketId::check(response.packet_id(), motor_id)?;
-				crate::InvalidParameterCount::check(response.parameters().len(), count)?;
-				Ok(response.into())
+				Ok(response.try_into()?)
 			});
 			on_response(data);
 		}
@@ -77,8 +76,7 @@ where
 		for &motor_id in motor_ids {
 			let data = self.read_status_response().and_then(|response| {
 				crate::InvalidPacketId::check(response.packet_id(), motor_id)?;
-				crate::InvalidParameterCount::check(response.parameters().len(), count)?;
-				Ok(response.into())
+				Ok(response.try_into()?)
 			});
 			on_response(data);
 		}
@@ -103,7 +101,7 @@ where
 			let data = self.read_status_response().and_then(|response| {
 				crate::InvalidPacketId::check(response.packet_id(), motor_id)?;
 				crate::InvalidParameterCount::check(response.parameters().len(), count)?;
-				Ok(response.into())
+				Ok(response.try_into()?)
 			});
 			on_response(data);
 		}
diff --git a/src/instructions/write.rs b/src/instructions/write.rs
index 452e751..aa883b2 100644
--- a/src/instructions/write.rs
+++ b/src/instructions/write.rs
@@ -13,8 +13,7 @@ where
 			write_u16_le(&mut buffer[0..], address);
 			buffer[2..].copy_from_slice(data)
 		})?;
-		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Write an 8 bit value to a specific motor.
@@ -23,8 +22,7 @@ where
 			write_u16_le(&mut buffer[0..], address);
 			buffer[2] = value;
 		})?;
-		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Write an 16 bit value to a specific motor.
@@ -33,8 +31,7 @@ where
 			write_u16_le(&mut buffer[0..], address);
 			write_u16_le(&mut buffer[2..], value);
 		})?;
-		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 
 	/// Write an 32 bit value to a specific motor.
@@ -43,7 +40,6 @@ where
 			write_u16_le(&mut buffer[0..], address);
 			write_u32_le(&mut buffer[2..], value);
 		})?;
-		crate::error::InvalidParameterCount::check(response.parameters().len(), 0).map_err(crate::ReadError::from)?;
-		Ok(response.into())
+		Ok(response.try_into()?)
 	}
 }

From ec7dcc2b6d5d228b5bddcd79d3f4eb1cb7105e54 Mon Sep 17 00:00:00 2001
From: Maarten de Vries <maarten@de-vri.es>
Date: Sat, 16 Dec 2023 12:13:11 +0100
Subject: [PATCH 14/15] Update changelog.

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b5884a7..d8f277e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,7 @@
 main:
   * Fix amount of data read in `sync_read_u16` and `sync_read_u16_cb`.
+  * Do not return `Err()` when the `alert` bit is set in a status packet from a motor.
+  * Report the `alert` bit in the returned values from commands in a new `Response` struct.
 
 v0.5.1 - 2023-12-07:
   * Parse all status messages when more than on has been read in a single `read()`.

From 64c73ba588e78dc93046a35aca72bd43e4738c15 Mon Sep 17 00:00:00 2001
From: Maarten de Vries <maarten@de-vri.es>
Date: Sat, 16 Dec 2023 12:13:26 +0100
Subject: [PATCH 15/15] Pass original BulkReadData command to user callback in
 bulk_read_cb().

---
 CHANGELOG.md                  | 1 +
 src/instructions/bulk_read.rs | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d8f277e..4761a6d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@ main:
   * Fix amount of data read in `sync_read_u16` and `sync_read_u16_cb`.
   * Do not return `Err()` when the `alert` bit is set in a status packet from a motor.
   * Report the `alert` bit in the returned values from commands in a new `Response` struct.
+  * Pass original `BulkReadData` command to user callback in `bulk_read_cb()`.
 
 v0.5.1 - 2023-12-07:
   * Parse all status messages when more than on has been read in a single `read()`.
diff --git a/src/instructions/bulk_read.rs b/src/instructions/bulk_read.rs
index 3c80a84..0f5f274 100644
--- a/src/instructions/bulk_read.rs
+++ b/src/instructions/bulk_read.rs
@@ -22,7 +22,7 @@ where
 	pub fn bulk_read_cb<Read, F>(&mut self, reads: &[Read], mut on_response: F) -> Result<(), WriteError>
 	where
 		Read: AsRef<BulkReadData>,
-		F: FnMut(Result<Response<Vec<u8>>, ReadError>),
+		F: FnMut(&BulkReadData, Result<Response<Vec<u8>>, ReadError>),
 	{
 		for i in 0..reads.len() {
 			for j in i + 1..reads.len() {
@@ -55,8 +55,8 @@ where
 			});
 
 			match response {
-				Ok(response) => on_response(Ok(response.into())),
-				Err(e) => on_response(Err(e)),
+				Ok(response) => on_response(read, Ok(response.into())),
+				Err(e) => on_response(read, Err(e)),
 			}
 		}
 		Ok(())
@@ -79,7 +79,7 @@ where
 		let mut responses = Vec::with_capacity(reads.len());
 		let mut read_error = None;
 
-		self.bulk_read_cb(reads, |data| {
+		self.bulk_read_cb(reads, |_read, data| {
 			if read_error.is_none() {
 				match data {
 					Err(e) => read_error = Some(e),