Skip to content

Add Watchdog trait#13

Open
williampMSFT wants to merge 2 commits intoOpenDevicePartnership:mainfrom
williampMSFT:user/williamp/watchdog
Open

Add Watchdog trait#13
williampMSFT wants to merge 2 commits intoOpenDevicePartnership:mainfrom
williampMSFT:user/williamp/watchdog

Conversation

@williampMSFT
Copy link
Contributor

This adds a trait to feed a watchdog timer

fn feed(&mut self) -> Result<(), Self::Error>;
}

impl<T: Watchdog> Watchdog for &mut T {
Copy link
Contributor Author

@williampMSFT williampMSFT Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looked odd to my eye, but apparently this pattern was established in embedded-hal by rust-embedded/embedded-hal#310. I guess the idea is that this lets a function that takes an impl Watchdog as an argument be passed a &mut impl Watchdog and still work? It's not clear to me why you wouldn't just say that functions have to declare that their argument type is &mut impl Watchdog instead, but I think we want to align on what embedded-hal is doing, so I went with it. If anyone knows what the upside of doing this is I'd be interested in your take on it :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah as we discussed offline, I think this pattern makes sense in some contexts/for some traits, but for a Watchdog specifically I can't think of a particular use case. Though I don't think it will necessarily hurt to have it here anyway as if someone really wants to pass an &mut impl Watchdog to a generic function there's nothing necessarily unsafe or problematic about that, since the compiler will just complain if they try to do anything funny.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I do understand your concern that the blanket impl means a function that takes an impl Watchdog means we can't be certain if it will take an owned T or a &T and don't know if the watchdog will be dropped and thus disabled. I guess my thoughts are the user is responsible for understanding that (e.g. if they pass an owned watchdog they should understand it will be dropped).

//! Traits for interactions with a processor's watchdog timer.

/// Feeds an existing watchdog to ensure the processor isn't reset.
pub trait Watchdog {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently there was previously a HAL Watchdog type in embedded-hal, but it was removed by rust-embedded/embedded-hal#324 because it had an unconstrained time type that it was using in the Enable trait, and there was a push to remove all unconstrained time types before the 1.0 release because they're not very useful for writing generic code.

I looked into what we might be able to do to solve for that, but from looking at the spec sheets for a few watchdogs, I came to the conclusion that there may not be a great hardware-agnostic way to describe configuration of a watchdog. Some watchdogs have multiple channels, some have a minimum delay between feedings in addition to a maximum delay, etc.

My current thinking is that if we introduce this feeding trait, that will allow for writing things like a generic service to 'share' a watchdog among a few different clients (i.e. only feed the hardware watchdog when X different things have fed the service since the last feeding of the hardware watchdog), but we can leave configuration to individual HALs. If anyone has thoughts on better options I'd love to hear them, though :)

Copy link

@kurtjd kurtjd Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I lean towards traits shouldn't try to describe HW configuration, it's too much of a headache. I think configuration should all be done before the peripheral is used in any generic context.

@williampMSFT williampMSFT marked this pull request as ready for review March 9, 2026 23:42
@williampMSFT williampMSFT requested a review from a team as a code owner March 9, 2026 23:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new Watchdog trait to the embedded-mcu-hal crate, providing an abstraction for feeding a processor's watchdog timer to prevent resets. It includes a blanket implementation for &mut T references.

Changes:

  • Added a new Watchdog trait with an associated Error type and a feed method
  • Added a blanket impl of Watchdog for &mut T where T: Watchdog
  • Registered the new watchdog module as a public module in lib.rs

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
embedded-mcu-hal/src/watchdog.rs New file defining the Watchdog trait with a feed method and a blanket impl for &mut T
embedded-mcu-hal/src/lib.rs Adds pub mod watchdog; to expose the new module

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants