hl: rework readers and writers to prevent misuse

hl/CHANGELOG.md

@@ -11,9 +11,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 - Change the return type of `Reader::done` from `Result<(), W5500::Error>` to `Result<&'a mut W5500, W5500::Error>`.
+- Split `Common::writer` into `Tcp::tcp_writer`, `Udp::udp_writer`, and `Udp::udp_writer_to`.
+
+#### Reader and Writer Changes
+
+Readers and writer functions previously returned a reader or writer, and it was up to the user to call `done` or `send` when finished. This was prone to misuse as the final `done` or `send` call was easily forgotten. Readers and writers now accept a closure which will automatically call `done` or `send` when finished.
+
+Code that previously looked like this:
+
+```rs
+let mut reader: UdpReader<_> = w5500.udp_reader(Sn0)?;
+let mut buf: [u8; 8] = [0; 8];
+reader.read_exact(&mut buf)?;
+reader.done()?;
+```
+
+Will now look like this:
+
+```rs
+let buf: [u8; 8] = w5500.udp_reader(Sn0, |reader| {
+ let mut buf: [u8; 8] = [0; 8];
+ reader.read_exact(&mut buf)?;
+ Ok(buf)
+})?;
+```
 
 ### Removed
-- Removed `non_exhaustive` from `Error`.
+- Removed `non_exhaustive` on `Error`.
 
 ## [0.8.0] - 2022-04-10
 ### Added

hl/src/lib.rs

@@ -90,8 +90,8 @@
 //!
 //! [`Registers`]: https://docs.rs/w5500-ll/latest/w5500_ll/trait.Registers.html
 //! [`std::net`]: https://doc.rust-lang.org/std/net/index.html
-//! [`Tcp`]: https://docs.rs/w5500-hl/0.7.1/w5500_hl/trait.Tcp.html
-//! [`Udp`]: https://docs.rs/w5500-hl/0.7.1/w5500_hl/trait.Udp.html
+//! [`Tcp`]: https://docs.rs/w5500-hl/0.9.0/w5500_hl/trait.Tcp.html
+//! [`Udp`]: https://docs.rs/w5500-hl/0.9.0/w5500_hl/trait.Udp.html
 //! [w5500-ll]: https://github.com/newAM/w5500-ll-rs
 //! [Wiznet W5500]: https://www.wiznet.io/product-item/w5500/
 #![cfg_attr(docsrs, feature(doc_cfg))]
@@ -306,55 +306,30 @@ pub trait Read<'a, W5500: Registers> {
  /// * [`Error::UnexpectedEof`]
  fn read_exact(&mut self, buf: &mut [u8]) -> Result<(), Error<W5500::Error>>;
 
- /// Mark the data as read, removing the data from the queue.
- ///
- /// For a TCP reader this removes all data up to the current pointer
- /// position from the queue.
- ///
- /// For a UDP reader this removes the UDP datagram from the queue.
- ///
- /// To complete a read without marking the data as read simply use
- /// [`mem::drop`](https://doc.rust-lang.org/core/mem/fn.drop.html) on the
- /// reader.
- fn done(self) -> Result<&'a mut W5500, W5500::Error>;
+ /// Make the data as unread, returning the data to the queue.
+ fn unread(&mut self);
+
+ /// Returns `true` if [`unread`] was called.
+ fn is_unread(&self) -> bool;
+
+ // TODO: re-home this UDP/TCP specific doc parts
+ // Mark the data as read, removing the data from the queue.
+ //
+ // For a TCP reader this removes all data up to the current pointer
+ // position from the queue.
+ //
+ // For a UDP reader this removes the UDP datagram from the queue.
+ //
+ // To complete a read without marking the data as read simply use
+ // [`mem::drop`](https://doc.rust-lang.org/core/mem/fn.drop.html) on the
+ // reader.
 }
 
 /// Streaming writer for a TCP or UDP socket buffer.
 ///
 /// This implements the [`Seek`] traits.
 ///
