[RFC] Timer & Alarm

[diondokter]

I propose we add a timer & alarm trait.

Currently the traits and RFC text are hosted here:

• github
• rfc
• crates.io

For general design, let's keep the discussion here.
For specific things like typos, small fixes, let's do that in the linked repo.

[bugadani]

I have a concern that this API is for libraries to use, but imposes no requirements for the range of the timer. If an operation overflows, there is no good way to know by how much it overflows, so a library can't really wait on the timer, short of binary searching for a delay amount that doen't fail and so it can be used to wait for in a loop. Trying to use the "current remaining time" functions is a TOC-TOU issue.

Waiting for any period of time shouldn't overflow in my opinion.

[jannic]

The "elapsed time is rounded down" requirements are tricky to get right for tick-based timers. The first tick may happen immediately after calling .start(), so all calculations must subtract one from the tick difference - making sure to not underflow below zero.
Eg., for a timer with one tick per microsecond:

    fn elapsed_micros(&self) -> Result<u64, embedded_hal_timer::OverflowError> {
        let ticks = self.elapsed_ticks().unwrap().saturating_sub(1);
        Ok(ticks)
    }

And effectively, this may do more than rounding down. The actual elapsed time may be 1.9µs, but only one tick happened in the mean time, so elapsed_micros must return 0.

To be clear, I think it is a good thing to clearly specify such behavior, so just removing the requirement wouldn't be better. Not sure what's best. 🤷

[diondokter]

Trying to use the "current remaining time" functions is a TOC-TOU issue.

True for virtual timers yes. Though that can be fixed by changing the requirement to give the max time the timer support from the perspective of the last start call. Then you could call start, query the max and see if there's enough space.

I have a concern that this API is for libraries to use, but imposes no requirements for the range of the timer.

The trait itself doesn't impose requirements no, just like the SpiDevice trait doesn't impose the frequency of the bus.
This is something the user should document: The timer must be able to run for at least a minute without overflow with a precision of at least 5ms or something like that

[bugadani]

The trait itself doesn't impose requirements no, just like the SpiDevice trait doesn't impose the frequency of the bus.

I find the analogy and the reasoning itself to be lacking tbh, but I can't really put into words why. But for SpiDevice the requirement usually comes from the hardware, not from the driver.

Next concern: there is no way to tell if a Timer is running. If start can restart the Timer, and users generally won't know if the timer is running, they may defensively call start without regard of other users of the same Timer.

[diondokter]

they may defensively call start without regard of other users of the same Timer.

There are no other users. start requires &mut Self

[jannic]

Instead of the max_ methods, I suggest to add a single guaranteed_range_ticks() -> u64 that returns the amount of ticks that are guaranteed to elapse without an overflow. That should be a constant as in "doesn't change between two calls", so a user only needs to call it once to decide if the timer fulfills the requirements.

In addition, we could add a has_overflow_detection() -> bool. If this is true, the driver reliably returns an overflow error if the timer did overflow.

[bugadani]

There are no other users. start requires &mut Self

Consider the following:

fn caller(mut timer: &mut impl Timer) {
    timer.start();
    third_party_driver::use_timer(timer);
    let elapsed = timer.elapsed();
}

mod third_party_driver {
    fn use_timer(mut timer: &mut impl Timer) {
        // not under our control, calls timer.start();
    }
}

Sure, it may be contrived, but nothing prevents this.

Thirdly, what's the expected behaviour for Alarm methods when the Timer hasn't been started?