Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2/3] Timer abstraction: add OneShotTimer/PeriodicTimer drivers, Timer trait #1570

Merged
merged 9 commits into from
May 23, 2024

Conversation

jessebraham
Copy link
Member

@jessebraham jessebraham commented May 21, 2024

This is still a bit messy overall, but I plan to clean things up in the next and final PR in this series. Refactoring and documentation will be improved in said PR.

At this time OneShotTimer is working correctly both when backed by TIMGx and SYSTIMER. PeriodicTimer is working when backed by TIMGx, however there is one small issue remaining when backed by SYSTIMER.

Given the following example:

#![no_std]
#![no_main]

use esp_backtrace as _;
use esp_hal::{
    peripherals::Peripherals,
    prelude::*,
    timer::{systimer::SystemTimer, PeriodicTimer},
};
use esp_println::println;

#[entry]
fn main() -> ! {
    let peripherals = Peripherals::take();

    let systimer = SystemTimer::new(peripherals.SYSTIMER);
    let mut periodic = PeriodicTimer::new(systimer.alarm0);

    println!("Countdown 5 times using PeriodicTimer...");
    periodic.start(1000.millis());
    let mut i = 5;
    while i > 0 {
        println!("{i}");
        nb::block!(periodic.wait()).unwrap();
        i -= 1;
    }
    periodic.cancel().unwrap();
    println!("Done!");

    loop {}
}

Both 5 and 4 are immediately printed, and then the countdown continues as expected from there. Not sure why the first iteration is clearing right away, still looking into it.

@bjoernQ
Copy link
Contributor

bjoernQ commented May 22, 2024

Solution to the systimer periodic timer problem seems to be this:

image

i.e. this seems to fix it for me (tested on C6 only for now)

    fn load_value(&self, value: MicrosDurationU64) {
        ....

        if auto_reload {
            // Period mode

            systimer
                .target_conf(CHANNEL as usize)
                .modify(|_, w| unsafe { w.period().bits(ticks as u32) });

            #[cfg(not(esp32s2))]
            systimer
                .comp_load(CHANNEL as usize)
                .write(|w| w.load().set_bit());

            // Clear and then set SYSTIMER_TARGETx_PERIOD_MODE to configure COMPx into period mode
            systimer.target_conf(CHANNEL as usize).modify(|_,w| w.period_mode().clear_bit());
            systimer.target_conf(CHANNEL as usize).modify(|_,w| w.period_mode().set_bit());
        } else {
            // Target mode

                     .....

5 is still printed immediately but not 4

@jessebraham jessebraham force-pushed the feature/timer-abstraction-1 branch from fb8bb3c to b302f62 Compare May 22, 2024 14:36
@jessebraham jessebraham marked this pull request as ready for review May 22, 2024 16:08
@jessebraham jessebraham force-pushed the feature/timer-abstraction-1 branch from bb14ffd to b37bde9 Compare May 22, 2024 16:10
@jessebraham jessebraham requested review from bjoernQ and MabezDev May 22, 2024 16:11
Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jessebraham jessebraham enabled auto-merge May 22, 2024 18:45
Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks!

@jessebraham jessebraham added this pull request to the merge queue May 23, 2024
Merged via the queue into esp-rs:main with commit 5facb75 May 23, 2024
22 checks passed
@jessebraham jessebraham deleted the feature/timer-abstraction-1 branch May 23, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants