Integrate mode change into `into...` methods

6 files changed, 75 insertions, 90 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e247183..5e6b7fd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,7 +7,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
-...
+### Changed
+- [breaking-change] Mode change has been integrated into `into_continuous` and
+  `into_one_shot` methods. This removes the need for a `start` method in continuous
+  mode and the `Error::NotStarted`. Everything is now encoded into the modes.
+  When changing into continuous mode the measurements are started and to stop one
+  can simply change into one-shot mode. (This is how the hardware does it anyway).
+  The one-shot mode is not affected.
+  When changing the mode an I2C communication error can occur but the unchanged device
+  can now be retrieved.
 
 ## 0.1.0 - 2018-11-21
 
diff --git a/src/devices/mode/continuous.rs b/src/devices/mode/continuous.rs
index 66951dd..29471de 100644
--- a/src/devices/mode/continuous.rs
+++ b/src/devices/mode/continuous.rs
@@ -3,7 +3,7 @@
 use super::super::OperatingMode;
 use channels::ChannelSelection;
 use core::marker::PhantomData;
-use {conversion, hal, interface, mode, Ads1x1x, Error, Register};
+use {conversion, hal, interface, mode, Ads1x1x, Error, ModeChangeError, Register};
 
 impl<DI, IC, CONV, E> Ads1x1x<DI, IC, CONV, mode::Continuous>
 where
