Merge branch 'bugfix/uart_vfs_read_behavior_v5.0' into 'release/v5.0'

fix(uart_vfs): read() now aligned to POSIX defined behavior (v5.0)

See merge request espressif/esp-idf!35397
This commit is contained in:
morris 2024-12-06 18:38:30 +08:00
commit 09e84586d1
2 changed files with 184 additions and 43 deletions

View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -74,6 +74,12 @@ TEST_CASE("CRs are removed from the stdin correctly", "[vfs]")
esp_vfs_dev_uart_port_set_tx_line_endings(CONFIG_ESP_CONSOLE_UART_NUM, ESP_LINE_ENDINGS_CRLF);
flush_stdin_stdout();
// A test case with no use of uart driver
// For non-uart-driver-involved uart vfs, all reads are non-blocking
// If no data at the moment, read() returns directly;
// If there is data available at the moment, read() also returns directly with the currently available size
const char* send_str = "1234567890\n\r123\r\n4\n";
/* with CONFIG_NEWLIB_STDOUT_ADDCR, the following will be sent on the wire.
* (last character of each part is marked with a hat)
@ -133,30 +139,46 @@ struct read_task_arg_t {
struct write_task_arg_t {
const char* str;
size_t str_len;
SemaphoreHandle_t done;
};
static void read_task_fn(void* varg)
static void read_blocking_task_fn(void* varg)
{
struct read_task_arg_t* parg = (struct read_task_arg_t*) varg;
parg->out_buffer[0] = 0;
memset(parg->out_buffer, 0, parg->out_buffer_len);
fgets(parg->out_buffer, parg->out_buffer_len, stdin);
xSemaphoreGive(parg->done);
vTaskDelete(NULL);
}
static void read_non_blocking_task_fn(void* varg)
{
struct read_task_arg_t* parg = (struct read_task_arg_t*) varg;
memset(parg->out_buffer, 0, parg->out_buffer_len);
char *ptr = parg->out_buffer;
while (fgets(ptr, parg->out_buffer_len, stdin) != NULL) {
while (*ptr != 0) {
ptr++;
}
}
xSemaphoreGive(parg->done);
vTaskDelete(NULL);
}
static void write_task_fn(void* varg)
{
struct write_task_arg_t* parg = (struct write_task_arg_t*) varg;
fwrite_str_loopback(parg->str, strlen(parg->str));
fwrite_str_loopback(parg->str, parg->str_len);
xSemaphoreGive(parg->done);
vTaskDelete(NULL);
}
TEST_CASE("can write to UART while another task is reading", "[vfs]")
TEST_CASE("read with uart driver (blocking)", "[vfs]")
{
char out_buffer[32];
char out_buffer[32] = {};
size_t out_buffer_len = sizeof(out_buffer);
struct read_task_arg_t read_arg = {
@ -165,9 +187,13 @@ TEST_CASE("can write to UART while another task is reading", "[vfs]")
.done = xSemaphoreCreateBinary()
};
// Send a string with length less than the read requested length
const char in_buffer[] = "!(@*#&(!*@&#((SDasdkjhadsl\n";
size_t in_buffer_len = sizeof(in_buffer);
struct write_task_arg_t write_arg = {
.str = "!(@*#&(!*@&#((SDasdkjhadsl\n",
.done = xSemaphoreCreateBinary()
.str = in_buffer,
.str_len = in_buffer_len,
.done = xSemaphoreCreateBinary()
};
flush_stdin_stdout();
@ -177,15 +203,18 @@ TEST_CASE("can write to UART while another task is reading", "[vfs]")
esp_vfs_dev_uart_use_driver(CONFIG_ESP_CONSOLE_UART_NUM);
xTaskCreate(&read_task_fn, "vfs_read", 4096, &read_arg, 5, NULL);
vTaskDelay(10);
// Start the read task first, it will block until data incoming
xTaskCreate(&read_blocking_task_fn, "vfs_read", 4096, &read_arg, 5, NULL);
int res = xSemaphoreTake(read_arg.done, 100 / portTICK_PERIOD_MS);
TEST_ASSERT_FALSE(res);
xTaskCreate(&write_task_fn, "vfs_write", 4096, &write_arg, 6, NULL);
int res = xSemaphoreTake(write_arg.done, 100 / portTICK_PERIOD_MS);
res = xSemaphoreTake(write_arg.done, 100 / portTICK_PERIOD_MS);
TEST_ASSERT(res);
res = xSemaphoreTake(read_arg.done, 100 / portTICK_PERIOD_MS);
res = xSemaphoreTake(read_arg.done, 100 / portTICK_PERIOD_MS); // read() returns with currently available size
TEST_ASSERT(res);
TEST_ASSERT_EQUAL(0, strcmp(write_arg.str, read_arg.out_buffer));
@ -196,6 +225,69 @@ TEST_CASE("can write to UART while another task is reading", "[vfs]")
vSemaphoreDelete(write_arg.done);
}
TEST_CASE("read with uart driver (non-blocking)", "[vfs]")
{
char out_buffer[32] = {};
size_t out_buffer_len = sizeof(out_buffer);
struct read_task_arg_t read_arg = {
.out_buffer = out_buffer,
.out_buffer_len = out_buffer_len,
.done = xSemaphoreCreateBinary()
};
// Send a string with length less than the read requested length
const char in_buffer[] = "!(@*#&(!*@&#((SDasdkjhad\nce"; // read should not early return on \n
size_t in_buffer_len = sizeof(in_buffer);
struct write_task_arg_t write_arg = {
.str = in_buffer,
.str_len = in_buffer_len,
.done = xSemaphoreCreateBinary()
};
flush_stdin_stdout();
ESP_ERROR_CHECK(uart_driver_install(CONFIG_ESP_CONSOLE_UART_NUM,
256, 0, 0, NULL, 0));
esp_vfs_dev_uart_use_driver(CONFIG_ESP_CONSOLE_UART_NUM);
esp_vfs_dev_uart_port_set_rx_line_endings(CONFIG_ESP_CONSOLE_UART_NUM, ESP_LINE_ENDINGS_LF);
esp_vfs_dev_uart_port_set_tx_line_endings(CONFIG_ESP_CONSOLE_UART_NUM, ESP_LINE_ENDINGS_LF);
int flags = fcntl(STDIN_FILENO, F_GETFL, 0);
fcntl(STDIN_FILENO, F_SETFL, flags | O_NONBLOCK);
// If start the read task first, it will return immediately
xTaskCreate(&read_non_blocking_task_fn, "vfs_read", 4096, &read_arg, 5, NULL);
int res = xSemaphoreTake(read_arg.done, 100 / portTICK_PERIOD_MS);
TEST_ASSERT(res);
xTaskCreate(&write_task_fn, "vfs_write", 4096, &write_arg, 6, NULL);
vTaskDelay(10);
xTaskCreate(&read_non_blocking_task_fn, "vfs_read", 4096, &read_arg, 5, NULL);
res = xSemaphoreTake(write_arg.done, 100 / portTICK_PERIOD_MS);
TEST_ASSERT(res);
res = xSemaphoreTake(read_arg.done, 1000 / portTICK_PERIOD_MS); // read() returns with currently available size
TEST_ASSERT(res);
// string compare
for (int i = 0; i < in_buffer_len; i++) {
TEST_ASSERT_EQUAL(in_buffer[i], out_buffer[i]);
}
esp_vfs_dev_uart_use_nonblocking(CONFIG_ESP_CONSOLE_UART_NUM);
fcntl(STDIN_FILENO, F_SETFL, flags);
esp_vfs_dev_uart_port_set_rx_line_endings(CONFIG_ESP_CONSOLE_UART_NUM, ESP_LINE_ENDINGS_CRLF);
esp_vfs_dev_uart_port_set_tx_line_endings(CONFIG_ESP_CONSOLE_UART_NUM, ESP_LINE_ENDINGS_CRLF);
uart_driver_delete(CONFIG_ESP_CONSOLE_UART_NUM);
vSemaphoreDelete(read_arg.done);
vSemaphoreDelete(write_arg.done);
vTaskDelay(2); // wait for tasks to exit
}
TEST_CASE("fcntl supported in UART VFS", "[vfs]")
{
int flags = fcntl(STDIN_FILENO, F_GETFL, 0);

View File

@ -48,14 +48,18 @@
typedef void (*tx_func_t)(int, int);
// UART read bytes function type
typedef int (*rx_func_t)(int);
// UART get available received bytes function type
typedef size_t (*get_available_data_len_func_t)(int);
// Basic functions for sending and receiving bytes over UART
// Basic functions for sending, receiving bytes, and get available data length over UART
static void uart_tx_char(int fd, int c);
static int uart_rx_char(int fd);
static size_t uart_get_avail_data_len(int fd);
// Functions for sending and receiving bytes which use UART driver
// Functions for sending, receiving bytes, and get available data length which use UART driver
static void uart_tx_char_via_driver(int fd, int c);
static int uart_rx_char_via_driver(int fd);
static size_t uart_get_avail_data_len_via_driver(int fd);
typedef struct {
// Pointers to UART peripherals
@ -77,6 +81,8 @@ typedef struct {
tx_func_t tx_func;
// Functions used to read bytes from UART. Default to "basic" functions.
rx_func_t rx_func;
// Function used to get available data bytes from UART. Default to "basic" functions.
get_available_data_len_func_t get_avail_data_len_func;
} vfs_uart_context_t;
#define VFS_CTX_DEFAULT_VAL(uart_dev) (vfs_uart_context_t) {\
@ -86,6 +92,7 @@ typedef struct {
.rx_mode = DEFAULT_RX_MODE,\
.tx_func = uart_tx_char,\
.rx_func = uart_rx_char,\
.get_avail_data_len_func = uart_get_avail_data_len,\
}
//If the context should be dynamically initialized, remove this structure
@ -156,6 +163,19 @@ static int uart_open(const char *path, int flags, int mode)
return fd;
}
size_t uart_get_avail_data_len(int fd)
{
uart_dev_t* uart = s_ctx[fd]->uart;
return uart_ll_get_rxfifo_len(uart);
}
size_t uart_get_avail_data_len_via_driver(int fd)
{
size_t buffered_size = 0;
uart_get_buffered_data_len(fd, &buffered_size);
return buffered_size;
}
static void uart_tx_char(int fd, int c)
{
uart_dev_t* uart = s_ctx[fd]->uart;
@ -247,38 +267,65 @@ static ssize_t uart_read(int fd, void* data, size_t size)
assert(fd >=0 && fd < 3);
char *data_c = (char *) data;
size_t received = 0;
size_t available_size = 0;
int c = NONE; // store the read char
_lock_acquire_recursive(&s_ctx[fd]->read_lock);
while (received < size) {
int c = uart_read_char(fd);
if (c == '\r') {
if (s_ctx[fd]->rx_mode == ESP_LINE_ENDINGS_CR) {
c = '\n';
} else if (s_ctx[fd]->rx_mode == ESP_LINE_ENDINGS_CRLF) {
/* look ahead */
int c2 = uart_read_char(fd);
if (c2 == NONE) {
/* could not look ahead, put the current character back */
uart_return_char(fd, c);
break;
}
if (c2 == '\n') {
/* this was \r\n sequence. discard \r, return \n */
if (!s_ctx[fd]->non_blocking) {
c = uart_read_char(fd); // blocking until data available for non-O_NONBLOCK mode
}
// find the actual fetch size
available_size += s_ctx[fd]->get_avail_data_len_func(fd);
if (c != NONE) {
available_size++;
}
if (s_ctx[fd]->peek_char != NONE) {
available_size++;
}
size_t fetch_size = MIN(available_size, size);
if (fetch_size > 0) {
do {
if (c == NONE) { // for non-O_NONBLOCK mode, there is already a pre-fetched char
c = uart_read_char(fd);
}
assert(c != NONE);
if (c == '\r') {
if (s_ctx[fd]->rx_mode == ESP_LINE_ENDINGS_CR) {
c = '\n';
} else {
/* \r followed by something else. put the second char back,
* it will be processed on next iteration. return \r now.
*/
uart_return_char(fd, c2);
} else if (s_ctx[fd]->rx_mode == ESP_LINE_ENDINGS_CRLF) {
/* look ahead */
int c2 = uart_read_char(fd);
fetch_size--;
if (c2 == NONE) {
/* could not look ahead, put the current character back */
uart_return_char(fd, c);
c = NONE;
break;
}
if (c2 == '\n') {
/* this was \r\n sequence. discard \r, return \n */
c = '\n';
} else {
/* \r followed by something else. put the second char back,
* it will be processed on next iteration. return \r now.
*/
uart_return_char(fd, c2);
fetch_size++;
}
}
}
} else if (c == NONE) {
break;
}
data_c[received] = (char) c;
++received;
if (c == '\n') {
break;
}
data_c[received] = (char) c;
++received;
c = NONE;
} while (received < fetch_size);
}
if (c != NONE) { // fetched, but not used
uart_return_char(fd, c);
}
_lock_release_recursive(&s_ctx[fd]->read_lock);
if (received > 0) {
@ -1044,6 +1091,7 @@ void esp_vfs_dev_uart_use_nonblocking(int uart_num)
_lock_acquire_recursive(&s_ctx[uart_num]->write_lock);
s_ctx[uart_num]->tx_func = uart_tx_char;
s_ctx[uart_num]->rx_func = uart_rx_char;
s_ctx[uart_num]->get_avail_data_len_func = uart_get_avail_data_len;
_lock_release_recursive(&s_ctx[uart_num]->write_lock);
_lock_release_recursive(&s_ctx[uart_num]->read_lock);
}
@ -1054,6 +1102,7 @@ void esp_vfs_dev_uart_use_driver(int uart_num)
_lock_acquire_recursive(&s_ctx[uart_num]->write_lock);
s_ctx[uart_num]->tx_func = uart_tx_char_via_driver;
s_ctx[uart_num]->rx_func = uart_rx_char_via_driver;
s_ctx[uart_num]->get_avail_data_len_func = uart_get_avail_data_len_via_driver;
_lock_release_recursive(&s_ctx[uart_num]->write_lock);
_lock_release_recursive(&s_ctx[uart_num]->read_lock);
}