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

Alternative Delay implementation #169

Closed
glindstedt opened this issue Nov 14, 2020 · 2 comments · Fixed by #170
Closed

Alternative Delay implementation #169

glindstedt opened this issue Nov 14, 2020 · 2 comments · Fixed by #170

Comments

@glindstedt
Copy link
Contributor

If I want to use scheduling in rtic the init function does not have access to the SYST timer as described here (end paragraph of first section): https://rtic.rs/0.5/book/en/by-example/timer-queue.html.

This means that there's no way to construct the current delay implementation which requires the SYST timer.

This comment on the issue over at cortex-m-rtic suggest that HAL crates provide alternate Delay implementations to support this use case, potentially using cortex_m::asm::delay.

I tested this a bit locally but a delay based on cortex_m::asm::delay seemed pretty unstable and delayed for less time than I had calculated. Later I found this issue seem to explain the problem: rust-embedded/cortex-m#236
The gist of it being that the current implementation is only accurate on M3/M4.

As far as I understand though, Delay in this crate is only really used for startup configuration where one has to wait at least some amount of time, so it could still be useful to have an implementation based on cortex_m::asm::delay if it's augmented to account for the inaccuracy of the current implementation, and properly documented to inform the user.

If this seems reasonable I could provide a PR for adding this.

@richardeoin
Copy link
Member

Thanks for the thorough summary of the problem! I think it would be reasonable to offer alternative delay implementations in the HAL, if only because it's a small piece of functionality to require an extra crate for.

I have this snippet for those times when I need multiple objects that implement DelayMs/DelayUs https://gist.github.com/richardeoin/ba57bbc0618676100bbffe2ae28d05bc Basing the implementation on a hardware timer avoids worrying about the core implementation. Feel free to use that snippet in a PR if you think it would be useful.

@glindstedt
Copy link
Contributor Author

The gist you linked was excellent, so I just used that without modification. It's much better than relying on the cortex_m::asm::delay function. I only had to change a few methods to take trait implementors instead of the Delay struct to make it work.

@bors bors bot closed this as completed in f3e10a1 Jan 10, 2021
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 a pull request may close this issue.

2 participants