From b9ba375ca2391b00cbfb96d4b4336a0c977fe15d Mon Sep 17 00:00:00 2001 From: Ryan Summers Date: Mon, 1 Jul 2024 17:54:13 +0200 Subject: [PATCH] Refactoring bus traits for embedded-hal 1.0 --- Cargo.toml | 4 +- src/bus/four_wire.rs | 101 ++++++++--------------------- src/bus/four_wire_ref.rs | 85 ------------------------ src/bus/mod.rs | 21 ------ src/bus/three_wire.rs | 25 ++++---- src/device.rs | 125 ++---------------------------------- src/lib.rs | 2 +- src/tcp.rs | 51 +-------------- src/udp.rs | 65 +------------------ src/uninitialized_device.rs | 17 +---- 10 files changed, 50 insertions(+), 446 deletions(-) delete mode 100644 src/bus/four_wire_ref.rs diff --git a/Cargo.toml b/Cargo.toml index 6652d97..9ee2dfd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ no-chip-version-assertion = [] [dependencies] byteorder = { version = "1.3.4", default-features = false } -embedded-hal = "0.2" +embedded-hal = "1" embedded-nal = "0.6.0" bit_field = "0.10" derive-try-from-primitive = "1" @@ -23,4 +23,4 @@ nb = "1.0.0" defmt = { version = "0.3", optional = true } [dev-dependencies] -embedded-hal-mock = "0.9" \ No newline at end of file +embedded-hal-mock = "0.9" diff --git a/src/bus/four_wire.rs b/src/bus/four_wire.rs index 4640191..d010856 100644 --- a/src/bus/four_wire.rs +++ b/src/bus/four_wire.rs @@ -1,8 +1,7 @@ #![allow(clippy::inconsistent_digit_grouping, clippy::unusual_byte_groupings)] use core::fmt; -use embedded_hal::blocking::spi::{Transfer, Write}; -use embedded_hal::digital::v2::OutputPin; +use embedded_hal::spi::{ErrorType, Operation, SpiDevice}; use crate::bus::Bus; @@ -11,100 +10,50 @@ const WRITE_MODE_MASK: u8 = 0b00000_1_00; // TODO This name is not ideal, should be renamed to VDM #[derive(Debug)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct FourWire + Write, ChipSelect: OutputPin> { - cs: ChipSelect, - spi: Spi, +pub struct FourWire { + spi: SPI, } -impl + Write, ChipSelect: OutputPin> FourWire { - pub fn new(spi: Spi, cs: ChipSelect) -> Self { - Self { cs, spi } +impl FourWire { + pub fn new(spi: SPI) -> Self { + Self { spi } } - pub fn release(self) -> (Spi, ChipSelect) { - (self.spi, self.cs) + pub fn release(self) -> SPI { + self.spi } } -impl + Write, ChipSelect: OutputPin> Bus for FourWire { - type Error = - FourWireError<>::Error, >::Error, ChipSelect::Error>; +impl Bus for FourWire { + type Error = ::Error; - fn read_frame(&mut self, block: u8, address: u16, data: &mut [u8]) -> Result<(), Self::Error> { + fn read_frame(&mut self, block: u8, address: u16, data: &mut [u8]) -> Result<(), SPI::Error> { let address_phase = address.to_be_bytes(); let control_phase = block << 3; - let data_phase = data; - // set Chip select to Low, i.e. prepare to receive data - self.cs.set_low().map_err(FourWireError::ChipSelectError)?; - let result = (|| { - self.spi - .write(&address_phase) - .and_then(|_| self.spi.write(&[control_phase])) - .map_err(FourWireError::WriteError)?; - self.spi - .transfer(data_phase) - .map_err(FourWireError::TransferError)?; - Ok(()) - })(); + self.spi.transaction(&mut [ + Operation::Write(&address_phase), + Operation::Write(&[control_phase]), + Operation::Write(data), + ])?; - // set Chip select to High, i.e. we've finished listening - self.cs.set_high().map_err(FourWireError::ChipSelectError)?; - - // then return the result of the transmission - result + Ok(()) } - fn write_frame(&mut self, block: u8, address: u16, data: &[u8]) -> Result<(), Self::Error> { - let address_phase = address.to_be_bytes(); + fn write_frame(&mut self, block: u8, address: u16, data: &[u8]) -> Result<(), SPI::Error> { let control_phase = block << 3 | WRITE_MODE_MASK; - let data_phase = data; - // set Chip select to Low, i.e. prepare to transmit - self.cs.set_low().map_err(FourWireError::ChipSelectError)?; - let result = self - .spi - .write(&address_phase) - .and_then(|_| self.spi.write(&[control_phase])) - .and_then(|_| self.spi.write(data_phase)) - .map_err(FourWireError::WriteError); + let address_phase = address.to_be_bytes(); + self.spi.transaction(&mut [ + Operation::Write(&address_phase), + Operation::Write(&[control_phase]), + Operation::Write(data), + ])?; - // set Chip select to High, i.e. we've finished transmitting - self.cs.set_high().map_err(FourWireError::ChipSelectError)?; - - // then return the result of the transmission - result + Ok(()) } } -// Must use map_err, ambiguity prevents From from being implemented -#[repr(u8)] -#[derive(Clone)] -pub enum FourWireError { - TransferError(TransferError), - WriteError(WriteError), - ChipSelectError(ChipSelectError), -} - -impl fmt::Debug - for FourWireError -{ - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "FourWireError::{}", - match self { - Self::TransferError(_) => "TransferError", - Self::WriteError(_) => "WriteError", - Self::ChipSelectError(_) => "ChipSelectError", - } - ) - } -} - -// TODO Improved error rendering could be done with specialization. -// https://github.com/rust-lang/rust/issues/31844 - #[cfg(test)] mod test { use embedded_hal::digital::v2::OutputPin; diff --git a/src/bus/four_wire_ref.rs b/src/bus/four_wire_ref.rs deleted file mode 100644 index 0a653a5..0000000 --- a/src/bus/four_wire_ref.rs +++ /dev/null @@ -1,85 +0,0 @@ -#![allow(clippy::inconsistent_digit_grouping, clippy::unusual_byte_groupings)] - -use core::fmt; -use embedded_hal::blocking::spi::{Transfer, Write}; -use embedded_hal::digital::v2::OutputPin; - -use crate::bus::{Bus, FourWire, FourWireError}; - -// TODO This name is not ideal, should be renamed to VDM -/// This is just like [crate::bus::FourWire] but takes references instead of ownership -/// for the SPI bus and the ChipSelect pin -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct FourWireRef<'a, Spi: Transfer + Write, ChipSelect: OutputPin>( - FourWire, OutputPinRef<'a, ChipSelect>>, -); - -impl<'a, Spi: Transfer + Write, ChipSelect: OutputPin> FourWireRef<'a, Spi, ChipSelect> { - pub fn new(spi: &'a mut Spi, cs: &'a mut ChipSelect) -> Self { - Self(FourWire::new(SpiRef(spi), OutputPinRef(cs))) - } - - // this is actually a bit silly, but maybe someday someone finds this useful - pub fn release(self) -> (&'a mut Spi, &'a mut ChipSelect) { - let (spi_ref, cs_ref) = self.0.release(); - (spi_ref.0, cs_ref.0) - } -} - -impl + Write, ChipSelect: OutputPin> Bus - for FourWireRef<'_, Spi, ChipSelect> -{ - type Error = - FourWireError<>::Error, >::Error, ChipSelect::Error>; - - #[inline] - fn read_frame(&mut self, block: u8, address: u16, data: &mut [u8]) -> Result<(), Self::Error> { - self.0.read_frame(block, address, data) - } - - #[inline] - fn write_frame(&mut self, block: u8, address: u16, data: &[u8]) -> Result<(), Self::Error> { - self.0.write_frame(block, address, data) - } -} - -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct SpiRef<'a, Spi: Transfer + Write>(pub &'a mut Spi); - -impl<'a, Spi: Transfer + Write> Transfer for SpiRef<'a, Spi> { - type Error = >::Error; - - #[inline] - fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Self::Error> { - self.0.transfer(words) - } -} - -impl<'a, Spi: Transfer + Write> Write for SpiRef<'a, Spi> { - type Error = >::Error; - - #[inline] - fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { - self.0.write(words) - } -} - -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct OutputPinRef<'a, P: OutputPin>(pub &'a mut P); - -impl<'a, P: OutputPin> OutputPin for OutputPinRef<'a, P> { - type Error = P::Error; - - #[inline] - fn set_low(&mut self) -> Result<(), Self::Error> { - self.0.set_low() - } - - #[inline] - fn set_high(&mut self) -> Result<(), Self::Error> { - self.0.set_high() - } -} diff --git a/src/bus/mod.rs b/src/bus/mod.rs index f7b7f17..fe446eb 100644 --- a/src/bus/mod.rs +++ b/src/bus/mod.rs @@ -1,14 +1,9 @@ use core::fmt::Debug; mod four_wire; -mod four_wire_ref; mod three_wire; pub use self::four_wire::FourWire; -pub use self::four_wire::FourWireError; -pub use self::four_wire_ref::FourWireRef; -pub use self::four_wire_ref::OutputPinRef; -pub use self::four_wire_ref::SpiRef; pub use self::three_wire::ThreeWire; pub use self::three_wire::ThreeWireError; @@ -19,19 +14,3 @@ pub trait Bus { fn write_frame(&mut self, block: u8, address: u16, data: &[u8]) -> Result<(), Self::Error>; } - -pub struct BusRef<'a, B: Bus>(pub &'a mut B); - -impl Bus for BusRef<'_, B> { - type Error = B::Error; - - #[inline] - fn read_frame(&mut self, block: u8, address: u16, data: &mut [u8]) -> Result<(), Self::Error> { - self.0.read_frame(block, address, data) - } - - #[inline] - fn write_frame(&mut self, block: u8, address: u16, data: &[u8]) -> Result<(), Self::Error> { - self.0.write_frame(block, address, data) - } -} diff --git a/src/bus/three_wire.rs b/src/bus/three_wire.rs index 1a21f82..111c6fc 100644 --- a/src/bus/three_wire.rs +++ b/src/bus/three_wire.rs @@ -1,7 +1,7 @@ #![allow(clippy::inconsistent_digit_grouping, clippy::unusual_byte_groupings)] use core::fmt; -use embedded_hal::blocking::spi::{Transfer, Write}; +use embedded_hal::spi::{ErrorType, Operation, SpiBus}; use crate::bus::Bus; @@ -14,22 +14,22 @@ const FIXED_DATA_LENGTH_MODE_4: u8 = 0b000000_11; // TODO This name is not ideal, should be renamed to FDM #[derive(Debug)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct ThreeWire + Write> { - spi: Spi, +pub struct ThreeWire { + spi: SPI, } -impl + Write> ThreeWire { - pub fn new(spi: Spi) -> Self { +impl ThreeWire { + pub fn new(spi: SPI) -> Self { Self { spi } } - pub fn release(self) -> Spi { + pub fn release(self) -> SPI { self.spi } } -impl + Write> Bus for ThreeWire { - type Error = ThreeWireError<>::Error, >::Error>; +impl Bus for ThreeWire { + type Error = ::Error; /// Transfers a frame with an arbitrary data length in FDM /// @@ -67,11 +67,9 @@ impl + Write> Bus for ThreeWire { let address_phase = address.to_be_bytes(); self.spi .write(&address_phase) - .and_then(|_| self.spi.write(&[control_phase])) - .map_err(ThreeWireError::WriteError)?; + .and_then(|_| self.spi.write(&[control_phase]))?; self.spi - .transfer(&mut data_phase[..last_length_written as usize]) - .map_err(ThreeWireError::TransferError)?; + .transfer_in_place(&mut data_phase[..last_length_written as usize])?; address += last_length_written; data_phase = &mut data_phase[last_length_written as usize..]; @@ -100,8 +98,7 @@ impl + Write> Bus for ThreeWire { self.spi .write(&address_phase) .and_then(|_| self.spi.write(&[control_phase])) - .and_then(|_| self.spi.write(&data_phase[..last_length_written as usize])) - .map_err(ThreeWireError::WriteError)?; + .and_then(|_| self.spi.write(&data_phase[..last_length_written as usize]))?; address += last_length_written; data_phase = &data_phase[last_length_written as usize..]; diff --git a/src/device.rs b/src/device.rs index 74ec66d..9ee966b 100644 --- a/src/device.rs +++ b/src/device.rs @@ -1,7 +1,6 @@ use bit_field::BitArray; -use embedded_hal::digital::v2::OutputPin; -use crate::bus::{Bus, BusRef, FourWire, SpiRef, ThreeWire}; +use crate::bus::{Bus, FourWire, ThreeWire}; use crate::host::Host; use crate::net::Ipv4Addr; use crate::socket::Socket; @@ -32,7 +31,7 @@ pub struct DeviceState { } pub struct Device { - bus: SpiBus, + pub(crate) bus: SpiBus, state: DeviceState, } @@ -55,129 +54,15 @@ impl Device { if self.state.sockets != [0b11111111] { Err(ResetError::SocketsNotReleased) } else { - self.clear_mode()?; + self.reset_device()?; Ok(UninitializedDevice::new(self.bus)) } } - fn clear_mode(&mut self) -> Result<(), SpiBus::Error> { - // Set RST common register of the w5500 - self.as_mut().reset_device() - } - - #[inline] - pub fn gateway(&mut self) -> Result { - self.as_mut().gateway() - } - - #[inline] - pub fn subnet_mask(&mut self) -> Result { - self.as_mut().subnet_mask() - } - - #[inline] - pub fn mac(&mut self) -> Result { - self.as_mut().mac() - } - - #[inline] - pub fn ip(&mut self) -> Result { - self.as_mut().ip() - } - - #[inline] - pub fn phy_config(&mut self) -> Result { - self.as_mut().phy_config() - } - - #[inline] - pub fn version(&mut self) -> Result { - self.as_mut().version() - } - - /// Get the currently set Retry Time-value Register. - /// - /// RTR (Retry Time-value Register) [R/W] [0x0019 – 0x001A] [0x07D0] - #[inline] - pub fn current_retry_timeout(&mut self) -> Result { - self.as_mut().current_retry_timeout() - } - - /// Set a new value for the Retry Time-value Register. - /// - /// RTR (Retry Time-value Register) [R/W] [0x0019 – 0x001A] [0x07D0] - /// - /// # Example - /// - /// ``` - /// use w5500::register::common::RetryTime; - /// - /// let default = RetryTime::from_millis(200); - /// assert_eq!(RetryTime::default(), default); - /// - /// // E.g. 4000 (register) = 400ms - /// let four_hundred_ms = RetryTime::from_millis(400); - /// assert_eq!(four_hundred_ms.to_u16(), 4000); - /// ``` - #[inline] - pub fn set_retry_timeout(&mut self, retry_time_value: RetryTime) -> Result<(), SpiBus::Error> { - self.as_mut().set_retry_timeout(retry_time_value) - } - - /// Get the current Retry Count Register value. - #[inline] - pub fn current_retry_count(&mut self) -> Result { - self.as_mut().current_retry_count() - } - - /// Set a new value for the Retry Count register. - #[inline] - pub fn set_retry_count(&mut self, retry_count: u8) -> Result<(), SpiBus::Error> { - self.as_mut().set_retry_count(retry_count) - } - - #[inline] - pub(crate) fn as_mut(&mut self) -> DeviceRefMut<'_, BusRef<'_, SpiBus>, HostImpl> { - DeviceRefMut { - bus: BusRef(&mut self.bus), - state: &mut self.state, - } - } - pub fn release(self) -> (SpiBus, HostImpl) { (self.bus, self.state.host) } - pub fn deactivate(self) -> (SpiBus, InactiveDevice) { - (self.bus, InactiveDevice(self.state)) - } -} - -#[derive(Debug)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub struct InactiveDevice(DeviceState); - -impl InactiveDevice { - /// Activates the device by taking ownership - pub fn activate(self, bus: SpiBus) -> Device { - Device { bus, state: self.0 } - } - - /// Activates the device by borrowing it - pub fn activate_ref(&mut self, bus: SpiBus) -> DeviceRefMut { - DeviceRefMut { - bus, - state: &mut self.0, - } - } -} - -pub struct DeviceRefMut<'a, SpiBus: Bus, HostImpl: Host> { - pub(crate) bus: SpiBus, - state: &'a mut DeviceState, -} - -impl DeviceRefMut<'_, SpiBus, HostImpl> { pub fn take_socket(&mut self) -> Option { // TODO maybe return Future that resolves when release_socket invoked for index in 0..8 { @@ -257,6 +142,8 @@ impl DeviceRefMut<'_, SpiBus, HostImpl> { Ok(version_register[0]) } + /// Set a new value for the Retry Time-value Register. + /// /// RTR (Retry Time-value Register) [R/W] [0x0019 – 0x001A] [0x07D0] /// /// # Example @@ -282,6 +169,8 @@ impl DeviceRefMut<'_, SpiBus, HostImpl> { Ok(()) } + /// Get the currently set Retry Time-value Register. + /// /// RTR (Retry Time-value Register) [R/W] [0x0019 – 0x001A] [0x07D0] /// /// E.g. 4000 = 400ms diff --git a/src/lib.rs b/src/lib.rs index 124ff6d..eba0c4b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,7 +17,7 @@ mod uninitialized_device; #[doc(inline)] pub use self::{ - device::{Device, DeviceRefMut, InactiveDevice}, + device::Device, host::{Dhcp, Host, HostConfig, Manual}, net::MacAddress, uninitialized_device::{InitializeError, UninitializedDevice}, diff --git a/src/tcp.rs b/src/tcp.rs index e57fca2..47cfb24 100644 --- a/src/tcp.rs +++ b/src/tcp.rs @@ -1,10 +1,4 @@ -use crate::{ - bus::Bus, - device::{Device, DeviceRefMut}, - host::Host, - register::socketn, - socket::Socket, -}; +use crate::{bus::Bus, device::Device, host::Host, register::socketn, socket::Socket}; use embedded_nal::{nb, IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4, TcpClientStack}; @@ -206,7 +200,7 @@ impl TcpSocket { } } -impl TcpClientStack for DeviceRefMut<'_, SpiBus, HostImpl> { +impl TcpClientStack for Device { type TcpSocket = TcpSocket; type Error = TcpSocketError; @@ -259,44 +253,3 @@ impl TcpClientStack for DeviceRefMut<'_, SpiBus, Ho Ok(()) } } - -impl TcpClientStack for Device { - type TcpSocket = TcpSocket; - type Error = TcpSocketError; - - fn socket(&mut self) -> Result { - self.as_mut().socket() - } - - fn connect( - &mut self, - socket: &mut Self::TcpSocket, - remote: SocketAddr, - ) -> nb::Result<(), Self::Error> { - self.as_mut().connect(socket, remote) - } - - fn is_connected(&mut self, socket: &Self::TcpSocket) -> Result { - self.as_mut().is_connected(socket) - } - - fn send( - &mut self, - socket: &mut Self::TcpSocket, - buffer: &[u8], - ) -> nb::Result { - self.as_mut().send(socket, buffer) - } - - fn receive( - &mut self, - socket: &mut Self::TcpSocket, - buffer: &mut [u8], - ) -> nb::Result { - self.as_mut().receive(socket, buffer) - } - - fn close(&mut self, socket: Self::TcpSocket) -> Result<(), Self::Error> { - self.as_mut().close(socket) - } -} diff --git a/src/udp.rs b/src/udp.rs index 7937f55..a7a2d15 100644 --- a/src/udp.rs +++ b/src/udp.rs @@ -4,7 +4,7 @@ use embedded_nal::{nb, IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4, UdpClientStac use crate::{ bus::Bus, - device::{Device, DeviceRefMut}, + device::Device, host::Host, register::socketn::{self, Status}, socket::Socket, @@ -478,48 +478,6 @@ where type UdpSocket = UdpSocket; type Error = UdpSocketError; - #[inline] - fn socket(&mut self) -> Result { - self.as_mut().socket() - } - - #[inline] - fn connect( - &mut self, - socket: &mut Self::UdpSocket, - remote: SocketAddr, - ) -> Result<(), Self::Error> { - self.as_mut().connect(socket, remote) - } - - #[inline] - fn send(&mut self, socket: &mut Self::UdpSocket, buffer: &[u8]) -> nb::Result<(), Self::Error> { - self.as_mut().send(socket, buffer) - } - - #[inline] - fn receive( - &mut self, - socket: &mut Self::UdpSocket, - buffer: &mut [u8], - ) -> nb::Result<(usize, SocketAddr), Self::Error> { - self.as_mut().receive(socket, buffer) - } - - #[inline] - fn close(&mut self, socket: Self::UdpSocket) -> Result<(), Self::Error> { - self.as_mut().close(socket) - } -} - -impl UdpClientStack for DeviceRefMut<'_, SpiBus, HostImpl> -where - SpiBus: Bus, - HostImpl: Host, -{ - type UdpSocket = UdpSocket; - type Error = UdpSocketError; - fn socket(&mut self) -> Result { if let Some(socket) = self.take_socket() { Ok(UdpSocket::new(socket)) @@ -564,27 +522,6 @@ where } impl UdpFullStack for Device -where - SpiBus: Bus, - HostImpl: Host, -{ - #[inline] - fn bind(&mut self, socket: &mut Self::UdpSocket, local_port: u16) -> Result<(), Self::Error> { - self.as_mut().bind(socket, local_port) - } - - #[inline] - fn send_to( - &mut self, - socket: &mut Self::UdpSocket, - remote: SocketAddr, - buffer: &[u8], - ) -> nb::Result<(), Self::Error> { - self.as_mut().send_to(socket, remote, buffer) - } -} - -impl UdpFullStack for DeviceRefMut<'_, SpiBus, HostImpl> where SpiBus: Bus, HostImpl: Host, diff --git a/src/uninitialized_device.rs b/src/uninitialized_device.rs index e6a7c2b..899b963 100644 --- a/src/uninitialized_device.rs +++ b/src/uninitialized_device.rs @@ -1,5 +1,4 @@ -use embedded_hal::blocking::spi::{Transfer, Write}; -use embedded_hal::digital::v2::OutputPin; +use embedded_hal::spi::SpiDevice; use embedded_nal::Ipv4Addr; use crate::bus::{Bus, FourWire, ThreeWire}; @@ -244,17 +243,3 @@ impl UninitializedDevice { } } } - -impl + Write, ChipSelect: OutputPin> - UninitializedDevice> -{ - pub fn deactivate(self) -> (Spi, ChipSelect) { - self.bus.release() - } -} - -impl + Write> UninitializedDevice> { - pub fn deactivate(self) -> Spi { - self.bus.release() - } -}