Allow a way to get >1 impl Delay type

[tgross35]

It seems like at this time, it is only possible to get one Delay : atsamd_hal::delay::Delay, which consumes the systick. I brought this issue up on the embedded HAL crate, and their suggestion was that there should be a way to get more than one delay via Clone` or otherwise. rust-embedded/embedded-hal#435

I'm not sure what the best way to go about this is, but it should be possible via this HAL. Being able to get more impl CountDown objects would be similarly helpful.

[bradleyharden]

@tgross35, yes, that seems like a reasonable feature. Feel free to make a PR. I'm not really familiar with the Delay API, but other maintainers should be.

[tgross35]

The inner workings aren't too complicated, but I am really not sure what the best way forward is that also avoids race conditions. This chunk specifically definitely seems problematic

atsamd/hal/src/delay.rs

Lines 65 to 67 in 7d85efd

	self.syst.set_reload(current_rvr);
	self.syst.clear_current();
	self.syst.enable_counter();

I don't mind doing the work, but I would appreciate some input from the proper maintainers if you know who to ping

[Sympatron]

A shallow google search seems to indicate that most (at least all I could find) Delay implementations work basically the same. They set the match register, reset the timer and wait synchronously (in a busy loop) for a match. No implementations I could find tries to be cloneable.

The only way I can see it work is by wrapping the Delay in a Mutex.
To make it compatible with APIs requiring a Delay object, one could perhaps implement Delay for Mutex<Delay> (or some kind of wrapper around that to avoid orphan rules).

That being said, I am no expert in Rust, so maybe there is another way.

[tgross35]

A mutex or RefCell is what jumped into my mind as well, but it doesn't seem like a good idea to add that overhead when some delays may be very short.

I am curious what Adam comes up with for cortex M, as mentioned here rust-embedded/embedded-hal#435 (comment)

edit: scratch my ISR example since I just realized the peripheral would have to be in a mutex anyway

[tgross35]

I think you could do things properly if the current Delay is used and allowed to Clone (with some interior mutability on SYST), but it would need to be !Send to make sure you couldn’t send it between threads or e.g. to an interrupt handler. But this would be a breaking change

[Sympatron]

If it's !Send anyway your should be able to get away with a RefCell because it can't be accessed concurrently in that case. I think that would work, but then you definitely cannot use it in ISRs without a Mutex.

[tgross35]

Qutoing from Adam's suggestion in rust-embedded/embedded-hal#435 (comment):

3. A struct that consumes SysTick (maybe with a release() method), configures it to run at some known rate, and then allows you to create as many new Delay objects as you want which only read the systick counter to work out how long has passed.

... For a HAL, something like option 3 but using an actual hardware timer is best, I think. It "uses up" the timer to ensure it's always running at a known frequency, then each of the Delays just watches until enough counts have passed on the timer (handling wraparound as required).

I would assume that changing the current Delay API is not acceptable, so perhaps something like this would work:

• Delay struct keeps its current external API
• Delay also allows a way to get a BorrowedDelay or something that takes an & to the original delay (thus in this case, forbidding Delay::delay_ms(&mut self)).
• Delay should keep SYST within an UnsafeCell to allow for interior mutability. It should provide a private API for the BorrowedDelays to get a simple counter of the systick plus something a wrap counter that they could compare to their timeouts
• BorrowedDelay would implement embedded_hal::Delay just using this API, rathter than going via the systick reload register

That way, there would only ever be one instance of Delay that interacts with the systick so worries about Send go away.

Alternatively, an entirely separate set of types like SharedDelay and BorrowedDelay could be introduced, or change the current Delay API.

Does any of this sound reasonable?

[jbeaurivage]

@tgross35, another approach would be using a timer queue. When scheduling timeouts, they are added to a queue, and the timer can then generate an event when the next timeout expires. This enables the timer to be Send + Sync and even be used using an API based on shared references only. RTIC has an utility crate that does exactly this (rtic-monotonics), though it's based on an async API. We could hopefully leverage this instead of rolling our own!

PS. there are ways to use async constructs in a sync program using simple, lightweight executors such as spin_on.