Why is saadc reading taking more than expected time and why do i have to put delay between readings

[robin]

Hi
i am trying to reduce the adc read times to a minimum we currently have it setup for 3us acq and 32 samples so expecting something close to 160us
Its getting around 250us.
Below is how I set it up and do the adc reads
You will see for each read I need to change gain and input pin

Q1 Am I doing the correct thing below (it works) or am i doing something unnecessary
and making the read cycles longer than necessary.
If my code is good any suggestions on how we can reduce the time

Q2 I am finding I have to put large delays in order of milliseconds to get good readings every time
if I dont some readings come back all FS IE 4095

FIRST TIME THROUGH

void saadc_adc_init(void) {

ret_code_t err_code;
nrf_drv_saadc_config_t adc_config;

adc_config.resolution = (nrf_saadc_resolution_t)NRF_SAADC_RESOLUTION_12BIT;
adc_config.oversample = (nrf_saadc_oversample_t)NRF_SAADC_OVERSAMPLE_DISABLED;
adc_config.interrupt_priority = NRFX_SAADC_CONFIG_IRQ_PRIORITY;
adc_config.low_power_mode = NRFX_SAADC_CONFIG_LP_MODE;

err_code = nrf_drv_saadc_init(&adc_config, saadc_callback);
APP_ERROR_CHECK(err_code);
}

CALL following function
to setup for the provided pin and gain

void saadc_channel_init(saadc_adc_input_t adc_input, saadc_adc_input_gain_t adc_ip_gain) {

ret_code_t err_code;
nrf_saadc_channel_config_t channel_config;

channel_config.resistor_p = NRF_SAADC_RESISTOR_DISABLED;
channel_config.resistor_n = NRF_SAADC_RESISTOR_DISABLED;
channel_config.gain = adc_ip_gain;
channel_config.reference = NRF_SAADC_REFERENCE_INTERNAL;
channel_config.acq_time = NRF_SAADC_ACQTIME_3US; // Note this was 3us on the prototype
channel_config.mode = NRF_SAADC_MODE_SINGLE_ENDED;
channel_config.burst = NRF_SAADC_BURST_DISABLED;

channel_config.pin_p = (nrf_saadc_input_t)(adc_input);
channel_config.pin_n = NRF_SAADC_INPUT_DISABLED;

// Channel number can be 0 -7 We may wish to use same channel for each ADC we read
err_code = nrf_drv_saadc_channel_init(0, &channel_config);
APP_ERROR_CHECK(err_code);

err_code = nrf_drv_saadc_buffer_convert(m_buffer_pool[0], ADC_SAMPLES_BUFFER_LEN);
APP_ERROR_CHECK(err_code);

err_code = nrf_drv_saadc_buffer_convert(m_buffer_pool[1], ADC_SAMPLES_BUFFER_LEN);
APP_ERROR_CHECK(err_code);

}

Call the following to start ADC
ret_code_t err_code = nrf_drv_ppi_channel_enable(m_ppi_channel);
APP_ERROR_CHECK(err_code);

THEN ON SUBSEQUENT adc reads I do this

nrf_drv_saadc_uninit();

