Interface refactor

pn532/src/interface/i2c.rs

@@ -1,85 +1,8 @@
-//! Transport abstraction and the I2C implementation.
-//!
-//! The PN532 communicates over a framed protocol regardless of the physical
-//! transport (I2C, SPI or HSU). The [`Interface`] trait mirrors that boundary:
-//! the driver builds logical commands, the interface handles framing and the
-//! acknowledgement/response handshake.
-
 use embedded_hal::delay::DelayNs;
-use embedded_hal::digital::{ErrorKind, ErrorType, InputPin, OutputPin};
+use embedded_hal::digital::{InputPin, OutputPin};
 use embedded_hal::i2c::{I2c, Operation};
+use crate::{Error, Interface, NoIrq, NoReset, PN532Command};
 use crate::types::constants::*;
-use crate::error::Error;
-use crate::PN532Command;
-
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-pub struct UnreachError;
-
-impl embedded_hal::digital::Error for UnreachError {
-    fn kind(&self) -> ErrorKind {
-        ErrorKind::Other
-    }
-}
-
-/// Marker type for when no reset pin is connected.
-#[derive(Clone, Copy, Debug, Default)]
-pub struct NoReset;
-
-impl ErrorType for NoReset { type Error = UnreachError; }
-
-impl OutputPin for NoReset {
-    fn set_low(&mut self) -> Result<(), Self::Error> {
-        Err(UnreachError)
-    }
-
-    fn set_high(&mut self) -> Result<(), Self::Error> {
-        Err(UnreachError)
-    }
-}
-
-/// Marker type for when no IRQ pin is connected.
-#[derive(Clone, Copy, Debug, Default)]
-pub struct NoIrq;
-
-impl ErrorType for NoIrq { type Error = UnreachError; }
-
-impl InputPin for NoIrq {
-    fn is_high(&mut self) -> Result<bool, Self::Error> {
-        panic!("NoIrq should not be used");
-    }
-
-    fn is_low(&mut self) -> Result<bool, Self::Error> {
-        panic!("NoIrq should not be used");
-    }
-}
-
-/// A framed transport to the PN532.
-///
-/// Implementations are responsible for the frame/ack handshake described in
-/// the PN532 user manual (UM0701-02).
-pub trait Interface {
-    /// The error type produced by the underlying physical transport.
-    type TransportError;
-
-    /// Initialise the hardware: pulse the reset pin (if provided) and wait for
-    /// the PN532 to become ready.
-    fn begin(&mut self) -> Result<(), Error<Self::TransportError>>;
-
-    /// Write a command frame (`header` + optional `body`) and wait for the ACK.
-    fn write_command(
-        &mut self,
-        header: &[u8],
-        body: &[u8],
-    ) -> Result<(), Error<Self::TransportError>>;
-
-    /// Read a response frame, returning the length of the payload (excluding
-    /// the `TFI` and command bytes) placed in `buf`.
-    fn read_response(
-        &mut self,
-        buf: &mut [u8],
-        timeout_ms: u16,
-    ) -> Result<usize, Error<Self::TransportError>>;
-}
 
 /// [`Interface`] implementation over a blocking `embedded_hal::i2c::I2c` bus.
 ///
