From d1593b82b09951983e1a779b8154fb1a235a367b Mon Sep 17 00:00:00 2001 From: "sonika.rathi" Date: Thu, 17 Aug 2023 09:36:32 +0200 Subject: [PATCH] fix(vfs/uart): Add support for multi-task call to uart select --- .../vfs/test_apps/main/test_vfs_select.c | 108 ++++++++++++++++-- components/vfs/vfs_uart.c | 24 ++-- 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/components/vfs/test_apps/main/test_vfs_select.c b/components/vfs/test_apps/main/test_vfs_select.c index ceab7004f7..08b0472806 100644 --- a/components/vfs/test_apps/main/test_vfs_select.c +++ b/components/vfs/test_apps/main/test_vfs_select.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -101,6 +101,23 @@ static void uart1_init(void) uart_param_config(UART_NUM_1, &uart_config); } +static void read_task(void *param) +{ + char recv_message[sizeof(message)]; + const test_task_param_t *test_task_param = param; + vTaskDelay(test_task_param->delay_ms / portTICK_PERIOD_MS); + read(test_task_param->fd, recv_message, sizeof(message)); + if (test_task_param->sem) { + xSemaphoreGive(test_task_param->sem); + } + vTaskDelete(NULL); +} + +static inline void start_read_task(const test_task_param_t *test_task_param) +{ + xTaskCreate(read_task, "read_task", 8*1024, (void *) test_task_param, 5, NULL); +} + static void send_task(void *param) { const test_task_param_t *test_task_param = param; @@ -112,7 +129,7 @@ static void send_task(void *param) vTaskDelete(NULL); } -static inline void start_task(const test_task_param_t *test_task_param) +static inline void start_write_task(const test_task_param_t *test_task_param) { xTaskCreate(send_task, "send_task", 8*1024, (void *) test_task_param, 5, NULL); } @@ -165,7 +182,7 @@ TEST_CASE("UART can do select()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); int s = select(uart_fd + 1, &rfds, NULL, NULL, &tv); TEST_ASSERT_EQUAL(s, 1); @@ -182,7 +199,7 @@ TEST_CASE("UART can do select()", "[vfs]") FD_SET(uart_fd, &rfds); FD_SET(socket_fd, &rfds); - start_task(&test_task_param); + start_write_task(&test_task_param); s = select(MAX(uart_fd, socket_fd) + 1, &rfds, NULL, NULL, &tv); TEST_ASSERT_EQUAL(s, 1); @@ -223,7 +240,7 @@ TEST_CASE("UART can do poll() with POLLIN event", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); int s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -241,7 +258,7 @@ TEST_CASE("UART can do poll() with POLLIN event", "[vfs]") poll_fds[1].fd = socket_fd; poll_fds[1].events = POLLIN; - start_task(&test_task_param); + start_write_task(&test_task_param); s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -284,7 +301,7 @@ TEST_CASE("UART can do poll() with POLLOUT event", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(uart_fd, poll_fds[0].fd); @@ -330,7 +347,7 @@ TEST_CASE("socket can do select()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); const int s = select(MAX(MAX(uart_fd, socket_fd), dummy_socket_fd) + 1, &rfds, NULL, NULL, &tv); TEST_ASSERT_EQUAL(1, s); @@ -379,7 +396,7 @@ TEST_CASE("socket can do poll()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_task(&test_task_param); + start_write_task(&test_task_param); int s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -474,8 +491,7 @@ static void select_task(void *task_param) { const test_select_task_param_t *param = task_param; - int s = select(param->maxfds, param->rdfds, param->wrfds, param->errfds, param->tv); - TEST_ASSERT_EQUAL(param->select_ret, s); + select(param->maxfds, param->rdfds, param->wrfds, param->errfds, param->tv); if (param->sem) { xSemaphoreGive(param->sem); @@ -489,6 +505,74 @@ static void inline start_select_task(test_select_task_param_t *param) } #if !CONFIG_IDF_TARGET_ESP32H2 // IDF-6782 +TEST_CASE("concurrent selects work for UART", "[vfs]") +{ + // This test case initiates two select tasks on the same UART FD, + // One task will wait for a write operation, while the other will wait for a read operation to occur. + // The first task will complete its operation before the second task proceeds with its operation on the same FD + // In this scenario, the write operation will be performed initially, + // followed by the subsequent continuation of the read operation. + + int uart_fd, socket_fd; + init(&uart_fd, &socket_fd); + + const test_task_param_t send_param = { + .fd = uart_fd, + .delay_ms = 0, + .sem = NULL, + }; + + fd_set wrfds1; + FD_ZERO(&wrfds1); + FD_SET(uart_fd, &wrfds1); + + test_select_task_param_t param_write = { + .rdfds = NULL, + .wrfds = &wrfds1, + .errfds = NULL, + .maxfds = uart_fd + 1, + .tv = NULL, + .select_ret = 1, + .sem = xSemaphoreCreateBinary(), + }; + TEST_ASSERT_NOT_NULL(param_write.sem); + //Start first task which will wait on select call for write operation on the UART FD + start_select_task(¶m_write); + + fd_set rdfds2; + FD_ZERO(&rdfds2); + FD_SET(uart_fd, &rdfds2); + + test_select_task_param_t param_read = { + .rdfds = &rdfds2, + .wrfds = NULL, + .errfds = NULL, + .maxfds = uart_fd + 1, + .tv = NULL, + .select_ret = 2, + .sem = xSemaphoreCreateBinary(), + }; + TEST_ASSERT_NOT_NULL(param_read.sem); + //Start second task which will wait on another select call for read operation on the same UART FD + start_select_task(¶m_read); + + //Start writing operation on the UART port + start_write_task(&send_param); + //Confirm the completion of the write operation + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param_write.sem, 1000 / portTICK_PERIOD_MS)); + vSemaphoreDelete(param_write.sem); + TEST_ASSERT(FD_ISSET(uart_fd, &wrfds1)); + + //Start reading operation on the same UART port + start_read_task(&send_param); + //Confirm the completion of the read operation + TEST_ASSERT_EQUAL(pdTRUE, xSemaphoreTake(param_read.sem, 1000 / portTICK_PERIOD_MS)); + vSemaphoreDelete(param_read.sem); + TEST_ASSERT(FD_ISSET(uart_fd, &rdfds2)); + + deinit(uart_fd, socket_fd); +} + TEST_CASE("concurrent selects work", "[vfs]") { int uart_fd, socket_fd; @@ -564,7 +648,7 @@ TEST_CASE("concurrent selects work", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(send_param.sem); - start_task(&send_param); // This task will write to UART which will be detected by select() + start_write_task(&send_param); // This task will write to UART which will be detected by select() start_select_task(¶m); vTaskDelay(100 / portTICK_PERIOD_MS); //make sure the task has started and waits in select() diff --git a/components/vfs/vfs_uart.c b/components/vfs/vfs_uart.c index 3e1fcf4a74..9e96d138c9 100644 --- a/components/vfs/vfs_uart.c +++ b/components/vfs/vfs_uart.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -21,6 +21,7 @@ #include "esp_rom_uart.h" #include "soc/soc_caps.h" #include "hal/uart_ll.h" +#include "freertos/semphr.h" #define UART_NUM SOC_UART_HP_NUM @@ -55,6 +56,7 @@ static int uart_rx_char(int fd); // Functions for sending and receiving bytes 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 SemaphoreHandle_t uart_select_mutex[UART_NUM]; typedef struct { // Pointers to UART peripherals @@ -150,6 +152,8 @@ static int uart_open(const char *path, int flags, int mode) return -1; } + uart_select_mutex[fd] = xSemaphoreCreateMutex(); + xSemaphoreGive(uart_select_mutex[fd]); s_ctx[fd]->non_blocking = ((flags & O_NONBLOCK) == O_NONBLOCK); return fd; @@ -298,6 +302,7 @@ static int uart_fstat(int fd, struct stat * st) static int uart_close(int fd) { assert(fd >=0 && fd < 3); + vSemaphoreDelete(uart_select_mutex[fd]); return 0; } @@ -437,6 +442,7 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, esp_vfs_select_sem_t select_sem, void **end_select_args) { const int max_fds = MIN(nfds, UART_NUM); + int fd = -1; *end_select_args = NULL; for (int i = 0; i < max_fds; ++i) { @@ -444,6 +450,7 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, if (!uart_is_driver_installed(i)) { return ESP_ERR_INVALID_STATE; } + fd = i; } } @@ -464,14 +471,11 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, FD_ZERO(writefds); FD_ZERO(exceptfds); + xSemaphoreTake(uart_select_mutex[fd], portMAX_DELAY); portENTER_CRITICAL(uart_get_selectlock()); //uart_set_select_notif_callback sets the callbacks in UART ISR - for (int i = 0; i < max_fds; ++i) { - if (FD_ISSET(i, &args->readfds_orig) || FD_ISSET(i, &args->writefds_orig) || FD_ISSET(i, &args->errorfds_orig)) { - uart_set_select_notif_callback(i, select_notif_callback_isr); - } - } + uart_set_select_notif_callback(fd, select_notif_callback_isr); for (int i = 0; i < max_fds; ++i) { if (FD_ISSET(i, &args->readfds_orig)) { @@ -500,17 +504,23 @@ static esp_err_t uart_start_select(int nfds, fd_set *readfds, fd_set *writefds, static esp_err_t uart_end_select(void *end_select_args) { uart_select_args_t *args = end_select_args; + int fd = -1; portENTER_CRITICAL(uart_get_selectlock()); esp_err_t ret = unregister_select(args); for (int i = 0; i < UART_NUM; ++i) { - uart_set_select_notif_callback(i, NULL); + if (FD_ISSET(i, &args->readfds_orig) || FD_ISSET(i, &args->writefds_orig) || FD_ISSET(i, &args->errorfds_orig)) { + uart_set_select_notif_callback(i, NULL); + fd = i; + break; + } } portEXIT_CRITICAL(uart_get_selectlock()); if (args) { free(args); } + xSemaphoreGive(uart_select_mutex[fd]); return ret; }