-/// Created with [`Common::writer`].
-///
-/// # Example
-///
-/// ```no_run
-/// # use embedded_hal_mock as h;
-/// # let mut w5500 = w5500_ll::blocking::vdm::W5500::new(h::spi::Mock::new(&[]), h::pin::Mock::new(&[]));
-/// use w5500_hl::{
-/// ll::{Registers, Sn::Sn0},
-/// net::{Ipv4Addr, SocketAddrV4},
-/// Udp,
-/// Common,
-/// Writer,
-/// };
-///
-/// const DEST: SocketAddrV4 = SocketAddrV4::new(Ipv4Addr::new(192, 0, 2, 1), 8081);
-///
-/// w5500.udp_bind(Sn0, 8080)?;
-///
-/// let mut udp_writer: Writer<_> = w5500.writer(Sn0)?;
-///
-/// let data_header: [u8; 10] = [0; 10];
-/// let n_written: u16 = udp_writer.write(&data_header)?;
-/// assert_eq!(usize::from(n_written), data_header.len());
-///
-/// let data: [u8; 123] = [0; 123];
-/// let n_written: u16 = udp_writer.write(&data)?;
-/// assert_eq!(usize::from(n_written), data.len());
-///
-/// udp_writer.udp_send_to(&DEST)?;
-/// # Ok::<(), w5500_hl::ll::blocking::vdm::Error<_, _>>(())
-/// ```
+/// Created with [`Udp::udp_writer`], [`Udp::udp_writer_to`] or [`Tcp::tcp_writer`].
 #[derive(Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct Writer<'a, W: Registers> {
@@ -363,6 +338,7 @@ pub struct Writer<'a, W: Registers> {
  head_ptr: u16,
  tail_ptr: u16,
  ptr: u16,
+ abort: bool,
 }
 
 impl<'a, W: Registers> Seek<W::Error> for Writer<'a, W> {
@@ -424,30 +400,10 @@ impl<'a, W: Registers> Writer<'a, W> {
  }
  }
 
- /// Send all data previously written with [`write`] and [`write_all`].
- ///
- /// For UDP sockets the destination is set by the last call to
- /// [`Registers::set_sn_dest`], [`Udp::udp_send_to`], or
- /// [`Writer::udp_send_to`].
- ///
- /// [`write`]: Writer::write
- /// [`write_all`]: Writer::write_all
- pub fn send(self) -> Result<(), W::Error> {
- self.w5500.set_sn_tx_wr(self.sn, self.ptr)?;
- self.w5500.set_sn_cr(self.sn, SocketCommand::Send)
- }
-
- /// Send all data previously written with [`Writer::write`] and
- /// [`Writer::write_all`] to the given address.
- ///
- /// # Panics
- ///
- /// * (debug) The socket must be opened as a UDP socket.
- pub fn udp_send_to(self, addr: &SocketAddrV4) -> Result<(), W::Error> {
- debug_assert_eq!(self.w5500.sn_sr(self.sn)?, Ok(SocketStatus::Udp));
-
- self.w5500.set_sn_dest(self.sn, addr)?;
- self.send()
+ /// Ignore all written data, and exit without sending anything.
+ #[inline]
+ pub fn abort(&mut self) {
+ self.abort = true
  }
 }
 
