Skip to content

Comparison of sub-second duration::Generic is broken #127

@DrTobe

Description

@DrTobe

When two Instants are subtracted, this results in a duration::Generic. This happens, for example, if we want to measure how much time has passed between two moments in time. To compare such a duration to another duration, the other one has to be converted into a duration::Generic, too. Unfortunately, comparison between two duration::Generics is broken:

use embedded_time::{clock::Clock, fraction::Fraction, Instant, duration::Milliseconds};

struct TwoInstantClock {
    instants: core::cell::RefCell<Vec<u32>>,
}

impl Clock for TwoInstantClock {
    type T = u32;
    const SCALING_FACTOR: Fraction = Fraction::new(1, 1000);
    fn try_now(&self) -> Result<Instant<Self>, embedded_time::clock::Error> {
        let i = self.instants.borrow_mut().remove(0);
        Ok(Instant::new(i))
    }
}

fn main() {
    let clock = TwoInstantClock {
        instants: core::cell::RefCell::new(vec![0, 5]),
    };
    let zero = clock.try_now().unwrap();
    let five = clock.try_now().unwrap();
    println!("{}", five-zero > Milliseconds(0u32).into()); // prints true!
}

The problem here is that for comparison, the integer part of a duration::Generic is multiplied by the scaling factor, i.e. this roughly floors the duration to full seconds. For sub-second durations, which can be expected to happen a lot in embedded contexts, this just does not work because both values will be zero.

This could be changed to compare self.integer * self.numerator * rhs.denominator to rhs.integer * rhs.numerator * self.denominator but this drastically increases the possibility of overflows. More elaborate comparison schemes are imaginable but may be costly. I would assume that overflows or errors can not be avoided completely without increasing the bit width of the numbers involved.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions