Fix panic on long waits (#3433)

Co-authored-by: Scott Mabin <scott@mabez.dev>

esp-hal-embassy/CHANGELOG.md

@@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Fixed a panic on very long wakeup times (#3433)
+
 ### Removed
 
 ## [0.7.0] - 2025-02-24

esp-hal-embassy/src/time_driver.rs

@@ -12,7 +12,7 @@ use esp_hal::{
     interrupt::{InterruptHandler, Priority},
     sync::Locked,
     time::{Duration, Instant},
-    timer::OneShotTimer,
+    timer::{Error, OneShotTimer},
 };
 
 pub type Timer = OneShotTimer<'static, Blocking>;
@@ -206,8 +206,20 @@ impl EmbassyTimer {
         let now = Instant::now().duration_since_epoch().as_micros();
 
         if timestamp > now {
-            let timeout = Duration::from_micros(timestamp - now);
+            let mut timeout = Duration::from_micros(timestamp - now);
-            unwrap!(timer.schedule(timeout));
+            loop {
+                // The timer API doesn't let us query a maximum timeout, so let's try backing
+                // off on failure.
+                match timer.schedule(timeout) {
+                    Ok(()) => break,
+                    Err(Error::InvalidTimeout) => {
+                        // It's okay to wake up earlier than scheduled.
+                        timeout = timeout / 2;
+                        assert_ne!(timeout, Duration::ZERO);
+                    }
+                    other => unwrap!(other),
+                }
+            }
             true
         } else {
             // If the timestamp is past, we return `false` to ask embassy to poll again

esp-hal/CHANGELOG.md

@@ -77,6 +77,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Async I2C is doesn't do blocking reads anymore (#3344)
 - Passing an invalid seven bit I2C address is now rejected (#3343)
 - PARL_IO: Use correct max transfer size (#3346)
+- `OneShot` timer now returns an InvalidTimeout from `schedule` instead of panicking (#3433)
 
 ### Removed
 

esp-hal/src/timer/timg.rs

@@ -472,7 +472,9 @@ impl Timer<'_> {
                 let clk_src = Clocks::get().apb_clock;
             }
         }
-        let ticks = timeout_to_ticks(value, clk_src, self.divider());
+        let Some(ticks) = timeout_to_ticks(value, clk_src, self.divider()) else {
+            return Err(Error::InvalidTimeout);
+        };
 
         // The counter is 54-bits wide, so we must ensure that the provided
         // value is not too wide:
@@ -580,13 +582,11 @@ fn ticks_to_timeout(ticks: u64, clock: Rate, divider: u32) -> u64 {
     ticks * period / 1_000_000
 }
 
-fn timeout_to_ticks(timeout: Duration, clock: Rate, divider: u32) -> u64 {
+fn timeout_to_ticks(timeout: Duration, clock: Rate, divider: u32) -> Option<u64> {
     let micros = timeout.as_micros();
+    let ticks_per_sec = (clock.as_hz() / divider) as u64;
 
-    // 1_000_000 is used to get rid of `float` calculations
+    micros.checked_mul(ticks_per_sec).map(|n| n / 1_000_000)
-    let period: u64 = 1_000_000 * 1_000_000 / ((clock.as_hz() / divider) as u64);
-
-    (1_000_000 * micros) / period
 }
 
 /// Behavior of the MWDT stage if it times out.

hil-test/tests/embassy_timers_executors.rs

@@ -6,6 +6,7 @@
 #![no_std]
 #![no_main]
 
+use embassy_futures::select::select;
 use embassy_time::{Duration, Ticker, Timer};
 #[cfg(not(feature = "esp32"))]
 use esp_hal::{
@@ -122,6 +123,7 @@ fn set_up_embassy_with_systimer(peripherals: Peripherals) {
 #[cfg(test)]
 #[embedded_test::tests(default_timeout = 3, executor = hil_test::Executor::new())]
 mod test {
+
     use super::*;
     use crate::test_cases::*;
     #[cfg(not(feature = "esp32"))]
@@ -271,4 +273,16 @@ mod test {
 
         assert!(false, "Test failed after 5 retries");
     }
+
+    /// Test that timg0 and systimer don't have vastly different tick rates.
+    #[test]
+    async fn test_that_a_very_long_wakeup_does_not_panic(peripherals: Peripherals) {
+        set_up_embassy_with_timg0(peripherals);
+
+        select(
+            Timer::after(Duration::from_micros(u64::MAX / 2)),
+            embassy_futures::yield_now(), // we don't actually want to wait forever
+        )
+        .await;
+    }
 }