Add embedded-hal-mock test suites to a4910

drivers/src/a4910/driver.rs

@@ -1,7 +1,7 @@
 //! Main Driver for A4910
 
-use embedded_hal::spi::SpiDevice;
 use crate::error::AllegroError;
+use embedded_hal::spi::SpiDevice;
 
 use super::{
     io::{Diagnostics, ReadRequest, ReadResponse, WriteRequest, WriteResponse},
@@ -16,7 +16,7 @@ pub struct A4910<SPI> {
 impl<SPI> A4910<SPI>
 where
     SPI: SpiDevice,
-    AllegroError: From<SPI::Error>
+    AllegroError: From<SPI::Error>,
 {
     /// Create a driver for the motor controller connected to the SPI bus
     pub fn new(spi: SPI) -> Self {
@@ -32,23 +32,23 @@ where
     /// # Errors
     /// Returns a `AllegroError::SpiError` if the SPI transaction fails.
     /// Stores a message but still returns a `AllegroError::MotorFault` if the fault bit is raised.
-    pub fn read_register(&mut self, register: A4910Reg) -> Result<&Self, AllegroError> {
+    pub fn read_register(&mut self, register: A4910Reg) -> Result<(), AllegroError> {
         let mut message = Self::read_request(register);
         self.spi.transfer_in_place(&mut message)?;
         self.read_response(register, message)?;
-        Ok(self)
+        Ok(())
     }
 
     /// Write to the specified register, and store the returned diagnostics.
     ///
     /// # Errors
     /// Returns a `AllegroError::SpiError` if the SPI transaction fails.
     /// Stores a message but still returns a `AllegroError::MotorFault` if the fault bit is raised.
-    pub fn write_register(&mut self, register: A4910Reg) -> Result<&Self, AllegroError> {
+    pub fn write_register(&mut self, register: A4910Reg) -> Result<(), AllegroError> {
         let mut message = self.write_request(register);
         self.spi.transfer_in_place(&mut message)?;
         self.write_response(message)?;
-        Ok(self)
+        Ok(())
     }
 
     /// Encode a request to read the desired register.
@@ -66,60 +66,102 @@ where
     }
 
     /// Decode the response from a SPI read transaction.
-    /// 
+    ///
     /// # Errors
     /// This function will check for an `AllegroError::MotorFault` and return it
     /// after parsing.
     fn read_response(&mut self, register: A4910Reg, message: [u8; 2]) -> Result<(), AllegroError> {
         let response = ReadResponse::from(u16::from_be_bytes(message));
         self.regs[register].set_value(response.register().into());
         self.status.set_header(response.status());
-        if response.status().ff() { return Err(AllegroError::MotorFault) }
+        if response.status().ff() {
+            return Err(AllegroError::MotorFault);
+        }
         Ok(())
     }
 
     /// Decode the response from a SPI write transaction.
-    /// 
+    ///
     /// # Errors
     /// This function will check for an `AllegroError::MotorFault` and return it
     /// after parsing.
     fn write_response(&mut self, message: [u8; 2]) -> Result<(), AllegroError> {
         self.status = WriteResponse::from(u16::from_be_bytes(message));
-        if self.status.header().ff() { return Err(AllegroError::MotorFault) }
+        if self.status.header().ff() {
+            return Err(AllegroError::MotorFault);
+        }
         Ok(())
     }
 }
 
+#[cfg(test)]
 mod tests {
-    #![allow(clippy::unusual_byte_groupings)]
-
+    #[allow(clippy::unusual_byte_groupings)]
     #[test]
-    fn test_spi_derive() {
+    fn test_setting_registers() {
         use super::*;
+        use crate::a4910::regs::config::*;
         use bilge::prelude::*;
         use embedded_hal_mock::eh1::spi::{Mock, Transaction};
 
-        let expected: [&Transaction; 0] = [];
+        let expected = [
+            // Read from Config1 register
+            &Transaction::transaction_start(),
+            &Transaction::transfer_in_place(vec![0x40, 0x00], vec![0x18, 0x20]),
+            &Transaction::transaction_end(),
+            // Write all Config1 register 1's to 0's, and vice versa
+            &Transaction::transaction_start(),
+            &Transaction::transfer_in_place(vec![0b01_1_00111, 0b0101_1111], vec![0x00, 0x00]),
+            &Transaction::transaction_end(),
+            // Read the switched values back from Config1
+            &Transaction::transaction_start(),
+            &Transaction::transfer_in_place(vec![0x40, 0x00], vec![0x1F, 0x5F]),
+            &Transaction::transaction_end(),
+        ];
+
         let mut a4910 = A4910::new(Mock::new(expected));
 
-        let config0_read_req = [0b00_0_00000, 0b0000_0000];
-        assert_eq!(
-            A4910::<Mock>::read_request(A4910Reg::Config0),
-            config0_read_req
-        );
+        a4910.read_register(A4910Reg::Config1).unwrap();
+        assert_eq!(a4910.regs.cfg.1, Config1::default());
+
+        a4910.regs.cfg.1.set_vt(u7::new(0b101_1111).into());
+        a4910
+            .regs
+            .cfg
+            .1
+            .set_dbm(DisableBootstrapManagement::Disabled);
+        a4910.regs.cfg.1.set_diag(DiagOutput::Temperature);
+        a4910.regs.cfg.1.set_esf(StopOnFault::Disabled);
+        a4910
+            .regs
+            .cfg
+            .1
+            .set_csb(CurrentSenseBandwidth::ReducedBandwidth);
+
+        a4910.write_register(A4910Reg::Config1).unwrap();
+        a4910.read_register(A4910Reg::Config1).unwrap();
+        assert_eq!(a4910.regs.cfg.1, u13::new(0x1F_5F).into());
 
-        let config1_read_req = [0b01_0_00000, 0b0000_0000];
-        assert_eq!(
-            A4910::<Mock>::read_request(A4910Reg::Config1),
-            config1_read_req
-        );
+        a4910.spi.done();
+    }
 
-        let config1_write_req = [0b01_1_1_1_00_0, 0b0_0100000];
-        assert_eq!(a4910.write_request(A4910Reg::Config1), config1_write_req);
+    #[test]
+    fn test_transaction_errors() {
+        use super::*;
+        use crate::a4910::regs::config::*;
+        use embedded_hal_mock::eh1::spi::{Mock, Transaction};
+
+        let expected = [
+            &Transaction::transaction_start(),
+            &Transaction::transfer_in_place(vec![0x40, 0x00], vec![0x98, 0x20]),
+            &Transaction::transaction_end(),
+        ];
+
+        let mut a4910 = A4910::new(Mock::new(expected));
 
-        let config1_read_resp = [0b00_0_0_0_11_1, 0b1_1011111];
-        a4910.read_response(A4910Reg::Config1, config1_read_resp).unwrap();
-        assert_eq!(a4910.regs.cfg.1.vt(), u7::new(0b101_1111).into());
+        let result = a4910.read_register(A4910Reg::Config1);
+        assert_eq!(result, Err(AllegroError::MotorFault));
+        assert_eq!(a4910.regs.cfg.1, Config1::default());
 
         a4910.spi.done();
     }

drivers/src/a4964/diagnostics.rs

@@ -1,8 +1,8 @@
 //! All diagnostics for the chip
 
-use bilge::prelude::*;
 use crate::a4964::readback::ReadbackDiagnostics;
 use crate::a4964::regs::diagnostic::*;
+use bilge::prelude::*;
 
 #[bitsize(24)]
 #[derive(Clone, Copy, DefaultBits, DebugBits, FromBits)]

drivers/src/a4964/driver.rs

@@ -2,14 +2,14 @@
 
 use embedded_hal::spi::SpiDevice;
 
-use crate::io::Parity;
 use crate::error::AllegroError;
+use crate::io::Parity;
 
 use super::{
+    diagnostics::Diagnostics,
     io::*,
-    regs::{*, readback::ReadbackSelect},
     readback::Readback,
-    diagnostics::Diagnostics,
+    regs::{readback::ReadbackSelect, *},
 };
 pub struct A4964<SPI> {
     spi: SPI,
@@ -38,26 +38,30 @@ where
     /// # Errors
     /// Returns an `AllegroError` if the SPI transaction fails.
     /// Returns an `InvalidRegister` if the user is trying to read from the write only register
-    pub fn read_register(&mut self, register: A4964Reg) -> Result<&Self, AllegroError> {
-        if register == A4964Reg::WriteOnly { return Err(AllegroError::InvalidRegister) }
+    pub fn read_register(&mut self, register: A4964Reg) -> Result<(), AllegroError> {
+        if register == A4964Reg::WriteOnly {
+            return Err(AllegroError::InvalidRegister);
+        }
 
         let mut message = Self::read_request(register);
         self.spi.transfer_in_place(&mut message)?;
         self.read_response(register, message)?;
-        Ok(self)
+        Ok(())
     }
 
     /// Write to the specified register, and store the returned diagnostics.
     ///
     /// # Errors
     /// Returns a `MotorFault` if the SPI transaction fails.
-    pub fn write_register(&mut self, register: A4964Reg) -> Result<&Self, AllegroError> {
-        if register == A4964Reg::ReadOnly { return Err(AllegroError::InvalidRegister) }
-
+    pub fn write_register(&mut self, register: A4964Reg) -> Result<(), AllegroError> {
+        if register == A4964Reg::ReadOnly {
+            return Err(AllegroError::InvalidRegister);
+        }
+
         let mut message = self.write_request(register);
         self.spi.transfer_in_place(&mut message)?;
         self.write_response(message)?;
-        Ok(self)
+        Ok(())
     }
 
     /// Encode a request to read the desired register.
@@ -69,12 +73,14 @@ where
     /// Encode a request to write to the desired register.
     fn write_request(&self, register: A4964Reg) -> [u8; 2] {
         let message: u16 = if register == A4964Reg::WriteOnly {
-            let reg_contents = unsafe { bilge::arbitrary_int::u10::new_unchecked(self.regs[register].value()) };
+            let reg_contents =
+                unsafe { bilge::arbitrary_int::u10::new_unchecked(self.regs[register].value()) };
             let mut request = WriteOnlyRequest::new(false, reg_contents, register.into());
             request.set_odd_parity();
             request.into()
         } else {
-            let reg_contents = unsafe { bilge::arbitrary_int::u9::new_unchecked(self.regs[register].value()) };
+            let reg_contents =
+                unsafe { bilge::arbitrary_int::u9::new_unchecked(self.regs[register].value()) };
             let mut request = WriteRequest::new(false, reg_contents, true, register.into());
             request.set_odd_parity();
             request.into()
@@ -83,7 +89,7 @@ where
     }
 
     /// Decode the response from a SPI read transaction.
-    /// 
+    ///
     /// # Errors
     /// If the message has `AllegroError::InvalidParity`, then it is not to be trusted.
     /// The message is not parsed, and the function returns.
@@ -92,24 +98,31 @@ where
     fn read_response(&mut self, register: A4964Reg, message: [u8; 2]) -> Result<(), AllegroError> {
         let mut result: Result<(), AllegroError> = Ok(());
         let message = u16::from_be_bytes(message);
-        if !crate::io::parity(message) { return Err(AllegroError::InvalidParity); }
-
+        if !crate::io::parity(message) {
+            return Err(AllegroError::InvalidParity);
+        }
+
         let register_contents: u16 = if register == A4964Reg::ReadOnly {
             let response = ReadOnlyResponse::from(message);
-            if response.status().ff() { result = Err(AllegroError::MotorFault) }
+            if response.status().ff() {
+                result = Err(AllegroError::MotorFault);
+            }
             self.diagnostics.set_main_diagnostics(response.status());
 
             let register_contents: u16 = response.register().into();
             self.readback[self.regs.readback_select.rbs()] = register_contents;
 
             if self.regs.readback_select.rbs() == ReadbackSelect::Diagnostic {
-                self.diagnostics.set_readback_diagnostics(response.register().into());
+                self.diagnostics
+                    .set_readback_diagnostics(response.register().into());
             }
 
             register_contents
         } else {
             let response = ReadResponse::from(message);
-            if response.status().ff() { result = Err(AllegroError::MotorFault) }
+            if response.status().ff() {
+                result = Err(AllegroError::MotorFault);
+            }
 
             self.diagnostics.set_main_diagnostics(response.status());
             response.register().into()
@@ -119,19 +132,43 @@ where
     }
 
     /// Decode the response from a SPI write transaction.
-    /// 
+    ///
     /// # Errors
     /// If the message has `AllegroError::InvalidParity`, then it is not to be trusted.
     /// The message is not parsed, and the function returns.
     /// This function will check for an `AllegroError::MotorFault` and return it
     /// after parsing.
     fn write_response(&mut self, message: [u8; 2]) -> Result<(), AllegroError> {
         let message = u16::from_be_bytes(message);
-        if !crate::io::parity(message) { return Err(AllegroError::InvalidParity) }
+        if !crate::io::parity(message) {
+            return Err(AllegroError::InvalidParity);
+        }
         let resp = WriteResponse::from(message);
         self.diagnostics.set_main_diagnostics(resp.header());
         self.diagnostics.set_status_diagnostics(resp.data());
-        if resp.header().ff() { return Err(AllegroError::MotorFault) }
+        if resp.header().ff() {
+            return Err(AllegroError::MotorFault);
+        }
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+
+    #[test]
+    fn a4964() {
+        use super::*;
+        use embedded_hal_mock::eh1::spi::{Mock, Transaction};
+
+        let expected: [&Transaction; 0] = [];
+        let mut a4964 = A4964::new(Mock::new(expected));
+
+        let a = 1;
+        let b = 1;
+
+        assert_eq!(a, b);
+
+        a4964.spi.done();
+    }
+}

drivers/src/a4964/mod.rs

@@ -1,7 +1,7 @@
 //! Allegro A4964 Sensorless Sinusoidal Drive BLDC Controller
 
+pub mod diagnostics;
 pub mod driver;
 pub mod io;
-pub mod regs;
 pub mod readback;
-pub mod diagnostics;
+pub mod regs;

drivers/src/a4964/readback.rs

@@ -1,8 +1,8 @@
 //! Readback telemetry
 
-use core::ops::{Index, IndexMut};
-use bilge::prelude::*;
 use super::regs::readback::ReadbackSelect;
+use bilge::prelude::*;
+use core::ops::{Index, IndexMut};
 
 #[bitsize(10)]
 #[derive(Copy, Clone, PartialEq, DebugBits, DefaultBits, FromBits)]
@@ -60,4 +60,4 @@ impl IndexMut<ReadbackSelect> for Readback {
             ReadbackSelect::AppliedPhaseAdvance => &mut self.applied_phase_advance,
         }
     }
-}
+}