search

espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.7k stars 7.3k forks source link

ADC DMA samples delaying (IDFGH-9519) #10875

Open andybyc13 opened 1 year ago

andybyc13 commented 1 year ago

Answers checklist.

IDF version.

v4.4

Operating System used.

Linux

How did you build your project?

Command line with idf.py

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

None

Development Kit.

ESP32-S3-WROOM-1

Power Supply used.

USB

What is the expected behavior?

Sample read from ADC DMA is always with newest value.

What is the actual behavior?

Sample from ADC DMA has been delayed by 1 sample. It means, that after change voltage on pin, old value is reported, and then new one.

Steps to reproduce.

Compile and run following code. It is simplification of example from ESP-IDF-v5.0, but behaviour is same as in described v4.4.

`

include

include

include "sdkconfig.h"

include "esp_log.h"

include "freertos/FreeRTOS.h"

include "freertos/task.h"

include "freertos/semphr.h"

include "esp_adc/adc_continuous.h"

define EXAMPLE_READ_LEN 4

define GET_UNIT(x) ((x>>3) & 0x1)

define ADC_CONV_MODE ADC_CONV_SINGLE_UNIT_1

define ADC_OUTPUT_TYPE ADC_DIGI_OUTPUT_FORMAT_TYPE2

static adc_channel_t channel[] = {ADC_CHANNEL_2};

static TaskHandle_t s_task_handle; static const char *TAG = "EXAMPLE";

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 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 = EXAMPLE_READ_LEN,
    .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 = 1 * 1000,
    .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;

}

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;

}

void app_main(void) { esp_err_t ret; uint32_t ret_num = 0; uint8_t result[EXAMPLE_READ_LEN] = {0}; memset(result, 0, 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,
};
ESP_ERROR_CHECK(adc_continuous_register_event_callbacks(handle, &cbs, NULL));
ESP_ERROR_CHECK(adc_continuous_start(handle));

while(1) {
    ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
    while (1) {
        ret = adc_continuous_read(handle, result, EXAMPLE_READ_LEN, &ret_num, 0);
        if (ret == ESP_OK) {
            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 (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);
                }
            }
            vTaskDelay(300);
            // following line solves problem
            // ret = adc_continuous_read(handle, result, EXAMPLE_READ_LEN, &ret_num, 0);
        } else if (ret == ESP_ERR_TIMEOUT) {
            break;
        }
    }
}

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

} `

Debug Logs.

No response

More Information.

I develop project on ESP32-S3 with ESP-IDF v.4.4. U use ADC in DMA mode. I set: sampling rate to 1 kHz, I need only 1 channel (ADC1 channel 2), and sample each time with buffer for only single sample (4 bytes). Every 3-5 seconds, I print out measured value to console.

To test this, I provide 2 voltage levels and check, when I will see reaction on console. I need to wait 1 more sample to be printed, to see the difference in read value.

As mentioned in added code, when I perform double read (just repeat the function), problem seems to be solved. Am I configuring something in my code wrong or what causes this behavior?

Icarus113 commented 1 year ago

@andybyc13 Hi, from the condition you described, is it possible that, right before you change the input level, there's one sample happens. Then the internal buffer (whose size is 4 bytes as you configured) is filled with this time sample (let's say result A). After you change the input level, the newer result will be filled in the internal buffer until the previous one (result A) is read out (as the internal buffer size is configured to 4B as well).

We're preparing another way that you can get the newest value easier.

For now, you can try this callback, the const adc_continuous_evt_data_t *edata.

Let me know the updates, I'll give feedback once I get time.

Icarus113 commented 1 year ago

Related to https://github.com/espressif/esp-idf/issues/10864

eos1d3 commented 1 year ago

I said this in last year here and also told your technical support via email before IDF 5.0. ESP never understands how people use ADC and make this useless ADC driver.

ESP treats ADC as audio recorders or oscilloscopes and tries to record every bits of ADC values. This is totally wrong and I never see any other MCUs do in this way.

For normal ADC usage, just give application the REAL TIME ADC values or the LATEST ADC values. Never save them in buffer! Those historical ADC values are completely useless and no one will need them.

For example, when using ADC for Moseft overcurrent protection. Your ADC driver can take many seconds to clean up and the Mosfet has always exploded!

The ring buffer is never required and it uses huge amount of memory. For each ADC channel, ESP use many KB for buffer. That is really crazy!

I also use SMT32 with their beautiful ADC. It just needs 2 bytes for each ADC channel. And it is able to scan continuously with DMA, and has hardware oversampling without any CPU resources.

For any ADC requirement, I always switch to other MCUs.

Icarus113 commented 1 year ago

@eos1d3 Hi, thanks for your input. Would you check if the callback usage can meet your requirements? Inside the callback, you can get the conversion frame buffer which is real time. New data will be filled into the buffer. If so, we can add a initialisation flag to not use the ringbuffer.

andybyc13 commented 1 year ago

@Icarus113 thank you for this information, however we develop out SW with ESP-IDF v.4.4.4, where this structure adc_continuous_evt_cbs_t is not present, so cannot use this callback.

@eos1d3 you are right, generally ADC in any other MCU returns you real time samples. Current implementation gives you 2 possibilities how you can use these data: latched samples sometimes can be useful too, but if you need real time, the only thing you need to do, is clear old samples and wait for buffer to be filled.

@Icarus113 is it possible to add in ESP-IDF v.4.4.4 at least initialization flag to not use the ring buffer? Also, callback will be useful too. At the moment, I fully resolved all the issues in my code with really big workaround, but it seems, this change would simplify my implementation.

Also, more details in documentation about how ADC with DMA works will be helpful too.

eos1d3 commented 1 year ago

@Icarus113 I do not think callback is available when continuous sampling is running. At least from ESP Docs, I do not find it.

Bad implementation of ADC API is one thing. Another bad design is the noise of ADC. Its noise can be 10-20 timer higher than any MCU ADC.

And finally, other MCUs do not need software over-sampling. Like new AVR and STM32, over-sampling is done by hardware and no need to calculate the average by CPU any more. ESP ADC is many generations behind.

The rest of ESP is really good but ADC is totally useless. I really hope Espressif update the ADC design in new chips.

ScumCoder commented 8 months ago

ESP never understands how people use ADC and make this useless ADC driver. This is totally wrong normal ADC usage Those historical ADC values are completely useless and no one will need them The ring buffer is never required

https://www.youtube.com/watch?v=j95kNwZw8YY

It's probably a good idea to never project your very personal experience, values and tastes to literally everyone, as it is to never consider your very subjective opinion to be an objective truth. In my current project, the IDF API does exactly what I need; was it designed by you, I'd have to implement the ringbuffer functionality by myself on top of it.