ADC continuous read stop working after call `esp_ota_begin()` (IDFGH-11488)

guo-max commented 1 year 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.

ESP-IDF v5.2-dev-3775-gb4268c874a

Espressif SoC revision.

ESP32S3

Operating System used.

Windows

How did you build your project?

Command line with idf.py

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

CMD

Development Kit.

LilyGo T-DISPLAY-S3

Power Supply used.

USB

What is the expected behavior?

ADC continuous read should work after call esp_ota_begin(). Calling esp_ota_begin() will erase the ota partition, which may take a while and block the adc reading task. But the adc reading should work after esp_ota_begin() finished.

What is the actual behavior?

ADC continuous read stop working after call esp_ota_begin()

Steps to reproduce.

Tested with esp-idf\examples\peripherals\adc\continuous_read\
Add a separate task that will call esp_ota_begin() after about 1s.
removed some unnecessary logs.
must build with an OTA partition.
Modified example code:

/*
 * 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 "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/semphr.h"
#include "esp_adc/adc_continuous.h"

#include <pthread.h>
#include <inttypes.h>
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "esp_pthread.h"
#include "esp_ota_ops.h"
static void *ota_thread(void *arg);

#define EXAMPLE_ADC_UNIT ADC_UNIT_1
#define _EXAMPLE_ADC_UNIT_STR(unit) #unit
#define EXAMPLE_ADC_UNIT_STR(unit) _EXAMPLE_ADC_UNIT_STR(unit)
#define EXAMPLE_ADC_CONV_MODE ADC_CONV_SINGLE_UNIT_1
#define EXAMPLE_ADC_ATTEN ADC_ATTEN_DB_0
#define EXAMPLE_ADC_BIT_WIDTH SOC_ADC_DIGI_MAX_BITWIDTH

#if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2
#define EXAMPLE_ADC_OUTPUT_TYPE ADC_DIGI_OUTPUT_FORMAT_TYPE1
#define EXAMPLE_ADC_GET_CHANNEL(p_data) ((p_data)->type1.channel)
#define EXAMPLE_ADC_GET_DATA(p_data) ((p_data)->type1.data)
#else
#define EXAMPLE_ADC_OUTPUT_TYPE ADC_DIGI_OUTPUT_FORMAT_TYPE2
#define EXAMPLE_ADC_GET_CHANNEL(p_data) ((p_data)->type2.channel)
#define EXAMPLE_ADC_GET_DATA(p_data) ((p_data)->type2.data)
#endif

#define EXAMPLE_READ_LEN 256

#if CONFIG_IDF_TARGET_ESP32
static adc_channel_t channel[2] = {ADC_CHANNEL_6, ADC_CHANNEL_7};
#else
static adc_channel_t channel[2] = {ADC_CHANNEL_2, ADC_CHANNEL_3};
#endif

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 = 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 = 20 * 1000,
        .conv_mode = EXAMPLE_ADC_CONV_MODE,
        .format = EXAMPLE_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++)
    {
        adc_pattern[i].atten = EXAMPLE_ADC_ATTEN;
        adc_pattern[i].channel = channel[i] & 0x7;
        adc_pattern[i].unit = EXAMPLE_ADC_UNIT;
        adc_pattern[i].bit_width = EXAMPLE_ADC_BIT_WIDTH;

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

    *out_handle = handle;
}

static void *ota_thread(void *arg)
{
    usleep(250 * 1000);
    printf("This thread has ID 0x%" PRIx32 " and %u bytes free stack\n", pthread_self(), uxTaskGetStackHighWaterMark(NULL));
    esp_ota_handle_t update_handle = 0;
    const esp_partition_t *update_partition = NULL;

    update_partition = esp_ota_get_next_update_partition(NULL);
    assert(update_partition != NULL);

    printf("ota will begin after 1s.\n");
    sleep(1);
    printf("begin ota.\n");
    esp_ota_begin(update_partition, OTA_SIZE_UNKNOWN, &update_handle);
    while (true)
    {
        sleep(1);
    }

    return NULL;
}

void app_main(void)
{

    pthread_attr_t attr;
    pthread_t thread1;
    esp_pthread_cfg_t esp_pthread_cfg;
    int res;

    res = pthread_attr_init(&attr);
    assert(res == 0);
    pthread_attr_setstacksize(&attr, 16384);
    res = pthread_create(&thread1, &attr, ota_thread, NULL);
    assert(res == 0);
    printf("Created larger stack thread 0x%" PRIx32 "\n", thread1);

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

    size_t timeout_counter = 0;

    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, pdMS_TO_TICKS(1000));

        char unit[] = EXAMPLE_ADC_UNIT_STR(EXAMPLE_ADC_UNIT);

        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 " bytes", ret, ret_num);
                // for (int i = 0; i < ret_num; i += SOC_ADC_DIGI_RESULT_BYTES) {
                //     adc_digi_output_data_t *p = (adc_digi_output_data_t*)&result[i];
                //     uint32_t chan_num = EXAMPLE_ADC_GET_CHANNEL(p);
                //     uint32_t data = EXAMPLE_ADC_GET_DATA(p);
                //     /* Check the channel number validation, the data is invalid if the channel num exceed the maximum channel */
                //     if (chan_num < SOC_ADC_CHANNEL_NUM(EXAMPLE_ADC_UNIT)) {
                //         ESP_LOGI(TAG, "Unit: %s, Channel: %"PRIu32", Value: %"PRIx32, unit, chan_num, data);
                //     } else {
                //         ESP_LOGW(TAG, "Invalid data [%s_%"PRIu32"_%"PRIx32"]", unit, chan_num, data);
                //     }
                // }
                /**
                 * 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);
            }
            else if (ret == ESP_ERR_TIMEOUT)
            {
                ESP_LOGI("TASK", "timeout");
                // We try to read `EXAMPLE_READ_LEN` until API returns timeout, which means there's no available data
                break;
            }
        }
    }

    ESP_LOGI("TASK","exit.");
    ESP_ERROR_CHECK(adc_continuous_stop(handle));
    ESP_ERROR_CHECK(adc_continuous_deinit(handle));
}

Debug Logs.

ADC works fine before `ota begin`
Call `esp_ota_begin()` from another thread.
ADC stop working after `ota begin`.

More Information.

I think any task/function that may block other tasks could trigger this bug. Here I use esp_ota_begin() to demonstrate it. I think this bug is not related to OTA at all. I have experience that writing large file to file system (esp_littlefs) could trigger this bug.

wanckl commented 1 year ago

@guo-max Hello, OTA update process is require FLASH write/change, while ADC(and others) Programs is also store in FLASH by default, So it is not allowed to happen in same time.

When OTA start, chip will disable Cache and interrupts, then it can be safe for FLASH option, so your ADC stop,

If you really need it, you may find is there any config option in menuconfig to place ADC programs in internal ram(IRAM), so that is can exec without FLASH access.

p-ranav commented 1 year ago

When esp_ota_begin() completes, it would be better if the ADC reader continued working OK like before? The component does not recover gracefully from the timeout error after the OTA function call has finished. Is there a technical reason why it does not recover?

espressif / esp-idf