espressif / esp-idf

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

uart_isr_register for idf v5 up (IDFGH-10195) #11463

Open caniot opened 1 year ago

caniot commented 1 year ago

Is your feature request related to a problem?

Please can you add the possibility to add own interrupt handling function with uart_isr_register to version 5.like in versions 4, because otherwise we can not use the v5 without this feature.

Describe the solution you'd like.

No response

Describe alternatives you've considered.

No response

Additional context.

No response

torkleyy commented 1 year ago

It seems there is no way to write low-latency UART code with IDF v5. Is there a specific reason this function got removed?

felixzero commented 10 months ago

Same for me, I can't upgrade to V5 for some of my boards, is there a workaround to implement low latency/high throughput UART avoiding the fat ISR set up by uart_driver_install?

EDIT: I feel like I should provide more argument/example.

I am interfacing a Modbus device through RS-485, running at 1Mbps. I use a RS-485 transceiver in half duplex mode and toggle between RX and TX by software activation of the RTS pin, like described in the documentation.

I naturally tried uart_set_mode(uart_num, UART_MODE_RS485_HALF_DUPLEX) using the driver implementation but it is far too slow to achieve reliable communication: even with the task setupping and running UART pinned to core 1 (et everything else on core 0) and the UART ISR configured with level 3, I still get a significant delay up to 100µs after the end of the transmission before the software sets the RTS off, especially when the ESP32 is busy running Wifi/HTTP. This makes it miss some of the responses from my device which answers very quickly after the 3.5 character timeout.

In ESP IDF v4, this is a very easy fix: overriding the ISR of the driver

uart_intr_config_t intr_config = {
    .intr_enable_mask = UART_TX_DONE_INT_ENA_M | UART_RXFIFO_TOUT_INT_ENA_M,
};
ESP_ERROR_CHECK(uart_isr_free(UART_NUM));
ESP_ERROR_CHECK(uart_isr_register(UART_NUM, uart_intr_handle, NULL, ESP_INTR_FLAG_IRAM, NULL));
ESP_ERROR_CHECK(uart_intr_config(UART_NUM, &intr_config));

And then I can define a simple ISR performing this tasks. This is particularly easy considering that in my application all my transmissions are smaller than the size of the RX/TX FIFO and therefore I don't need all the ring buffer management code from the IDF driver.

static void IRAM_ATTR uart_intr_handle(void *arg)
{
    uint32_t uart_intr_status = uart_hal_get_intsts_mask(&hal_context);

    if (uart_intr_status & UART_INTR_TX_DONE) {
        uart_hal_set_rts(&hal_context, 1);
        uart_hal_clr_intsts_mask(&hal_context, UART_INTR_TX_DONE);
        return;
    } else if (uart_intr_status & UART_INTR_RXFIFO_TOUT) {
        // handle rx...
    } else {
        uart_hal_clr_intsts_mask(&hal_context, uart_intr_status);
        return;
    }
}
phatpaul commented 10 months ago

This deprecation has me worried as well. I've wasted many hours trying to get a basic LINbus driver working reliably on the high-level UART driver. I need to go lower-level to make the BREAK detection and synchronization robust.

Some references to my LIN driver woes: https://github.com/espressif/esp-idf/issues/4537#issuecomment-1433671118 https://esp32.com/viewtopic.php?t=8181

BTW I would love to see some example code using the UART at low-level with interrupts. It would be great if this low-level driver can work on UART2, while leaving the default UART for console output.

GideonZ commented 7 months ago

I just ran into the same problem when porting my application from 4.4 to 5.1.2. I just copied the code from 4.4 into my driver:

#include "esp_check.h"
esp_err_t uart_isr_register(uart_port_t uart_num, void (*fn)(void *), void *arg, int intr_alloc_flags,  uart_isr_handle_t *handle)
{
    int ret;
    ESP_RETURN_ON_FALSE((uart_num < UART_NUM_MAX), ESP_FAIL, UART_TAG, "uart_num error");
    UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock));
    ret = esp_intr_alloc(uart_periph_signal[uart_num].irq, intr_alloc_flags, fn, arg, handle);
    UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock));
    return ret;
}
fahirad commented 5 months ago

