ADC Continuous mode sometimes returns invalid channel number (IDFGH-13842)

ermacv commented 2 weeks ago

Answers checklist.

[X] I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
[X] I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
[X] I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.3.1

Espressif SoC revision.

ESP32-S3

Operating System used.

Windows

How did you build your project?

Command line with CMake

If you are using Windows, please specify command line type.

None

Development Kit.

Custom Board

Power Supply used.

USB

What is the expected behavior?

I expect that ADC in continuous mode returns valid channel in adc_digi_output_data_t structure.

What is the actual behavior?

ADC in continuous mode sometimes returns invalid channel in adc_digi_output_data_t structure.

Steps to reproduce.

Hello!

I read an ADC channel in continuous mode using the following function plat_analog_get_voltage. Sometimes I got assertion failed on line assert(channel == p->type2.channel);. The function plat_analog_get_voltage is called in polling task several times (~10) and then the task is sleep for 100 ms. The assertion could be triggered just after device boot and after few hours, so I don't know how to reproduce it correctly. The plat_analog_init is called only on device boot.

#define SAMPLE_RATE_HZ (80 * 1000)
#define NO_OF_SAMPLES (64)  //< ADC multisampling number of samples
#define TAG "plat_analog"

static const adc_continuous_handle_cfg_t init_config1 = {
    .max_store_buf_size = NO_OF_SAMPLES * SOC_ADC_DIGI_DATA_BYTES_PER_CONV,
    .conv_frame_size = NO_OF_SAMPLES * SOC_ADC_DIGI_DATA_BYTES_PER_CONV,
    .flags.flush_pool = 0,
};
static const adc_cali_curve_fitting_config_t cali_config = {
    .unit_id = ADC_UNIT_1,
    .atten = ADC_ATTEN_DB_12,
    .bitwidth = ADC_BITWIDTH_12,
};

static SemaphoreHandle_t access_mtx;
static adc_continuous_handle_t adc1_handle;
static adc_cali_handle_t adc1_cali_handle = NULL;

static adc_digi_pattern_config_t adc_pattern[] = {
    [0] =
        {
            .atten = ADC_ATTEN_DB_12,
            .bit_width = SOC_ADC_DIGI_MIN_BITWIDTH,
            .unit = ADC_UNIT_1,
        },
};

static_assert(SOC_ADC_DIGI_MIN_BITWIDTH == 12, "Tune bit width of the calibration and pattern config");

static const adc_continuous_config_t config = {
    .sample_freq_hz = SAMPLE_RATE_HZ,
    .conv_mode = ADC_CONV_SINGLE_UNIT_1,
    .format = ADC_DIGI_OUTPUT_FORMAT_TYPE2,
    .pattern_num = sizeof(adc_pattern) / sizeof(adc_pattern[0]),
    .adc_pattern = adc_pattern,
};

// GCOVR_EXCL_START
void plat_analog_init(void) {
  ESP_ERROR_CHECK(adc_continuous_new_handle(&init_config1, &adc1_handle));
  ESP_ERROR_CHECK(adc_cali_create_scheme_curve_fitting(&cali_config, &adc1_cali_handle));
  access_mtx = xSemaphoreCreateMutex();
  assert(access_mtx);
}

void plat_analog_deinit(void) {
  ESP_ERROR_CHECK(adc_cali_delete_scheme_curve_fitting(adc1_cali_handle));
  ESP_ERROR_CHECK(adc_continuous_deinit(adc1_handle));
  vSemaphoreDelete(access_mtx);
  access_mtx = NULL;
}
// GCOVR_EXCL_STOP

