Merge branch 'master' into feat/retry-time-and-count

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add `defmt` features for enabling `defmt::Format` to most structs and errors by [@elpiel](https://github.com/elpiel) ([#39](https://github.com/kellerkindt/w5500/issues/39))
 - Fixed an issue where internal function names were conflicting with trait names by [@ryan-summers](https://github.com/ryan-summers) ([#36](https://github.com/kellerkindt/w5500/issues/36))
 
+### Fixed
+
+- Fixed an issue that caused corruption when reading partial MACRAW frames by [@Felix-El](https://github.com/Felix-El) ([#47](https://github.com/kellerkindt/w5500/pull/47))
+
 ## [0.4.1] - January 2nd, 2023
 
 ### Added

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "w5500"
-version = "0.4.0"
+version = "0.4.1"
 authors = ["Michael Watzko <michael@watzko.de>", "Jonah Dahlquist <hi@jonah.name>", "Ryan Summers <ryan.summers@vertigo-designs.com"]
 repository = "https://github.com/kellerkindt/w5500.git"
 description = "W5500 IoT Controller implementation."

src/cursor.rs

@@ -0,0 +1,126 @@
+use crate::bus::Bus;
+use crate::register::socketn::Command;
+use crate::socket::Socket;
+
+pub(crate) struct RxCursor<'a, SpiBus>
+where
+    SpiBus: Bus,
+{
+    sock: &'a Socket,
+    bus: &'a mut SpiBus,
+    ptr: u16,
+    size: u16,
+}
+
+impl<'a, SpiBus> RxCursor<'a, SpiBus>
+where
+    SpiBus: Bus,
+{
+    pub fn new(sock: &'a Socket, bus: &'a mut SpiBus) -> Result<Self, SpiBus::Error> {
+        let size = sock.get_receive_size(bus)?;
+        let ptr = sock.get_rx_read_pointer(bus)?;
+        Ok(Self {
+            sock,
+            bus,
+            ptr,
+            size,
+        })
+    }
+
+    #[inline]
+    pub fn available(&self) -> u16 {
+        self.size
+    }
+
+    /// Read up to `buf.len()` bytes. The actual number of bytes read is bounded by `available()`.
+    pub fn read(&mut self, buf: &mut [u8]) -> Result<u16, SpiBus::Error> {
+        if buf.is_empty() {
+            return Ok(0);
+        }
+
+        let count = self.available().min(buf.len() as u16);
+        self.bus
+            .read_frame(self.sock.rx_buffer(), self.ptr, &mut buf[..count as _])?;
+        Ok(self.skip(count))
+    }
+
+    /// Read up to `max` bytes. The actual number of bytes read is bounded by buf.len() and available().
+    pub fn read_upto(&mut self, buf: &mut [u8], max: u16) -> Result<u16, SpiBus::Error> {
+        let bounded_buf = if buf.len() > max as usize {
+            &mut buf[..max as _]
+        } else {
+            buf
+        };
+        self.read(bounded_buf)
+    }
+
+    /// Skip up to count bytes. The actual number of bytes skipped is bounded by available().
+    pub fn skip(&mut self, count: u16) -> u16 {
+        let bounded_count = self.available().min(count);
+        self.ptr = self.ptr.wrapping_add(bounded_count);
+        self.size -= bounded_count;
+        bounded_count
+    }
+
+    /// Return ownership of the portion of the RX buffer that has already been read back to the
+    /// chip and issue the next receive command.
+    pub fn commit(mut self) -> Result<(), SpiBus::Error> {
+        self.sock.set_rx_read_pointer(self.bus, self.ptr)?;
+        self.sock.command(self.bus, Command::Receive)?;
+        Ok(())
+    }
+}
+
+pub(crate) struct TxCursor<'a, SpiBus>
+where
+    SpiBus: Bus,
+{
+    sock: &'a Socket,
+    bus: &'a mut SpiBus,
+    ptr: u16,
+    size: u16,
+}
+
+impl<'a, SpiBus> TxCursor<'a, SpiBus>
+where
+    SpiBus: Bus,
+{
+    pub fn new(sock: &'a Socket, bus: &'a mut SpiBus) -> Result<Self, SpiBus::Error> {
+        let size = sock.get_tx_free_size(bus)?;
+        let ptr = sock.get_tx_write_pointer(bus)?;
+        Ok(Self {
+            sock,
+            bus,
+            ptr,
+            size,
+        })
+    }
+
+    #[inline]
+    pub fn available(&self) -> u16 {
+        self.size
+    }
+
+    /// Write all bytes in buf to the current TX buffer position and update the cursor position
+    /// and remaining size on success.
+    pub fn write(&mut self, buf: &[u8]) -> Result<u16, SpiBus::Error> {
+        if buf.is_empty() || buf.len() > self.available() as _ {
+            return Ok(0);
+        }
+
+        let count = buf.len() as u16;
+        self.bus
+            .write_frame(self.sock.tx_buffer(), self.ptr, &buf[..count as _])?;
+        self.ptr = self.ptr.wrapping_add(count);
+        self.size -= count;
+        Ok(count)
+    }
+
+    /// Pass ownership of the portion of the TX buffer that has already been written back to the
+    /// chip and issue the next send command.
+    pub fn commit(mut self) -> Result<(), SpiBus::Error> {
+        self.sock.set_tx_write_pointer(self.bus, self.ptr)?;
+        self.sock.command(self.bus, Command::Send)?;
+        Ok(())
+    }
+}

src/lib.rs

@@ -4,6 +4,7 @@
 #![doc = include_str!("../README.md")]
 
 pub mod bus;
+mod cursor;
 mod device;
 mod host;
 pub mod net;

src/raw_device.rs

@@ -39,38 +39,6 @@ impl<SpiBus: Bus> RawDevice<SpiBus> {
         Ok(Self { bus, raw_socket })
     }
 
-    // Read bytes from the RX buffer.
-    //
-    // # Args
-    // * `buffer` - The location to read data into. The length of this slice determines how much
-    // data is read.
-    // * `offset` - The offset into current RX data to start reading from in bytes.
-    //
-    // # Returns
-    // The number of bytes successfully read.
-    fn read_bytes(&mut self, buffer: &mut [u8], offset: u16) -> Result<usize, SpiBus::Error> {
-        let rx_size = self.raw_socket.get_receive_size(&mut self.bus)? as usize;
-
-        let read_buffer = if rx_size > buffer.len() + offset as usize {
-            buffer
-        } else {
-            &mut buffer[..rx_size - offset as usize]
-        };
-
-        let read_pointer = self
-            .raw_socket
-            .get_rx_read_pointer(&mut self.bus)?
-            .wrapping_add(offset);
-        self.bus
-            .read_frame(self.raw_socket.rx_buffer(), read_pointer, read_buffer)?;
-        self.raw_socket.set_rx_read_pointer(
-            &mut self.bus,
-            read_pointer.wrapping_add(read_buffer.len() as u16),
-        )?;
-
-        Ok(read_buffer.len())
-    }
-
     /// Read an ethernet frame from the device.
     ///
     /// # Args
@@ -79,40 +47,29 @@ impl<SpiBus: Bus> RawDevice<SpiBus> {
     /// # Returns
     /// The number of bytes read into the provided frame buffer.
     pub fn read_frame(&mut self, frame: &mut [u8]) -> Result<usize, SpiBus::Error> {
+        let mut rx_cursor = crate::cursor::RxCursor::new(&self.raw_socket, &mut self.bus)?;
+
         // Check if there is anything to receive.
-        let rx_size = self.raw_socket.get_receive_size(&mut self.bus)? as usize;
-        if rx_size == 0 {
+        if rx_cursor.available() == 0 {
             return Ok(0);
         }
 
         // The W5500 specifies the size of the received ethernet frame in the first two bytes.
         // Refer to https://forum.wiznet.io/t/topic/979/2 for more information.
-        let expected_frame_size: usize = {
+        let expected_frame_size = {
             let mut frame_bytes = [0u8; 2];
-            assert!(self.read_bytes(&mut frame_bytes[..], 0)? == 2);
+            assert!(rx_cursor.read(&mut frame_bytes[..])? == 2);
 
-            u16::from_be_bytes(frame_bytes) as usize - 2
+            u16::from_be_bytes(frame_bytes).saturating_sub(2)
         };
 
-        // Read the ethernet frame
-        let read_buffer = if frame.len() > expected_frame_size {
-            &mut frame[..expected_frame_size]
-        } else {
-            frame
-        };
-
-        let received_frame_size = self.read_bytes(read_buffer, 2)?;
-
-        // Register the reception as complete.
-        self.raw_socket
-            .command(&mut self.bus, register::socketn::Command::Receive)?;
-
-        // If we couldn't read the whole frame, drop it instead.
+        let received_frame_size = rx_cursor.read_upto(frame, expected_frame_size)?;
         if received_frame_size < expected_frame_size {
-            Ok(0)
-        } else {
-            Ok(received_frame_size)
+            rx_cursor.skip(expected_frame_size - received_frame_size);
         }
+
+        rx_cursor.commit()?;
+        Ok(received_frame_size as _)
     }
 
     /// Write an ethernet frame to the device.
@@ -123,37 +80,20 @@ impl<SpiBus: Bus> RawDevice<SpiBus> {
     /// # Returns
     /// The number of bytes successfully transmitted from the provided buffer.
     pub fn write_frame(&mut self, frame: &[u8]) -> Result<usize, SpiBus::Error> {
-        let max_size = self.raw_socket.get_tx_free_size(&mut self.bus)? as usize;
-
-        let write_data = if frame.len() < max_size {
-            frame
-        } else {
-            &frame[..max_size]
-        };
-
-        // Append the data to the write buffer after the current write pointer.
-        let write_pointer = self.raw_socket.get_tx_write_pointer(&mut self.bus)?;
-
-        // Write data into the buffer and update the writer pointer.
-        self.bus
-            .write_frame(self.raw_socket.tx_buffer(), write_pointer, write_data)?;
-        self.raw_socket.set_tx_write_pointer(
-            &mut self.bus,
-            write_pointer.wrapping_add(write_data.len() as u16),
-        )?;
-
-        // Wait for the socket transmission to complete.
+        // Reset the transmission complete flag, we'll wait on it later.
         self.raw_socket
             .reset_interrupt(&mut self.bus, register::socketn::Interrupt::SendOk)?;
 
-        self.raw_socket
-            .command(&mut self.bus, register::socketn::Command::Send)?;
+        let mut tx_cursor = crate::cursor::TxCursor::new(&self.raw_socket, &mut self.bus)?;
+        let count = tx_cursor.write(frame)?;
+        tx_cursor.commit()?;
 
+        // Wait for the socket transmission to complete.
         while !self
             .raw_socket
             .has_interrupt(&mut self.bus, register::socketn::Interrupt::SendOk)?
         {}
 
-        Ok(write_data.len())
+        Ok(count as _)
     }
 }