-
Notifications
You must be signed in to change notification settings - Fork 1
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
delay: Add delay module #23
Conversation
d894ad9
to
549d8b1
Compare
Have you done any timing tests on the implementation to make sure it's correct? |
I believe so, but I'll play around with the blinky example and double check that. |
I ran some tests and it's a few milliseconds fast per second if I sleep repeatedly for a second. Not sure if that's worth chasing down though: it could just be core clock inaccuracy (due to internal reference). |
Hmm, a delay should always delay for the minimum time specified. If it can delay less, that sounds problematic |
I'll look further into it then |
@ryan-summers From what I can tell, it looks like the slight delay inaccuracy is due to inaccuracy in the HSI with which I'm running the MCUs I have. I tested other clocks and they're also running a little faster than the configured frequency (which translate to HSI deviation within datasheet limits). I did pick up a bug in the original implementation that was making the delay actually slightly slower than it should have been and have corrected the implementation in the latest commit. I also looked at the So I think with the latest fix, this delay logic is correct. And if one wants more accurate delays, one should probably use an external oscillator, or trim the HSI differently from the factory cal somehow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me :) Feel free to merge when you feel its ready! Thanks for diving into it
This implements the embedded-hal 1.0
DelayNs
trait. The cortex-m crate has essentially the same logic, but the published version is based on embedded-hal 0.2. This can be removed (repurposed for use with timers?) when a new version of the cortex-m crate is published.The blinky example has been updated to use the delay module.