uint32_t plat_analog_get_voltage(gpio_num_t pin) {
  BaseType_t mutex_rc = xSemaphoreTake(access_mtx, portMAX_DELAY);
  assert(mutex_rc == pdTRUE);
  adc_channel_t channel;
  adc_unit_t unit_id;
  ESP_ERROR_CHECK(adc_continuous_io_to_channel(pin, &unit_id, &channel));
  assert(unit_id == ADC_UNIT_1);
  adc_pattern[0].channel = channel;
  adc_iir_filter_handle_t adc_filter_handle;
  const adc_continuous_iir_filter_config_t filter_config = {
      .channel = channel,
      .coeff = ADC_DIGI_IIR_FILTER_COEFF_64,
      .unit = ADC_UNIT_1,
  };
  ESP_ERROR_CHECK(adc_new_continuous_iir_filter(adc1_handle, &filter_config, &adc_filter_handle));
  ESP_ERROR_CHECK(adc_continuous_iir_filter_enable(adc_filter_handle));
  ESP_ERROR_CHECK(adc_continuous_config(adc1_handle, &config));
  ESP_ERROR_CHECK(adc_continuous_start(adc1_handle));
  static uint8_t buf[NO_OF_SAMPLES * SOC_ADC_DIGI_DATA_BYTES_PER_CONV];
  memset(buf, 0, sizeof(buf));
  uint32_t out_length;
  ESP_ERROR_CHECK(adc_continuous_read(adc1_handle, buf, sizeof(buf), &out_length, ADC_MAX_DELAY));
  assert(out_length == sizeof(buf));
  ESP_ERROR_CHECK(adc_continuous_stop(adc1_handle));
  ESP_ERROR_CHECK(adc_continuous_flush_pool(adc1_handle));
  ESP_ERROR_CHECK(adc_continuous_iir_filter_disable(adc_filter_handle));
  uint32_t adc_reading = 0;
  // // Multisampling
  for (uint32_t i = 0; i < out_length; i += SOC_ADC_DIGI_RESULT_BYTES) {
    adc_digi_output_data_t *p = (adc_digi_output_data_t *)&buf[i];
    if (channel != p->type2.channel) {
      ESP_LOGW(TAG, "channel = %d, p->type2.channel = %d", channel, p->type2.channel);
    }
    if (unit_id != p->type2.unit) {
      ESP_LOGW(TAG, "unit_id = %d, p->type2.unit = %d", unit_id, p->type2.unit);
    }
    assert(channel == p->type2.channel);
    assert(unit_id == p->type2.unit);
    adc_reading += p->type2.data;
  }

  adc_reading /= NO_OF_SAMPLES;

  int mv;
  // Convert adc_reading to voltage in mV
  ESP_ERROR_CHECK(adc_del_continuous_iir_filter(adc_filter_handle));
  ESP_ERROR_CHECK(adc_cali_raw_to_voltage(adc1_cali_handle, adc_reading, &mv));
  ESP_LOGV(TAG, "ADC[%d] = %d mV", channel, mv);
  mutex_rc = xSemaphoreGive(access_mtx);
  assert(mutex_rc == pdTRUE);
  return mv;
}

Debug Logs.

W (15736771) plat_analog: channel = 7, p->type2.channel = 6

assert failed: 0x4200efa0
0x4200efa0: plat_analog_get_voltage at /home/ermacv/dev/proj/smart_air_key/git/ac20-fw/components/platform/private/plat_analog_cont.c:73 (discriminator 1)

Backtrace: 0x403756c6:0x3c137540 0x4037e035:0x3c137560 0x40385e25:0x3c137580 0x4200efa0:0x3c1375c0 0x4201118d:0x3c137610 0x42027135:0x3c137630
0x403756c6: panic_abort at /home/ermacv/esp/esp-idf/components/esp_system/panic.c:463
0x4037e035: esp_system_abort at /home/ermacv/esp/esp-idf/components/esp_system/port/esp_system_chip.c:92
0x40385e25: __assert_func at /home/ermacv/esp/esp-idf/components/newlib/assert.c:39
0x4200efa0: plat_analog_get_voltage at /home/ermacv/dev/proj/smart_air_key/git/ac20-fw/components/platform/private/plat_analog_cont.c:73 (discriminator 1)
0x4201118d: plat_gp_in_get_active at /home/ermacv/dev/proj/smart_air_key/git/ac20-fw/components/platform/plat_gp_in.c:150
0x42027135: get_active at /home/ermacv/dev/proj/smart_air_key/git/ac20-fw/components/app_acs/src/app_acs_controller_input.c:134
 (inlined by) input_get_state_int at /home/ermacv/dev/proj/smart_air_key/git/ac20-fw/components/app_acs/src/app_acs_controller_input.c:322
 (inlined by) input_poll_task at /home/ermacv/dev/proj/smart_air_key/git/ac20-fw/components/app_acs/src/app_acs_controller_input.c:339

