Incorrect sample order in high frequency ADC sampling + BLE transfer

[David]

Hello Guys,

I am running an application which is sampling 3 ADC channels at 50 MHz. I am using sample buffers of size 150 and PPI to generate a sample event every 20 microseconds. At lower sampler rates, the data position in the buffers is consistent. But as I go below a sample period of 100 microseconds, the buffer order is corrupted.

This seems to be a very common and important issue but there is really no clear cut instructions on how to fix the issue.

I have tried the tips provided here, but none of them seem to work.

void saadc_callback(nrf_drv_saadc_evt_t const * p_event)
{
    
    if (p_event->type == NRF_DRV_SAADC_EVT_DONE)
    {
        ret_code_t err_code;
        err_code = nrf_drv_saadc_buffer_convert(p_event->data.done.p_buffer, SAMPLES_IN_BUFFER);
        APP_ERROR_CHECK(err_code);

        /* Get average battery value */
        uint16_t bat_total = 0;
        
        /* Create Bucket from ADC Data */
        sample_bucket_t* pbucket = malloc(sizeof(sample_bucket_t));
        if(pbucket == NULL) return;
        for(uint16_t i=0; i<SAMPLE_BUCKET_SIZE; i++){
          int16_t mic_val = p_event->data.done.p_buffer[i*3];
          int16_t acc_val = p_event->data.done.p_buffer[i*3+1];
          bat_total += p_event->data.done.p_buffer[i*3+2];
printf("%d\n", mic_val);
          pbucket->samples[i].data[0] = (uint8_t)(mic_val);
          pbucket->samples[i].data[1] = (uint8_t)((mic_val >> 8) & 0x0F) | ( (uint8_t)(acc_val) << 4 );
          pbucket->samples[i].data[2] = (uint8_t)(acc_val >> 4);
        }
        
        battery_val = bat_total / SAMPLE_BUCKET_SIZE;

        /* Run Sampling FSM */
        switch(device_status.sampling_state){
          case SAMPLE_NONE:
            free(pbucket);
            break;

          case SAMPLE_POLLING:
            ring_buffer_add(buckets_before, pbucket);   /* Add bucket to buckets before buffer */

            /* Check if accel threshold is exceeded */
            uint16_t accel_val = bucket_acc_average(pbucket);
            //printf("%d\n", accel_val);
            if(accel_val > fusion_config.shot_detection_acc_threshold){
              //LOGI("Accelerometer threshold exceeded");
              device_status.sampling_state = SAMPLE_ACCEL_THRESHOLD;
              device_status.sampling_status.acc_exceed_time = global_ticks;
            }
            break;

          case SAMPLE_ACCEL_THRESHOLD:
            /* Continue adding buckets to before_buffer even after accel threshold
              has been exceeded. */
            ring_buffer_add(buckets_before, pbucket);   

            /* Check if threshold has been exceeded */;
            uint16_t audio_val = bucket_mic_average(pbucket);
            if(audio_val > fusion_config.shot_detection_aud_threshold){
              //LOGI("Shot Detected !");
              device_status.sampling_state = SAMPLE_SHOT_DETECTED;
            }

            /* Check if time has been elapsed */
            else if(TIME_ELAPSED(device_status.sampling_status.acc_exceed_time, AUDIO_WAIT_TIMEOUT)){
              //LOGI("Audio Signal not detected ");
              device_status.sampling_state = SAMPLE_POLLING;
            }

            break;

          case SAMPLE_SHOT_DETECTED:
            /* Add buckets to buckets_after ring buffer */
            //printf("%d\n", buckets_after->size);
            if(buckets_after->size < buckets_after->capacity){
              ring_buffer_add(buckets_after, pbucket); 
            }

            /* When bucket is full, create shot_data_t */
            else{
              free(pbucket);  /* free bucket or add to ring buffer */
              uint8_t shot_id = get_shot_count();
              //device_status.sampling_state = SAMPLE_SHOT_COOL_DOWN;
              //break;

              /* Create ble packets from bucket data */
              /* Iterate through all buckets in buckets before */
              while(buckets_before->size){
                sample_bucket_t* bucket = front(buckets_before);
                
                ble_packet_t* ble_packet = malloc(sizeof(ble_packet_t));
                //if(ble_packet == NULL) continue;

                ble_packet->buf[0] = shot_id;
                memcpy(ble_packet->buf+2,  &bucket->samples, 150);
                ble_packet->packet_length = 152;
                
                ble_add_to_queue(LOW_PRIORITY_PACKET, ble_packet);

                dequeue(buckets_before);
                free(bucket);
              }


              /* Iterate through buckets_after */
              while(buckets_after->size){
                sample_bucket_t* bucket = front(buckets_after);

                ble_packet_t* ble_packet = malloc(sizeof(ble_packet_t));
                //if(ble_packet == NULL) continue;

                ble_packet->buf[0] = shot_id;
                memcpy(ble_packet->buf+2,  &bucket->samples, 150);
                ble_packet->packet_length = 152;
                
                ble_add_to_queue(LOW_PRIORITY_PACKET, ble_packet);
                dequeue(buckets_after);

                /* Add bucket to buckets before buffer so that next shot can use it */
                ring_buffer_add(buckets_before, bucket); 
              }
              
              /* Go To Cool Down Phase */
              device_status.sampling_state = SAMPLE_SHOT_COOL_DOWN;
              //LOGD("SHOT_DETECTED -> SHOT_COOL_DOWN");
            }
            break;

          case SAMPLE_SHOT_COOL_DOWN:
            device_status.sampling_state = SAMPLE_POLLING;
            free(pbucket);
            break;

          default:
            free(pbucket);
            break;
        }
    }
}


/**
 * Initialize ADC (saadc)
 * Params:
 *  bool bat_only: Flag to determine if only Battery ADC should be enabled
 * Ret: None
 */
static void init_adc(bool bat_only){
  ret_code_t err_code;

  nrf_saadc_channel_config_t channel_config_mic =
      NRF_DRV_SAADC_DEFAULT_CHANNEL_CONFIG_SE(MIC_ADC);
  channel_config_mic.acq_time = NRF_SAADC_ACQTIME_3US;

  nrf_saadc_channel_config_t channel_config_acc =
      NRF_DRV_SAADC_DEFAULT_CHANNEL_CONFIG_SE(ACC_ADC);
  channel_config_acc.acq_time = NRF_SAADC_ACQTIME_3US;

  nrf_saadc_channel_config_t channel_config_bat =
      NRF_DRV_SAADC_DEFAULT_CHANNEL_CONFIG_SE(BAT_ADC);
  channel_config_bat.acq_time = NRF_SAADC_ACQTIME_3US;

  nrf_drv_saadc_config_t saadc_config = {
    .resolution = NRF_SAADC_RESOLUTION_12BIT,
    .oversample = NRF_SAADC_OVERSAMPLE_DISABLED,
    .interrupt_priority = 2,
    .low_power_mode = false
  };

  err_code = nrf_drv_saadc_init(&saadc_config, saadc_callback);
  APP_ERROR_CHECK(err_code);

  /* Initialize channels */
  
  err_code = nrf_drv_saadc_channel_init(0, &channel_config_mic);
  APP_ERROR_CHECK(err_code);

  err_code = nrf_drv_saadc_channel_init(1, &channel_config_acc);
  APP_ERROR_CHECK(err_code);

  err_code = nrf_drv_saadc_channel_init(BAT_CHANNEL, &channel_config_bat);
  APP_ERROR_CHECK(err_code);


  /* Set buffers */
  err_code = nrf_drv_saadc_buffer_convert(buffer_pool[0], SAMPLES_IN_BUFFER);
  APP_ERROR_CHECK(err_code);

  /* Initialize all buffers only when accel and microphone are being sampled */
 
  err_code = nrf_drv_saadc_buffer_convert(buffer_pool[1], SAMPLES_IN_BUFFER);
  APP_ERROR_CHECK(err_code);
  

  APP_ERROR_CHECK(nrf_drv_ppi_init());

  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(&adc_timer, &timer_cfg, timer_handler);
  APP_ERROR_CHECK(err_code);

  /* If only battery sampling is needed, use a lower sampling rate
   * All channels will be sampled on every iteration.
   */
  uint32_t ticks;
  if(bat_only){
    ticks = nrf_drv_timer_ms_to_ticks(&adc_timer, ADC_BATT_SAMPLE_INTERVAL);
  }
  else{
    ticks = nrf_drv_timer_us_to_ticks(&adc_timer, ADC_FULL_SAMPLE_INTERVAL);
  }
  
  nrf_drv_timer_extended_compare(&adc_timer,
                                 NRF_TIMER_CC_CHANNEL0,
                                 ticks,
                                 NRF_TIMER_SHORT_COMPARE0_CLEAR_MASK,
                                 false);
  
  uint32_t timer_compare_event_addr = nrf_drv_timer_compare_event_address_get(&adc_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(&ppi_channel);
  APP_ERROR_CHECK(err_code);

  err_code = nrf_drv_ppi_channel_alloc(&ppi_channel2);
  APP_ERROR_CHECK(err_code);

  err_code = nrf_drv_ppi_channel_assign(ppi_channel,
                                        timer_compare_event_addr,
                                        saadc_sample_task_addr);
  APP_ERROR_CHECK(err_code);
  
  err_code = nrf_drv_ppi_channel_assign(ppi_channel2, 
    nrf_saadc_event_address_get(NRF_SAADC_EVENT_END), 
    nrf_saadc_task_address_get(NRF_SAADC_TASK_START)
  );
  APP_ERROR_CHECK(err_code);

  nrf_drv_timer_enable(&adc_timer);

  periph_status.adc_status = (bat_only) ? ADC_BATTERY_ONLY : ADC_FULL ;
}


/**
 * Start ADC Sampling
 * Params: None
 * Ret: None
 */
static void start_adc_sampling(void){
  APP_ERROR_CHECK(nrf_drv_ppi_channel_enable(ppi_channel));
  APP_ERROR_CHECK(nrf_drv_ppi_channel_enable(ppi_channel2));
}

The SAADC runs with priority level = 2, and there is not much else that interrupts it other than the softdevice. In the near future I will incoorporate a feature to send a BLE packet every 10 ms.

Please can you shed more light on how the solution 1 (Use PPI to trigger START task on an END event. This will avoid the delayed triggering og the START task due to a queued interrupt generated by the END event,) from the link above should be implemented. I have tried the implied implementation but this doesnt solve this problem.

I am using the latest nRF17.0.0 SDK so I am suprised this issue still persists after soo many years. 

Please what is the best workaround for this?

Is a sample rate of 50 kHz achievable using CPU interrupts?

[haakonsh]

Hey David,

What's the worst-case run time of saadc_callback? If it's more than the sample rate multiplied by the number of samples divided by the number of channels, you will have issues serving the SAADC interrupts.

Mixing of samples in buffers is usually due to running the offset calibration after triggering the START task, running the calibration without first stopping the SAADC, triggering the START task too early/late, or mismanaging the buffers. 

My first guess would be that you're triggering the START task after you've started sampling with the call to nrfx_saadc_buffer_convert in the saadc_callback. If you've connected the END event to the START task you should update the SAADC's buffer registers immediately after the STARTED event has fired without triggering another START task. 

FYI:
Saadc_callback runs inside the SAADC ISR at whatever priority this ISR is set to. The SoftDevice will eventually crash if it is blocked for too long at prio 2 given a particular work-load, ie. as the SAADC sample rate increases there will be a point where the SoftDevice crashes. 

[David]

where do I catch the STARTED EVENT?\

There is SAADC_CALLBACK Only throws the following events:

typedef enum
{
NRFX_SAADC_EVT_DONE, ///< Event generated when the buffer is filled with samples.
NRFX_SAADC_EVT_LIMIT, ///< Event generated after one of the limits is reached.
NRFX_SAADC_EVT_CALIBRATEDONE ///< Event generated when the calibration is complete.
} nrfx_saadc_evt_type_t;

[David]

I went through the library source code and I have seen that these values can be updated in nrfx_saadc.c irq function. but it is still not clear what should be done wit the buffers.

I have also done more research and trails with the board and obtained the following results:

- With BLE disabled, no swapping occurs and ADC buffers are in correct order,

- With BLE enabled but not connection, swapping occurs

- With BLE enabled and connected to smartphone, more frequent swapping occurs.

It is safe to say that the whole situation stems for the softdevice interrupts.

1) My application needs to sample at a minimum sample rate of 40 kHz while maintaining a BLE connection. Do you have any pointers on the interrupt/peripheral configuration that can be used to achieve this?

2) Is it possible to disable softdevice interrupts in a critical part of the code while still maintaining BLE connection.

https://github.com/NordicPlayground/nRF52-ADC-examples/blob/master/ble_app_uart__saadc_timer_driven__scan_mode/main.c

This example from your team tries to perform BLE transfer and SAADC Scan sampling at the same time, but it only samples at 250 Hz.

[haakonsh]

David said:

It is safe to say that the whole situation stems for the softdevice interrupts.

 Most likely, and that also indicate that the issue is that the START task is trigger both in HW and SW, where the SW triggered START task is delayed by a preemtive SoftDevice interrupt. 

Do you see the issue if you do not connect the END task to the START task? 

1) If you are using the END->START PPI channel then you should not use the buffer_convert functionality of the driver, as it will itself call the START task. I rather suggest that you set up the buffers manually in the SAADC ISR following a STARTED event. 

2) Yes. See Multiprotocol support. A Radio Timeslot can be used to execute code with guaranteed execution. ie. no innterupts from the SoftDevice during a Timeslot.

[David]

The first solution gives results that are a little bit more stable. The ADC never seems to start in a specific order. the buffer arrangement is different every time the program starts.

Is there a specific order of instructions that must be used to start the ADC to ensure correct buffer placement everytime?

[haakonsh]

David said:

The ADC never seems to start in a specific order. the buffer arrangement is different every time the program starts.

What do you mean? 
The samples should always be in the format specified in EasyDMA. If they are not, and you're not using calibration, then there is a mismatch between when the START task is triggered and when the SAMPLE tasks are triggered. 

Can you share more of your code, at least what pertains to the SAADC operation? 

[David]

#define SAMPLES_IN_BUFFER   3 * 50

void saadc_callback(nrf_drv_saadc_evt_t const * p_event)
{
    
    if (p_event->type == NRF_DRV_SAADC_EVT_DONE)
    {
        ret_code_t err_code;
        
        int16_t mic_val = p_event->data.done.p_buffer[1*3];
        int16_t acc_val = p_event->data.done.p_buffer[1*3+1];
        int16_t b_val = p_event->data.done.p_buffer[1*3+2];
        printf("%d %d %d\n", mic_val, acc_val, b_val);
    }
}



/**
 * Callback for Timer
 * Params: Timer Event type
 * Ret: None
 */
void timer_handler(nrf_timer_event_t event_type, void * p_context){
}


/**
 * Initialize ADC (saadc)
 * Params:
 *  bool bat_only: Flag to determine if only Battery ADC should be enabled
 * Ret: None
 */
static void init_adc(void){
  ret_code_t err_code;

  nrf_saadc_channel_config_t channel_config_mic = NRF_DRV_SAADC_DEFAULT_CHANNEL_CONFIG_SE(MIC_ADC);
  channel_config_mic.acq_time                   = NRF_SAADC_ACQTIME_3US;

  nrf_saadc_channel_config_t channel_config_acc = NRF_DRV_SAADC_DEFAULT_CHANNEL_CONFIG_SE(ACC_ADC);
  channel_config_acc.acq_time                   = NRF_SAADC_ACQTIME_3US;

  nrf_saadc_channel_config_t channel_config_bat = NRF_DRV_SAADC_DEFAULT_CHANNEL_CONFIG_SE(BAT_ADC);
  channel_config_bat.acq_time                   = NRF_SAADC_ACQTIME_3US;

  nrf_drv_saadc_config_t saadc_config = {
    .resolution = NRF_SAADC_RESOLUTION_12BIT,
    .oversample = NRF_SAADC_OVERSAMPLE_DISABLED,
    .interrupt_priority = 5,
    .low_power_mode = false
  };

  APP_ERROR_CHECK(nrf_drv_saadc_init(&saadc_config, saadc_callback));

  /* Initialize channels */
  APP_ERROR_CHECK(nrf_drv_saadc_channel_init(1, &channel_config_mic));
  APP_ERROR_CHECK(nrf_drv_saadc_channel_init(2, &channel_config_acc));
  APP_ERROR_CHECK(nrf_drv_saadc_channel_init(BAT_CHANNEL, &channel_config_bat));

  /* Set buffers */
  APP_ERROR_CHECK(nrf_drv_saadc_buffer_convert(buffer_pool[0], SAMPLES_IN_BUFFER));
  APP_ERROR_CHECK(nrf_drv_saadc_buffer_convert(buffer_pool[1], SAMPLES_IN_BUFFER));
  
  APP_ERROR_CHECK(nrf_drv_ppi_init());

  nrf_drv_timer_config_t timer_cfg  = NRF_DRV_TIMER_DEFAULT_CONFIG;
  timer_cfg.bit_width               = NRF_TIMER_BIT_WIDTH_32;

  APP_ERROR_CHECK(nrf_drv_timer_init(&adc_timer, &timer_cfg, timer_handler));

  uint32_t ticks = nrf_drv_timer_us_to_ticks(&adc_timer, ADC_FULL_SAMPLE_INTERVAL);

  nrf_drv_timer_extended_compare(&adc_timer, NRF_TIMER_CC_CHANNEL0, ticks, 
      NRF_TIMER_SHORT_COMPARE0_CLEAR_MASK, false);
  
  uint32_t timer_cmp_evt_addr   = nrf_drv_timer_compare_event_address_get(&adc_timer,
                                                                              NRF_TIMER_CC_CHANNEL0);
  uint32_t adc_sample_task_addr = nrf_drv_saadc_sample_task_get();

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

  APP_ERROR_CHECK(nrf_drv_ppi_channel_alloc(&ppi_channel2));

  APP_ERROR_CHECK(nrf_drv_ppi_channel_assign(ppi_channel, timer_cmp_evt_addr, adc_sample_task_addr));
  
  err_code = nrf_drv_ppi_channel_assign(ppi_channel2, 
    nrf_saadc_event_address_get(NRF_SAADC_EVENT_END), 
    nrf_saadc_task_address_get(NRF_SAADC_TASK_START)
  );
  APP_ERROR_CHECK(err_code);

  APP_ERROR_CHECK(nrf_drv_ppi_channel_enable(ppi_channel));
  APP_ERROR_CHECK(nrf_drv_ppi_channel_enable(ppi_channel2));

  //nrf_saadc_task_trigger(NRF_SAADC_TASK_SAMPLE);
  nrf_drv_timer_enable(&adc_timer);
  nrf_drv_timer_clear(&adc_timer);
}

I this is what my adc setup looks like. I call the init function once to startup the ADC.

The END event is used to trigger the next start like we discussed above.

[haakonsh]

That looks alright, but how do you set the buffers following the STARTED event? 

Also, I suggest that you increase the buffer size significantly, as it will greatly reduce the number of interrupts that you need to service. 

[David]

I assumed the nrf_saadc handles the setting of the buffer inside the driver. But it seems this is wrong. I guess I have to make changes to the saadc driver.  How should the buffer be set?

Also my application needs to see the sampled data really often. What buffer size do you think will be sufficient?

[haakonsh]

I'm looking at the new SAADC driver v2 in sdk17 and it has this already implemented, including triggering the START task following an END event. 

See modules\nrfx\drivers\include\nrfx_saadc_v2.h and modules\nrfx\drivers\src\nrfx_saadc.c. 

nrfx_saadc_init

nrfx_saadc_channels_config

nrfx_saadc_advanced_mode_set, set start_on_end to 'true' in nrfx_saadc_adv_config_t, and supply an event handler to enable non-blocking operation. 

nrfx_saadc_mode_trigger

nrfx_saadc_buffer_set(buffer1)

nrfx_saadc_buffer_set(buffer2)

[David]

So this means I don't need to do anything with the driver. What could be the source of the swapped buffer positions? 

Also what buffer size do you think is sufficient?