Refactor Command, a bit more cleanup

src/coe/abort_code.rs

@@ -1,5 +1,3 @@
-use core::fmt;
-
 /// Defined in ETG1000.6 Table 41 – SDO Abort Codes
 #[derive(Debug, Copy, Clone, PartialEq, Eq, num_enum::FromPrimitive, num_enum::IntoPrimitive)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
@@ -84,8 +82,8 @@ pub enum AbortCode {
     Unknown(u32),
 }
 
-impl fmt::Display for AbortCode {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+impl core::fmt::Display for AbortCode {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         let num = u32::from(*self);
 
         f.write_fmt(format_args!("{:#010x}", num))?;

src/coe/services.rs

@@ -93,7 +93,7 @@ pub trait CoeServiceResponse: PackedStruct + PackedStructInfo {
 }
 
 /// Must be implemented for any type used to send a CoE service.
-pub trait CoeServiceRequest: PackedStruct + Display {
+pub trait CoeServiceRequest: PackedStruct {
     type Response: CoeServiceResponse;
 
     /// Get the auto increment counter value for this request.

src/command.rs

@@ -1,6 +1,17 @@
-use crate::generate::le_u16;
-use core::fmt;
-use nom::{combinator::map, error::ParseError, sequence::pair, IResult};
+use crate::{fmt, generate::le_u16};
+use nom::{combinator::map, sequence::pair, IResult};
+
+const NOP: u8 = 0x00;
+const APRD: u8 = 0x01;
+const FPRD: u8 = 0x04;
+const BRD: u8 = 0x07;
+const LRD: u8 = 0x0A;
+const BWR: u8 = 0x08;
+const APWR: u8 = 0x02;
+const FPWR: u8 = 0x05;
+const FRMW: u8 = 0x0E;
+const LWR: u8 = 0x0B;
+const LRW: u8 = 0x0c;
 
 /// PDU command.
 #[derive(Default, PartialEq, Eq, Debug, Copy, Clone)]
@@ -85,8 +96,8 @@ pub enum Command {
     },
 }
 
-impl fmt::Display for Command {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+impl core::fmt::Display for Command {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         match self {
             Command::Nop => write!(f, "NOP"),
             Command::Aprd { address, register } => {
@@ -119,25 +130,25 @@ impl fmt::Display for Command {
 
 impl Command {
     /// Get just the command code for a command.
-    pub const fn code(&self) -> CommandCode {
+    pub const fn code(&self) -> u8 {
         match self {
-            Self::Nop => CommandCode::Nop,
+            Self::Nop => NOP,
 
             // Reads
-            Self::Aprd { .. } => CommandCode::Aprd,
-            Self::Fprd { .. } => CommandCode::Fprd,
-            Self::Brd { .. } => CommandCode::Brd,
-            Self::Lrd { .. } => CommandCode::Lrd,
+            Self::Aprd { .. } => APRD,
+            Self::Fprd { .. } => FPRD,
+            Self::Brd { .. } => BRD,
+            Self::Lrd { .. } => LRD,
 
             // Writes
-            Self::Bwr { .. } => CommandCode::Bwr,
-            Self::Apwr { .. } => CommandCode::Apwr,
-            Self::Fpwr { .. } => CommandCode::Fpwr,
-            Self::Lwr { .. } => CommandCode::Lwr,
-            Self::Frmw { .. } => CommandCode::Frmw,
+            Self::Bwr { .. } => BWR,
+            Self::Apwr { .. } => APWR,
+            Self::Fpwr { .. } => FPWR,
+            Self::Frmw { .. } => FRMW,
+            Self::Lwr { .. } => LWR,
 
             // Read/writes
-            Self::Lrw { .. } => CommandCode::Lrw,
+            Self::Lrw { .. } => LRW,
         }
     }
 
@@ -167,76 +178,56 @@ impl Command {
             }
         }
     }
-}
-
-/// Broadcast or configured station addressing.
-#[derive(Default, Copy, Clone, Debug, PartialEq, Eq, num_enum::TryFromPrimitive)]
-#[repr(u8)]
-pub enum CommandCode {
-    #[default]
-    Nop = 0x00,
-
-    // Reads
-    Aprd = 0x01,
-    Fprd = 0x04,
-    Brd = 0x07,
-    Lrd = 0x0A,
-
-    // Writes
-    Bwr = 0x08,
-    Apwr = 0x02,
-    Fpwr = 0x05,
-    Frmw = 0x0E,
-    Lwr = 0x0B,
-
-    // Read/writes
-    Lrw = 0x0c,
-}
 
-impl CommandCode {
-    /// Parse an address, producing a [`Command`].
-    pub fn parse_address<'a, E>(self, i: &'a [u8]) -> IResult<&'a [u8], Command, E>
-    where
-        E: ParseError<&'a [u8]>,
-    {
+    /// Parse a command from a code and address data (e.g. `(u16, u16)` or `u32`), producing a [`Command`].
+    pub fn parse(command_code: u8, i: &[u8]) -> IResult<&[u8], Self> {
         use nom::number::complete::{le_u16, le_u32};
 
-        match self {
-            Self::Nop => Ok((i, Command::Nop)),
+        match command_code {
+            NOP => Ok((i, Command::Nop)),
 
-            Self::Aprd => map(pair(le_u16, le_u16), |(address, register)| Command::Aprd {
+            APRD => map(pair(le_u16, le_u16), |(address, register)| Command::Aprd {
                 address,
                 register,
             })(i),
-            Self::Fprd => map(pair(le_u16, le_u16), |(address, register)| Command::Fprd {
+            FPRD => map(pair(le_u16, le_u16), |(address, register)| Command::Fprd {
                 address,
                 register,
             })(i),
-            Self::Brd => map(pair(le_u16, le_u16), |(address, register)| Command::Brd {
+            BRD => map(pair(le_u16, le_u16), |(address, register)| Command::Brd {
                 address,
                 register,
             })(i),
-            Self::Lrd => map(le_u32, |address| Command::Lrd { address })(i),
+            LRD => map(le_u32, |address| Command::Lrd { address })(i),
 
-            Self::Bwr => map(pair(le_u16, le_u16), |(address, register)| Command::Bwr {
+            BWR => map(pair(le_u16, le_u16), |(address, register)| Command::Bwr {
                 address,
                 register,
             })(i),
-            Self::Apwr => map(pair(le_u16, le_u16), |(address, register)| Command::Apwr {
+            APWR => map(pair(le_u16, le_u16), |(address, register)| Command::Apwr {
                 address,
                 register,
             })(i),
-            Self::Fpwr => map(pair(le_u16, le_u16), |(address, register)| Command::Fpwr {
+            FPWR => map(pair(le_u16, le_u16), |(address, register)| Command::Fpwr {
                 address,
                 register,
             })(i),
-            Self::Frmw => map(pair(le_u16, le_u16), |(address, register)| Command::Frmw {
+            FRMW => map(pair(le_u16, le_u16), |(address, register)| Command::Frmw {
                 address,
                 register,
             })(i),
-            Self::Lwr => map(le_u32, |address| Command::Lwr { address })(i),
+            LWR => map(le_u32, |address| Command::Lwr { address })(i),
+
+            LRW => map(le_u32, |address| Command::Lrw { address })(i),
 
-            Self::Lrw => map(le_u32, |address| Command::Lrw { address })(i),
+            other => {
+                fmt::error!("Invalid command code {:#02x}", other);
+
+                Err(nom::Err::Failure(nom::error::Error {
+                    input: i,
+                    code: nom::error::ErrorKind::Tag,
+                }))
+            }
         }
     }
 }

src/pdi.rs

@@ -1,4 +1,4 @@
-use core::{fmt, ops::Range};
+use core::ops::Range;
 
 /// An accumulator that stores the bit and byte offsets in the PDI so slave IO data can be mapped
 /// to/from the PDI using FMMUs.
@@ -87,7 +87,7 @@ impl PdiSegment {
     }
 }
 
-impl fmt::Display for PdiSegment {
+impl core::fmt::Display for PdiSegment {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         if self.bit_len > 0 {
             write!(

src/pdu_loop/frame_element/sendable_frame.rs

@@ -88,7 +88,7 @@ impl<'sto> SendableFrame<'sto> {
 
         let buf = le_u16(header.0, buf);
 
-        let buf = le_u8(frame.command.code() as u8, buf);
+        let buf = le_u8(frame.command.code(), buf);
         let buf = le_u8(frame.index, buf);
 
         // Write address and register data

src/pdu_loop/pdu_rx.rs

@@ -1,6 +1,6 @@
 use super::storage::PduStorageRef;
 use crate::{
-    command::CommandCode,
+    command::Command,
     error::{Error, PduError, PduValidationError},
     fmt,
     pdu_loop::{frame_header::FrameHeader, pdu_flags::PduFlags},
@@ -47,7 +47,7 @@ impl<'sto> PduRx<'sto> {
         // Only take as much as the header says we should
         let (_rest, i) = take(header.payload_len())(i)?;
 
-        let (i, command_code) = map_res(u8, CommandCode::try_from)(i)?;
+        let (i, command_code) = u8(i)?;
         let (i, index) = u8(i)?;
 
         let mut frame = self
@@ -64,7 +64,7 @@ impl<'sto> PduRx<'sto> {
             )));
         }
 
-        let (i, command) = command_code.parse_address(i)?;
+        let (i, command) = Command::parse(command_code, i)?;
 
         // Check for weird bugs where a slave might return a different command than the one sent for
         // this PDU index.

src/register.rs

@@ -2,7 +2,6 @@ use crate::{
     error::WrappedPackingError,
     pdu_data::{PduData, PduRead},
 };
-use core::fmt;
 use packed_struct::{prelude::*, PackingError};
 
 /// Slave device register address abstraction.
@@ -309,8 +308,8 @@ pub struct SupportFlags {
     pub special_fmmu: bool,
 }
 
-impl fmt::Display for SupportFlags {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+impl core::fmt::Display for SupportFlags {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         // Just print DC for now
         f.write_str("DC: ")?;
 

src/slave/mod.rs

@@ -441,7 +441,6 @@ where
     ) -> Result<(), Error>
     where
         T: PduData,
-        <T as PduRead>::Error: Debug,
     {
         let sub_index = sub_index.into();
 
@@ -570,7 +569,6 @@ where
     pub async fn sdo_read<T>(&self, index: u16, sub_index: impl Into<SubIndex>) -> Result<T, Error>
     where
         T: PduData,
-        <T as PduRead>::Error: Debug,
     {
         let sub_index = sub_index.into();
 
@@ -629,7 +627,6 @@ impl<'a, S> SlaveRef<'a, S> {
     pub async fn register_read<T>(&self, register: impl Into<u16>) -> Result<T, Error>
     where
         T: PduRead,
-        <T as PduRead>::Error: Debug,
     {
         self.read_ignore_wkc(register).await?.wkc(1, "raw read")
     }
@@ -641,7 +638,6 @@ impl<'a, S> SlaveRef<'a, S> {
     pub async fn register_write<T>(&self, register: impl Into<u16>, value: T) -> Result<T, Error>
     where
         T: PduData,
-        <T as PduRead>::Error: Debug,
     {
         self.write_ignore_wkc(register, value)
             .await?
@@ -654,7 +650,6 @@ impl<'a, S> SlaveRef<'a, S> {
     ) -> Result<PduResponse<T>, Error>
     where
         T: PduRead,
-        <T as PduRead>::Error: Debug,
     {
         self.client.fprd(self.configured_address, register).await
     }
@@ -667,7 +662,6 @@ impl<'a, S> SlaveRef<'a, S> {
     ) -> Result<PduResponse<T>, Error>
     where
         T: PduData,
-        <T as PduRead>::Error: Debug,
     {
         self.client
             .fpwr(self.configured_address, register, value)
@@ -681,9 +675,11 @@ impl<'a, S> SlaveRef<'a, S> {
     ) -> Result<T, Error>
     where
         T: PduRead,
-        <T as PduRead>::Error: Debug,
     {
-        self.read_ignore_wkc(register.into()).await?.wkc(1, context)
+        self.client
+            .fprd(self.configured_address, register)
+            .await?
+            .wkc(1, context)
     }
 
     /// A wrapper around an FPWR service to this slave's configured address.
@@ -695,9 +691,9 @@ impl<'a, S> SlaveRef<'a, S> {
     ) -> Result<T, Error>
     where
         T: PduData,
-        <T as PduRead>::Error: Debug,
     {
-        self.write_ignore_wkc(register, value)
+        self.client
+            .fpwr(self.configured_address, register, value)
             .await?
             .wkc(1, context)
     }

src/slave_state.rs

@@ -1,4 +1,3 @@
-use core::fmt;
 use packed_struct::prelude::*;
 
 /// AL (application layer) device state.
@@ -35,8 +34,8 @@ pub enum SlaveState {
     Unknown = 0xff,
 }
 
-impl fmt::Display for SlaveState {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+impl core::fmt::Display for SlaveState {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         let s = match self {
             SlaveState::None => "None",
             SlaveState::Init => "Init",