@@ -11,8 +11,12 @@ where
     CONV: conversion::ConvertMeasurement,
 {
     /// Change operating mode to OneShot
-    pub fn into_one_shot(mut self) -> Result<Ads1x1x<DI, IC, CONV, mode::OneShot>, Error<E>> {
-        self.set_operating_mode(OperatingMode::OneShot)?;
+    pub fn into_one_shot(
+        mut self,
+    ) -> Result<Ads1x1x<DI, IC, CONV, mode::OneShot>, ModeChangeError<E, Self>> {
+        if let Err(Error::I2C(e)) = self.set_operating_mode(OperatingMode::OneShot) {
+            return Err(ModeChangeError::I2C(e, self));
+        }
         Ok(Ads1x1x {
             iface: self.iface,
             config: self.config,
@@ -24,37 +28,12 @@ where
         })
     }
 
-    /// Start continuous conversions
-    ///
-    /// _Note:_ this method is only available in continuous mode.
-    pub fn start(&mut self) -> Result<(), Error<E>> {
-        self.set_operating_mode(OperatingMode::Continuous)?;
-        self.a_conversion_was_started = true;
-        Ok(())
-    }
-
     /// Read the most recent measurement
-    ///
-    /// The continuous measurement must be started with [`start()`] before
-    /// calling this method. Otherwise, `Error::NotStarted` will be returned.
-    ///
-    /// _Note:_ this method is only available in continuous mode.
-    ///
-    /// [`start()`]: struct.Ads1x1x.html#method.start
     pub fn read(&mut self) -> Result<i16, Error<E>> {
-        if !self.a_conversion_was_started {
-            return Err(Error::NotStarted);
-        }
         let value = self.iface.read_register(Register::CONVERSION)?;
         Ok(CONV::convert_measurement(value))
     }
-}
 
-impl<DI, IC, CONV, E> Ads1x1x<DI, IC, CONV, mode::Continuous>
-where
-    DI: interface::ReadData<Error = E> + interface::WriteData<Error = E>,
-    CONV: conversion::ConvertMeasurement,
-{
     /// Select the channel for measurements.
     ///
     /// Note that when changing the channel in continuous conversion mode, the
diff --git a/src/devices/mode/oneshot.rs b/src/devices/mode/oneshot.rs
index 3595c26..612d5a4 100644
--- a/src/devices/mode/oneshot.rs
+++ b/src/devices/mode/oneshot.rs
@@ -1,9 +1,10 @@
 //! Common functions
 
+use super::super::OperatingMode;
 use channels::ChannelSelection;
 use core::marker::PhantomData;
 use {conversion, hal, interface, nb};
-use {mode, Ads1x1x, BitFlags, Config, Error, Register};
+use {mode, Ads1x1x, BitFlags, Config, Error, ModeChangeError, Register};
 
 impl<DI, IC, CONV, E> Ads1x1x<DI, IC, CONV, mode::OneShot>
 where
@@ -11,12 +12,17 @@ where
     CONV: conversion::ConvertMeasurement,
 {
     /// Change operating mode to Continuous
-    pub fn into_continuous(self) -> Result<Ads1x1x<DI, IC, CONV, mode::Continuous>, Error<E>> {
+    pub fn into_continuous(
+        mut self,
+    ) -> Result<Ads1x1x<DI, IC, CONV, mode::Continuous>, ModeChangeError<E, Self>> {
+        if let Err(Error::I2C(e)) = self.set_operating_mode(OperatingMode::Continuous) {
+            return Err(ModeChangeError::I2C(e, self));
+        }
         Ok(Ads1x1x {
             iface: self.iface,
             config: self.config,
             fsr: self.fsr,
-            a_conversion_was_started: self.a_conversion_was_started,
+            a_conversion_was_started: true,
             _conv: PhantomData,
             _ic: PhantomData,
             _mode: PhantomData,
diff --git a/src/lib.rs b/src/lib.rs
index 559a069..1422277 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -140,21 +140,25 @@
 //!
 //! ### Change into continuous conversion mode and read the last measurement
 //!
+//! Changing the mode may fail in case there was a communication error.
+//! In this case, you can retrieve the unchanged device from the error type.
+//!
 //! ```no_run
 //! extern crate linux_embedded_hal as hal;
 //! extern crate ads1x1x;
-//! use ads1x1x::{ Ads1x1x, SlaveAddr };
+//! use ads1x1x::{ Ads1x1x, SlaveAddr, ModeChangeError };
 //!
 //! # fn main() {
 //! let dev = hal::I2cdev::new("/dev/i2c-1").unwrap();
 //! let address = SlaveAddr::default();
 //! let adc = Ads1x1x::new_ads1013(dev, address);
-//! let mut adc = adc.into_continuous().unwrap();
-//! adc.start().unwrap();
-//! while(adc.is_measurement_in_progress().unwrap()) {
-//!     // some delay...
+//! match adc.into_continuous() {
+//!     Err(ModeChangeError::I2C(e, adc)) => /* mode change failed handling */ panic!(),
+//!     Ok(mut adc) => {
+//!         let measurement = adc.read().unwrap();
+//!         // ...
+//!     }
 //! }
-//! let measurement = adc.read().unwrap();
 //! # }
 //! ```
 //!
@@ -212,15 +216,24 @@ extern crate embedded_hal as hal;
 extern crate nb;
 use core::marker::PhantomData;
 
-/// All possible errors in this crate
+/// Errors in this crate
 #[derive(Debug)]
 pub enum Error<E> {
     /// I²C bus error
     I2C(E),
     /// Invalid input data provided
     InvalidInputData,
-    /// Continuous measurement was not started
-    NotStarted,
+}
+
+/// Error type for mode changes.
+///
+/// This allows to retrieve the unchanged device in case of an error.
+pub enum ModeChangeError<E, DEV> {
+    /// I²C bus error while changing mode.
+    ///
+    /// `E` is the error that happened.
+    /// `DEV` is the device with the mode unchanged.
+    I2C(E, DEV),
 }
 
 const DEVICE_BASE_ADDRESS: u8 = 0b100_1000;
diff --git a/tests/mux.rs b/tests/mux.rs
index f67690f..b961fc7 100644
--- a/tests/mux.rs
+++ b/tests/mux.rs
@@ -51,10 +51,16 @@ macro_rules! mux_test {
 
             #[test]
             fn continuous_can_select_channel() {
-                let config = Config::default().with_high($config_bits);
-                let transactions = [ I2cTrans::write(DEV_ADDR, vec![Register::CONFIG, config.msb(), config.lsb()]) ];
+                let config1 = Config::default().with_low(BF::OP_MODE);
+                let config2 = config1.with_high($config_bits);
+                let transactions = [
+                    I2cTrans::write(DEV_ADDR,
+                    vec![Register::CONFIG, config1.msb(), config1.lsb()]),
+                    I2cTrans::write(DEV_ADDR,
+                    vec![Register::CONFIG, config2.msb(), config2.lsb()])
+                ];
                 let dev = new(&transactions);
-                let mut dev = dev.into_continuous().unwrap();
+                let mut dev = dev.into_continuous().ok().unwrap();
                 dev.select_channel(&mut channel::$CS).unwrap();
                 destroy(dev);
             }
diff --git a/tests/tier1.rs b/tests/tier1.rs
index 1def511..de44b3c 100644
--- a/tests/tier1.rs
+++ b/tests/tier1.rs
@@ -4,7 +4,7 @@ extern crate embedded_hal;
 extern crate embedded_hal_mock as hal;
 use hal::i2c::Transaction as I2cTrans;
 extern crate ads1x1x;
-use ads1x1x::{ channel, DataRate12Bit, DataRate16Bit, Error };
+use ads1x1x::{channel, DataRate12Bit, DataRate16Bit};
 
 mod common;
 use common::{ new_ads1013, destroy_ads1013, new_ads1113, destroy_ads1113,
@@ -49,8 +49,7 @@ macro_rules! measure_tests {
                 let transactions = [ I2cTrans::write(DEV_ADDR, vec![Register::CONFIG, config.msb(), config.lsb()]),
                                     I2cTrans::write_read(DEV_ADDR, vec![Register::CONVERSION], vec![0x80, 0x00] ) ];
                 let dev = $create(&transactions);
-                let mut dev = dev.into_continuous().unwrap();
-                dev.start().unwrap();
+                let mut dev = dev.into_continuous().ok().unwrap();
                 let measurement = dev.read().unwrap();
                 assert_eq!($expected, measurement);
                 $destroy(dev);
@@ -130,55 +129,29 @@ fn can_read_measurement_not_in_progress() {
 
 #[test]
 fn can_convert_to_continuous() {
-    let dev = new_ads1013(&[]);
-    let dev = dev.into_continuous().unwrap();
+    let config = Config::default().with_low(BitFlags::OP_MODE);
+    let transactions = [I2cTrans::write(
+        DEV_ADDR,
+        vec![Register::CONFIG, config.msb(), config.lsb()],
+    )];
+    let dev = new_ads1013(&transactions);
+    let dev = dev.into_continuous().ok().unwrap();
     destroy_ads1013(dev);
 }
 
 #[test]
 fn can_convert_to_one_shot() {
+    let config_cont = Config::default().with_low(BitFlags::OP_MODE);
     let config_os = Config::default();
-    let transactions = [ I2cTrans::write(DEV_ADDR, vec![Register::CONFIG, config_os.msb(),   config_os.lsb()]) ];
+    let transactions = [I2cTrans::write(
+        DEV_ADDR,
+        vec![Register::CONFIG, config_cont.msb(), config_cont.lsb()]),
+        I2cTrans::write(
+        DEV_ADDR,
+        vec![Register::CONFIG, config_os.msb(), config_os.lsb()],
+    )];
     let dev = new_ads1013(&transactions);
-    let dev = dev.into_continuous().unwrap();
-    let dev = dev.into_one_shot().unwrap();
+    let dev = dev.into_continuous().ok().unwrap();
+    let dev = dev.into_one_shot().ok().unwrap();
     destroy_ads1013(dev);
 }
-
-mod continuous {
-    use super::*;
-    #[test]
-    fn can_start() {
-        let config = Config::default().with_low(BitFlags::OP_MODE);
-        let transactions = [ I2cTrans::write(DEV_ADDR, vec![Register::CONFIG, config.msb(), config.lsb()]) ];
-        let dev = new_ads1013(&transactions);
-        let mut dev = dev.into_continuous().unwrap();
-        dev.start().unwrap();
-        destroy_ads1013(dev);
-    }
-
-    fn assert_not_started<T, E>(result: Result<T, Error<E>>) {
-        match result {
-            Err(Error::NotStarted) => (),
-            _ => panic!("Error::NotStarted not returned.")
-        }
-    }
-    #[test]
-    fn check_assert_matches() {
-        assert_not_started::<(), ()>(Err(Error::NotStarted));
-    }
-
-    #[test]
-    #[should_panic]
-    fn check_assert_fails() {
-        assert_not_started::<(), ()>(Ok(()));
-    }
-
-    #[test]
-    fn cannot_read_if_not_started() {
-        let dev = new_ads1013(&[]);
-        let mut dev = dev.into_continuous().unwrap();
-        assert_not_started(dev.read());
-        destroy_ads1013(dev);
-    }
-}