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

ESP32 ADC DMA sampling rate different than configured sampling frequency with ESP-IDF 5.0 (IDFGH-9225) #10612

Open
3 tasks done
milindmovasha opened this issue Jan 25, 2023 · 32 comments
Labels
Status: Selected for Development Issue is selected for development

Comments

@milindmovasha
Copy link

Answers checklist.

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

General issue report

Hello, I have modified the ESP-IDF examples/peripherals/adc/continuous_read/main/continuous_read_main.c as follows to check the samples generated per second

/*
 * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
 *
 * SPDX-License-Identifier: Apache-2.0
 */

#include <string.h>
#include <stdio.h>
#include "sdkconfig.h"
#include "esp_log.h"
#include "esp_timer.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/semphr.h"
#include "esp_adc/adc_continuous.h"

#define EXAMPLE_READ_LEN   256
#define GET_UNIT(x)        ((x>>3) & 0x1)

#if CONFIG_IDF_TARGET_ESP32
#define ADC_CONV_MODE       ADC_CONV_SINGLE_UNIT_1  //ESP32 only supports ADC1 DMA mode
#define ADC_OUTPUT_TYPE     ADC_DIGI_OUTPUT_FORMAT_TYPE1
#elif CONFIG_IDF_TARGET_ESP32S2
#define ADC_CONV_MODE       ADC_CONV_BOTH_UNIT
#define ADC_OUTPUT_TYPE     ADC_DIGI_OUTPUT_FORMAT_TYPE2
#elif CONFIG_IDF_TARGET_ESP32C3 || CONFIG_IDF_TARGET_ESP32H2 || CONFIG_IDF_TARGET_ESP32C2
#define ADC_CONV_MODE       ADC_CONV_ALTER_UNIT     //ESP32C3 only supports alter mode
#define ADC_OUTPUT_TYPE     ADC_DIGI_OUTPUT_FORMAT_TYPE2
#elif CONFIG_IDF_TARGET_ESP32S3
#define ADC_CONV_MODE       ADC_CONV_BOTH_UNIT
#define ADC_OUTPUT_TYPE     ADC_DIGI_OUTPUT_FORMAT_TYPE2
#endif

#if CONFIG_IDF_TARGET_ESP32C3 || CONFIG_IDF_TARGET_ESP32S3 || CONFIG_IDF_TARGET_ESP32H2 || CONFIG_IDF_TARGET_ESP32C2
static adc_channel_t channel[3] = {ADC_CHANNEL_2, ADC_CHANNEL_3, (ADC_CHANNEL_0 | 1 << 3)};
#endif
#if CONFIG_IDF_TARGET_ESP32S2
static adc_channel_t channel[3] = {ADC_CHANNEL_2, ADC_CHANNEL_3, (ADC_CHANNEL_0 | 1 << 3)};
#endif  
#if CONFIG_IDF_TARGET_ESP32
static adc_channel_t channel[1] = {ADC_CHANNEL_7};
#endif
static TaskHandle_t s_task_handle;
static const char *TAG = "EXAMPLE";
int overflow = 0;

static bool IRAM_ATTR s_conv_done_cb(adc_continuous_handle_t handle, const adc_continuous_evt_data_t *edata, void *user_data)
{
    BaseType_t mustYield = pdFALSE;
    //Notify that ADC continuous driver has done enough number of conversions
    vTaskNotifyGiveFromISR(s_task_handle, &mustYield);

    return (mustYield == pdTRUE);
}

static bool IRAM_ATTR s_on_pool_ovf(adc_continuous_handle_t handle, const adc_continuous_evt_data_t *edata, void *user_data)
{
    BaseType_t mustYield = pdFALSE;
    overflow++;
    //Notify that ADC continuous driver has done enough number of conversions
    vTaskNotifyGiveFromISR(s_task_handle, &mustYield);
    return (mustYield == pdTRUE);
}

static void continuous_adc_init(adc_channel_t *channel, uint8_t channel_num, adc_continuous_handle_t *out_handle)
{
    adc_continuous_handle_t handle = NULL;

    adc_continuous_handle_cfg_t adc_config = {
        .max_store_buf_size = 1024,
        .conv_frame_size = EXAMPLE_READ_LEN,
    };
    ESP_ERROR_CHECK(adc_continuous_new_handle(&adc_config, &handle));

    adc_continuous_config_t dig_cfg = {
        .sample_freq_hz = 80000,
        .conv_mode = ADC_CONV_MODE,
        .format = ADC_OUTPUT_TYPE,
    };
    adc_digi_pattern_config_t adc_pattern[SOC_ADC_PATT_LEN_MAX] = {0};
    dig_cfg.pattern_num = channel_num;
    for (int i = 0; i < channel_num; i++) {
        uint8_t unit = GET_UNIT(channel[i]);
        uint8_t ch = channel[i] & 0x7;
        adc_pattern[i].atten = ADC_ATTEN_DB_0;
        adc_pattern[i].channel = ch;
        adc_pattern[i].unit = unit;
        adc_pattern[i].bit_width = SOC_ADC_DIGI_MAX_BITWIDTH;

        ESP_LOGI(TAG, "adc_pattern[%d].atten is :%x", i, adc_pattern[i].atten);
        ESP_LOGI(TAG, "adc_pattern[%d].channel is :%x", i, adc_pattern[i].channel);
        ESP_LOGI(TAG, "adc_pattern[%d].unit is :%x", i, adc_pattern[i].unit);
    }
    dig_cfg.adc_pattern = adc_pattern;
    ESP_ERROR_CHECK(adc_continuous_config(handle, &dig_cfg));

    *out_handle = handle;
}

#if !CONFIG_IDF_TARGET_ESP32
static bool check_valid_data(const adc_digi_output_data_t *data)
{
    const unsigned int unit = data->type2.unit;
    if (unit > 2) return false;
    if (data->type2.channel >= SOC_ADC_CHANNEL_NUM(unit)) return false;

    return true;
}
#endif

void app_main(void)
{
    esp_err_t ret;
    uint32_t ret_num = 0;
    uint8_t result[EXAMPLE_READ_LEN] = {0};

    int64_t samples = 0;
    int64_t current_us;
    int64_t previous_us;

    memset(result, 0xcc, EXAMPLE_READ_LEN);

    s_task_handle = xTaskGetCurrentTaskHandle();
    adc_continuous_handle_t handle = NULL;
    continuous_adc_init(channel, sizeof(channel) / sizeof(adc_channel_t), &handle);

    adc_continuous_evt_cbs_t cbs = {
        .on_conv_done = s_conv_done_cb,
        .on_pool_ovf = s_on_pool_ovf,
    };
    ESP_ERROR_CHECK(adc_continuous_register_event_callbacks(handle, &cbs, NULL));
    ESP_ERROR_CHECK(adc_continuous_start(handle));

    previous_us = esp_timer_get_time();
    while(1) {

        /**
         * This is to show you the way to use the ADC continuous mode driver event callback.
         * This `ulTaskNotifyTake` will block when the data processing in the task is fast.
         * However in this example, the data processing (print) is slow, so you barely block here.
         *
         * Without using this event callback (to notify this task), you can still just call
         * `adc_continuous_read()` here in a loop, with/without a certain block timeout.
         */
        ulTaskNotifyTake(pdTRUE, portMAX_DELAY);

        while (1) {
            ret = adc_continuous_read(handle, result, EXAMPLE_READ_LEN, &ret_num, 0);
            if (ret == ESP_OK) {
#define MAX_SAMPLES     80000
                samples += (ret_num/SOC_ADC_DIGI_RESULT_BYTES);
                if(samples >= MAX_SAMPLES) {
                    current_us = esp_timer_get_time();
                    int64_t sps = samples*1000000/ (current_us-previous_us);
                    printf("samples = %lld, time = %lld us, sps = %lld, overflows = %d\n",samples,((current_us-previous_us)),sps,overflow);
                    previous_us = esp_timer_get_time();
                    samples  = 0;
                }
#if 0
                ESP_LOGI("TASK", "ret is %x, ret_num is %"PRIu32, ret, ret_num);
                for (int i = 0; i < ret_num; i += SOC_ADC_DIGI_RESULT_BYTES) {
                    adc_digi_output_data_t *p = (void*)&result[i];
        #if CONFIG_IDF_TARGET_ESP32
                    ESP_LOGI(TAG, "Unit: %d, Channel: %d, Value: %x", 1, p->type1.channel, p->type1.data);
        #else
                    if (ADC_CONV_MODE == ADC_CONV_BOTH_UNIT || ADC_CONV_MODE == ADC_CONV_ALTER_UNIT) {
                        if (check_valid_data(p)) {
                            ESP_LOGI(TAG, "Unit: %d,_Channel: %d, Value: %x", p->type2.unit+1, p->type2.channel, p->type2.data);
                        } else {
                            ESP_LOGI(TAG, "Invalid data [%d_%d_%x]", p->type2.unit+1, p->type2.channel, p->type2.data);
                        }
                    }
        #if CONFIG_IDF_TARGET_ESP32S2
                    else if (ADC_CONV_MODE == ADC_CONV_SINGLE_UNIT_2) {
                        ESP_LOGI(TAG, "Unit: %d, Channel: %d, Value: %x", 2, p->type1.channel, p->type1.data);
                    } else if (ADC_CONV_MODE == ADC_CONV_SINGLE_UNIT_1) {
                        ESP_LOGI(TAG, "Unit: %d, Channel: %d, Value: %x", 1, p->type1.channel, p->type1.data);
                    }
        #endif  //#if CONFIG_IDF_TARGET_ESP32S2
        #endif
                }
                /**
                 * Because printing is slow, so every time you call `ulTaskNotifyTake`, it will immediately return.
                 * To avoid a task watchdog timeout, add a delay here. When you replace the way you process the data,
                 * usually you don't need this delay (as this task will block for a while).
                 */
                vTaskDelay(1);
#endif
            } else if (ret == ESP_ERR_TIMEOUT) {
                //We try to read `EXAMPLE_READ_LEN` until API returns timeout, which means there's no available data
                break;
            }
        }
    }

    ESP_ERROR_CHECK(adc_continuous_stop(handle));
    ESP_ERROR_CHECK(adc_continuous_deinit(handle));
}


Getting the following output when the above example is run on ESP32-DevKit-V1 board:

--- idf_monitor on /dev/ttyUSB0 115200 ---
--- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H ---
ets Jun  8 2016 00:22:57

rst:0x1 (POWERON_RESET),boot:0x17 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff0030,len:7000
load:0x40078000,len:15452
ho 0 tail 12 room 4
load:0x40080400,len:3840
0x40080400: _init at ??:?

entry 0x4008064c
I (29) boot: ESP-IDF v5.0-dirty 2nd stage bootloader
I (29) boot: compile time 13:30:47
I (29) boot: chip revision: v1.0
I (32) boot_comm: chip revision: 1, min. bootloader chip revision: 0
I (39) boot.esp32: SPI Speed      : 40MHz
I (44) boot.esp32: SPI Mode       : DIO
I (49) boot.esp32: SPI Flash Size : 2MB
I (53) boot: Enabling RNG early entropy source...
I (58) boot: Partition Table:
I (62) boot: ## Label            Usage          Type ST Offset   Length
I (69) boot:  0 nvs              WiFi data        01 02 00009000 00006000
I (77) boot:  1 phy_init         RF data          01 01 0000f000 00001000
I (84) boot:  2 factory          factory app      00 00 00010000 00100000
I (92) boot: End of partition table
I (96) boot_comm: chip revision: 1, min. application chip revision: 0
I (103) esp_image: segment 0: paddr=00010020 vaddr=3f400020 size=09a4ch ( 39500) map
I (126) esp_image: segment 1: paddr=00019a74 vaddr=3ffb0000 size=02678h (  9848) load
I (130) esp_image: segment 2: paddr=0001c0f4 vaddr=40080000 size=03f24h ( 16164) load
I (138) esp_image: segment 3: paddr=00020020 vaddr=400d0020 size=1874ch (100172) map
I (176) esp_image: segment 4: paddr=00038774 vaddr=40083f24 size=08860h ( 34912) load
I (191) esp_image: segment 5: paddr=00040fdc vaddr=50000000 size=00010h (    16) load
I (198) boot: Loaded app from partition at offset 0x10000
I (198) boot: Disabling RNG early entropy source...
I (211) cpu_start: Pro cpu up.
I (211) cpu_start: Starting app cpu, entry point is 0x40081168
0x40081168: call_start_cpu1 at /space/projects/esp32/esp-idf-v5.0/components/esp_system/port/cpu_start.c:142

I (0) cpu_start: App cpu up.
I (225) cpu_start: Pro cpu start user code
I (225) cpu_start: cpu freq: 160000000 Hz
I (226) cpu_start: Application information:
I (230) cpu_start: Project name:     continuous_read
I (236) cpu_start: App version:      v5.0-dirty
I (241) cpu_start: Compile time:     Jan 25 2023 16:51:15
I (247) cpu_start: ELF file SHA256:  a47a436d0c39ad40...
I (253) cpu_start: ESP-IDF:          v5.0-dirty
I (259) heap_init: Initializing. RAM available for dynamic allocation:
I (266) heap_init: At 3FFAE6E0 len 00001920 (6 KiB): DRAM
I (272) heap_init: At 3FFB2F70 len 0002D090 (180 KiB): DRAM
I (278) heap_init: At 3FFE0440 len 00003AE0 (14 KiB): D/IRAM
I (284) heap_init: At 3FFE4350 len 0001BCB0 (111 KiB): D/IRAM
I (291) heap_init: At 4008C784 len 0001387C (78 KiB): IRAM
I (298) spi_flash: detected chip: generic
I (302) spi_flash: flash io: dio
W (306) spi_flash: Detected size(4096k) larger than the size in the binary image header(2048k). Using the size in the binary image header.
I (320) cpu_start: Starting scheduler on PRO CPU.
I (0) cpu_start: Starting scheduler on APP CPU.
I (330) EXAMPLE: adc_pattern[0].atten is :0
I (330) EXAMPLE: adc_pattern[0].channel is :7
I (340) EXAMPLE: adc_pattern[0].unit is :0
I (350) gpio: GPIO[35]| InputEn: 0| OutputEn: 0| OpenDrain: 0| Pullup: 0| Pulldown: 0| Intr:0 
samples = 80000, time = 1225531 us, sps = 65277, overflows = 0
samples = 80000, time = 1225185 us, sps = 65296, overflows = 0
samples = 80000, time = 1225348 us, sps = 65287, overflows = 0
samples = 80000, time = 1225347 us, sps = 65287, overflows = 0
samples = 80000, time = 1225357 us, sps = 65287, overflows = 0
samples = 80000, time = 1225349 us, sps = 65287, overflows = 0
samples = 80000, time = 1225352 us, sps = 65287, overflows = 0
samples = 80000, time = 1225344 us, sps = 65287, overflows = 0

Done

As seen in the results the samples are generated at rate 65287 samples/second whereas configured sampling frequency is 80000. I have tried with different values of desired sample frequency. There is always a ratio of 1.22 between expected desired frequency to achieved sampling rates.

