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

Access DMA buffered data from RTIC task #171

Open
jismithc opened this issue Jan 15, 2025 · 2 comments
Open

Access DMA buffered data from RTIC task #171

jismithc opened this issue Jan 15, 2025 · 2 comments

Comments

@jismithc
Copy link

jismithc commented Jan 15, 2025

Hi I'm new to rust and stm32s so please bare with me.

Main issue: I cannot find a way to access buffered data from a RTIC task triggered by a DMA transfer complete interrupt.

Preferred functionality from a stm32f4xx_hal example - https://github.com/stm32-rs/stm32f4xx-hal/blob/master/examples/rtic-adc-dma.rs
This example configures DMA to trigger an interrupt when its buffer is full (in this case a quantity of 2).
transfer is set as a shared variable, and given a &'static mut [u16; 2] buffer to fill.

In the dma interrupt task (triggered when the first buffer is full), the task locks shared.transfer, and (from what I understand) passes ownership of the filled buffer from the transfer to the interrupt task, replacing it with a second buffer.

            let (buffer, _) = transfer
                .next_transfer(local.buffer.take().unwrap())
                .unwrap();

I have attempted to do something similar with stm32g4xx_hal without success. There is no next_trasfer() available.
I am not necessarily requiring a double buffer strategy, but simply a way to access the full buffer's data from the interrupt task utilizing the tools provided from this hal, avoiding unsafe blocks.


Side note:
I have developed a somewhat usable way using unsafe blocks to access static mut array. This is not my preferred method.
In fact, rustc gives me this warning warning: creating a mutable reference to mutable static is discouraged.

const BUFFER_SIZE: usize = 200;

type AdcDma = CircTransfer<
    Stream0<stm32g4xx_hal::stm32::DMA1>,
    Adc<stm32g4xx_hal::stm32::ADC1, DMA>,
    &'static mut [u16; BUFFER_SIZE],
>;

#[app(device = stm32g4xx_hal::stm32, dispatchers = [EXTI0])]
mod app {
    ...
    #[local]
    struct Local {
        ...
        adc_dma: AdcDma,
        adc_buffer: &'static mut [u16; BUFFER_SIZE],
    }

    #[init]
    fn init(ctx: init::Context) -> (Shared, Local, init::Monotonics) {
        let dma_streams = dp.DMA1.split(&mut rcc);
        let config = DmaConfig::default()
            .transfer_complete_interrupt(true)
            .circular_buffer(true)
            .memory_increment(true)
            .peripheral_increment(false); // Don't increment peripheral address

        // 4) Prepare a buffer for the ADC samples
        static mut ADC_BUFFER: [u16; BUFFER_SIZE] = [0; BUFFER_SIZE];

        // Initialize ADC with delay for proper initialization timing
        let mut adc_controller =
            adc::AdcController::new(dp.ADC1, board_pins.adc1, &mut rcc, dma_streams, delay);

        // Enable ADC DMA with local buffer
        let (adc_dma_enabled, stream) = adc_controller.enable_dma();
        let mut adc_dma = unsafe {
            stream.into_circ_peripheral_to_memory_transfer(adc_dma_enabled, &mut ADC_BUFFER, config)
        };

        // Start DMA transfer and ADC conversion
        adc_dma.start(|adc| adc.start_conversion());
        (
            Local {
                adc_dma: adc_dma,
                adc_buffer: unsafe { &mut ADC_BUFFER },
            },
            init::Monotonics(mono),
        )
    }

    #[task(binds = DMA1_CH1, local = [adc_dma, adc_buffer])]
    fn dma_isr(ctx: dma_isr::Context) {
        if adc_dma.get_transfer_complete_flag() {
            process_samples(&ctx.local.adc_buffer[BUFFER_SIZE / 2..]);
        }
    }
}

// From another local file.
    pub fn enable_dma(&mut self) -> (Adc<ADC1, DMA>, Stream0<DMA1>) {
        // Take ownership of ADC and stream
        let adc = self.adc.take().unwrap();
        let stream = self.stream.take().unwrap();

        // Enable DMA mode first, then enable ADC
        let adc_dma = adc.enable_dma(AdcDma::Continuous);

        (adc_dma, stream)
    }

Is there already a supported way to access the dma buffer data? Or is this functionality lacking.
Thanks for the help!

@usbalbin
Copy link
Contributor

usbalbin commented Jan 15, 2025

Thank you for the report! :)

Yeah the DMA story is currently less than perfect... This is issue is proof of that. I have opened #170 to try and find solutions to make working with DMA more nice.

I believe this is a good example of something that should be easier to do.

As for your code snippet above. Mutable references are supposed to be unique.

[...]
        let mut adc_dma = unsafe {//              First mutable borrow ------v
            stream.into_circ_peripheral_to_memory_transfer(adc_dma_enabled, &mut ADC_BUFFER, config)
        };

        // Start DMA transfer and ADC conversion
        adc_dma.start(|adc| adc.start_conversion());
        (
            Local {
                adc_dma: adc_dma,//     v------ Second mutable borrow - UB💥
                adc_buffer: unsafe { &mut ADC_BUFFER },
            },
            init::Monotonics(mono),
        )
[...]

The second mutable borrow causes UB. So this should really be avoided. This why the warning tries to prevent you from borrowing the static mut since it can not check for correctness.

From a quick look I do not see anything preventing implementing something like next_tansfer from f4xx-hal (but without the double buffer stuff). @AdinAck any thoughts?

@AdinAck
Copy link
Contributor

AdinAck commented Jan 15, 2025

I encountered this exact same roadblock last week while working on something. I believe the original designers intended for transfers to be constructed and then freed upon completion to be re-configured again for the next transfer.

...very difficult to embed in any kind of abstracted code due to move semantics.

But yes this is a good use-case example we outlined in #170 for interface improvements.

As a side note, it's usually good practice to minimize the size of interrupt functions, unless immediate response time is necessary. If it's not, you can await the interrupt event with rtic_sync::signal::Signal or Channel.

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

No branches or pull requests

3 participants