Skip to content

Backport UART fixes #110

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.

Sign up for GitHub

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

Merged
floitsch merged 3 commits into from
Dec 12, 2025
Merged

Backport UART fixes #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2038df8
fix(uart): correct uart_get_tx_buffer_free_size calculation
songruo Oct 17, 2025
2a6a1e9
fix(uart): fix uart_write_bytes gets stuck internally if ring buffer …
songruo Nov 3, 2025
eeaec6f
fix(uart): fix send_size calculation in uart_write_bytes
songruo Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion components/esp_driver_uart/include/driver/uart.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,9 @@ esp_err_t uart_flush_input(uart_port_t uart_num);
esp_err_t uart_get_buffered_data_len(uart_port_t uart_num, size_t* size);

/**
* @brief UART get TX ring buffer free space size
* @brief UART get TX ring buffer free space size for the next data to be enqueued
*
* It returns the tight conservative bound for NOSPLIT ring buffer overall enqueueable payload across up to two chunks.
*
* @param uart_num UART port number, the max port number is (UART_NUM_MAX -1).
* @param size Pointer of size_t to accept the free space size
Expand Down
118 changes: 97 additions & 21 deletions components/esp_driver_uart/src/uart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ typedef struct {
bool coll_det_flg; /*!< UART collision detection flag */
bool rx_always_timeout_flg; /*!< UART always detect rx timeout flag */
int rx_buffered_len; /*!< UART cached data length */
int rx_buf_size; /*!< RX ring buffer size */
bool rx_buffer_full_flg; /*!< RX ring buffer full flag. */
uint8_t *rx_data_buf; /*!< Data buffer to stash FIFO data*/
uint8_t rx_stash_len; /*!< stashed data length.(When using flow control, after reading out FIFO data, if we fail to push to buffer, we can just stash them.) */
Expand All @@ -145,8 +144,8 @@ typedef struct {
bool tx_waiting_fifo; /*!< this flag indicates that some task is waiting for FIFO empty interrupt, used to send all data without any data buffer*/
uint8_t *tx_ptr; /*!< TX data pointer to push to FIFO in TX buffer mode*/
uart_tx_data_t *tx_head; /*!< TX data pointer to head of the current buffer in TX ring buffer*/
uint32_t tx_len_tot; /*!< Total length of current item in ring buffer*/
uint32_t tx_len_cur;
uint32_t trans_total_remaining_len; /*!< Remaining data length of the current processing transaction in TX ring buffer*/
uint32_t trans_chunk_remaining_len; /*!< Remaining data length of the current processing chunk of the transaction in TX ring buffer*/
uint8_t tx_brk_flg; /*!< Flag to indicate to send a break signal in the end of the item sending procedure */
uint8_t tx_brk_len; /*!< TX break signal cycle length/number */
uint8_t tx_waiting_brk; /*!< Flag to indicate that TX FIFO is ready to send break signal after FIFO is empty, do not push data into TX FIFO right now.*/
Expand Down Expand Up @@ -1047,15 +1046,15 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
//That would cause a watch_dog reset because empty interrupt happens so often.
//Although this is a loop in ISR, this loop will execute at most 128 turns.
while (tx_fifo_rem) {
if (p_uart->tx_len_tot == 0 || p_uart->tx_ptr == NULL || p_uart->tx_len_cur == 0) {
if (p_uart->trans_total_remaining_len == 0 || p_uart->tx_ptr == NULL || p_uart->trans_chunk_remaining_len == 0) {
size_t size;
p_uart->tx_head = (uart_tx_data_t *) xRingbufferReceiveFromISR(p_uart->tx_ring_buf, &size);
if (p_uart->tx_head) {
//The first item is the data description
//Get the first item to get the data information
if (p_uart->tx_len_tot == 0) {
if (p_uart->trans_total_remaining_len == 0) {
p_uart->tx_ptr = NULL;
p_uart->tx_len_tot = p_uart->tx_head->tx_data.size;
p_uart->trans_total_remaining_len = p_uart->tx_head->tx_data.size;
if (p_uart->tx_head->type == UART_DATA_BREAK) {
p_uart->tx_brk_flg = 1;
p_uart->tx_brk_len = p_uart->tx_head->tx_data.brk_len;
Expand All @@ -1067,30 +1066,30 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
//Update the TX item pointer, we will need this to return item to buffer.
p_uart->tx_ptr = (uint8_t *)p_uart->tx_head;
en_tx_flg = true;
p_uart->tx_len_cur = size;
p_uart->trans_chunk_remaining_len = size;
}
} else {
//Can not get data from ring buffer, return;
break;
}
}
if (p_uart->tx_len_tot > 0 && p_uart->tx_ptr && p_uart->tx_len_cur > 0) {
if (p_uart->trans_total_remaining_len > 0 && p_uart->tx_ptr && p_uart->trans_chunk_remaining_len > 0) {
// To fill the TX FIFO.
uint32_t send_len = uart_enable_tx_write_fifo(uart_num, (const uint8_t *) p_uart->tx_ptr,
MIN(p_uart->tx_len_cur, tx_fifo_rem));
MIN(p_uart->trans_chunk_remaining_len, tx_fifo_rem));
p_uart->tx_ptr += send_len;
p_uart->tx_len_tot -= send_len;
p_uart->tx_len_cur -= send_len;
p_uart->trans_total_remaining_len -= send_len;
p_uart->trans_chunk_remaining_len -= send_len;
tx_fifo_rem -= send_len;
if (p_uart->tx_len_cur == 0) {
if (p_uart->trans_chunk_remaining_len == 0) {
//Return item to ring buffer.
vRingbufferReturnItemFromISR(p_uart->tx_ring_buf, p_uart->tx_head, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
p_uart->tx_head = NULL;
p_uart->tx_ptr = NULL;
//Sending item done, now we need to send break if there is a record.
//Set TX break signal after FIFO is empty
if (p_uart->tx_len_tot == 0 && p_uart->tx_brk_flg == 1) {
if (p_uart->trans_total_remaining_len == 0 && p_uart->tx_brk_flg == 1) {
uart_hal_clr_intsts_mask(&(uart_context[uart_num].hal), UART_INTR_TX_BRK_DONE);
UART_ENTER_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
uart_hal_tx_break(&(uart_context[uart_num].hal), p_uart->tx_brk_len);
Expand Down Expand Up @@ -1417,6 +1416,8 @@ int uart_tx_chars(uart_port_t uart_num, const char *buffer, uint32_t len)
return tx_len;
}

// Per transaction in the ring buffer:
// A data description item, followed by one or more data chunk items
static int uart_tx_all(uart_port_t uart_num, const char *src, size_t size, bool brk_en, int brk_len)
{
if (size == 0) {
Expand All @@ -1431,7 +1432,6 @@ static int uart_tx_all(uart_port_t uart_num, const char *src, size_t size, bool
#endif
p_uart_obj[uart_num]->coll_det_flg = false;
if (p_uart_obj[uart_num]->tx_buf_size > 0) {
size_t max_size = xRingbufferGetMaxItemSize(p_uart_obj[uart_num]->tx_ring_buf);
int offset = 0;
uart_tx_data_t evt;
evt.tx_data.size = size;
Expand All @@ -1443,11 +1443,14 @@ static int uart_tx_all(uart_port_t uart_num, const char *src, size_t size, bool
}
xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void *) &evt, sizeof(uart_tx_data_t), portMAX_DELAY);
while (size > 0) {
size_t send_size = size > max_size / 2 ? max_size / 2 : size;
xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void *)(src + offset), send_size, portMAX_DELAY);
size -= send_size;
offset += send_size;
uart_enable_tx_intr(uart_num, 1, UART_THRESHOLD_NUM(uart_num, UART_EMPTY_THRESH_DEFAULT));
size_t free_size = xRingbufferGetCurFreeSize(p_uart_obj[uart_num]->tx_ring_buf);
size_t send_size = MIN(size, free_size);
if (send_size > 0) {
xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void *)(src + offset), send_size, portMAX_DELAY);
size -= send_size;
offset += send_size;
uart_enable_tx_intr(uart_num, 1, UART_THRESHOLD_NUM(uart_num, UART_EMPTY_THRESH_DEFAULT));
}
}
} else {
while (size) {
Expand Down Expand Up @@ -1571,7 +1574,79 @@ esp_err_t uart_get_tx_buffer_free_size(uart_port_t uart_num, size_t *size)
ESP_RETURN_ON_FALSE((uart_num < UART_NUM_MAX), ESP_ERR_INVALID_ARG, UART_TAG, "uart_num error");
ESP_RETURN_ON_FALSE((p_uart_obj[uart_num]), ESP_ERR_INVALID_ARG, UART_TAG, "uart driver error");
ESP_RETURN_ON_FALSE((size != NULL), ESP_ERR_INVALID_ARG, UART_TAG, "arg pointer is NULL");
*size = p_uart_obj[uart_num]->tx_buf_size - p_uart_obj[uart_num]->tx_len_tot;

// If tx buffer is disabled or ring buffer is full, overall enqueueable payload is 0
if (p_uart_obj[uart_num]->tx_buf_size == 0 || xRingbufferGetCurFreeSize(p_uart_obj[uart_num]->tx_ring_buf) == 0) {
*size = 0;
return ESP_OK;
}

// Tight conservative bound for NOSPLIT ring buffer overall enqueueable payload across up to two segments
const size_t RINGBUF_ITEM_HDR_SIZE = 8; // per public ringbuf API docs

// Per-item cap in current state and basis to infer minimal buffer size
size_t max_item = xRingbufferGetMaxItemSize(p_uart_obj[uart_num]->tx_ring_buf);

// Get current ring buffer pointer offsets and items waiting to detect empty
UBaseType_t off_free = 0;
UBaseType_t off_acq = 0;
UBaseType_t items_waiting = 0;
vRingbufferGetInfo(p_uart_obj[uart_num]->tx_ring_buf, &off_free, NULL, NULL, &off_acq, &items_waiting);

// Minimal possible total buffer size for NOSPLIT: see ringbuf initialization logic
// xMaxItemSize = ALIGN4(xSize/2) - header => xSize_min = 2 * (xMaxItemSize + header - up_to_3_alignment)
size_t buf_size_min = 2 * (max_item + RINGBUF_ITEM_HDR_SIZE - 3);
buf_size_min &= ~((size_t)3); // align down to 4 bytes

size_t total_payload = 0;
if (off_acq == off_free && items_waiting == 0) {
// Empty buffer: conservatively treat as a single large contiguous segment
total_payload = p_uart_obj[uart_num]->tx_buf_size - RINGBUF_ITEM_HDR_SIZE;
} else if (off_acq <= off_free) {
// Single contiguous free segment
size_t seg = (size_t)off_free - (size_t)off_acq;
if (seg > RINGBUF_ITEM_HDR_SIZE) {
size_t usable = seg - RINGBUF_ITEM_HDR_SIZE;
usable &= ~((size_t)3);
if (usable > max_item) {
usable = max_item;
}
total_payload = usable;
}
} else {
// Free space wraps: two segments [acq..tail) and [head..free)
size_t seg1 = buf_size_min - (size_t)off_acq;
size_t seg2 = (size_t)off_free; // from head (offset 0) to free
size_t payload1 = 0;
if (seg1 > RINGBUF_ITEM_HDR_SIZE) {
size_t usable1 = seg1 - RINGBUF_ITEM_HDR_SIZE;
usable1 &= ~((size_t)3);
if (usable1 > max_item) {
usable1 = max_item;
}
payload1 = usable1;
}
size_t payload2 = 0;
if (seg2 > RINGBUF_ITEM_HDR_SIZE) {
size_t usable2 = seg2 - RINGBUF_ITEM_HDR_SIZE;
usable2 &= ~((size_t)3);
if (usable2 > max_item) {
usable2 = max_item;
}
payload2 = usable2;
}
total_payload = payload1 + payload2;
}

// Subtract the cost of the transaction's data description item (header + aligned struct)
size_t desc_cost = RINGBUF_ITEM_HDR_SIZE + (((sizeof(uart_tx_data_t)) + 3) & ~((size_t)3));
if (total_payload > desc_cost) {
total_payload -= desc_cost;
} else {
total_payload = 0;
}

*size = total_payload;
return ESP_OK;
}

Expand Down Expand Up @@ -1747,7 +1822,8 @@ esp_err_t uart_driver_install(uart_port_t uart_num, int rx_buffer_size, int tx_b
p_uart_obj[uart_num]->event_queue_size = event_queue_size;
p_uart_obj[uart_num]->tx_ptr = NULL;
p_uart_obj[uart_num]->tx_head = NULL;
p_uart_obj[uart_num]->tx_len_tot = 0;
p_uart_obj[uart_num]->trans_total_remaining_len = 0;
p_uart_obj[uart_num]->trans_chunk_remaining_len = 0;
p_uart_obj[uart_num]->tx_brk_flg = 0;
p_uart_obj[uart_num]->tx_brk_len = 0;
p_uart_obj[uart_num]->tx_waiting_brk = 0;
Expand Down
66 changes: 58 additions & 8 deletions components/esp_driver_uart/test_apps/uart/main/test_uart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,23 +418,73 @@ TEST_CASE("uart tx with ringbuffer test", "[uart]")
rd_data[i] = 0;
}

size_t tx_buffer_free_space;
uart_get_tx_buffer_free_size(uart_num, &tx_buffer_free_space);
TEST_ASSERT_EQUAL_INT(2048, tx_buffer_free_space); // full tx buffer space is free
uart_write_bytes(uart_num, (const char *)wr_data, 1024);
uart_get_tx_buffer_free_size(uart_num, &tx_buffer_free_space);
TEST_ASSERT_LESS_THAN(2048, tx_buffer_free_space); // tx transmit in progress: tx buffer has content
TEST_ASSERT_GREATER_OR_EQUAL(1024, tx_buffer_free_space);
uart_wait_tx_done(uart_num, portMAX_DELAY);
uart_get_tx_buffer_free_size(uart_num, &tx_buffer_free_space);
TEST_ASSERT_EQUAL_INT(2048, tx_buffer_free_space); // tx done: tx buffer back to empty

uart_read_bytes(uart_num, rd_data, 1024, pdMS_TO_TICKS(1000));
TEST_ASSERT_EQUAL_HEX8_ARRAY(wr_data, rd_data, 1024);

TEST_ESP_OK(uart_driver_delete(uart_num));
free(rd_data);
free(wr_data);
}

TEST_CASE("uart tx ring buffer free space test", "[uart]")
{
uart_port_param_t port_param = {};
TEST_ASSERT(port_select(&port_param));
// This is a test on the driver API, no need to test for both HP/LP uart port, call port_select() to be compatible with pytest
// Let's only test on HP UART
if (port_param.port_num < SOC_UART_HP_NUM) {
uart_port_t uart_num = port_param.port_num;
uint8_t *rd_data = (uint8_t *)malloc(1024);
TEST_ASSERT_NOT_NULL(rd_data);
uint8_t *wr_data = (uint8_t *)malloc(2048);
TEST_ASSERT_NOT_NULL(wr_data);
uart_config_t uart_config = {
.baud_rate = 2000000,
.data_bits = UART_DATA_8_BITS,
.parity = UART_PARITY_DISABLE,
.stop_bits = UART_STOP_BITS_1,
.flow_ctrl = UART_HW_FLOWCTRL_CTS_RTS,
.rx_flow_ctrl_thresh = port_param.rx_flow_ctrl_thresh,
.source_clk = port_param.default_src_clk,
};
uart_wait_tx_idle_polling(uart_num);
TEST_ESP_OK(uart_param_config(uart_num, &uart_config));
TEST_ESP_OK(uart_driver_install(uart_num, 256, 1024 * 2, 20, NULL, 0));
// Let CTS be high, so that transmission is blocked
esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ONE_INPUT, uart_periph_signal[uart_num].pins[SOC_UART_PERIPH_SIGNAL_CTS].signal, false);

// When nothing pushed to the TX ring buffer, the free space should be the full capacity
size_t tx_buffer_free_space;
uart_get_tx_buffer_free_size(uart_num, &tx_buffer_free_space);
TEST_ASSERT_EQUAL_INT(2020, tx_buffer_free_space); // no-split ring buffer: 2048 - 20 (data description item) - 8 (header)

// Push 1024 bytes to the TX ring buffer
uart_write_bytes(uart_num, (const char *)wr_data, 1024); // two chunks
vTaskDelay(pdMS_TO_TICKS(500));
uart_get_tx_buffer_free_size(uart_num, &tx_buffer_free_space);
TEST_ASSERT_LESS_THAN(2020, tx_buffer_free_space); // tx buffer has content
TEST_ASSERT_GREATER_OR_EQUAL(952, tx_buffer_free_space);

// Fill the remaining space in the TX ring buffer
uart_write_bytes(uart_num, (const char *)wr_data, tx_buffer_free_space);
uart_get_tx_buffer_free_size(uart_num, &tx_buffer_free_space);
TEST_ASSERT_EQUAL_INT(0, tx_buffer_free_space); // tx buffer is full

// Let CTS be low, so that transmission is unblocked
esp_rom_gpio_connect_in_signal(GPIO_MATRIX_CONST_ZERO_INPUT, uart_periph_signal[uart_num].pins[SOC_UART_PERIPH_SIGNAL_CTS].signal, false);
uart_wait_tx_done(uart_num, portMAX_DELAY);
uart_get_tx_buffer_free_size(uart_num, &tx_buffer_free_space);
TEST_ASSERT_EQUAL_INT(2020, tx_buffer_free_space); // tx buffer is back to full capacity

TEST_ESP_OK(uart_driver_delete(uart_num));
free(rd_data);
free(wr_data);
}
}

TEST_CASE("uart int state restored after flush", "[uart]")
{
uart_port_param_t port_param = {};
Expand Down