diff --git a/components/vfs/test_apps/main/test_vfs_select.c b/components/vfs/test_apps/main/test_vfs_select.c index 08b0472806..ceab7004f7 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-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2022 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -101,23 +101,6 @@ 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; @@ -129,7 +112,7 @@ static void send_task(void *param) vTaskDelete(NULL); } -static inline void start_write_task(const test_task_param_t *test_task_param) +static inline void start_task(const test_task_param_t *test_task_param) { xTaskCreate(send_task, "send_task", 8*1024, (void *) test_task_param, 5, NULL); } @@ -182,7 +165,7 @@ TEST_CASE("UART can do select()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_write_task(&test_task_param); + start_task(&test_task_param); int s = select(uart_fd + 1, &rfds, NULL, NULL, &tv); TEST_ASSERT_EQUAL(s, 1); @@ -199,7 +182,7 @@ TEST_CASE("UART can do select()", "[vfs]") FD_SET(uart_fd, &rfds); FD_SET(socket_fd, &rfds); - start_write_task(&test_task_param); + start_task(&test_task_param); s = select(MAX(uart_fd, socket_fd) + 1, &rfds, NULL, NULL, &tv); TEST_ASSERT_EQUAL(s, 1); @@ -240,7 +223,7 @@ TEST_CASE("UART can do poll() with POLLIN event", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_write_task(&test_task_param); + start_task(&test_task_param); int s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -258,7 +241,7 @@ TEST_CASE("UART can do poll() with POLLIN event", "[vfs]") poll_fds[1].fd = socket_fd; poll_fds[1].events = POLLIN; - start_write_task(&test_task_param); + start_task(&test_task_param); s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -301,7 +284,7 @@ TEST_CASE("UART can do poll() with POLLOUT event", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_write_task(&test_task_param); + start_task(&test_task_param); poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(uart_fd, poll_fds[0].fd); @@ -347,7 +330,7 @@ TEST_CASE("socket can do select()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_write_task(&test_task_param); + start_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); @@ -396,7 +379,7 @@ TEST_CASE("socket can do poll()", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(test_task_param.sem); - start_write_task(&test_task_param); + start_task(&test_task_param); int s = poll(poll_fds, sizeof(poll_fds)/sizeof(poll_fds[0]), 100); TEST_ASSERT_EQUAL(s, 1); @@ -491,7 +474,8 @@ static void select_task(void *task_param) { const test_select_task_param_t *param = task_param; - select(param->maxfds, param->rdfds, param->wrfds, param->errfds, param->tv); + int s = select(param->maxfds, param->rdfds, param->wrfds, param->errfds, param->tv); + TEST_ASSERT_EQUAL(param->select_ret, s); if (param->sem) { xSemaphoreGive(param->sem); @@ -505,74 +489,6 @@ 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; @@ -648,7 +564,7 @@ TEST_CASE("concurrent selects work", "[vfs]") .sem = xSemaphoreCreateBinary(), }; TEST_ASSERT_NOT_NULL(send_param.sem); - start_write_task(&send_param); // This task will write to UART which will be detected by select() + start_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 9e96d138c9..3e1fcf4a74 100644 --- a/components/vfs/vfs_uart.c +++ b/components/vfs/vfs_uart.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -21,7 +21,6 @@ #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 @@ -56,7 +55,6 @@ 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 @@ -152,8 +150,6 @@ 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; @@ -302,7 +298,6 @@ 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; } @@ -442,7 +437,6 @@ 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) { @@ -450,7 +444,6 @@ 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; } } @@ -471,11 +464,14 @@ 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 - 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) || FD_ISSET(i, &args->writefds_orig) || FD_ISSET(i, &args->errorfds_orig)) { + uart_set_select_notif_callback(i, select_notif_callback_isr); + } + } for (int i = 0; i < max_fds; ++i) { if (FD_ISSET(i, &args->readfds_orig)) { @@ -504,23 +500,17 @@ 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) { - 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; - } + uart_set_select_notif_callback(i, NULL); } portEXIT_CRITICAL(uart_get_selectlock()); if (args) { free(args); } - xSemaphoreGive(uart_select_mutex[fd]); return ret; }