Skip to content
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

Fix off-by-one-tick in timer period calculation #491

Merged
merged 1 commit into from
Apr 28, 2024

Conversation

dne
Copy link
Contributor

@dne dne commented Apr 24, 2024

STM32 timers, in upcounting mode, count from 0 to ARR (inclusive) before restarting at 0/generating an overflow event, so we need to subtract 1 from the number of desired ticks to get the correct period.

@richardeoin
Copy link
Member

Thanks for the PR and catching this off-by-one error! This change invalidates a doc comment on timer.rs:427, with this change the comment needs to state that the counter reaches the ARR after N-1 ticks and restarts at zero after N ticks. Could you update it?
This change will be marked Breaking in the CHANGELOG, feel free to do this now or I can add it later.

@dne dne force-pushed the timer-freq-off-by-one branch from eb4532d to dc6338c Compare April 26, 2024 22:17
@dne
Copy link
Contributor Author

dne commented Apr 26, 2024

This change invalidates a doc comment on timer.rs:427, with this change the comment needs to state that the counter reaches the ARR after N-1 ticks and restarts at zero after N ticks. Could you update it?

Thanks for the review. I've updated the doc comment I had missed.

Copy link
Member

@richardeoin richardeoin left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@richardeoin richardeoin added this pull request to the merge queue Apr 28, 2024
Merged via the queue into stm32-rs:master with commit ac64582 Apr 28, 2024
22 checks passed
@dne dne deleted the timer-freq-off-by-one branch April 28, 2024 21:39
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