-
Notifications
You must be signed in to change notification settings - Fork 39
Independent Watchdog peripheral #118
base: master
Are you sure you want to change the base?
Conversation
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.
You forgot to implement WatchdogDisable.
The datasheet doesn't mention anything about stopping the IWDG. Is there an unofficial way? |
Sorry I can't find any way to implement DisableWatchdog. Sorry for the noise. |
Rebased and addressed your concerns. |
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 added some comments about potential improvements.
|
||
/// Reason for controller reset | ||
#[derive(Clone, Copy, Debug)] | ||
pub enum ResetReason { |
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.
How about adding documentation on the variants of this enum? I would say some are not immediately obvious.
pub const MAX_TIMEOUT_MS: u32 = 26214; | ||
|
||
const MAX_RELOAD: u16 = 0xFFF; | ||
const MAX_PRESCALE: u8 = 6; |
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.
It would be nice to add some comment on what this prescale value corresponds to, if that makes sense.
impl WatchdogEnable for IndependentWatchdog { | ||
type Time = u32; | ||
|
||
fn start<T>(&mut self, period: T) where T: Into<Self::Time> { |
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 think it would be good to add some information about the requirements of period
and state what happens when these checks fail. (e.g. max timeout check)
@@ -20,6 +21,10 @@ pub struct KiloHertz(pub u32); | |||
#[derive(Clone, Copy)] | |||
pub struct MegaHertz(pub u32); |
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.
Should we make the other structs also PartialOrd
and PartialEq
?
Depends on the unproven+unreleased watchdog traits from
embedded-hal