@@ -182,39 +105,10 @@ where
         body: &[u8],
     ) -> Result<(), Error<Self::TransportError>> {
         self.command = Some(PN532Command::from_bits(header[0]));
-
+        let frame = super::to_request_frame(header, body)?;
-        let data_len = header.len() + body.len() + 1; // TFI + payload
+        crate::trace!("pn532: write cmd=0x{:02X} len={}", header[0], frame.len());
-        if data_len > 0xFF {
-            return Err(Error::NoSpace);
-        }
-
-        // PREAMBLE + STARTCODE1 + STARTCODE2 + LEN + LCS + TFI + payload + DCS + POSTAMBLE
-        let frame_len = 6 + header.len() + body.len() + 2;
-        let mut frame = [0u8; MAX_DATA_LEN];
-        if frame_len > frame.len() {
-            return Err(Error::NoSpace);
-        }
-
-        frame[0] = PN532_PREAMBLE;
-        frame[1] = PN532_START_CODE_1;
-        frame[2] = PN532_START_CODE_2;
-        frame[3] = data_len as u8;
-        frame[4] = (!(data_len as u8)).wrapping_add(1);
-        frame[5] = PN532_HOST_TO_PN532;
-
-        let mut sum = PN532_HOST_TO_PN532;
-        let mut idx = 6;
-        for &b in header.iter().chain(body.iter()) {
-            frame[idx] = b;
-            idx += 1;
-            sum = sum.wrapping_add(b);
-        }
-        frame[idx] = (!sum).wrapping_add(1);
-        frame[idx + 1] = PN532_POSTAMBLE;
-
-        crate::trace!("pn532: write cmd=0x{:02X} len={}", header[0], frame_len);
         self.i2c
-            .write(PN532_I2C_ADDRESS, &frame[..frame_len])
+            .write(PN532_I2C_ADDRESS, &frame)
             .map_err(Error::Transport)?;
         crate::trace!("pn532: write ACKed, reading ACK frame");
         self.read_ack_frame()
@@ -231,10 +125,10 @@ where
         // Read the full response frame in a single transaction, stripping the
         // RDY byte. Frame layout: PREAMBLE(3) + LEN + LCS + TFI + CMD + payload + DCS + POSTAMBLE.
         let frame_len = buf.len() + 9;
-        let mut frame = [0u8; MAX_DATA_LEN + 10];
+        if frame_len > MAX_DATA_LEN + 10 {
-        if frame_len > frame.len() {
             return Err(Error::NoSpace);
         }
+        let mut frame = [0u8; MAX_DATA_LEN + 10];
         self.i2c
             .transaction(
                 PN532_I2C_ADDRESS,
@@ -244,37 +138,7 @@ where
                 ],
             )
             .map_err(Error::Transport)?;
-
+        super::from_response_frame(self.command.unwrap(), &frame[..frame_len], buf)
-        if frame[0] != PN532_PREAMBLE
-            || frame[1] != PN532_START_CODE_1
-            || frame[2] != PN532_START_CODE_2
-        {
-            return Err(Error::InvalidFrame);
-        }
-        let length = frame[3] as usize;
-        if frame[3].wrapping_add(frame[4]) != 0 {
-            return Err(Error::InvalidFrame);
-        }
-
-        let cmd = self.command.unwrap().as_response();
-        if frame[5] != PN532_PN532_TO_HOST || frame[6] != cmd {
-            return Err(Error::InvalidFrame);
-        }
-
-        let data_len = length.saturating_sub(2);
-        if data_len > buf.len() {
-            return Err(Error::NoSpace);
-        }
-        let mut sum = frame[5].wrapping_add(frame[6]);
-        for i in 0..data_len {
-            buf[i] = frame[7 + i];
-            sum = sum.wrapping_add(frame[7 + i]);
-        }
-        if sum.wrapping_add(frame[7 + data_len]) != 0 {
-            return Err(Error::InvalidFrame);
-        }
-
-        Ok(data_len)
     }
 }
 
@@ -340,4 +204,4 @@ where
             Err(Error::InvalidAck)
         }
     }
-}
+}

pn532/src/interface/mod.rs