@@ -607,64 +563,6 @@ pub trait Common: Registers {
  fn is_state_udp(&mut self, sn: Sn) -> Result<bool, Self::Error> {
  Ok(self.sn_sr(sn)? == Ok(SocketStatus::Udp))
  }
-
- /// Create a socket writer.
- ///
- /// This returns a [`Writer`] structure, which contains functions to
- /// stream data into the W5500 socket buffers incrementally.
- ///
- /// This is useful for writing large packets that are too large to stage
- /// in the memory of your microcontroller.
- ///
- /// The socket should be opened as a TCP or UDP socket before calling this
- /// method.
- ///
- /// # Example
- ///
- /// ```no_run
- /// # use embedded_hal_mock as h;
- /// # let mut w5500 = w5500_ll::blocking::vdm::W5500::new(h::spi::Mock::new(&[]), h::pin::Mock::new(&[]));
- /// use w5500_hl::{
- /// ll::{Registers, Sn::Sn0},
- /// net::{Ipv4Addr, SocketAddrV4},
- /// Udp,
- /// Writer,
- /// Common,
- /// };
- ///
- /// const DEST: SocketAddrV4 = SocketAddrV4::new(Ipv4Addr::new(192, 0, 2, 1), 8081);
- ///
- /// w5500.udp_bind(Sn0, 8080)?;
- ///
- /// let mut writer: Writer<_> = w5500.writer(Sn0)?;
- ///
- /// // write part of a packet
- /// let buf: [u8; 8] = [0; 8];
- /// writer.write_all(&buf)?;
- ///
- /// // write another part
- /// let other_buf: [u8; 16] = [0; 16];
- /// writer.write_all(&buf)?;
- ///
- /// // send all previously written data to the destination
- /// writer.udp_send_to(&DEST)?;
- /// # Ok::<(), w5500_hl::Error<_>>(())
- /// ```
- fn writer(&mut self, sn: Sn) -> Result<Writer<Self>, Self::Error>
- where
- Self: Sized,
- {
- let sn_tx_fsr: u16 = self.sn_tx_fsr(sn)?;
- let sn_tx_wr: u16 = self.sn_tx_wr(sn)?;
-
- Ok(Writer {
- w5500: self,
- sn,
- head_ptr: sn_tx_wr,
- tail_ptr: sn_tx_wr.wrapping_add(sn_tx_fsr),
- ptr: sn_tx_wr,
- })
- }
 }
 
 /// Implement the common socket trait for any structure that implements [`w5500_ll::Registers`].

hl/src/tcp.rs

@@ -1,4 +1,4 @@
-use crate::{port_is_unique, Error, Read, Seek, SeekFrom};
+use crate::{port_is_unique, Error, Read, Seek, SeekFrom, Writer};
 use core::cmp::min;
 use w5500_ll::{
  net::SocketAddrV4, Protocol, Registers, Sn, SocketCommand, SocketMode, SocketStatus,
@@ -39,19 +39,19 @@ use w5500_ll::{
 ///
 /// // ... wait for a RECV interrupt
 ///
-/// let mut reader: TcpReader<_> = w5500.tcp_reader(MQTT_SOCKET)?;
-/// let mut buf = [0; 2];
+/// let mut buf: [u8; 2] = w5500.tcp_reader(MQTT_SOCKET, |reader| {
+///  let mut buf: [u8; 2] = [0; 2];
 ///
-/// // read the first two bytes
-/// reader.read_exact(&mut buf)?;
-/// // ... do something with the data
+///  // read the first two bytes
+///  reader.read_exact(&mut buf)?;
+///  // ... do something with the data
 ///
-/// // read another two bytes into the same buffer
-/// reader.read_exact(&mut buf)?;
-/// // ... do something with the data
+/// // read another two bytes into the same buffer
+/// reader.read_exact(&mut buf)?;
 ///
-/// // mark the data as read
-/// reader.done()?;
+/// // return the data to the outer function
+/// Ok(buf)
+/// })?;
 /// # Ok::<(), w5500_hl::Error<_>>(())
 /// ```
 #[derive(Debug, PartialEq, Eq)]
@@ -62,6 +62,7 @@ pub struct TcpReader<'a, W: Registers> {
  pub(crate) head_ptr: u16,
  pub(crate) tail_ptr: u16,
  pub(crate) ptr: u16,
+ pub(crate) unread: bool,
 }
 
 impl<'a, W: Registers> Seek<W::Error> for TcpReader<'a, W> {
@@ -113,10 +114,14 @@ impl<'a, W: Registers> Read<'a, W> for TcpReader<'a, W> {
  }
  }
 
- fn done(self) -> Result<&'a mut W, W::Error> {
- self.w5500.set_sn_rx_rd(self.sn, self.ptr)?;
- self.w5500.set_sn_cr(self.sn, SocketCommand::Recv)?;
- Ok(self.w5500)
+ #[inline]
+ fn unread(&mut self) {
+ self.unread = true
+ }
+
+ #[inline]
+ fn is_unread(&self) -> bool {
+ self.unread
  }
 }
 
