rust-embedded/embedded-hal

Consider improving watchdog API using move semantics

hannobraun opened this issue · 3 comments

@astro suggested the following:

/// Feeds an existing watchdog to ensure the processor isn't reset. Sometimes
/// commonly referred to as "kicking" or "refreshing".
#[cfg(feature = "unproven")]
pub trait Watchdog {
    /// Triggers the watchdog. This must be done once the watchdog is started
    /// to prevent the processor being reset.
    fn feed(&mut self);
}

/// Enables A watchdog timer to reset the processor if software is frozen or 
/// stalled.
pub trait WatchdogEnable {
    /// Unit of time used by the watchdog
    type Time;
    /// The started watchdog that should be `feed()`
    type Target: Watchdog;
    /// Starts the watchdog with a given period, typically once this is done 
    /// the watchdog needs to be kicked periodically or the processor is reset. 
    ///
    /// This consumes the value and returns the `Watchdog` trait that you must
    /// `feed()`.
    fn start<T>(self, period: T) -> Self::Target where T: Into<Self::Time>;
}

/// Disables a running watchdog timer so the processor won't be reset.
#[cfg(feature = "unproven")]
pub trait WatchdogDisable {
    type Target: WatchdogEnable;
    /// Disables the watchdog
    ///
    /// This stops the watchdog and returns the `WatchdogEnable` trait so that
    /// it can be started again.
    fn disable(self) -> Self::Target;
}

I think this is an interesting approach that's worth thinking about.

This would work well on my HAL crate: https://github.com/gd32v-rust/gd32vf103-hal/blob/68a9d3b594038480504e3b52abea1a48aefaf148/src/wdog.rs

This new API could support the following two scenarios:

  1. The Watchdog after enabled is turned into another struct, like Watchdog<Enabled> or other probable designs. This could take ownership of old struct to prevent double start calls in compile time, and prevent disable calls if the watchdog is already disabled.

  2. If we don't need to implement different traits and functions for a disabled watchdog and an enabled one, we may use type Target = Self, which can also be adapted to the older WatchdogEnable trait.

By now the current API only support the second one. By this new design we support both second one and the first one, and fallback to second one if library designers still uses the second one by now.

PR #222 by @luojia65 was merged and will be released in v1.0.0.
Can this be closed now?

Yes, I think so.