Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
DrTobe opened this issue Nov 10, 2022 · 0 comments
Open

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

DrTobe opened this issue Nov 10, 2022 · 0 comments

Comments

@DrTobe
Copy link

DrTobe commented Nov 10, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant