Skip to content

Add timer #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add timer #59

wants to merge 2 commits into from

Conversation

tytouf
Copy link

@tytouf tytouf commented Jun 12, 2025

Copy code from stm32h7 to implement basic timer on stm32h5.
Add an example using Timer2: blinky_timer

@tytouf tytouf mentioned this pull request Jun 12, 2025
Copy link
Contributor

@astapleton astapleton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The main changes I think this needs:

  • remove the dependency on embedded-hal 0.2 (see comments), and replace the functionality that the trait implementations provided
  • remove references of the stm32h7xx repository (unless intentional)

@@ -60,10 +60,14 @@ log-semihost = ["log"]
cortex-m = { version = "^0.7.7", features = ["critical-section-single-core"] }
stm32h5 = { package = "stm32h5", version = "0.16.0" }
fugit = "0.3.7"
embedded-hal-02 = { package = "embedded-hal", version = "0.2.6", features = ["unproven"] }
Copy link
Contributor

@astapleton astapleton Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any reason to support embedded-hal 0.2 at this point. I've intentionally avoided doing so because embedded-hal 1.0 was released before any progress was made on this project.

embedded-hal = "1.0.0"
defmt = { version = "1.0.0", optional = true }
paste = "1.0.15"
log = { version = "0.4.20", optional = true}
nb = "1.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've avoided using the nb crate due to previous feedback that it's likely to be deprecated. See this thread: #27 (comment)

Comment on lines +5 to +6
//! - [Blinky using a Timer](https://github.com/stm32-rs/stm32h7xx-hal/blob/master/examples/blinky_timer.rs)
//! - [64 bit microsecond timer](https://github.com/stm32-rs/stm32h7xx-hal/blob/master/examples/tick_timer.rs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of mentions and links to stm32h7xx-hal in this file

Comment on lines +99 to +101
impl embedded_hal_02::timer::Periodic for Timer<$TIMX> {}

impl embedded_hal_02::timer::CountDown for Timer<$TIMX> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, there's no need to support embedded-hal 0.2 in this crate, so we can remove these trait implementations (although we'll probably need to keep a similar start method)

impl Timer<$TIMX> {
/// Configures a TIM peripheral as a periodic count down timer,
/// without starting it
pub fn $timX(tim: $TIMX, prec: rec::$Rec, clocks: &CoreClocks) -> Self
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be public, as the expectation is that one uses the TIMX.timer(...) method to get a Timer instance.

Comment on lines +34 to +35
fn get_clk(clocks: &CoreClocks) -> Option<Hertz> {
Some(clocks.$ckX())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to return an Option here as the clock, as stored in CoreClocks is not an optional. It will always be valid.

embedded-hal = "1.0.0"
defmt = { version = "1.0.0", optional = true }
paste = "1.0.15"
log = { version = "0.4.20", optional = true}
nb = "1.0.0"
void = { version = "1.0.2", default-features = false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will be unnecessary with the embedded-hal 0.2 dependency removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants