[Solved] Design pattern: how to cleanly exit from a task that is waiting on a UART queue

[RobMeades]

I have a component which creates a dynamic task that waits on a UART queue for received characters, like so:

Code: Select all

static void rx_task(void *pvParameters)
{
    uart_event_t uart_event;
    while (1) {
        if (xQueueReceive(_queue_uart, (void *) &uart_event,
                          (portTickType) portMAX_DELAY)) {
            if (uart_event.size > 0) {
                          //... do stuff with the received data
            }
        }
    }
}

I would like the component to delete that dynamic task cleanly when it exits. A simple way would be to write an unusual value to the UART queue myself, in other words make the dynamic task something like this:

Code: Select all

static void rx_task(void *pvParameters)
{
    uart_event_t uart_event;

    xSemaphoreTake(_mtx_rx_task_running, (portTickType) portMAX_DELAY);

    uart_event.type = UART_DATA;
    
    while (uart_event.type < UART_EVENT_MAX) {
        if (xQueueReceive(_queue_uart, (void *) &uart_event,
                          (portTickType) portMAX_DELAY)) {
            if ((uart_event.type == UART_DATA) && (uart_event.size > 0)) {
                          //... do stuff with the received data
            }
        }
    }

    xSemaphoreGive(_mtx_rx_task_running);

}

...then from my tidy-up code I can do something like this:

Code: Select all

    uart_event_t uart_event;
    uart_event.type = UART_EVENT_MAX;
    uart_event.size = 0;
    xQueueSend(_queue_uart, (void *) &uart_event, (portTickType) portMAX_DELAY);
    xSemaphoreTake(_mtx_rx_task_running, (portTickType) portMAX_DELAY);
    xSemaphoreGive(_mtx_rx_task_running);
    vTaskDelete(_task_handle_rx);

However, when I implement this and my tidy-up code runs I get a panic in the ESP32 UART interrupt handler (just where it exits a critical section):

Code: Select all

Guru Meditation Error: Core  0 panic'ed (IllegalInstruction). Exception was unhandled.
Core 0 register dump:
PC      : 0x400e30fa  PS      : 0x00060630  A0      : 0x00000000  A1      : 0x3ffb3f60
0x400e30fa: uart_rx_intr_handler_default at C:/msys32/home/rob/esp/esp-idf/components/driver/uart.c:1241

My questions are:

• should what I'm suggesting work, if so can anyone spot my deliberate mistake?
• if what I am suggesting is not going to work (e.g. FreeRTOS has some concept of queue ownership which I'm violating, etc.) is there an alternative accepted design pattern for this (e.g. do I have to use queue sets?)?

[ESP_Sprite]

Which esp-idf do you use, and/or can you tell me what's on the offending line in the backtrace of driver/uart.c?

[RobMeades]

I've included the function in question, marking the specific line, below. Git log shows that the last thing in my tree was v3.1.1, sha 3b92e85b308578c24f14f57371498303c198c7cb.

Code: Select all

esp_err_t uart_flush_input(uart_port_t uart_num)
{
    UART_CHECK((uart_num < UART_NUM_MAX), "uart_num error", ESP_FAIL);
    UART_CHECK((p_uart_obj[uart_num]), "uart driver error", ESP_FAIL);
    uart_obj_t* p_uart = p_uart_obj[uart_num];
    uint8_t* data;
    size_t size;

    //rx sem protect the ring buffer read related functions
    xSemaphoreTake(p_uart->rx_mux, (portTickType)portMAX_DELAY);
    uart_disable_rx_intr(p_uart_obj[uart_num]->uart_num);
    while(true) {
        if(p_uart->rx_head_ptr) {
            vRingbufferReturnItem(p_uart->rx_ring_buf, p_uart->rx_head_ptr);
            UART_ENTER_CRITICAL(&uart_spinlock[uart_num]);
            p_uart_obj[uart_num]->rx_buffered_len -= p_uart->rx_cur_remain;
            uart_pattern_queue_update(uart_num, p_uart->rx_cur_remain);
            UART_EXIT_CRITICAL(&uart_spinlock[uart_num]);
            p_uart->rx_ptr = NULL;
            p_uart->rx_cur_remain = 0;
            p_uart->rx_head_ptr = NULL;
        }
        data = (uint8_t*) xRingbufferReceive(p_uart->rx_ring_buf, &size, (portTickType) 0);
        if(data == NULL) {
            if( p_uart_obj[uart_num]->rx_buffered_len != 0 ) {
                ESP_LOGE(UART_TAG, "rx_buffered_len error");
                p_uart_obj[uart_num]->rx_buffered_len = 0;
            }
            //We also need to clear the `rx_buffer_full_flg` here.
            UART_ENTER_CRITICAL(&uart_spinlock[uart_num]);
            p_uart_obj[uart_num]->rx_buffer_full_flg = false;
            UART_EXIT_CRITICAL(&uart_spinlock[uart_num]);
            break;
        }
        UART_ENTER_CRITICAL(&uart_spinlock[uart_num]);
        p_uart_obj[uart_num]->rx_buffered_len -= size;
        uart_pattern_queue_update(uart_num, size);
        UART_EXIT_CRITICAL(&uart_spinlock[uart_num]);
        vRingbufferReturnItem(p_uart->rx_ring_buf, data);
        if(p_uart_obj[uart_num]->rx_buffer_full_flg) {
            BaseType_t res = xRingbufferSend(p_uart_obj[uart_num]->rx_ring_buf, p_uart_obj[uart_num]->rx_data_buf, p_uart_obj[uart_num]->rx_stash_len, 1);
            if(res == pdTRUE) {
                UART_ENTER_CRITICAL(&uart_spinlock[uart_num]);
                p_uart_obj[uart_num]->rx_buffered_len += p_uart_obj[uart_num]->rx_stash_len;
                p_uart_obj[uart_num]->rx_buffer_full_flg = false;
                UART_EXIT_CRITICAL(&uart_spinlock[uart_num]); // <-- THIS IS THE LINE
            }
        }
    }
    p_uart->rx_ptr = NULL;
    p_uart->rx_cur_remain = 0;
    p_uart->rx_head_ptr = NULL;
    uart_reset_rx_fifo(uart_num);
    uart_enable_rx_intr(p_uart_obj[uart_num]->uart_num);
    xSemaphoreGive(p_uart->rx_mux);
    return ESP_OK;
}

[ESP_Sprite]

Ah, on second look, I see the issue: you exit your task function, which is not valid in FreeRTOS. Either loop at the end (and let your tidy-up routine clean up the task), or call vTaskDelete(NULL) to let it clean up itself.

[RobMeades]

Ah, yes, well done, that was it. I obviously need to be more au fait with Free RTOSness.