@@ -497,9 +502,10 @@ pub trait Tcp: Registers {
  /// # Example
  ///
  /// See [`TcpReader`].
- fn tcp_reader(&mut self, sn: Sn) -> Result<TcpReader<Self>, Error<Self::Error>>
+ fn tcp_reader<F, T>(&mut self, sn: Sn, mut f: F) -> Result<T, Error<Self::Error>>
  where
  Self: Sized,
+ F: FnMut(&mut TcpReader<Self>) -> Result<T, Error<Self::Error>>,
  {
  debug_assert!(!matches!(
  self.sn_sr(sn)?,
@@ -513,13 +519,90 @@ pub trait Tcp: Registers {
 
  let sn_rx_rd: u16 = self.sn_rx_rd(sn)?;
 
- Ok(TcpReader {
+ let mut reader = TcpReader {
  w5500: self,
  sn,
  head_ptr: sn_rx_rd,
  tail_ptr: sn_rx_rd.wrapping_add(sn_rx_rsr),
  ptr: sn_rx_rd,
- })
+ unread: false,
+ };
+
+ let ret = f(&mut reader)?;
+
+ if !reader.is_unread() {
+ reader.w5500.set_sn_rx_rd(sn, reader.ptr)?;
+ reader.w5500.set_sn_cr(sn, SocketCommand::Recv)?;
+ }
+
+ Ok(ret)
+ }
+
+ /// Create a TCP writer.
+ ///
+ /// This returns a [`Writer`] structure, which contains functions to
+ /// stream data into the W5500 socket buffers incrementally.
+ ///
+ /// This is useful for writing large packets that are too large to stage
+ /// in the memory of your microcontroller.
+ ///
+ /// The socket should be opened as a TCP socket before calling this
+ /// method.
+ ///
+ /// # Example
+ ///
+ /// ```no_run
+ /// # use embedded_hal_mock as h;
+ /// # let mut w5500 = w5500_ll::blocking::vdm::W5500::new(h::spi::Mock::new(&[]), h::pin::Mock::new(&[]));
+ /// use w5500_hl::{
+ /// ll::{Registers, Sn, SocketInterrupt},
+ /// net::{Ipv4Addr, SocketAddrV4},
+ /// Tcp,
+ /// };
+ ///
+ /// const MQTT_SOCKET: Sn = Sn::Sn0;
+ /// const MQTT_SOURCE_PORT: u16 = 33650;
+ /// const MQTT_SERVER: SocketAddrV4 = SocketAddrV4::new(Ipv4Addr::new(192, 168, 2, 10), 1883);
+ ///
+ /// w5500.tcp_connect(MQTT_SOCKET, MQTT_SOURCE_PORT, &MQTT_SERVER)?;
+ ///
+ /// // ... wait for a CON interrupt
+ ///
+ /// const CONNECT: [u8; 14] = [
+ /// 0x10, 0x0C, 0x00, 0x04, b'M', b'Q', b'T', b'T', 0x04, 0x02, 0x0E, 0x10, 0x00, 0x00,
+ /// ];
+ /// w5500.tcp_writer(MQTT_SOCKET, |writer| writer.write_all(&CONNECT))?;
+ /// # Ok::<(), w5500_hl::Error<_>>(())
+ /// ```
+ fn tcp_writer<F, T>(&mut self, sn: Sn, mut f: F) -> Result<T, Error<Self::Error>>
+ where
+ Self: Sized,
+ F: FnMut(&mut Writer<Self>) -> Result<T, Error<Self::Error>>,
+ {
+ debug_assert!(!matches!(
+ self.sn_sr(sn)?,
+ Ok(SocketStatus::Udp) | Ok(SocketStatus::Init) | Ok(SocketStatus::Macraw)
+ ));
+
+ let sn_tx_fsr: u16 = self.sn_tx_fsr(sn)?;
+ let sn_tx_wr: u16 = self.sn_tx_wr(sn)?;
+
+ let mut writer = Writer {
+ w5500: self,
+ sn,
+ head_ptr: sn_tx_wr,
+ tail_ptr: sn_tx_wr.wrapping_add(sn_tx_fsr),
+ ptr: sn_tx_wr,
+ abort: false,
+ };
+ let ret = f(&mut writer)?;
+
+ if !writer.abort {
+ writer.w5500.set_sn_tx_wr(sn, writer.ptr)?;
+ writer.w5500.set_sn_cr(sn, SocketCommand::Send)?;
+ }
+
+ Ok(ret)
  }
 }