diff --git a/components/esp_system/include/esp_private/usb_console.h b/components/esp_system/include/esp_private/usb_console.h index 028b83a2f2..9ddfe0e44f 100644 --- a/components/esp_system/include/esp_private/usb_console.h +++ b/components/esp_system/include/esp_private/usb_console.h @@ -1,6 +1,6 @@ /* - * SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -101,6 +101,14 @@ esp_err_t esp_usb_console_set_cb(esp_usb_console_cb_t rx_cb, esp_usb_console_cb_ */ bool esp_usb_console_is_installed(void); +/** + * @brief Call the USB interrupt handler while any interrupts + * are pending, but not more than a few times. Non-static to + * allow placement into IRAM by ldgen. + * + */ +void esp_usb_console_poll_interrupts(void); // [refactor-todo] Remove when implementing IDF-12175 + #ifdef __cplusplus } #endif diff --git a/components/esp_vfs_console/test_apps/usb_cdc_vfs/main/test_app_main.c b/components/esp_vfs_console/test_apps/usb_cdc_vfs/main/test_app_main.c index c72692dc75..67fe5a7fa4 100644 --- a/components/esp_vfs_console/test_apps/usb_cdc_vfs/main/test_app_main.c +++ b/components/esp_vfs_console/test_apps/usb_cdc_vfs/main/test_app_main.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -11,15 +11,28 @@ #include "esp_private/usb_console.h" #include "esp_vfs_cdcacm.h" -static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struct timeval tv) +static void flush_write(void) +{ + fflush(stdout); + fsync(fileno(stdout)); +} + +static void test_setup(const char* func_name, const size_t func_name_size) +{ + /* write the name of the test back to the pytest environment to start the test */ + write(fileno(stdout), func_name, func_name_size); + write(fileno(stdout), "\n", 1); + flush_write(); +} + +static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struct timeval* tv) { int fd = fileno(stream); fd_set read_fds; FD_ZERO(&read_fds); FD_SET(fd, &read_fds); - - /* call select with to wait for either a read ready or timeout to happen */ - int nread = select(fd + 1, &read_fds, NULL, NULL, &tv); + /* call select to wait for either a read ready or an except to happen */ + int nread = select(fd + 1, &read_fds, NULL, NULL, tv); if (nread < 0) { return -1; } else if (FD_ISSET(fd, &read_fds)) { @@ -27,7 +40,7 @@ static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struc int total_read = 0; do { - read_count = read(fileno(stream), buf + total_read, 1); + read_count = read(fd, buf + total_read, buf_len - total_read); if (read_count < 0 && errno != EWOULDBLOCK) { return -1; } else if (read_count > 0) { @@ -41,12 +54,31 @@ static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struc return total_read; } else { + /* select timed out */ return -2; } + return nread; +} + +static bool wait_for_read_ready(FILE *stream) +{ + int fd = fileno(stream); + fd_set read_fds; + FD_ZERO(&read_fds); + FD_SET(fd, &read_fds); + /* call select to wait for either a read ready or an except to happen */ + int nread = select(fd + 1, &read_fds, NULL, NULL, NULL); + if ((nread >= 0) && (FD_ISSET(fd, &read_fds))) { + return true; + } else { + return false; + } } -void app_main(void) +static void test_usb_cdc_select(void) { + test_setup(__func__, sizeof(__func__)); + struct timeval tv; tv.tv_sec = 1; tv.tv_usec = 0; @@ -70,7 +102,7 @@ void app_main(void) bool message_received = false; size_t char_read = 0; while (!message_received && out_buffer_len > char_read) { - int nread = read_bytes_with_select(stdin, out_buffer + char_read, out_buffer_len - char_read, tv); + int nread = read_bytes_with_select(stdin, out_buffer + char_read, out_buffer_len - char_read, &tv); if (nread > 0) { char_read += nread; if (out_buffer[char_read - 1] == '\n') { @@ -83,6 +115,7 @@ void app_main(void) // function since the string is expected as is by the test environment char timeout_msg[] = "select timed out\n"; write(fileno(stdout), timeout_msg, sizeof(timeout_msg)); + flush_write(); } else { break; } @@ -91,6 +124,59 @@ void app_main(void) // write the received message back to the test environment. The test // environment will check that the message received matches the one sent write(fileno(stdout), out_buffer, char_read); + flush_write(); vTaskDelay(10); // wait for the string to send } + +static void test_usb_cdc_read_non_blocking(void) +{ + test_setup(__func__, sizeof(__func__)); + + const size_t max_read_bytes = 16; + char read_data_buffer[max_read_bytes]; + + memset(read_data_buffer, 0x00, max_read_bytes); + + /* reset errno to 0 for the purpose of the test to make sure it is no + * longer set to EWOULDBLOCK later in the test. */ + errno = 0; + + /* make sure to read in non blocking mode */ + int stdin_fd = fileno(stdin); + int flags = fcntl(stdin_fd, F_GETFL); + flags |= O_NONBLOCK; + int res = fcntl(stdin_fd, F_SETFL, flags); + TEST_ASSERT(res == 0); + + /* call read when no data is available for reading. + * expected result: + * - read returns -1 and errno has been set to EWOULDBLOCK */ + int nread = read(stdin_fd, read_data_buffer, max_read_bytes); + TEST_ASSERT(nread == -1); + TEST_ASSERT(errno == EWOULDBLOCK); + + /* reset errno to 0 for the purpose of the test to make sure it is no + * longer set to EWOULDBLOCK later in the test. */ + errno = 0; + + /* call read X number of bytes when less than X number of + * bytes are available for reading (Y bytes). + * expected result: + * - read returns at least 1 bytes errno is not set to EWOULDBLOCK */ + char message_8[] = "send_bytes\n"; + write(fileno(stdout), message_8, sizeof(message_8)); + flush_write(); + + const bool is_ready = wait_for_read_ready(stdin); + TEST_ASSERT(is_ready); + nread = read(stdin_fd, read_data_buffer, max_read_bytes); + TEST_ASSERT(nread >= 1); + TEST_ASSERT(errno != EWOULDBLOCK); +} + +void app_main(void) +{ + test_usb_cdc_select(); + test_usb_cdc_read_non_blocking(); +} diff --git a/components/esp_vfs_console/test_apps/usb_cdc_vfs/pytest_usb_cdc_vfs.py b/components/esp_vfs_console/test_apps/usb_cdc_vfs/pytest_usb_cdc_vfs.py index 9c3d626b9b..b6c8503d45 100644 --- a/components/esp_vfs_console/test_apps/usb_cdc_vfs/pytest_usb_cdc_vfs.py +++ b/components/esp_vfs_console/test_apps/usb_cdc_vfs/pytest_usb_cdc_vfs.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: CC0-1.0 import pytest from pytest_embedded import Dut @@ -14,6 +14,13 @@ from pytest_embedded import Dut indirect=True,) @pytest.mark.parametrize('test_message', ['test123456789!@#%^&*']) def test_usb_cdc_vfs_default(dut: Dut, test_message: str) -> None: + # test run: test_usb_cdc_select + dut.expect_exact('test_usb_cdc_select', timeout=2) dut.expect_exact('select timed out', timeout=2) dut.write(test_message) dut.expect_exact(test_message, timeout=2) + + # test run: test_usb_cdc_read_non_blocking + dut.expect_exact('test_usb_cdc_read_non_blocking', timeout=2) + dut.expect_exact('send_bytes', timeout=2) + dut.write('abcdefgh') diff --git a/components/esp_vfs_console/vfs_cdcacm.c b/components/esp_vfs_console/vfs_cdcacm.c index ed66e77033..8a725e6d52 100644 --- a/components/esp_vfs_console/vfs_cdcacm.c +++ b/components/esp_vfs_console/vfs_cdcacm.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -181,15 +181,15 @@ static ssize_t cdcacm_read(int fd, void *data, size_t size) ssize_t received = 0; _lock_acquire_recursive(&s_read_lock); - while (cdcacm_data_length_in_buffer() < size) { - if (!s_blocking) { - errno = EWOULDBLOCK; - _lock_release_recursive(&s_read_lock); - return -1; + if (s_blocking) { + while (cdcacm_data_length_in_buffer() < size) { + xSemaphoreTake(s_rx_semaphore, portMAX_DELAY); } - xSemaphoreTake(s_rx_semaphore, portMAX_DELAY); + } else { + /* process pending interrupts before requesting available data */ + esp_usb_console_poll_interrupts(); + size = MIN(size, cdcacm_data_length_in_buffer()); } - if (s_rx_mode == ESP_LINE_ENDINGS_CR || s_rx_mode == ESP_LINE_ENDINGS_LF) { /* This is easy. Just receive, and if needed replace \r by \n. */ received = esp_usb_console_read_buf(data_c, size); @@ -443,6 +443,9 @@ static esp_err_t cdcacm_start_select(int nfds, fd_set *readfds, fd_set *writefds return ret; } + /* make sure the driver processes any pending interrupts before checking read or write + * readiness */ + esp_usb_console_poll_interrupts(); bool trigger_select = false; if (FD_ISSET(USB_CDC_LOCAL_FD, &args->readfds_orig) && esp_usb_console_available_for_read() > 0) {