-
Notifications
You must be signed in to change notification settings - Fork 48
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
Use Delay implementation from cortex-m crate. #8
Conversation
fn delay_ms(&mut self, ms: u16) { | ||
self.delay_ms(ms as u32); | ||
} | ||
pub trait SYSTDelayExt { |
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.
Renamed this because the DelayExt
would fit more for extensions of Delay
since this crate isn't yet widely used and is v0.0.0
, changes like this could be acceptable.
fn delay(self, clocks: &Clocks) -> Delay { | ||
Delay::new(self, clocks) | ||
impl DelayExt for Delay { | ||
fn delay<T>(&mut self, delay: T) |
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.
I contemplated removing this function, but time
is a nicer abstraction than simple u32
. Possibly in the future if cortex-m
starts using embedded-time
they will have this interface too.
The implementation from cortex-m crate uses the Core-provided clock for the timer instead of the default External this gives 8 times better resolution. Additionally the crate's implementation can handle much longer delays as it uses 64 bit integer to calulate needed cycles for sleep.
99a48b7
to
2383283
Compare
@dotcypress, I switched the Delay implementation to use one from cortex_m. Its not a critical change :) but brings in some nice improvements over the previous implementation - namely around handling larger intervals. Can you take look ? |
@pawelchcki yup, will take a look but a bit later today or tomorrow. |
Thanks @dotcypress! Absolutely no rush on this PR as its mostly cosmetics. But figured I'll use my weekly quota of pinging You directly on it 😂.
Likewise! |
Thanks ! |
While working on other changes I noticed the latest
cortex-m
crate got the Delay implementation that is tiny bit more powerful than the current one in this crate. So I updated the dependency and removed most of Delay implementation, I left thetime
based function as a trait for the mainDelay
, and added them toprelude
, not sure if that is the best approach so any comments highly welcome.The implementation from
cortex-m
crate uses the Core-provided clockfor the timer instead of the default External this gives 8 times better resolution.
Additionally the crate's implementation can handle much longer delays
as it uses 64 bit integer to calculate needed cycles for sleep.