Clocks/Delay papercut.

[jonathanpallant]

You have the system clock speed in the results of hal::clocks::init_clocks_and_plls. But you can't just pass it to cortex_m::delay::Delay. And it feels like you should be able to.

[jannic]

What's wrong with the current situation?

      let clocks = init_clocks_and_plls( [...] ).ok().unwrap();
      let sys_hz: u32 = clocks.system_clock.freq().integer();
      let mut delay = cortex_m::delay::Delay::new(core.SYST, sys_hz);

Too complicated?

[jonathanpallant]

Yes. At worst it should be:

      let clocks = init_clocks_and_plls( [...] ).ok().unwrap();
      let mut delay = cortex_m::delay::Delay::new(core.SYST, clocks.system_clock.into());

Don't forget your example requires I think two traits to be in scope.

[jannic]

Looking at other HALs, one option is to just not use cortex_m::delay::Delay but implement our own Delay, like here:

• https://github.com/stm32-rs/stm32f3xx-hal/blob/master/src/delay.rs
• https://github.com/atsamd-rs/atsamd/blob/master/hal/src/delay.rs

Interestingly, I found those by searching for Delay::new in the https://github.com/rust-embedded organization, the very one where the cortex_m crate also lives. Perhaps cortex_m::delay::Delay is just too low-level?

Also noteworthy: rust-embedded/embedded-hal#103

[jannic]

@jonathanpallant: Now that Timer implements the delay traits, it shouldn't be necessary to use cortex_m::delay::Delay. Can we close this ticket, or do you think it's still worth improving the usability of cortex_m::delay::Delay?

The usage with Timer is now:

let clocks = init_clocks_and_plls( [...] ).ok().unwrap();
let mut timer = rp2040_hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks);
timer.delay_ms(500);

[jannic]

As mentioned above, I think this is solved with the current API. Timer should be the common delay implementation, so the usability of cortex_m::delay::Delay isn't as important as before.

@thejpster, feel free to reopen if you disagree.