More Information.

No response

cristianfunes79 commented 2 weeks ago

I'll take a look.

ermacv commented 2 weeks ago

Hello! I still cannot create 100% reproducible test app but I have some extra results.

I included private headers to the source file and read the number of ADC internal ring buffer items before starting a new ADC conversion. The code:

  UBaseType_t uxItemsWaiting;
  vRingbufferGetInfo(adc1_handle->ringbuf_hdl, NULL, NULL, NULL, NULL, &uxItemsWaiting);

  if (uxItemsWaiting != 0) {
    ESP_LOGW(TAG, "uxItemsWaiting = %u", uxItemsWaiting);
  }

  ESP_ERROR_CHECK(adc_continuous_config(adc1_handle, &config));
  ESP_ERROR_CHECK(adc_continuous_start(adc1_handle));
  ...

And when the assertion is fired I also see the following log:

W (615) plat_analog: uxItemsWaiting = 256
W (615) plat_analog: channel = 7, p->type2.channel = 6

So it seems that calling:

  ESP_ERROR_CHECK(adc_continuous_stop(adc1_handle));
  ESP_ERROR_CHECK(adc_continuous_flush_pool(adc1_handle));

after successful adc_continuous_read is not enough and data are pushed to the ring buffer from ISR handler.

ermacv commented 2 weeks ago

Wow! I have more interesting results! I modified the read code:

  UBaseType_t uxItemsWaiting;
  vRingbufferGetInfo(adc1_handle->ringbuf_hdl, NULL, NULL, NULL, NULL, &uxItemsWaiting);

  if (uxItemsWaiting != 0) {
    ESP_LOGW(TAG, "uxItemsWaiting (before adc_continuous_config) = %u", uxItemsWaiting);
  }

  ESP_ERROR_CHECK(adc_continuous_config(adc1_handle, &config));
  ESP_ERROR_CHECK(adc_continuous_start(adc1_handle));
  static uint8_t buf[NO_OF_SAMPLES * SOC_ADC_DIGI_DATA_BYTES_PER_CONV];
  uint32_t out_length;
  ESP_ERROR_CHECK(adc_continuous_read(adc1_handle, buf, sizeof(buf), &out_length, ADC_MAX_DELAY));
  assert(out_length == sizeof(buf));
  ESP_ERROR_CHECK(adc_continuous_stop(adc1_handle));
  ESP_ERROR_CHECK(adc_continuous_flush_pool(adc1_handle));

  vTaskDelay(0);

  vRingbufferGetInfo(adc1_handle->ringbuf_hdl, NULL, NULL, NULL, NULL, &uxItemsWaiting);

  if (uxItemsWaiting != 0) {
    ESP_LOGW(TAG, "uxItemsWaiting (after adc_continuous_flush_pool) = %u", uxItemsWaiting);
  }

And I observe this log (after several attempts):

W (1606) plat_analog: uxItemsWaiting (after adc_continuous_flush_pool) = 256
W (1616) plat_analog: uxItemsWaiting (before adc_continuous_config) = 256
W (1626) plat_analog: channel = 6, p->type2.channel = 0

So it means that ADC HW could sometimes generate an interrupt even after adc_continuous_stop call and it is the reason why ring buffer is filled on plat_analog_get_voltage new function call.

espressif / esp-idf