Rahix / avr-hal

embedded-hal abstractions for AVR microcontrollers
Apache License 2.0
1.33k stars 225 forks source link

Timer-based Delay & CountDown #9

Open Rahix opened 5 years ago

Rahix commented 5 years ago

Right now, avr-hal only supports busy-loop delay which is bad for power-consumption and inaccurate with longer wait times. We should also add support for timer based delay and while working on that, also add implementations for the embedded-hal timer trait: CountDown

Maybe, along the same lines, an implementation of embedded_time::Clock could also be useful.

OliverEvans96 commented 4 years ago

Is there currently any way to access the clock, as in Arduino's millis()?

lights0123 commented 4 years ago

nope. I was going to take a look at this, but I don't really like reading datasheets :man_shrugging:

Nexyll commented 4 years ago

I'm trying to use the Ws2812 driver with timer based delays library and the new method require a 3 MHz Timer

The new method needs a periodic timer running at 3 MHz

If it's too slow (e.g. e.g. all/some leds are white or display the wrong color) you may want to try the slow feature.

Source : https://docs.rs/ws2812-timer-delay/0.3.0/ws2812_timer_delay/

From what I understand, the feature described in this issue is necessary to satisfy this parameter but is there a technique to work around it while waiting for an implementation ?

Rahix commented 4 years ago

@Nexyll: I'm a bit sceptical whether this would work at all. Usually, for controlling neopixels on AVR, you'd need some hand-written assembly to make sure the timing is right. E.g. this is what the commonly used Arduino implementation is doing: https://github.com/adafruit/Adafruit_NeoPixel/blob/8595ee3f5880d5096d71551c621d7a2d3818f974/Adafruit_NeoPixel.cpp#L1026-L1069

Rahix commented 4 years ago

@OliverEvans96, and anyone else who is searching for a millis() function in avr-hal: I wrote an example implementation in uno-millis.rs and a walkthrough blog-post: https://blog.rahix.de/005-avr-hal-millis

Rahix commented 3 years ago

@Nexyll, you might be interested in this example which just got merged into smart-leds-rs: https://github.com/smart-leds-rs/smart-leds-samples/tree/master/avr-examples

nathansamson commented 2 years ago

Any progress on this (especially the CountDown timer)? Any pointers on how one would be able to implement this? (especially for arduino uno)

Rahix commented 2 years ago

What exactly are you trying to do?

If you just need something implementing the CountDown trait, you could probably get away with writing a thin wrapper ontop of the millis() implementation mentioned above. If you need it more precise, the uno-hc-sr04.rs example contains some code using a timer directly for timing purposes.

nathansamson commented 2 years ago

I am trying to do exactly what Nexyll is tring, to get https://docs.rs/ws2812-timer-delay/0.3.0/ws2812_timer_delay/ to work with a 3MHz timer. (hence millis or even delay_us is out)

I guess I could use the timers code example, assume it gives me a very precise timer count and go out of the loop if bit count > a given number. I am bit worried this wont be exact enough either (and I would basically writing a busy loop reading btis from the timer counter)

nathansamson commented 2 years ago

So my first attempt was something like this. So basically count the number of ticks until they are more than you needed for your particular delay (Assuming a 16MHz timer, not prescaled, not taking into account any overflow issues)

Sidenote: I defined the count parameter to be in nanoseconds for this implementation, the trait is supisciously not defining that and making it a bit useless as a trait I think, but nevermind that.

So I think I have 2 overflow issues, one in calculating the ticks per wait(if the number of nanoseconds would overflow one tick cycle) I am also ignoring when the counter overflows/resets and not taking that into account.

pub struct BrokenTimer {
    pub timer: arduino_hal::pac::TC1,
    pub ticks_per_wait: u16,
}

impl BrokenTimer {
    pub fn new(timer: arduino_hal::pac::TC1) -> BrokenTimer {
        BrokenTimer {
            timer: timer,
            ticks_per_wait: 0,
        }
    }
}

impl embedded_hal::timer::CountDown for BrokenTimer {
    type Time = u16;

    fn start<T>(&mut self, count: T) where
        T: Into<Self::Time> {
        self.ticks_per_wait = Into::<Self::Time>::into(count) * 10u16 / 625u16;
    }