Call my function above
saadc_adc_init(..)
Call my function above to init with new pin and gain
saadc_channel_init(...

Then enable it
ret_code_t err_code = nrf_drv_ppi_channel_enable(m_ppi_channel);
APP_ERROR_CHECK(err_code);

[haakonsh]

Do you only take one sample before you change the SAADC settings? 

Do you disable the ppi channel after you've taken one sample? 

What do you use to trigger m_ppi_channel? 

Why do you not want to use more than one channel?

[robin]

Do you only take one sample before you change the SAADC settings? 

         Its set up to take 32 samples controlled by the PPI, so we kick it off and when its done we have 32 samples in the buffer. We would then change the input and gain and run again on the new input.

Do you disable the ppi channel after you've taken one sample?

           No , see the event handler at the bottom of this reply, you now have all the code involved,
            NOTE I commented out all my logging when I was measuring the time

 What do you use to trigger m_ppi_channel?

          As per my code above I call   nrf_drv_ppi_channel_enable(m_ppi_channel);

Why do you not want to use more than one channel?

            I didnt think of that or was aware I could, I am relatively new to this. 
            So are you suggesting I can pre-configure 3 channels for the 3 inputs I have and then just switch the channel for each read cycle. If so can you point me to an example ?
I assume I would be able to keep the remaining code the same using the one timer and ppi channel ?

In our application we have 3 gpios connected to 3 different analog sources, apart from the input pin being different the gain is different for all 3.  We need this to be as fast as possible. We dont scan all 3 sequentially in the same order, sometimes its 2 sometimes all 3 and in different order. 

Thanks Robin

[haakonsh]

"As per my code above I call   nrf_drv_ppi_channel_enable(m_ppi_channel);"

Yes, but what event triggers this channel? Are you using a TIMER?

"I didnt think of that or was aware I could, I am relatively new to this. "
From the SAADC specification:

When multiple channels are enabled, they are sampled successively in a sequence starting with the lowest channel number. The time it takes to sample all enabled channels is given as follows:

Total time < Sum(CH[x].tACQ+tCONV), x is the number of enabled channels

A DONE event is generated for every single completed conversion, and an END event is generated when multiple samples, as specified in RESULT.MAXCNT, have been written to memory.

You can enable three channels with different gain settings, increase the buffer to 96, trigger the sample task 32 times to fill the buffer. 
See Scan mode. 

"We need this to be as fast as possible. "
This will take 32samples x 3channels x (3µs+2µs) = 480µs if the sample task is triggered as fast as possible, f.ex every 5µs, or 80 ticks of a 16MHz TIMER. 

 "We dont scan all 3 sequentially in the same order, sometimes its 2 sometimes all 3 and in different order. "
You can't change the order the channels are sampled in when using scan mode, but you can enable/disable a channel during between samples. 

Can you specify exactly what it is you're trying to achieve with the SAADC? 

[robin]

Hi

I will explain it again hopefully it will be clearer. (The code is at the bottom with some I missed)
We have 3 ADC inputs that require different gain settings, we use PPI/TIMER/SAADC to read the ADC as required. We DONT want to ever scan all 3 sequentially, it will vary between scanning individually and some times scan a combination of any 2. 
We need to keep scan time and current to minimum possible.
I know theoretically the scan time for one ADC is going to be 5us x 32 = 160us , the only over head is the code to set it up and then collect the result. That is why I wanted to check that I am not doing any unnecessary work, and from what you have said it seems I could pre-configure 3 channels at start up and then just enable the ones I need on each scan call. 

Please the code/functions at the bottom here are the calls I make to those functions, how can this be improved and changed so I can use less code ?

1) POWER UP: I call
    saadc_sampling_event_init();
    saadc_adc_init();

2) VERY FIRST ADC READ: I call
    saadc_channel_init( <the adc input I want IE AIN0....AINx>, <gain required> );
    saadc_sampling_event_enable();

3) SUBSEQUENT ADC READS: I call
     saadc_setup_next_adc_read(<the adc input I want IE AIN0....AINx>);   # Gain is determined in function

----------------------------

1) POWER UP

This is where our timer is setup

void saadc_sampling_event_init(void) {
ret_code_t err_code;

err_code = nrf_drv_ppi_init();
APP_ERROR_CHECK(err_code);

nrf_drv_timer_config_t timer_cfg = NRF_DRV_TIMER_DEFAULT_CONFIG;
timer_cfg.bit_width = NRF_TIMER_BIT_WIDTH_32;
err_code = nrf_drv_timer_init(&m_timer, &timer_cfg, timer_handler);
APP_ERROR_CHECK(err_code);

/* setup m_timer for compare event every 5us 200Khz */
uint32_t ticks = nrf_drv_timer_us_to_ticks(&m_timer, 5); // Note same as prototype

nrf_drv_timer_extended_compare(&m_timer,
NRF_TIMER_CC_CHANNEL0,
ticks,
NRF_TIMER_SHORT_COMPARE0_CLEAR_MASK,
false);
nrf_drv_timer_enable(&m_timer);

uint32_t timer_compare_event_addr = nrf_drv_timer_compare_event_address_get(&m_timer,
NRF_TIMER_CC_CHANNEL0);
uint32_t saadc_sample_task_addr = nrf_drv_saadc_sample_task_get();

/* setup ppi channel so that timer compare event is triggering sample task in SAADC */
err_code = nrf_drv_ppi_channel_alloc(&m_ppi_channel);
APP_ERROR_CHECK(err_code);

err_code = nrf_drv_ppi_channel_assign(m_ppi_channel,
timer_compare_event_addr,
saadc_sample_task_addr);
APP_ERROR_CHECK(err_code);
}

