Arkaleks / ms5611

no_std driver for the MS5611 (barometric pressure sensor)
Apache License 2.0
5 stars 2 forks source link

Use Timer instead of Delay #3

Open Arkaleks opened 5 years ago

Arkaleks commented 5 years ago

Current implementation of this driver takes ownership of a Delay in order to block get_compensated_sample() and alike operations. While this is fine as a very simple solution, not all implementations of embedded_hal provide multiple Delay object, e.g. atsamd implementation only provides one Delay which is using Systick as delay provider.

Instead of replacing the API to borrow a Delay whenever an operation need it, the API could be changed to be non-blocking by using a Timer:

// Old
pub fn get_sample(&mut self, osr: Oversampling) -> Result<Sample, E> {}

// New
pub fn get_sample(&mut self, osr: Oversampling) -> nb::Result<Sample, Error<SPI>>{}

Where get_sample``returnsnb::WouldBlock ` as long as a conversion is ongoing, allowing user's code to do other operation in the mean time.

Arkaleks commented 5 years ago

It's not clear how to use human time (milliseconds) with the trait CountDown. Most embedded-hal implementations use clock as a time reference, hence Time = Herz. Because external trait cannot be implemented for external types, something like the following is not possible:

mod my_device {
pub struct MyDevice<T>(T,)

impl<T, U> MyDevice<T>
where
    T: hal::timer::CountDown<Time=U>
    U: From<Milliseconds>
{
    pub fn new(timer: T) -> Self {
        let mut dev = Self(timer);
        dev.start(Milliseconds(3));
        (dev)
   }

    pub fn wait(&mut self) -> Result<(), Void> {
        (self.timer.wait())
    }
}

pub struct Milliseconds(u8);
}

// ...
fn main() {}

// Not possible
impl From<my_device::Milliseconds> for hal::Herz {
    fn from(duration: my_device::Milliseconds) -> Self {
        Herz(duration.0 * 1000)
    }
}

Adding a conversion trait could be a solution however that doesn't sound really like a good one.

Ref: rust-embedded/embedded-hal#129