From ca36814a09928cd8ea6702adef486d05e4d1de1b Mon Sep 17 00:00:00 2001 From: Diego Barrios Romero Date: Wed, 31 Oct 2018 06:50:26 +0100 Subject: [PATCH] Cache the status of the control register in the driver to avoid reads --- src/ds323x/configuration.rs | 37 +++++++++++++++++-------------------- src/lib.rs | 7 ++++++- tests/common/mod.rs | 4 +++- tests/configuration.rs | 33 +++++++++++++++++++++------------ 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/src/ds323x/configuration.rs b/src/ds323x/configuration.rs index 0d07f9a..472ab31 100644 --- a/src/ds323x/configuration.rs +++ b/src/ds323x/configuration.rs @@ -9,25 +9,15 @@ where DI: ReadData + WriteData { /// Enable the oscillator (set the clock running). - /// - /// (Does not alter the device register if already running). pub fn enable(&mut self) -> Result<(), Error> { - let control = self.iface.read_register(Register::CONTROL)?; - if (control & BitFlags::EOSC) != 0 { - self.iface.write_register(Register::CONTROL, control & !BitFlags::EOSC)?; - } - Ok(()) + let control = self.control; + self.write_control(control & !BitFlags::EOSC) } /// Disable the oscillator (stops the clock). - /// - /// (Does not alter the device register if already stopped). pub fn disable(&mut self) -> Result<(), Error> { - let control = self.iface.read_register(Register::CONTROL)?; - if (control & BitFlags::EOSC) == 0 { - self.iface.write_register(Register::CONTROL, control | BitFlags::EOSC)?; - } - Ok(()) + let control = self.control; + self.write_control(control | BitFlags::EOSC) } /// 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> { 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> { - let control = self.iface.read_register(Register::STATUS)?; - if (control & BitFlags::EN32KHZ) == 0 { - self.iface.write_register(Register::STATUS, control | BitFlags::EN32KHZ)?; + let status = self.iface.read_register(Register::STATUS)?; + if (status & BitFlags::EN32KHZ) == 0 { + 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> { - let control = self.iface.read_register(Register::STATUS)?; - if (control & BitFlags::EN32KHZ) != 0 { - self.iface.write_register(Register::STATUS, control & !BitFlags::EN32KHZ)?; + let status = self.iface.read_register(Register::STATUS)?; + if (status & BitFlags::EN32KHZ) != 0 { + 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> { self.iface.write_register(Register::AGING_OFFSET, offset as u8) } + + fn write_control(&mut self, control: u8) -> Result<(), Error> { + self.iface.write_register(Register::CONTROL, control)?; + self.control = control; + Ok(()) + } } diff --git a/src/lib.rs b/src/lib.rs index eeea93b..a4878e0 100644 --- a/src/lib.rs +++ b/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 { iface: DI, + control: u8, _ic: PhantomData } @@ -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 } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 4f4a7e7..cac730a 100644 --- a/tests/common/mod.rs +++ b/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; diff --git a/tests/configuration.rs b/tests/configuration.rs index 2f88b8a..960bfd5 100644 --- a/tests/configuration.rs +++ b/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, - [ I2cTrans::write_read(DEV_ADDR, vec![Register::$register], vec![$value_enabled]) ], - [ SpiTrans::transfer(vec![Register::$register, 0], vec![Register::$register, $value_enabled]) ]); + call_triple_test!(do_nothing_if_not_necessary, $method, + [ I2cTrans::write_read(DEV_ADDR, vec![Register::$register], vec![$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); -change_if_necessary_test!(disable, disable, CONTROL, 0xFF, 0xFF & !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, 0xFF, 0xFF & !BF::EN32KHZ); +call_method_test!(enable, enable, CONTROL, CONTROL_POR_VALUE & !BF::EOSC); +call_method_test!(disable, disable, CONTROL, CONTROL_POR_VALUE | BF::EOSC); +change_if_necessary_test!(en_32khz_out, enable_32khz_output, STATUS, BF::EN32KHZ, 0); 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!(aging_offset_max, set_aging_offset, AGING_OFFSET, 127, 127); +set_param_test!(set_aging_offset_min, set_aging_offset, AGING_OFFSET, -128, 0b1000_0000); +set_param_test!(set_aging_offset_max, set_aging_offset, AGING_OFFSET, 127, 127);