void saadc_adc_init(void) {

ret_code_t err_code;
nrf_drv_saadc_config_t adc_config;

adc_config.resolution = (nrf_saadc_resolution_t)NRF_SAADC_RESOLUTION_12BIT;
adc_config.oversample = (nrf_saadc_oversample_t)NRF_SAADC_OVERSAMPLE_DISABLED;
adc_config.interrupt_priority = NRFX_SAADC_CONFIG_IRQ_PRIORITY;
adc_config.low_power_mode = NRFX_SAADC_CONFIG_LP_MODE;

err_code = nrf_drv_saadc_init(&adc_config, saadc_callback);
APP_ERROR_CHECK(err_code);
}

---------------------------------------

2) VERY FIRST ADC READ

In here we setup the channel to use, I assume we could do this for all 3 inputs using different channels on power up, but will this enable them ?  We want to enable the one or two we want at the time and then read

void saadc_channel_init(saadc_adc_input_t adc_input, saadc_adc_input_gain_t adc_ip_gain) {

ret_code_t err_code;
nrf_saadc_channel_config_t channel_config;

channel_config.resistor_p = NRF_SAADC_RESISTOR_DISABLED;
channel_config.resistor_n = NRF_SAADC_RESISTOR_DISABLED;
channel_config.gain = adc_ip_gain;
channel_config.reference = NRF_SAADC_REFERENCE_INTERNAL;
channel_config.acq_time = NRF_SAADC_ACQTIME_3US; // Note this was 3us on the prototype
channel_config.mode = NRF_SAADC_MODE_SINGLE_ENDED;
channel_config.burst = NRF_SAADC_BURST_DISABLED;

channel_config.pin_p = (nrf_saadc_input_t)(adc_input);
channel_config.pin_n = NRF_SAADC_INPUT_DISABLED;

// Channel number can be 0 -7 We may wish to use same channel for each ADC we read
err_code = nrf_drv_saadc_channel_init(0, &channel_config);
APP_ERROR_CHECK(err_code);

err_code = nrf_drv_saadc_buffer_convert(m_buffer_pool[0], ADC_SAMPLES_BUFFER_LEN);
APP_ERROR_CHECK(err_code);

err_code = nrf_drv_saadc_buffer_convert(m_buffer_pool[1], ADC_SAMPLES_BUFFER_LEN);
APP_ERROR_CHECK(err_code);

}

void saadc_sampling_event_enable(void)
{

ret_code_t err_code = nrf_drv_ppi_channel_enable(m_ppi_channel);
APP_ERROR_CHECK(err_code);
}

-----------------------------------

3) SUBSEQUENT ADC READS
This is called for subsequent reads which in turn calls functions above. Could this be simplified I just want to change the ADC input and then read again.

void saadc_setup_next_adc_read(saadc_adc_input_t adc_input) {

saadc_adc_uninit();

 saadc_ctrl_channel_uninit(last_adc_read_state);

saadc_adc_init();

if (adc_input == SAADC_ADC_INPUT_VREF)
saadc_channel_init(adc_input, SAADC_ADC_INPUT_GAIN_1_2);
else if (adc_input == SAADC_ADC_INPUT_OPAMP_OUT)
saadc_channel_init(adc_input, SAADC_ADC_INPUT_GAIN_1_4);
else if (adc_input == SAADC_ADC_INPUT_BATTERY)
saadc_channel_init(adc_input,SAADC_ADC_INPUT_GAIN_1_6);
else
NRF_LOG_ERROR("saadc_setup_next_adc_read() Invalid ADC input\n");

ret_code_t err_code = nrf_drv_ppi_channel_enable(m_ppi_channel);
APP_ERROR_CHECK(err_code);
}

[haakonsh]

There's is something that you can do that will lower the overhead by quite a bit. When you initialize the SAADC you should also initialize all three channels with their proper settings, then call:

//Disable all three channels
NRF_SAADC->CH[0].PSELP = 0;
NRF_SAADC->CH[1].PSELP = 0;
NRF_SAADC->CH[2].PSELP = 0;