@@ -0,0 +1,145 @@
+//! Transport abstraction and the I2C implementation.
+//!
+//! The PN532 communicates over a framed protocol regardless of the physical
+//! transport (I2C, SPI or HSU). The [`Interface`] trait mirrors that boundary:
+//! the driver builds logical commands, the interface handles framing and the
+//! acknowledgement/response handshake.
+
+mod i2c;
+pub use i2c::I2cInterface;
+
+use embedded_hal::digital::{ErrorKind, ErrorType, InputPin, OutputPin};
+use crate::error::Error;
+use crate::PN532Command;
+use crate::types::constants::{MAX_DATA_LEN, PN532_HOST_TO_PN532, PN532_PN532_TO_HOST, PN532_POSTAMBLE, PN532_PREAMBLE, PN532_START_CODE_1, PN532_START_CODE_2};
+
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+pub struct UnreachError;
+
+impl embedded_hal::digital::Error for UnreachError {
+    fn kind(&self) -> ErrorKind {
+        ErrorKind::Other
+    }
+}
+
+/// Marker type for when no reset pin is connected.
+#[derive(Clone, Copy, Debug, Default)]
+pub struct NoReset;
+
+impl ErrorType for NoReset { type Error = UnreachError; }
+
+impl OutputPin for NoReset {
+    fn set_low(&mut self) -> Result<(), Self::Error> {
+        Err(UnreachError)
+    }
+
+    fn set_high(&mut self) -> Result<(), Self::Error> {
+        Err(UnreachError)
+    }
+}
+
+/// Marker type for when no IRQ pin is connected.
+#[derive(Clone, Copy, Debug, Default)]
+pub struct NoIrq;
+
+impl ErrorType for NoIrq { type Error = UnreachError; }
+
+impl InputPin for NoIrq {
+    fn is_high(&mut self) -> Result<bool, Self::Error> {
+        panic!("NoIrq should not be used");
+    }
+
+    fn is_low(&mut self) -> Result<bool, Self::Error> {
+        panic!("NoIrq should not be used");
+    }
+}
+
+/// A framed transport to the PN532.
+///
+/// Implementations are responsible for the frame/ack handshake described in
+/// the PN532 user manual (UM0701-02).
+pub trait Interface {
+    /// The error type produced by the underlying physical transport.
+    type TransportError;
+
+    /// Initialise the hardware: pulse the reset pin (if provided) and wait for
+    /// the PN532 to become ready.
+    fn begin(&mut self) -> Result<(), Error<Self::TransportError>>;
+
+    /// Write a command frame (`header` + optional `body`) and wait for the ACK.
+    fn write_command(
+        &mut self,
+        header: &[u8],
+        body: &[u8],
+    ) -> Result<(), Error<Self::TransportError>>;
+
+    /// Read a response frame, returning the length of the payload (excluding
+    /// the `TFI` and command bytes) placed in `buf`.
+    fn read_response(
+        &mut self,
+        buf: &mut [u8],
+        timeout_ms: u16,
+    ) -> Result<usize, Error<Self::TransportError>>;
+}
+
+pub(crate) fn to_request_frame<E>(header: &[u8], body: &[u8]) -> Result<heapless::Vec<u8, MAX_DATA_LEN>, Error<E>> {
+    let data_len = header.len() + body.len() + 1; // TFI + payload
+    if data_len > 0xFF {
+        return Err(Error::NoSpace);
+    }
+
+    // PREAMBLE + STARTCODE1 + STARTCODE2 + LEN + LCS + TFI + payload + DCS + POSTAMBLE
+    let frame_len = 6 + header.len() + body.len() + 2;
+    if frame_len > MAX_DATA_LEN {
+        return Err(Error::NoSpace);
+    }
+    let mut frame = heapless::Vec::new();
+    frame.push(PN532_PREAMBLE).unwrap();
+    frame.push(PN532_START_CODE_1).unwrap();
+    frame.push(PN532_START_CODE_2).unwrap();
+    frame.push(data_len as u8).unwrap();
+    frame.push((!(data_len as u8)).wrapping_add(1)).unwrap();
+    frame.push(PN532_HOST_TO_PN532).unwrap();
+
+    let mut sum = PN532_HOST_TO_PN532;
+    for &b in header.iter().chain(body.iter()) {
+        frame.push(b).unwrap();
+        sum = sum.wrapping_add(b);
+    }
+    frame.push((!sum).wrapping_add(1)).unwrap();
+    frame.push(PN532_POSTAMBLE).unwrap();
+    Ok(frame)
+}
+
+pub(crate) fn from_response_frame<E>(command: PN532Command, frame: &[u8], buf: &mut [u8]) -> Result<usize, Error<E>> {
+    if frame[0] != PN532_PREAMBLE
+        || frame[1] != PN532_START_CODE_1
+        || frame[2] != PN532_START_CODE_2
+    {
+        return Err(Error::InvalidFrame);
+    }
+    let length = frame[3] as usize;
+    if frame[3].wrapping_add(frame[4]) != 0 {
+        return Err(Error::InvalidFrame);
+    }
+
+    let cmd = command.as_response();
+    if frame[5] != PN532_PN532_TO_HOST || frame[6] != cmd {
+        return Err(Error::InvalidFrame);
+    }
+
+    let data_len = length.saturating_sub(2);
+    if data_len > buf.len() {
+        return Err(Error::NoSpace);
+    }
+    let mut sum = frame[5].wrapping_add(frame[6]);
+    for i in 0..data_len {
+        buf[i] = frame[7 + i];
+        sum = sum.wrapping_add(frame[7 + i]);
+    }
+    if sum.wrapping_add(frame[7 + data_len]) != 0 {
+        return Err(Error::InvalidFrame);
+    }
+
+    Ok(data_len)
+} 

pn532/src/types/frame.rs

@@ -1,12 +0,0 @@
-use crate::{Error, PN532Command};
-use crate::types::constants::*;
-
-pub struct DataFrame {
-    command: PN532Command
-}
-
-impl DataFrame {
-    pub fn new<E>(header: &[u8], body: &[u8]) -> Result<Self, Error<E>> {
-        Ok(Self { command: PN532Command::TgGetData } )
-    }
-}

pn532/src/types/mod.rs

@@ -3,7 +3,6 @@ use bitfields::bitflag;
 #[cfg(feature = "felica")]
 pub mod felica;
 pub mod constants;
-pub mod frame;
 
 /// Passive target baud rates supported by [`crate::Pn532::read_passive_target_id`].
 #[derive(PartialEq, Eq, Debug)]