embassy-rs / embassy

Modern embedded framework, using Rust and async.
https://embassy.dev
Apache License 2.0
5.31k stars 732 forks source link

ring buffered adc v2 #3116

Closed sethkrie closed 3 months ago

sethkrie commented 3 months ago

new PR, taking Dirbao's advice to make the DMA impl in a separate struct that consumes Adc to make RingBufferedAdc. Handling overrun similar to RingBufferedUart.

The API is therefore entirely disjoint from the original ADC API.

sethkrie commented 3 months ago

Still have a few things to check regarding overrun. Not ready for a merge/review quite yet.

sethkrie commented 3 months ago

Ready for review @Dirbaio

andresv commented 2 months ago

@sethkrie if I have declared for example 4 sequences and my dma buffer size is 16 and then I read out 8 samples I get seq0 seq1 seq2 seq3 seq0 seq1 seq2 seq3 right?

    let adc_dma_buf = [0u16; 16];
    let mut adc: RingBufferedAdc<embassy_stm32::peripherals::ADC1> = adc.into_ring_buffered(p.DMA2_CH0, adc_dma_buf);

    adc.set_sample_sequence(Sequence::One, &mut p.PA0, SampleTime::CYCLES112);
    adc.set_sample_sequence(Sequence::Two, &mut p.PA1, SampleTime::CYCLES112);
    adc.set_sample_sequence(Sequence::Two, &mut p.PA2, SampleTime::CYCLES112);
    adc.set_sample_sequence(Sequence::Two, &mut p.PA3, SampleTime::CYCLES112);

    let mut read_buf = [0u16; 8];
    let _ = adc.start();
    loop {
        match adc.read_exact(&mut read_buf).await {
            Ok(_data) => {
                let toc = Instant::now();
                info!(
                    "\n adc1: {} dt = {}, n = {}",
                    buffer1[0..16],
                    (toc - tic).as_micros(),
                    _data
                );
                tic = toc;
            }
            Err(e) => {
                warn!("Error: {:?}", e);
                read_buf = [0u16; 8];
                let _ = adc.start();
            }
        }
    }

If I would have read only 3 readings with one go I would have got seq0 seq1 seq2 and then seq3 seq0 seq1 right?

Is currently continous mode working as fast as it can and I cannot define like sample with 100 Hz?

sethkrie commented 2 months ago

@andresv

Yeah so you're right in that the ADC will always follow it's sequencing as configured, however, if there is overrun, the context of 'where' you are in the sequence is lost. The driver will stop everything and start from scratch. This is slow and undesirable.

Since async is somewhat non-deterministic in when it polls a task, you can run into trouble here and you'll find that your buffer will always overrun.

So, the idea is that you want to create a buffer that is sufficiently large that your app will most certainly loop back around and wake the read task before it overruns.

For example, I'll give DMA a 512 length buffer and then after starting continuous reads, call read_exact() with a 256 length buffer. The task will sleep until the DMA peripheral flags an interrupt for either a half-transfer or full-transfer event. This means the read_exact() call will have 256 words to read since the previous call, unless one was not read in time, in which case, you'll have overrun.

So two takeaways:

Regarding sampling frequency -- the above issue comes down to the fact that, yes the ADC is running at full-tilt until the conversions/DMA are stopped, however, you may do a one-shot of the scanning continuous mode and use a TIM or EXTI interrupt to call .start() -> .read_exact() -> . teardown_adc() to sample your sequence very quickly, but doing so at an overall slower rate.

Note that teardown ADC does not actually turn off the ADC peripheral, it just clears the DMA flag and stops conversions, so the next .start() won't require all the initial setup of Adc::new().

andresv commented 2 months ago

Currently both read and read_exact are implemented, I wonder if there is an usecase for read at all. Like you most likely always want to use read_exact.

Also for using read_exact there are many footguns and therefore in depth knowledge is needed. Your summary should be somehow condensed and written to read_exact documentation.

sethkrie commented 2 months ago

I think there's docs covering the space issues (N and N/2) and overrun, but not on the implications of how they interact with async. Note that my .read_exact() is just exposing the ReadableRingBuffef API, which is very well documented. There's definitely some caveats with this implementation though, so I'll open a PR that puts a condensed version of what I wrote above in the adc_ringbuff's read_exact().

One more question, what's the best way to engage more people in the community in conversations around improving the guard rails around this? Maybe someone can shed more light and I can do the leg work, just not sure how to best reach the right people.

Thanks!

andresv commented 2 months ago

@sethkrie I added some docs: https://github.com/embassy-rs/embassy/pull/3143