For each time you want to sample a channel:

// Disable last channel used
NRF_SAADC->CH[<previous channel>].PSELP = 0;

// Enable the next channel
NRF_SAADC->CH[<next channel>].PSELP = <pin number>;

// Prepare the data buffer
err_code = nrf_drv_saadc_buffer_convert(m_buffer_pool[0], ADC_SAMPLES_BUFFER_LEN);
APP_ERROR_CHECK(err_code);

The channel settings will be stored even though they are disabled, you do not need to un-initialize the SAADC driver. 

[robin]

Thanks Haakonsh I was looking at this approach and wondered if it would work,
So using your code for each adc read  i would simply do this 
NRF_SAADC->CH[<previous channel>].PSELP = 0;
NRF_SAADC->CH[<next channel>].PSELP = <pin number>;
err_code = nrf_drv_saadc_buffer_convert(m_buffer_pool[0], ADC_SAMPLES_BUFFER_LEN);
Then this to start the next read
nrf_drv_ppi_channel_enable(m_ppi_channel);    

Is that correct ?

Why do I have to call  nrf_drv_saadc_buffer_convert(m_buffer_pool[0], ADC_SAMPLES_BUFFER_LEN);
I have a call in the event handler    
nrf_drv_saadc_buffer_convert(p_event->data.done.p_buffer, ADC_SAMPLES_BUFFER_LEN);
Do I need to still do this, or do I only need this one in the event handler as I have 2 buffers, which I may not need thinking about this as we dont read the adc continuously and therefore need time to process a read while another is going on.

When I enable 2 of the ADC inputs I see it will then scan these sequentially, will the results be put in one buffer or is this why I would need 2 buffers. If its just one then I would need to double its size and I believe the results would be interleaved.

UPDATE: I tried this code but I get an BUSy ERROR when I call
nrf_drv_saadc_buffer_convert(m_buffer_pool[0], ADC_SAMPLES_BUFFER_LEN);

to start the next adc read on different pin

Robin

[haakonsh]

"Then this to start the next read
nrf_drv_ppi_channel_enable(m_ppi_channel);    

Is that correct ?"

No, this line enables the given channe, just replace <next channel> and <pin number> whith the values you want :
NRF_SAADC->CH[<next channel>].PSELP = <pin number>;

"Why do I have to call  nrf_drv_saadc_buffer_convert(m_buffer_pool[0], ADC_SAMPLES_BUFFER_LEN);
I have a call in the event handler    
nrf_drv_saadc_buffer_convert(p_event->data.done.p_buffer, ADC_SAMPLES_BUFFER_LEN);
Do I need to still do this, or do I only need this one in the event handler as I have 2 buffers, which I may not need thinking about this as we dont read the adc continuously and therefore need time to process a read while another is going on."

This will probably work, but I have not tried to change the channel settings after I've called the START task, therefore I thought it best to do it after.

"When I enable 2 of the ADC inputs I see it will then scan these sequentially, will the results be put in one buffer or is this why I would need 2 buffers. If its just one then I would need to double its size and I believe the results would be interleaved."

One buffer, you need to double the size, and they will be stored sequentially(interleaved), f.ex

[Chan1][Chan2][Chan1][Chan2]...
[32-bit word 0 ][32-bit word 1  ]...

[robin]

Thanks Haarkonsh
Currently in my existing code I am calling 
nrf_drv_ppi_channel_disable(m_ppi_channel);
In the ADC read complete event handler as it looked like another ADC read was being started.
And this is why I am calling    nrf_drv_ppi_channel_enable(m_ppi_channel);    to start the next ADC read.

So looking at my code i provided have I setup the SAADCC/PPI/TIMER feature correctly so it runs once for the number of channels enabled and samples ?
If it is ok and I shouldnt have to call nrf_drv_ppi_channel_disable(m_ppi_channel);  in the saadc read event complete handler , then how do I restart it again for the next read after I disable and enable channels for the next read using NRF_SAADC->CH[<next channel>].PSELP = <pin number>;

Is this call  nrf_drv_saadc_buffer_convert(p_event->data.done.p_buffer, ADC_SAMPLES_BUFFER_LEN);
in the event handler starting the next read cycle while I am still processing the first 

Robin