Hello @GideonZ and the rest of the squad, is it possible to actually just manually add it to the driver, does it work and will it create any problems further on? As you can imagine, I too have stumbled upon the same problem..

GideonZ commented 5 months ago

Manually changing files of the ESP-IDF is something I wouldn't recommend. I have just added this code into my project. It works and I haven't come across any issues so far.

MikeMyhre1 commented 5 months ago

For conversion from IDF V4 to V5 drop the 'install driver' and uart_isr_free/register commands. Keep the uart_param_config functions and use esp_intr_alloc to hook the interrupt. Something like this:

include "hal/uart_ll.h"

include "soc/interrupts.h"

static void IRAM_ATTR my_uart_isr( void *arg ) { if( UART1.int_st.txfifo_empty_int_st ) { // build reply uart_ll_write_txfifo(&UART1, "sendstr",7 ); uart_clear_intr_status(UART_NUM_1, UART_TXFIFO_EMPTY_INT_CLR); return; } if( UART1.int_st.parity_err_int_st ) { uart_clear_intr_status(UART_NUM_1, UART_PARITY_ERR_INT_CLR); return; // handle other interrupt sources and clear them. if( UART1.status.rxfifo_cnt ) { // read bytes from UART1.fifo.rxfifo_rd_byte until UART1.status.rxfifo_cnt == 0 uart_clear_intr_status(UART_NUM_1, UART_RXFIFO_FULL_INT_CLR); } }

const uart_config_t uart_config = { .baud_rate = 9600, .data_bits = UART_DATA_8_BITS, .parity = UART_PARITY_EVEN, .stop_bits = UART_STOP_BITS_1, .flow_ctrl = UART_HW_FLOWCTRL_DIABLE, .source_clk = UART_SCLK_APB, };

uart_isr_handle_t isr_handle; ESP_ERROR_CHECK(uart_param_config(UART_NUM_1, &uart_config )); ESP_ERROR_CHECK(uart_set_pin(UART_NUM_1, 17, 18, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE)); ESP_ERROR_CHECK(uart_set_line_inverse(UART_NUM_1,UART_SIGNAL_TXD_INV)); ESP_ERROR_CHECK(esp_intr_alloc(ETS_UART1_INTR_SOURCE, (void *)NULL, my_uart_isr, NULL, &isr_handle,));

uart_intr_config_t uintr_cfg = { .rxfifo_full_thresh = 1, .intr_enable_mask = (UART_RXFIFO_FULL_INT_ENA_M) }; uintr_cfg.txfifo_empty_intr_thresh = 1; uintr_cfg.intr_enable_mask = (UART_RXFIFO_FULL_INT_ENA_M | UART_TXFIFO_EMPTY_INT_ENA_M); ESP_ERROR_CHECK(uart_intr_config(UART_NUM_1,&uintr_cfg));

uart_intr_config_t intr_config = { .intr_enable_mask = UART_TX_DONE_INT_ENA_M | UART_RXFIFO_TOUT_INT_ENA_M, };

GideonZ commented 5 months ago

ESP_ERROR_CHECK(uart_isr_register(UART_NUM, uart_intr_handle, NULL, ESP_INTR_FLAG_IRAM, NULL));

Aren't you calling exactly the function that is not available in V5? This was the whole point of the post.

MikeMyhre1 commented 5 months ago

ESP_ERROR_CHECK(uart_isr_register(UART_NUM, uart_intr_handle, NULL, ESP_INTR_FLAG_IRAM, NULL));

Aren't you calling exactly the function that is not available in V5? This was the whole point of the post.

Error in copying. I changed it. By mistake I had quoted the previous comment. Those three lines are not needed.