Please help me understand why there is a difference between achieved sample rate vs configured sampling frequency.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jan 25, 2023
@github-actions github-actions bot changed the title ESP32 ADC DMA sampling rate different than configured sampling frequency with ESP-IDF 5.0 ESP32 ADC DMA sampling rate different than configured sampling frequency with ESP-IDF 5.0 (IDFGH-9225) Jan 25, 2023
@felixcollins
Copy link

I've just found the exact same thing. I've arrived at my calculation of 1.2195 independently so it seem there is a real bug here. Interestingly the data captured does seem complete. I fed a sine wave in and it looks correct when plotted. There must be a bug in the clock configuration for the ADC.

@felixcollins
Copy link

felixcollins commented Apr 6, 2023

Relevant code is here:
C:\Users\felix\esp5\esp-idf\components\hal\adc_hal.c

/**
* For esp32s2 and later chips
* - Set ADC digital controller clock division factor. The clock is divided from `APLL` or `APB` clock.
*   Expression: controller_clk = APLL/APB * (div_num  + div_a / div_b + 1).
* - Enable clock and select clock source for ADC digital controller.
* For esp32, use I2S clock
*/
static void adc_hal_digi_sample_freq_config(adc_hal_dma_ctx_t *hal, uint32_t freq)
{
#if !CONFIG_IDF_TARGET_ESP32
   uint32_t interval = APB_CLK_FREQ / (ADC_LL_CLKM_DIV_NUM_DEFAULT + ADC_LL_CLKM_DIV_A_DEFAULT / ADC_LL_CLKM_DIV_B_DEFAULT + 1) / 2 / freq;
   //set sample interval
   adc_ll_digi_set_trigger_interval(interval);
   //Here we set the clock divider factor to make the digital clock to 5M Hz
   adc_ll_digi_controller_clk_div(ADC_LL_CLKM_DIV_NUM_DEFAULT, ADC_LL_CLKM_DIV_B_DEFAULT, ADC_LL_CLKM_DIV_A_DEFAULT);
   adc_ll_digi_clk_sel(0);   //use APB
#else
   i2s_ll_rx_clk_set_src(hal->dev, I2S_CLK_SRC_DEFAULT);    /*!< Clock from PLL_D2_CLK(160M)*/
   uint32_t bclk_div = 16;
   uint32_t bclk = freq * 2;
   uint32_t mclk = bclk * bclk_div;
   uint32_t mclk_div = I2S_BASE_CLK / mclk;
   i2s_ll_rx_set_mclk(hal->dev, I2S_BASE_CLK, mclk, mclk_div);
   i2s_ll_rx_set_bck_div_num(hal->dev, bclk_div);
#endif
}

@lychee512
Copy link

lychee512 commented Apr 26, 2023

I have found the same issue on an ESP32, using a modified continuous_read_main.c that prints the sample waveforms to the output, allowing me to capture the waveforms. I sent in a 10 KHz triangle wave, and got this:

ADC_test_skip_10KHz

From this data, I found that the adc_continuous_read driver skips 2 samples every 11 samples, which is where the observed sampling frequency discrepancy comes from (11/9 = 1.22).

Modifying the sampling time data to correct for these skipped samples, we can get the correct measurements of the waveform (and get the correct frequency).
ADC_test_skip_10KHz_corrected

This behaviour is only apparent when using adc_continuous_read(), using the i2s adc driver doesn't show this behaviour.

@higaski
Copy link
Contributor

higaski commented May 2, 2023

6 months have passed since I've last posted something related to the ongoing ADC continuous mode problems here #8874. I've re-visited the issue now and I still have no clue how this driver is supposed to behave/work...

I have taken the initial example from this issue and removed code for "non-S3" chips. I've setup the ADC to take 100.000 samples @ 10kHz. My highly advanced math skills tell me that it should take roughly 10s to take those, yet the loop which collects it finishes within 2.5s. So I'm not off by a factor of 1.22 but a factor of 4.

The ESP-IDF version I'm using is v5.2-dev-350-g56123c52aa

Any ideas, suggestions...?

#include <esp_adc/adc_continuous.h>
#include <esp_task.h>
#include <esp_timer.h>
#include <freertos/FreeRTOS.h>
#include <freertos/task.h>
#include <array>
#include <cstdio>

static constexpr auto max_number_of_samples{100'000uz};
static constexpr auto sampling_frequency{10u * 1000u};
static constexpr auto expected_time{static_cast<double>(max_number_of_samples) /
                                    static_cast<double>(sampling_frequency)};
adc_continuous_handle_t adc1_handle{};
std::array<uint8_t, 256uz> result{};

// Times how long ADC takes to take "max_number_of_samples" at configured
// sampling frequency
void task_function(void*) {
  uint32_t ret_num{};
  size_t number_of_samples{};

  // Start ADC
  int64_t previous_us{esp_timer_get_time()};
  ESP_ERROR_CHECK(adc_continuous_start(adc1_handle));

  for (;;) {
    // Read samples
    if (adc_continuous_read(
          adc1_handle, data(result), size(result), &ret_num, 0u) != ESP_OK)
      continue;

    // Sum all, once done print time it took to measure them
    number_of_samples += ret_num;
    if (number_of_samples < max_number_of_samples) continue;
    number_of_samples = 0uz;

    // Convert time it took from us to s (10^6)
    auto const actual_us{esp_timer_get_time()};
    auto const delta_time{(actual_us - previous_us) / 1e6};
    printf("Taking %zu samples @ %uHz took %fs instead of %fs\n",
           max_number_of_samples,
           sampling_frequency,
           delta_time,
           expected_time);

    // Stop ADC and read till empty (might print error message that ADC is
    // already stopped since print before takes long)
    adc_continuous_stop(adc1_handle);
    while (adc_continuous_read(
             adc1_handle, data(result), size(result), &ret_num, 0u) == ESP_OK)
      ;

    // Restart ADC
    previous_us = esp_timer_get_time();
    ESP_ERROR_CHECK(adc_continuous_start(adc1_handle));
  }
}

extern "C" void app_main() {
  // Init ADC in continuous mode
  adc_continuous_handle_cfg_t adc_config{
    .max_store_buf_size = size(result) * SOC_ADC_DIGI_RESULT_BYTES * 8u,
    .conv_frame_size = size(result),
  };
  ESP_ERROR_CHECK(adc_continuous_new_handle(&adc_config, &adc1_handle));

  std::array patterns{
    adc_digi_pattern_config_t{
      .atten = ADC_ATTEN_DB_0,
      .channel = ADC_CHANNEL_0,
      .unit = ADC_UNIT_1,
      .bit_width = ADC_BITWIDTH_12,
    },
    adc_digi_pattern_config_t{
      .atten = ADC_ATTEN_DB_0,
      .channel = ADC_CHANNEL_0,
      .unit = ADC_UNIT_1,
      .bit_width = ADC_BITWIDTH_12,
    },
  };

  adc_continuous_config_t dig_cfg{
    .pattern_num = size(patterns),
    .adc_pattern = data(patterns),
    .sample_freq_hz = sampling_frequency,
    .conv_mode = ADC_CONV_SINGLE_UNIT_1,
    .format = ADC_DIGI_OUTPUT_FORMAT_TYPE2,
  };
  ESP_ERROR_CHECK(adc_continuous_config(adc1_handle, &dig_cfg));

  // Create a max priority task on core 1
  xTaskCreatePinnedToCore(
    task_function, NULL, 4096uz, NULL, ESP_TASK_PRIO_MAX, NULL, 1);

  for (;;)
    vTaskDelay(pdMS_TO_TICKS(1000u));
}

I've also created a zip which contains this example:
esp_idf_issue_10612-master.zip

/edit
Ok, I'm sorry I've been an idiot again. Factor 4... is the difference between the size in bytes and the resulting integer.... oh boy.

@felixcollins
Copy link

@higaski it looks like you are configuring two patterns with the same channel.

@higaski
Copy link
Contributor

higaski commented May 3, 2023

I'm aware of that, but since I didn't care about the conversion results anyhow it didn't matter. I've just written that snippet to estimate the sampling frequency.

@felixcollins
Copy link

Could it be causing the sampling rate to be doubled?

@higaski
Copy link
Contributor

higaski commented May 3, 2023

Could it be causing the sampling rate to be doubled?

No definitely not. The number of channels defined in the pattern does not have any effect on the sampling rate, at least not in the ESP-IDF version I've been using.

I've fixed my stupid mistake and updated the example:
#10612

Using the high resolution timer I can hardly find any discrepancy between the set sampling rate and the actual one. When taking 100.000 samples @ 10kHz the error measured this way is ~ <0.03% which is good enough for my use-cases.

@Tom-Lichtinghagen
Copy link

Hi,

I'm also facing the problem, that the real sampling freqeuncy is about 0.8 times the configured sampling frequency. Is there still no solution for this?

@felixcollins
Copy link

Has anyone from the IDF team even seen this yet? Hello out there!? This is a pretty serious bug.

@felixcollins
Copy link

@espressif-bot espressif-bot added Status: Selected for Development Issue is selected for development and removed Status: Opened Issue is new labels May 10, 2023
@higaski
Copy link
Contributor

higaski commented May 10, 2023

https://www.esp32.com/viewtopic.php?t=31407

The issue seems to be platform dependent. I've run your code on an ESP32-S3 and the error rate there is nowhere even near that high.

=~=~=~=~=~=~=~=~=~=~=~= PuTTY log 2023.05.10 12:22:44 =~=~=~=~=~=~=~=~=~=~=~=
Time: 76863611, samples per second: 40250
Time: 77863611, samples per second: 40250
Time: 78863611, samples per second: 40250
Time: 79863611, samples per second: 40250
Time: 80863611, samples per second: 40500

@milindmovasha
Copy link
Author

https://www.esp32.com/viewtopic.php?t=31407

The issue seems to be platform dependent. I've run your code on an ESP32-S3 and the error rate there is nowhere even near that high.

=~=~=~=~=~=~=~=~=~=~=~= PuTTY log 2023.05.10 12:22:44 =~=~=~=~=~=~=~=~=~=~=~=
Time: 76863611, samples per second: 40250
Time: 77863611, samples per second: 40250
Time: 78863611, samples per second: 40250
Time: 79863611, samples per second: 40250
Time: 80863611, samples per second: 40500

Yes, I can confirm that I have tried the original code I submitted in bug report on ESP32-S2 and ESP32-S3 modules and it works as expected. It is specific to ESP32 module.

@felixcollins
Copy link

Could this PR fix the issue? #11411

@klaus-liebler
Copy link

klaus-liebler commented May 23, 2023

I am facing similar issues with my ESP32-S3 and IDF 5.0.2
My test application is

#include <esp_log.h>
#include <esp_attr.h>
#include <freertos/FreeRTOS.h>
#include <freertos/task.h>
#include <esp_adc/adc_continuous.h>

#define TAG "MAIN"

#if (SOC_ADC_DIGI_RESULT_BYTES == 2)
#define ADC_TEST_OUTPUT_TYPE    ADC_DIGI_OUTPUT_FORMAT_TYPE1
#else
#define ADC_TEST_OUTPUT_TYPE    ADC_DIGI_OUTPUT_FORMAT_TYPE2
#endif

constexpr uint32_t ADC_TEST_FREQ_HZ{20 * 1000};
constexpr uint32_t CONVERSIONS_IN_ONE_CALLBACK{1};
constexpr uint32_t MEASUREMENT_DURATION_SECS{10};

static volatile DRAM_ATTR uint32_t successfulSamples{0};
static volatile DRAM_ATTR uint32_t errors{0};
adc_continuous_handle_t handle{nullptr};

extern "C" bool IRAM_ATTR NOINLINE_ATTR s_conv_done_cb(adc_continuous_handle_t handle, const adc_continuous_evt_data_t *edata, void *user_data)
{
    if(edata->size==CONVERSIONS_IN_ONE_CALLBACK*SOC_ADC_DIGI_RESULT_BYTES){
        successfulSamples=successfulSamples+CONVERSIONS_IN_ONE_CALLBACK;
    }else{
        errors=errors+1;
    }
    return false;
}


extern "C" void app_main(void)
{
    adc_continuous_handle_cfg_t adc_config = {};
    adc_config.max_store_buf_size = 1024;
    adc_config.conv_frame_size = CONVERSIONS_IN_ONE_CALLBACK*SOC_ADC_DIGI_DATA_BYTES_PER_CONV;
    adc_continuous_new_handle(&adc_config, &handle);

    adc_digi_pattern_config_t adc_pattern[SOC_ADC_PATT_LEN_MAX];
    adc_pattern[0].atten = ADC_ATTEN_DB_0;
    adc_pattern[0].channel = ADC_CHANNEL_0;
    adc_pattern[0].unit = ADC_UNIT_1;
    adc_pattern[0].bit_width = SOC_ADC_DIGI_MAX_BITWIDTH;
    adc_continuous_config_t dig_cfg = {};
    dig_cfg.sample_freq_hz = ADC_TEST_FREQ_HZ,
    dig_cfg.conv_mode = ADC_CONV_SINGLE_UNIT_1;
    dig_cfg.format = ADC_TEST_OUTPUT_TYPE;
    dig_cfg.adc_pattern = adc_pattern;
    dig_cfg.pattern_num = 1;
    adc_continuous_config(handle, &dig_cfg);

    adc_continuous_evt_cbs_t cbs = {};
    cbs.on_conv_done = s_conv_done_cb;
    adc_continuous_register_event_callbacks(handle, &cbs, nullptr);

    ESP_LOGI(TAG, "Start ADC with %luHz and %lu samples per callback and %lu seconds measurement time", ADC_TEST_FREQ_HZ, CONVERSIONS_IN_ONE_CALLBACK, MEASUREMENT_DURATION_SECS);
    adc_continuous_start(handle);
    vTaskDelay(pdMS_TO_TICKS(MEASUREMENT_DURATION_SECS*1000));
    adc_continuous_stop(handle);
    ESP_LOGI(TAG, "Stop ADC: successful:%lu (expecting: %lu!), errors %lu", successfulSamples, MEASUREMENT_DURATION_SECS*ADC_TEST_FREQ_HZ, errors);
    while (true)
    {
        ESP_LOGI(TAG, "Heap %6lu", esp_get_free_heap_size());
        vTaskDelay(pdMS_TO_TICKS(10000));
    }
}

The relevant log is

I (302) MAIN: Start ADC with 20000Hz and 1 samples per callback and 10 seconds measurement time
I (10312) MAIN: Stop ADC: successful:176266 (expecting: 200000!), errors 0

So, instead of 200,000 samples in 10 seconds, I get 176,266.

Regards, Klaus

@Tom-Lichtinghagen
Copy link

@klaus-liebler that result is strange because higaski said that it works when using an ESP32-S3 derivate. The only difference I can see is that you are using IDF 5.0.2 and higaski is using IDF 5.2.

Can you switch to the IDF 5.2 and doublecheck whether your code works with that version? Maybe @higaski can also try it with IDF 5.0.2 to have a better evidence?

@higaski
Copy link
Contributor

higaski commented Jun 1, 2023

I'm getting 0.18 to 0.19% error with 5.0.2

@klaus-liebler
Copy link

klaus-liebler commented Jun 1, 2023

I gave v5.2-dev-823-g903af13e84 a try. The results are more or less the same as before:

I (335) MAIN: Start ADC with 20000Hz and 1 samples per callback and 10 seconds measurement time
I (10345) MAIN: Stop ADC: successful:175425 (expecting: 200000.000000!), errors 0, deviation: -12.287500%

Well, I've tried to narrow down the problem further and changed the samples per callback drastically to 256. That means, there are not 20000, but only 78 ISR calls per second. With both IDF versions, the result ist the same.

I (335) MAIN: Start ADC with 20000Hz and 256 samples per callback and 10 seconds measurement time
I (10345) MAIN: Stop ADC: successful:199680 (expecting: 200000.000000!), errors 0, deviation: -0.160000%

So, the problem is not the frequency itself, but the very frequent calls into my ISR routine. Obviously, the ESP32 hardware or software architecture seems to be overwhelmed with 20k calls per second.

I then checked it exactly and tried different callback frequencies. The table and the diagram shows the dependency of the sample rare error [%] on the callback frequency [Hz] for different ADC frequencies (20kHz, 40kHz, 60kHz, 80kHz)

image

So, the error is more or less proportional to the callback frequency and somehow dependend on the ADC frequency. At least on my test setup, 60kHz ADC frequency seems to be the "sweet spot".

Cheers,

Klaus

@Tom-Lichtinghagen
Copy link

Thanks @klaus-liebler and @higaski, but this effect seems to be a different problem. In case of using the ESP32 changing the amount of callbacks does not have any impact of the factor of 0.8.

@klaus-liebler
Copy link

@Tom-Lichtinghagen , do you recommend to file another issue for this or is this just beyond the limits of the architecture?

@Tom-Lichtinghagen
Copy link

Hi,

switching to the ESP32-S3 solved the problem regarding the sample rate deviation. But now I'm facing the problem, that the pool overflow interrupt is triggered although I read out the pool fast enough. Does anyone has face this problem also?

Another question from my side is if the conversion frame size and the maximal pool buffer size refer to all configured adc channels? That would imply that the time between two conversion done interrupts would depend on the number of used adc channel (with the conditional that the conversion frame size is the same). But tests did not show the behavior.

@phoddie
Copy link
Contributor

phoddie commented Sep 1, 2023

FWIW – on ESP32 with ESP-IDF v5.1.1 the the sampling rate is still incorrect. When used to record audio through an analog input, it plays back at noticeably a higher pitch than the input.

@cazou
Copy link

cazou commented Mar 2, 2024

Hi !

I am confronted to this issue it seems, with indeed, my requested frequency (44100 Hz) divided by ~1.22 on ESP32.

From what I can see, the ADC continuous driver uses the i2s DMA, which is configured where @felixcollins mentioned in #10612 (comment)

According to the esp32 TRM:

For high
performance audio applications, the analog PLL output clock source APLL_CLK must be used to acquire
highly accurate I2Sn_CLK and BCK.

(Page 310, 12.3 The Clock of I2S Module)

But in the initialization code, of the adc_hal, I2S_CLK_SRC_DEFAULT is used, which is set to the PLL_D2_CLK(160M) clock: The clock the TRM says is not stable enough for audio.

The DAC continuous driver has a clk_src configuration field that can be set to DAC_DIGI_CLK_SRC_APLL, but is not present for ADC. I will be looking at porting this to the ADC continuous driver to see if the clock is more precise.

Did anyone successfully got the correct frequency with PLL_D2_CLK(160M) ?

@cazou
Copy link

cazou commented Mar 4, 2024

There is no noticeable improvement when using APLL as clock source. But I was able to confirm that the I2S clocks are correctly configured to closely match the requested sampling frequency.
It is also configured the same way when using the continuous DAC, which is outputting the signal at the correct frequency.

In my test program, the interrupt routine is also called correctly:

  • Increasing a value with edata->size in adc_continuous_evt_cbs_t::on_conv_done will get this value to the same as counting the number of bytes returned by adc_continuous_read() since the call to start()
  • adc_continuous_evt_cbs_t::on_pool_ovf is never called

But still, the effective sampling frequency is 36082 Hz instead of 44100. (/1.22)

In v4.4, with a similar program (the adc_continuous and i2s APIs have changed a lot between v4 and v5), the effective frequency is 43690 Hz, which is acceptable for me but could probably be better. That means that the issue is probably introduced with the new API in v5, but more importantly, that the issue is in software, not in HW (or at least it has a SW solution).

I will be looking into the DMA configuration next. It could be misconfigured and ends up timeout-ing sometimes.

@cazou
Copy link

cazou commented Mar 7, 2024

I pushed a simple fix, which is more a workaround, but can help for now. Next, I'd like to analyze why those 2 samples are lost in continuous mode.
Note that continuous mode will not start if the max_num is disabled.

@felixcollins
Copy link

Now, will Espressif investigate a proper fix for this!? It seems like @cazou has found an important clue.

@cazou
Copy link

cazou commented Mar 9, 2024

This is definitely an issue on the ESP32. The same code on esp32-H2 (or others as mentioned by other people) yields the correct frequency.
Only on ESP32 is there a comment saying:
//On esp32, ADC can only be continuously triggered when ADC_LL_DEFAULT_CONV_LIMIT_EN == 1, ADC_LL_DEFAULT_CONV_LIMIT_NUM != 0

All the other ones have it deactivated by default (and the API only uses the default value):

$ git grep ADC_LL_DEFAULT_CONV_LIMIT_EN
components/hal/esp32/include/hal/adc_ll.h:#define ADC_LL_DEFAULT_CONV_LIMIT_EN      1
components/hal/esp32c3/include/hal/adc_ll.h:#define ADC_LL_DEFAULT_CONV_LIMIT_EN      0
components/hal/esp32c6/include/hal/adc_ll.h:#define ADC_LL_DEFAULT_CONV_LIMIT_EN      0
components/hal/esp32h2/include/hal/adc_ll.h:#define ADC_LL_DEFAULT_CONV_LIMIT_EN      0
components/hal/esp32s2/include/hal/adc_ll.h:#define ADC_LL_DEFAULT_CONV_LIMIT_EN      0
components/hal/esp32s3/include/hal/adc_ll.h:#define ADC_LL_DEFAULT_CONV_LIMIT_EN      0

Also, the ESP32 is the only one using the i2s DMA. Some use SPI or GDMA. There is no GDMA on ESP32 but there is a SPI DMA, maybe using that one would work better, but I don't know if that is possible.

It would be nice to have an investigation on espressif side, at least to confirm that it is an issue on the ESP32 and that it won't be solved.

Maybe @mythbuster5 could shine some light on this ?

@jsb1
Copy link

jsb1 commented Mar 29, 2024

Found something
(ESP 32, Arduino 2.04, ESP.getChipRevision() is 1 )

Setting SYSCON.saradc_ctrl2.meas_num_limit=0; after starting DMA, I'm getting exact sample rates - as far as clock dividers allow.

Before:
#12 first buffer values: 1210, 1205, 1205, 1203, 1206, 1206, 1207, 1206, 1204, 1206,
Buffer#787140, Bytes=99600, Lastbytes=1200, sps=49800.000000, ret=0, is2ret=0
After:
Buffer#41877006, Bytes=100000, Lastbytes=1200, sps=50000.000000 ret=0, is2ret=0
#640 first buffer values:1206, 1203, 1207, 1206, 1206, 1206, 1207, 1206, 1206, 1206, `

ADC values seem to be stable. I'll let it run for a day.

The question is now: Where is the Bug? Maybe in chip documentation? And in all code derived from that?

BTW: With APLL there are only 0 values from adc. Maybe clock is not routed to it.

@cazou
Copy link

cazou commented Mar 30, 2024

Indeed ! because of ADC can only be continuously triggered when ADC_LL_DEFAULT_CONV_LIMIT_EN == 1, I assumed it had to stay that way, but it doesn't have to apparently.

I'm also getting way better results this way. So, as a workaround if people have this issue:

  • Add the following includes in the file calling adc_continuous_start:
#include <hal/misc.h>
#include <soc/syscon_struct.h>

And add this after the call to adc_continuous_start:

ESP_LOGI(TAG, "Deactivating meas_num_limit...");
HAL_FORCE_MODIFY_U32_REG_FIELD(SYSCON.saradc_ctrl2, meas_num_limit, 0);

The ESP_LOGI seems necessary for some reason, maybe a delay issue. You may still miss 2 samples at the beginning though.

I'll create a PR to force espressif to hopefully move a bit on this

@jsb1
Copy link

jsb1 commented Apr 5, 2024

Just an Info for Arduino users:
DMA code on Arduino 2.0.4 / ESP-IDF 4.x is completely nonsense.
Use Arduino 3 beta based on ESP-IDF 5.1+ and continous ADC API !!!

@jsb1
Copy link

jsb1 commented Apr 5, 2024

And Yes, there might be a bug on chip.
Sometimes when setting setting SYSCON.saradc_ctrl2.meas_num_limit=0, DMA stands still.
Prepare Your software for reboot when DMA does not deliver data!

@jsb1
Copy link

jsb1 commented Apr 6, 2024

My startup code looks like this now:


	volatile uint32_t sumBytesAtIRQ;
....
		for (int retry = 0; sumBytesAtIRQ == 0 && retry < 1000; retry++)
			vTaskDelay(1 / portTICK_PERIOD_MS);
		SYSCON.saradc_ctrl2.meas_num_limit = 0;
static bool IRAM_ATTR convDoneCB(adc_continuous_handle_t handle, const adc_continuous_evt_data_t *edata, void *user_data)
{
	MyType *self = (MyType *)user_data;

	self->cpuCyclesAtIRQ = esp_cpu_get_cycle_count();
	self->sumBytesAtIRQ += edata->size;

    return pdFALSE;
}

Seems to be safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Selected for Development Issue is selected for development
Projects
None yet
Development

No branches or pull requests

10 participants