ADC - how to consume fast samples while avoiding race conditions?

[mikemike651]

Hi everyone,

I have a fairly simple use for ADC: sample 4 channels (scan mode) every 10ms and store in RAM.

Every now and then (asynchronously) there will be a function or a task (not in ISR context) that will want to read the latest values of the 4 channels.

I've been struggling to implement that without a potential for getting a race condition (where the task might read the values as they are written), and I wanted to ask for your advice on that.

My (simplified) code for configuring the ADC is as follows:

#define NUM_SAADC_CHANNELS 4
#define ADC_NUM_SAMPLES_IN_BUFFER NUM_SAADC_CHANNELS
static nrf_saadc_value_t m_buffer_pool[2][ADC_NUM_SAMPLES_IN_BUFFER];
static volatile uint32_t m_buffer_index = 0;  // current buffer idx the DMA will be using to copy the ADC samples to (alternates between 0 and 1)

static void saadc_callback(nrfx_saadc_evt_t const * p_event)
{
    if (p_event->type == NRFX_SAADC_EVT_DONE)
    {
        // p_buffer is a pointer to the buffer with converted samples.
        /* set buffers for the next ADC event */
        APP_ERROR_CHECK(nrfx_saadc_buffer_convert(p_event->data.done.p_buffer, ADC_NUM_SAMPLES_IN_BUFFER));
        m_buffer_index ^= 1; // 0 becomes 1, 1 becomes 0
    }
}

ret_code_t adc_init(void)
{
    ret_code_t err_code = NRF_SUCCESS;

    // Initialize SAADC
    nrfx_saadc_init(...);

    // Initialize ADC channels
    for (int i=0; i<NUM_SAADC_CHANNELS; i++) {
        nrf_saadc_channel_config_t channel_config = NRFX_SAADC_DEFAULT_CHANNEL_CONFIG_SE(...);
        err_code = nrfx_saadc_channel_init(...); 
        APP_ERROR_CHECK(err_code);
    }

    // Configure buffers to ensure double buffering of samples, to avoid data loss when the sampling frequency is high
    err_code = nrfx_saadc_buffer_convert(m_buffer_pool[0],ADC_NUM_SAMPLES_IN_BUFFER);
    APP_ERROR_CHECK(err_code);   
    err_code = nrfx_saadc_buffer_convert(m_buffer_pool[1],ADC_NUM_SAMPLES_IN_BUFFER);
    APP_ERROR_CHECK(err_code);

    // Initialize PPI and Timer, as in the ADC example
    // ...

    return err_code;
}

In a nutshell it's identical to the example ADC code, where we have a double buffer, and PPI+Timer to sample every 10ms.

As for the asynchronous function run from a task, it will occasionally need to read the latest values stored by the ADC. I was thinking about this code:

void adc_get_latest_values(int16_t* p_array)
{
    // grab the values from the pool that is not currently being updated
    memcpy(p_array, &m_buffer_pool[m_buffer_index^1], ADC_NUM_SAMPLES_IN_BUFFER*sizeof(m_buffer_pool[0][0]));
}

My idea is that the ADC will update the buffer index m_buffer_index (0 or 1) and the task will read from m_buffer_index^1 (1 or 0).

My concern here is that there is a potential for a race condition, where m_buffer_index would get updated by the ISR while the function above is being run.

Your advice would be greatly appreciated!

[Einar Thorsrud]

Hi,

In principle 10 ms should be enough time for most tasks. Do you have a rush to use the values from the SAADC? If you do not need the latest samples every 10 ms, then you could use larger buffers, so that you would not have to swap buffers every 10 ms. Lets say you use set ADC_NUM_SAMPLES_IN_BUFFER to (NUM_SAADC_CHANNELS * 10) that would give you more time to process the data before they are updated. (I just suggested 10 here, a different number might make more sense in your application).

[mikemike651]

Thanks for your answer Einar.

Even if we use larger buffers, there is still the chance that while (asynchronously) reading the buffer that's not being currently used by hardware, a new ISR will kick in and change the buffer pointer. Are you aware of any software tricks that might help with that, or maybe have a code example that would show a guaranteed way to avoid a race condition?

Usually in such scenarios, disabling interrupts is a decent solution, but I read that in the nRF SDK it may mess up the SoftDevice stack (which in our case needs to be running while using the ADC).

Appreciate your help!

[Einar Thorsrud]

Hi,

I am not sure I understand the context here. If you sample every 10 ms and use the buffer size I suggested, that means buffers swap every 100 ms (you can use lager buffers as well). And so, from you get the event/interrupt with the buffer filled you know it will take at least 100 ms (in this example) before any data can change. That should give you a lot of time to use this data, copy it somewhere or whatever you need to do before there is any risk of it being corrupted.

I do not see why disabling interrupts would be relevant in this situation, as we are not talking about short times here. So you are right it would not work, but also It does not seem relevant. Or perhaps I don't understand where you mean there is a race condition? If so, perhaps you can explain in more detail in that case?

[mikemike651]

Thanks for your patience Einar, sure I'll try to explain.

With your suggestion I will have a dual buffer scheme that is updated every 100ms:

static nrf_saadc_value_t m_buffer_pool[2][ADC_NUM_SAMPLES_IN_BUFFER];

While the hardware updates index 0, the user can consume index 1, and vice-versa (the active buffer is marked by m_buffer_index in my code). My concern is that because the consumption is done asynchronously (it is unaware of when the last batch of fresh samples has arrived), it can happen at the end of the 100ms window just before the ISR fires with new samples.

In the code I posted, the ISR updates m_buffer_index, and I'm worried it will happen while the user is trying to access it (and the buffer), in the function adc_get_latest_values() which I posted originally.

What I'm trying to ask is, is there a software solution, say a FIFO, a ring buffer, or some synchronization trick, that can help with this potential issue?

Thanks again for your help.

[Einar Thorsrud]

Hi,

OK, now I see. This is not directly related to the SAADC driver, then. If you are only interested inthe very last sample and you want to do this simply and safely, perhaps you could stay with the buffer size you had (one sample for each channel in each buffer), and simply copy out to a variable which is not one of the two buffers in the event handler. This is a quite fast operation, so it should be OK to do this in the SAADC interrupt.

The above can be done atomically using the LDREX/STREX instructions, and the SDK has a helper library for that: nrf_atomic. IThat could be used direclty, but it is used to implement other modules as well, such as nrf_ringbuf. I am sure there are other methods as well, some may be better for your use case, but this gives an idea of the toolbox you have at hand.