Cache the status of the control register in the driver to avoid reads

src/ds323x/configuration.rs

@@ -9,25 +9,15 @@ where
     DI: ReadData<Error = E> + WriteData<Error = E>
 {
     /// Enable the oscillator (set the clock running).
-    ///
-    /// (Does not alter the device register if already running).
     pub fn enable(&mut self) -> Result<(), Error<E>> {
-        let control = self.iface.read_register(Register::CONTROL)?;
+        let control = self.control;
-        if (control & BitFlags::EOSC) != 0 {
+        self.write_control(control & !BitFlags::EOSC)
-            self.iface.write_register(Register::CONTROL, control & !BitFlags::EOSC)?;
-        }
-        Ok(())
     }
 
     /// Disable the oscillator (stops the clock).
-    ///
-    /// (Does not alter the device register if already stopped).
     pub fn disable(&mut self) -> Result<(), Error<E>> {
-        let control = self.iface.read_register(Register::CONTROL)?;
+        let control = self.control;
-        if (control & BitFlags::EOSC) == 0 {
+        self.write_control(control | BitFlags::EOSC)
-            self.iface.write_register(Register::CONTROL, control | BitFlags::EOSC)?;
-        }
-        Ok(())
     }
 
     /// Force a temperature conversion and time compensation with TXCO algorithm.
@@ -35,6 +25,7 @@ where
     /// The *busy* status should be checked before doing this. See [`is_busy()`](#method.is_busy)
     pub fn convert_temperature(&mut self) -> Result<(), Error<E>> {
         let control = self.iface.read_register(Register::CONTROL)?;
+        // do not overwrite if a conversion is in progress
         if (control & BitFlags::TEMP_CONV) == 0 {
             self.iface.write_register(Register::CONTROL, control | BitFlags::TEMP_CONV)?;
         }
@@ -45,9 +36,9 @@ where
     ///
     /// (Does not alter the device register if already enabled).
     pub fn enable_32khz_output(&mut self) -> Result<(), Error<E>> {
-        let control = self.iface.read_register(Register::STATUS)?;
+        let status = self.iface.read_register(Register::STATUS)?;
-        if (control & BitFlags::EN32KHZ) == 0 {
+        if (status & BitFlags::EN32KHZ) == 0 {
-            self.iface.write_register(Register::STATUS, control | BitFlags::EN32KHZ)?;
+            self.iface.write_register(Register::STATUS, status | BitFlags::EN32KHZ)?;
         }
         Ok(())
     }
@@ -56,9 +47,9 @@ where
     ///
     /// (Does not alter the device register if already disabled).
     pub fn disable_32khz_output(&mut self) -> Result<(), Error<E>> {
-        let control = self.iface.read_register(Register::STATUS)?;
+        let status = self.iface.read_register(Register::STATUS)?;
-        if (control & BitFlags::EN32KHZ) != 0 {
+        if (status & BitFlags::EN32KHZ) != 0 {
-            self.iface.write_register(Register::STATUS, control & !BitFlags::EN32KHZ)?;
+            self.iface.write_register(Register::STATUS, status & !BitFlags::EN32KHZ)?;
         }
         Ok(())
     }
@@ -67,4 +58,10 @@ where
     pub fn set_aging_offset(&mut self, offset: i8) -> Result<(), Error<E>> {
         self.iface.write_register(Register::AGING_OFFSET, offset as u8)
     }
+
+    fn write_control(&mut self, control: u8) -> Result<(), Error<E>> {
+        self.iface.write_register(Register::CONTROL, control)?;
+        self.control = control;
+        Ok(())
+    }
 }

src/lib.rs

@@ -362,7 +362,8 @@ impl BitFlags {
     const OSC_STOP   : u8 = 0b1000_0000;
 }
 
-const DEVICE_ADDRESS: u8 = 0b110_1000;
+const DEVICE_ADDRESS   : u8 = 0b110_1000;
+const CONTROL_POR_VALUE: u8 = 0b0001_1100;
 
 /// IC markers
 pub mod ic {
@@ -380,6 +381,7 @@ use interface::{ I2cInterface, SpiInterface };
 #[derive(Debug, Default)]
 pub struct Ds323x<DI, IC> {
     iface: DI,
+    control: u8,
     _ic: PhantomData<IC>
 }
 
@@ -393,6 +395,7 @@ where
             iface: I2cInterface {
                 i2c,
             },
+            control: CONTROL_POR_VALUE,
             _ic: PhantomData
         }
     }
@@ -413,6 +416,7 @@ where
             iface: I2cInterface {
                 i2c,
             },
+            control: CONTROL_POR_VALUE,
             _ic: PhantomData
         }
     }
@@ -435,6 +439,7 @@ where
                 spi,
                 cs: chip_select
             },
+            control: CONTROL_POR_VALUE,
             _ic: PhantomData
         }
     }

