stm32-rs / stm32l0xx-hal

A hardware abstraction layer (HAL) for the STM32L0 series microcontrollers written in Rust
BSD Zero Clause License
96 stars 60 forks source link

Add `Timer::new` to create timer without starting #152

Closed michaelbeaumont closed 3 years ago

michaelbeaumont commented 3 years ago

Useful for initializing a Timer to call start() on later.

I suppose timer() creates it and starts it as a sort of convenience, but it's a little silly having to pass a dummy timeout. More seriously, calling timer() with a dummy timeout but without listen() and later calling listen() and start() can cause a subtle issue, I believe. If the dummy timeout has already overflowed, the interrupt is already pending and will immediately fire, before the new start has expired (necessitating a clear_irq() call in addition to listen and start).

This PR doesn't fix that issue per se, it only avoids it for creation but I think the expected behavior for start() would be that it also calls clear_irq, WDYT?

This is a breaking change for the GeneralPurposeTimer trait but it feels reasonable, though we could also add an additional trait to hold enable.

dbrgn commented 3 years ago

Thanks! This looks reasonable to me, but could you add documentation to the trait and its methods?

(Also, we should really set up a CHANGELOG. I think I'll open a PR with a suggestion.)

michaelbeaumont commented 3 years ago

Done, let me know if that's sufficient. I think the trait is sort of only internally used at the moment, so there's not much to say about them. I added changelog entries as well.

dbrgn commented 3 years ago

Thanks!