Bugfixing; async timer example

examples/timer_async.rs

@@ -0,0 +1,19 @@
+use esp_idf_hal::sys::EspError;
+
+fn main() -> Result<(), EspError> {
+    // It is necessary to call this function once. Otherwise some patches to the runtime
+    // implemented by esp-idf-sys might not link properly. See https://github.com/esp-rs/esp-idf-template/issues/71
+    esp_idf_hal::sys::link_patches();
+
+    let per = esp_idf_hal::peripherals::Peripherals::take().unwrap();
+
+    let timer_conf = esp_idf_hal::timer::config::Config::new().auto_reload(true);
+    let mut timer = esp_idf_hal::timer::TimerDriver::new(per.timer00, &timer_conf)?;
+
+    esp_idf_hal::task::block_on(async move {
+        loop {
+            timer.delay(timer.tick_hz()).await?; // Every second
+            println!("Tick");
+        }
+    })
+}

examples/timer_notify.rs

@@ -1,6 +1,6 @@
 use std::num::NonZeroU32;
 
-use esp_idf_hal::sys::{EspError, TaskHandle_t}; // If using the `binstart` feature of `esp-idf-sys`, always keep this module imported
+use esp_idf_hal::{sys::EspError, task::notification::Notification}; // If using the `binstart` feature of `esp-idf-sys`, always keep this module imported
 
 fn main() -> Result<(), EspError> {
     // It is necessary to call this function once. Otherwise some patches to the runtime
@@ -9,9 +9,8 @@ fn main() -> Result<(), EspError> {
 
     let per = esp_idf_hal::peripherals::Peripherals::take().unwrap();
 
-    // This handle will be used as the address for the event in the callback.
+    // A safer abstraction over FreeRTOS/ESP-IDF task notifications.
-    // Make sure that the handle / thread lives always longer as the callback it is used in
+    let notification = Notification::new();
-    let main_task_handle: TaskHandle_t = esp_idf_hal::task::current().unwrap();
 
     // BaseClock for the Timer is the APB_CLK that is running on 80MHz at default
     // The default clock-divider is -> 80
@@ -19,30 +18,29 @@ fn main() -> Result<(), EspError> {
     let timer_conf = esp_idf_hal::timer::config::Config::new().auto_reload(true);
     let mut timer = esp_idf_hal::timer::TimerDriver::new(per.timer00, &timer_conf)?;
 
-    // Calculate value needed for alarm in seconds
+    // Every half a second
-    // (APB_CLK_FREQ / DEVIDER ) * seconds = count
+    timer.set_alarm(timer.tick_hz() / 2)?;
-    // example every 200 us
-    // ( 80*10^6 / 80 ) * 200 *10^(-6) = 200
-    timer.set_alarm(200)?;
 
-    // Saftey: make sure the task handle stays valid for longer than the subscribtion
+    let notifier = notification.notifier();
-    // is active
+
+    // Saftey: make sure the `Notification` object is not dropped while the subscription is active
     unsafe {
         timer.subscribe(move || {
             let bitset = 0b10001010101;
-            esp_idf_hal::task::notify_and_yield(main_task_handle, NonZeroU32::new(bitset).unwrap());
+            notifier.notify_and_yield(NonZeroU32::new(bitset).unwrap());
         })?;
     }
 
+    timer.enable_interrupt()?;
     timer.enable_alarm(true)?;
     timer.enable(true)?;
 
     loop {
         // Notify approach
         // The benefit with this approach over checking a global static variable is
-        // that the scheduler can hold the task, and resume when signaled
+        // that the scheduler can block the task, and quickly resume it when notified
-        // so no spinlock is needed
+        // so no spinlock is needed / the CPU does not waste cycles.
-        let bitset = esp_idf_hal::task::wait_notification(esp_idf_hal::delay::BLOCK);
+        let bitset = notification.wait(esp_idf_hal::delay::BLOCK);
 
         if let Some(bitset) = bitset {
             println!("got event with bits {bitset:#b} from ISR");

src/gpio.rs

@@ -1134,6 +1134,14 @@ impl<'d, T: Pin, MODE> PinDriver<'d, T, MODE> {
         Ok(())
     }
 
+    /// Subscribes the provided callback for ISR notifications.
+    /// As a side effect, interrupts will be disabled, so to receive a notification, one has
+    /// to also call `PinDriver::enable_interrupt` after calling this method.
+    ///
+    /// Note that `PinDriver::enable_interrupt` should also be called after
+    /// each received notification **from non-ISR context**, because the driver will automatically
+    /// disable ISR interrupts on each received ISR notification (so as to avoid IWDT triggers).
+    ///
     /// # Safety
     ///
     /// Care should be taken not to call STD, libc or FreeRTOS APIs (except for a few allowed ones)
@@ -1151,7 +1159,7 @@ impl<'d, T: Pin, MODE> PinDriver<'d, T, MODE> {
         chip::PIN_ISR_HANDLER[self.pin.pin() as usize] =
             Some(unsafe { core::mem::transmute(callback) });
 
-        self.enable_interrupt()
+        Ok(())
     }
 
     #[cfg(not(feature = "riscv-ulp-hal"))]

src/timer.rs

@@ -74,6 +74,7 @@ pub trait Timer: Send {
 pub struct TimerDriver<'d> {
     timer: u8,
     divider: u32,
+    isr_registered: bool,
     _p: PhantomData<&'d mut ()>,
 }
 
@@ -112,11 +113,12 @@ impl<'d> TimerDriver<'d> {
         Ok(Self {
             timer: ((TIMER::group() as u8) << 4) | (TIMER::index() as u8),
             divider: config.divider,
+            isr_registered: false,
             _p: PhantomData,
         })
     }
 
-    pub fn tick_hz(&self) -> u32 {
+    pub fn tick_hz(&self) -> u64 {
         let hz;
 
         #[cfg(esp_idf_version_major = "4")]
@@ -129,7 +131,7 @@ impl<'d> TimerDriver<'d> {
             hz = APB_CLK_FREQ / self.divider;
         }
 
-        hz
+        hz as _
     }
 
     pub fn enable(&mut self, enable: bool) -> Result<(), EspError> {
@@ -217,22 +219,36 @@ impl<'d> TimerDriver<'d> {
     pub fn enable_interrupt(&mut self) -> Result<(), EspError> {
         self.check();
 
-        esp!(unsafe {
+        if !self.isr_registered {
-            timer_isr_callback_add(
+            // Driver will complain if we try to register when ISR CB is already registered
-                self.group(),
+            esp!(unsafe {
-                self.index(),
+                timer_isr_callback_add(
-                Some(Self::handle_isr),
+                    self.group(),
-                (self.group() * timer_group_t_TIMER_GROUP_MAX + self.index())
+                    self.index(),
-                    as *mut core::ffi::c_void,
+                    Some(Self::handle_isr),
-                0,
+                    (self.group() * timer_group_t_TIMER_GROUP_MAX + self.index())
-            )
+                        as *mut core::ffi::c_void,
-        })
+                    0,
+                )
+            })?;
+
+            self.isr_registered = true;
+        }
+
+        Ok(())
     }
 
     pub fn disable_interrupt(&mut self) -> Result<(), EspError> {
         self.check();
 
-        esp!(unsafe { timer_isr_callback_remove(self.group(), self.index()) })
+        if self.isr_registered {
+            // Driver will complain if we try to deregister when ISR callback is not registered
+            esp!(unsafe { timer_isr_callback_remove(self.group(), self.index()) })?;
+
+            self.isr_registered = false;
+        }
+
+        Ok(())
     }
 
     pub async fn delay(&mut self, counter: u64) -> Result<(), EspError> {
@@ -265,6 +281,10 @@ impl<'d> TimerDriver<'d> {
         Ok(())
     }
 
+    /// Subscribes the provided callback for ISR notifications.
+    /// As a side effect, interrupts will be disabled, so to receive a notification, one has
+    /// to also call `TimerDriver::enable_interrupt` after calling this method.
+    ///
     /// # Safety
     ///
     /// Care should be taken not to call STD, libc or FreeRTOS APIs (except for a few allowed ones)
@@ -280,8 +300,6 @@ impl<'d> TimerDriver<'d> {
         ISR_HANDLERS[(self.group() * timer_group_t_TIMER_GROUP_MAX + self.index()) as usize] =
             Some(unsafe { core::mem::transmute(callback) });
 
-        self.enable_interrupt()?;
-
         Ok(())
     }
 
@@ -352,12 +370,14 @@ unsafe impl<'d> Send for TimerDriver<'d> {}
 #[cfg(feature = "nightly")]
 impl<'d> embedded_hal_async::delay::DelayUs for TimerDriver<'d> {
     async fn delay_us(&mut self, us: u32) {
-        let counter = (self.tick_hz() as u64 * us as u64) / 1000000;
+        let counter = core::cmp::max((self.tick_hz() * us as u64) / 1000000, 1);
+
         self.delay(counter).await.unwrap();
     }
 
     async fn delay_ms(&mut self, ms: u32) {
-        let counter = (self.tick_hz() as u64 * ms as u64) / 1000;
+        let counter = core::cmp::max((self.tick_hz() * ms as u64) / 1000, 1);
+
         self.delay(counter).await.unwrap();
     }
 }