tests/common/mod.rs

@@ -4,7 +4,9 @@ use self::ds323x::{ Ds323x, interface, ic };
 use hal::i2c::{ Mock as I2cMock, Transaction as I2cTrans };
 use hal::spi::{ Mock as SpiMock, Transaction as SpiTrans };
 
-pub const DEVICE_ADDRESS: u8 = 0b110_1000;
+pub const DEVICE_ADDRESS   : u8 = 0b110_1000;
+#[allow(unused)]
+pub const CONTROL_POR_VALUE: u8 = 0b0001_1100;
 
 pub struct Register;
 

tests/configuration.rs

@@ -6,9 +6,9 @@ use hal::spi::Transaction as SpiTrans;
 mod common;
 use common::{ DEVICE_ADDRESS as DEV_ADDR, Register, new_ds3231,
               new_ds3232, new_ds3234, destroy_ds3231, destroy_ds3232,
-              destroy_ds3234, BitFlags as BF };
+              destroy_ds3234, BitFlags as BF, CONTROL_POR_VALUE };
 
-macro_rules! call_method_test {
+macro_rules! call_triple_test {
     ($name:ident, $method:ident, $i2c_transactions:expr, $spi_transactions:expr) => {
         mod $name {
             use super::*;
@@ -19,15 +19,24 @@ macro_rules! call_method_test {
     };
 }
 
+macro_rules! call_method_test {
+    ($name:ident, $method:ident, $register:ident, $value_enabled:expr) => {
+        call_triple_test!($name, $method,
+            [ I2cTrans::write(DEV_ADDR, vec![Register::$register, $value_enabled]) ],
+            [ SpiTrans::write(vec![Register::$register + 0x80, $value_enabled])    ]);
+    };
+}
+
+
 macro_rules! change_if_necessary_test {
     ($name:ident, $method:ident, $register:ident, $value_enabled:expr, $value_disabled:expr) => {
         mod $name {
             use super::*;
-            call_method_test!(do_nothing_if_not_necessary, $method,
+            call_triple_test!(do_nothing_if_not_necessary, $method,
-                [ I2cTrans::write_read(DEV_ADDR, vec![Register::$register], vec![$value_enabled]) ],
+                 [ I2cTrans::write_read(DEV_ADDR, vec![Register::$register], vec![$value_enabled]) ],
-                [ SpiTrans::transfer(vec![Register::$register, 0], vec![Register::$register, $value_enabled]) ]);
+                 [ SpiTrans::transfer(vec![Register::$register, 0], vec![Register::$register, $value_enabled]) ]);
 
-            call_method_test!(change, $method,
+            call_triple_test!(change, $method,
                 [ I2cTrans::write_read(DEV_ADDR, vec![Register::$register], vec![$value_disabled]),
                   I2cTrans::write(DEV_ADDR, vec![Register::$register, $value_enabled]) ],
 
@@ -37,12 +46,12 @@ macro_rules! change_if_necessary_test {
     };
 }
 
-change_if_necessary_test!(enable, enable, CONTROL, 0xFF & !BF::EOSC,  0xFF);
+call_method_test!(enable, enable, CONTROL, CONTROL_POR_VALUE & !BF::EOSC);
-change_if_necessary_test!(disable, disable, CONTROL, 0xFF, 0xFF & !BF::EOSC);
+call_method_test!(disable, disable, CONTROL, CONTROL_POR_VALUE | BF::EOSC);
-change_if_necessary_test!(conv_temp, convert_temperature, CONTROL, 0xFF, 0xFF & !BF::TEMP_CONV);
+change_if_necessary_test!(en_32khz_out,  enable_32khz_output,  STATUS, BF::EN32KHZ, 0);
-change_if_necessary_test!(en_32khz_out,  enable_32khz_output,  STATUS, 0xFF, 0xFF & !BF::EN32KHZ);
 change_if_necessary_test!(dis_32khz_out, disable_32khz_output, STATUS, 0xFF & !BF::EN32KHZ, 0xFF);
 change_if_necessary_test!(clr_stop, clear_has_been_stopped_flag, STATUS, 0xFF & !BF::OSC_STOP, 0xFF);
+change_if_necessary_test!(conv_temp, convert_temperature, CONTROL, CONTROL_POR_VALUE | BF::TEMP_CONV, CONTROL_POR_VALUE & !BF::TEMP_CONV);
 
-set_param_test!(aging_offset_min, set_aging_offset, AGING_OFFSET, -128, 128);
+set_param_test!(set_aging_offset_min, set_aging_offset, AGING_OFFSET, -128, 0b1000_0000);
-set_param_test!(aging_offset_max, set_aging_offset, AGING_OFFSET,  127, 127);
+set_param_test!(set_aging_offset_max, set_aging_offset, AGING_OFFSET,  127, 127);