FluenTech / embedded-time

Time(ing) library (Instant/Duration/Clock/Timer/Period/Frequency) for bare-metal embedded systems
Apache License 2.0
87 stars 17 forks source link

Duration type conversions are difficult to use in practice #121

Open ryan-summers opened 2 years ago

ryan-summers commented 2 years ago

In a hypothetical scenario where a user wants to divide a Seconds<u32> in half, and the value is unknown, the library performs truncation on the result because the result type assumes the LHS type. For example, Seconds(1) / 2 is equal to zero duration. This is obviously not desired, so code should protect against this condition.

To guard against this, a logical step is to then instead convert from seconds -> milliseconds before dividing. Milliseconds::from(Seconds(1)) / 2. However, because From<Seconds> is not implemented for Milliseconds<u32>, but rather only Milliseconds<u64>, this means that a library that is generic over the Clock has to implement an additional traitbound over the clock type.

fn half_duration_in_future<C>(seconds: u32, clock: C) -> Instant<C>
where
    C: embedded_time::Clock,

    // Necessary trait bound to divide seconds reliably
    C::T: core::convert::TryFrom<u64>
{
    let now = clock.try_now().unwrap();

    let step = Milliseconds::from(Seconds(seconds)) / 2;

    now + step
} 

This then propagates throughout the whole code base, as all functions taking the generic clock are also required to carry the C::T: core::convert::TryFrom<u64> bound as well (e.g. if the division is done on some internal object).

ryan-summers commented 2 years ago

One work around to this is to just directly construct Milliseconds<u32> from the internal seconds:

let seconds = Seconds(1);
let ms = (seconds.0 * 1000).milliseconds();

let step = ms / 2;

But it would be much nicer to expose a TryFrom<Seconds<u32>> for Milliseconds<u32> and let the application determine whether or not unwrapping is safe (in our case, Seconds is actually bounded to 16-bit anyways, so an unwrap is always safe)