    fn wait(&mut self) -> Result<(), nb::Error<void::Void>> {
        // looping until a number of timer ticks have passed
        let tick1 = self.timer.tcnt1.read().bits();
        while self.timer.tcnt1.read().bits() <= tick1 + self.ticks_per_wait {

        }

        return Result::Ok(());
    }
}

impl embedded_hal::timer::Periodic for BrokenTimer {
}

So in principle this timer would work assuming I would have no overflow issues. Note that for a 3MHz timer (one I need), I need to define 333.33 nanoseconds (which I already can't but lets ignore and go slightly under for 333).

So I would need 5.3 ticks (my code rounds this to 5 currently), Note I am already having twice a relatively significant rounding error for this that I don't know if I can workaround, but lets ignore that for a moment.

But now I am seeing the following problem when executing the following bit of code (just for my sanity to double check what timers are doing)

So when executing the following loop

    loop {
        let tick1 = timer1.tcnt1.read().bits();
        let tick2 = timer1.tcnt1.read().bits();

        ufmt::uwriteln!(&mut serial, "Hello from Arduino 12 {} = {}!\r", tick1, tick2).void_unwrap();

I get this result

Hello from Arduino 12 21995 = 21999!
Hello from Arduino 12 5758 = 5762!
Hello from Arduino 12 54269 = 54273!
Hello from Arduino 12 4990 = 4994!
Hello from Arduino 12 53503 = 53507!

Here you can clearly see that just reading the current bit counter already takes 4 ticks. But my code would do this at least twice (one for the initial tick, once at least for the next tick to see if time has passed). Now since the second tick would only be 4 more than the first one, it would read it out a third time for in total 12 ticks. Almost triple from what I needed (for this short timer, bigger amounts would be less significantly impacted)

So this approach won't work for this use case

@Nexyll I've found a way though to make the "Ws2812 driver with timer based delays" work though (assuming you did not figure out in the mean time, and are still intrested -- regardless this might be helpfull for the next person coming along who hopefully does not need to spend 2 evenings getting this to work, like I did)

I created a hardcoded 3Mhz timer (which basically does one read of the timer bits since that seems to be around the time we need) + you need to enable the "slow" feature of the https://github.com/smart-leds-rs/ws2812-timer-delay-rs crate

Note this is for the arduino uno, when the moon is in the Waxing Gibbous phase. Your mileage might vary depending on multiple factors

[dependencies.ws2812-timer-delay]
version = "0.3.0"
features = ["slow"]
/* A broken timer but seems to work for me... */
pub struct Broken3MHzTimer {
    pub timer: arduino_hal::pac::TC1,
}

impl embedded_hal::timer::CountDown for Broken3MHzTimer {
    type Time = u16;

    fn start<T>(&mut self, count: T) where
        T: Into<Self::Time> {
    }

    fn wait(&mut self) -> Result<(), nb::Error<void::Void>> {
        // Reading once the timer seems to be slightly too fast but okay...
       // Note: we could do anything here, and probably don't need the timer after all
       // but doing this experimentally coincidently gave almost the right amount of sleep time needed that we wanted, so why change a winning horse.
        self.timer.tcnt1.read().bits();

        return Result::Ok(());
    }
}

impl embedded_hal::timer::Periodic for Broken3MHzTimer {
}
nathansamson commented 2 years ago

One more note: I think this would still be considered a busy loop, so I don't think it really solves anything?

RecursiveError commented 9 months ago

Any update on this issue? I don't understand much about how macros work, but couldn't a Timer-based Delay be implemented like USART? a struct that holds some data like clock, timer size.... with a raw_read and a macro like "default_serial!" for timer0 as default delay

delay just puts data in OCR#A, goes to idle, wakes up, checks if it was a timer interruption with raw_read, repeats the process

I don't think it's possible to create CountDown based on this implementation, but that could be two separate things

Rahix commented 9 months ago

@RecursiveError unfortunately, it is not quite that simple because an implementation like you suggest can only support delays up to the timer overflow. However, a lot of use-cases need delays longer than that so we need an implementation that counts timer overflows as well. To do this, you need to hook yourself into the respective timer interrupt. While doing that in your own code is easily possible, abstracting away this "hook into timer interrupt" into some generic API is an unsolved problem...