From b13ade8e0c122033cbe75ac3ec3583f4a041f837 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Thu, 29 May 2025 13:30:19 +0200 Subject: [PATCH 1/4] fix(driver): remove unecessary if conditions in the read function This changes affect usb_serial_jtag_vfs and cdcacm_vfs read functions. This commit removes the exit condition on reception of \n character. --- .../esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c | 3 --- components/esp_vfs_console/vfs_cdcacm.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c index d24ef0c351..e0a169f9fa 100644 --- a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c +++ b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c @@ -257,9 +257,6 @@ static ssize_t usb_serial_jtag_read(int fd, void* data, size_t size) } data_c[received] = (char) c; ++received; - if (c == '\n') { - break; - } } _lock_release_recursive(&s_ctx.read_lock); if (received > 0) { diff --git a/components/esp_vfs_console/vfs_cdcacm.c b/components/esp_vfs_console/vfs_cdcacm.c index 3402e34220..65dca2aa6e 100644 --- a/components/esp_vfs_console/vfs_cdcacm.c +++ b/components/esp_vfs_console/vfs_cdcacm.c @@ -196,9 +196,6 @@ static ssize_t cdcacm_read(int fd, void *data, size_t size) break; } data_c[received++] = (char) c; - if (c == '\n') { - break; - } } } _lock_release_recursive(&s_read_lock); From 0650ee6900818be66f6d009a24e200d8560bd8f6 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Tue, 3 Jun 2025 13:56:25 +0200 Subject: [PATCH 2/4] feat(usb_serial_tag_vfs): Add test for read exit conditions Add a test to make sure the VFS read does not return on reception of the \n character --- .../src/usb_serial_jtag_vfs.c | 2 + .../main/test_vfs_usb_serial_jtag.c | 61 ++++++++++++++++--- .../pytest_usb_serial_jtag_vfs.py | 21 ++++++- .../usb_cdc_vfs/main/test_app_main.c | 48 +++++++++++++++ .../usb_cdc_vfs/pytest_usb_cdc_vfs.py | 5 ++ 5 files changed, 129 insertions(+), 8 deletions(-) diff --git a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c index e0a169f9fa..9bbcbc3e88 100644 --- a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c +++ b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c @@ -226,9 +226,11 @@ static void usb_serial_jtag_return_char(int fd, int c) static ssize_t usb_serial_jtag_read(int fd, void* data, size_t size) { + assert(fd == USJ_LOCAL_FD); char *data_c = (char *) data; size_t received = 0; _lock_acquire_recursive(&s_ctx.read_lock); + while (received < size) { int c = usb_serial_jtag_read_char(fd); if (c == '\r') { diff --git a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.c b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.c index c349aab14c..9c418c57fe 100644 --- a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.c +++ b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.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 */ @@ -23,11 +23,6 @@ #include "test_utils.h" #include "sdkconfig.h" -struct task_arg_t { - FILE* stream; - SemaphoreHandle_t done; -}; - static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struct timeval tv) { @@ -74,7 +69,7 @@ static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struc * write calls. At the end of the test, the variable is check to make sure the select returned * for each of the write calls. */ -TEST_CASE("test select read, write and timeout", "[usb_serial_jtag]") +TEST_CASE("test select read, write and timeout", "[usb_serial_jtag vfs]") { struct timeval tv; tv.tv_sec = 1; @@ -128,3 +123,55 @@ TEST_CASE("test select read, write and timeout", "[usb_serial_jtag]") usb_serial_jtag_vfs_use_nonblocking(); usb_serial_jtag_driver_uninstall(); } + +TEST_CASE("read with usj driver (non-blocking)", "[usb serial jtag vfs]") +{ + // Send a string with length less than the read requested length + const char in_buffer[] = "!(@*#&(!*@&#((SDasdkjhad\nce"; // read should not early return on \n + const size_t in_buffer_len = sizeof(in_buffer); + + const size_t out_buffer_len = in_buffer_len - 1; // don't compare the null character at the end of in_buffer string + char out_buffer[out_buffer_len] = {}; + + // flush_stdin_stdout(); + + usb_serial_jtag_driver_config_t usj_config = USB_SERIAL_JTAG_DRIVER_CONFIG_DEFAULT(); + ESP_ERROR_CHECK(usb_serial_jtag_driver_install(&usj_config)); + usb_serial_jtag_vfs_use_driver(); + + usb_serial_jtag_vfs_set_rx_line_endings(ESP_LINE_ENDINGS_LF); + usb_serial_jtag_vfs_set_tx_line_endings(ESP_LINE_ENDINGS_LF); + + int flags = fcntl(STDIN_FILENO, F_GETFL, 0); + fcntl(STDIN_FILENO, F_SETFL, flags | O_NONBLOCK); + + // trigger the test environment to send the test message + char ready_msg[] = "ready to receive\n"; + write(fileno(stdout), ready_msg, sizeof(ready_msg)); + + // wait for the string to be sent and buffered + vTaskDelay(pdMS_TO_TICKS(500)); + + char *out_buffer_ptr = out_buffer; + size_t bytes_read = 0; + do { + int nread = read(STDIN_FILENO, out_buffer_ptr, out_buffer_len); + printf("%d\n", nread); + if (nread > 0) { + out_buffer_ptr += nread; + bytes_read += nread; + } + } while (bytes_read < in_buffer_len); + + // string compare + for (size_t i = 0; i < out_buffer_len; i++) { + TEST_ASSERT_EQUAL(in_buffer[i], out_buffer[i]); + } + + usb_serial_jtag_vfs_use_nonblocking(); + fcntl(STDIN_FILENO, F_SETFL, flags); + usb_serial_jtag_vfs_set_rx_line_endings(ESP_LINE_ENDINGS_CRLF); + usb_serial_jtag_vfs_set_tx_line_endings(ESP_LINE_ENDINGS_CRLF); + ESP_ERROR_CHECK(usb_serial_jtag_driver_uninstall()); + vTaskDelay(2); // wait for tasks to exit +} diff --git a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py index 897f3179ee..c8a7f07e44 100644 --- a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py +++ b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py @@ -16,10 +16,29 @@ from pytest_embedded import Dut ], indirect=True,) @pytest.mark.parametrize('test_message', ['test123456789!@#%^&*']) -def test_usj_vfs_release(dut: Dut, test_message: list) -> None: +@idf_parametrize('target', ['esp32s3', 'esp32c3', 'esp32c6', 'esp32h2'], indirect=['target']) +def test_usj_vfs_select(dut: Dut, test_message: list) -> None: dut.expect_exact('Press ENTER to see the list of tests') dut.write('\"test select read, write and timeout\"') dut.expect_exact('select timed out', timeout=2) dut.write(test_message) dut.expect_exact(test_message, timeout=2) dut.expect(r'\d{1} Tests 0 Failures 0 Ignored', timeout=10) + + +@pytest.mark.usb_serial_jtag +@pytest.mark.parametrize( + 'port, config', + [ + pytest.param('/dev/ttyACM1', 'release'), + ], + indirect=True, +) +@pytest.mark.parametrize('test_message', ['!(@*#&(!*@&#((SDasdkjhad\nce']) +@idf_parametrize('target', ['esp32s3', 'esp32c3', 'esp32c6', 'esp32h2'], indirect=['target']) +def test_usj_vfs_read(dut: Dut, test_message: list) -> None: + dut.expect_exact('Press ENTER to see the list of tests') + dut.write('"read with usj driver (non-blocking)"') + dut.expect_exact('ready to receive', timeout=2) + dut.write(test_message) + dut.expect(r'\d{1} Tests 0 Failures 0 Ignored', timeout=10) 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 4e0b84d5bb..e4372bb38d 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 @@ -86,7 +86,55 @@ static void test_usb_cdc_read_non_blocking(void) TEST_ASSERT(errno != EWOULDBLOCK); } +static void test_usb_cdc_read_no_exit_on_newline_reception(void) +{ + test_setup(__func__, sizeof(__func__)); + + // Send a string with length less than the read requested length + const char in_buffer[] = "!(@*#&(!*@&#((SDasdkjhad\nce"; // read should not early return on \n + const size_t in_buffer_len = sizeof(in_buffer); + + const size_t out_buffer_len = in_buffer_len - 1; // don't compare the null character at the end of in_buffer string + char out_buffer[out_buffer_len] = {}; + + ESP_ERROR_CHECK(esp_vfs_dev_cdcacm_register()); + + esp_vfs_dev_cdcacm_set_rx_line_endings(ESP_LINE_ENDINGS_LF); + esp_vfs_dev_cdcacm_set_tx_line_endings(ESP_LINE_ENDINGS_LF); + + int flags = fcntl(STDIN_FILENO, F_GETFL, 0); + fcntl(STDIN_FILENO, F_SETFL, flags | O_NONBLOCK); + + // trigger the test environment to send the test message + char ready_msg[] = "ready to receive\n"; + write(fileno(stdout), ready_msg, sizeof(ready_msg)); + + // wait for the string to be sent and buffered + vTaskDelay(pdMS_TO_TICKS(500)); + + char *out_buffer_ptr = out_buffer; + size_t bytes_read = 0; + do { + int nread = read(STDIN_FILENO, out_buffer_ptr, out_buffer_len); + if (nread > 0) { + out_buffer_ptr += nread; + bytes_read += nread; + } + } while (bytes_read < in_buffer_len); + + // string compare + for (size_t i = 0; i < out_buffer_len; i++) { + TEST_ASSERT_EQUAL(in_buffer[i], out_buffer[i]); + } + + fcntl(STDIN_FILENO, F_SETFL, flags); + esp_vfs_dev_cdcacm_set_rx_line_endings(ESP_LINE_ENDINGS_CRLF); + esp_vfs_dev_cdcacm_set_tx_line_endings(ESP_LINE_ENDINGS_CRLF); + vTaskDelay(2); // wait for tasks to exit +} + void app_main(void) { test_usb_cdc_read_non_blocking(); + test_usb_cdc_read_no_exit_on_newline_reception(); } 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 b462d0f8b2..0e1cde5cbc 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 @@ -18,3 +18,8 @@ def test_usb_cdc_vfs_default(dut: Dut, test_message: str) -> None: dut.expect_exact('test_usb_cdc_read_non_blocking', timeout=2) dut.expect_exact('send_bytes', timeout=2) dut.write('abcdefgh') + + # test run: test_usb_cdc_read_no_exit_on_newline_reception + dut.expect_exact('test_usb_cdc_read_no_exit_on_newline_reception', timeout=2) + dut.expect_exact('ready to receive', timeout=2) + dut.write('!(@*#&(!*@&#((SDasdkjhad\nce') From fd1103d39c16a215b6846da207e77053338621e8 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Thu, 5 Jun 2025 12:15:53 +0200 Subject: [PATCH 3/4] feat(usb_cdc): Update vfs read() to comply with POSIX standards --- .../main/test_vfs_usb_serial_jtag.c | 37 ++++++- .../pytest_usb_serial_jtag_vfs.py | 24 ++++- .../usb_cdc_vfs/main/test_app_main.c | 99 +++++++++++++++++++ .../usb_cdc_vfs/pytest_usb_cdc_vfs.py | 5 + components/esp_vfs_console/vfs_cdcacm.c | 35 +++---- 5 files changed, 174 insertions(+), 26 deletions(-) diff --git a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.c b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.c index 9c418c57fe..4e305baa87 100644 --- a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.c +++ b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.c @@ -124,7 +124,9 @@ TEST_CASE("test select read, write and timeout", "[usb_serial_jtag vfs]") usb_serial_jtag_driver_uninstall(); } -TEST_CASE("read with usj driver (non-blocking)", "[usb serial jtag vfs]") +/* Test that the read function does not return prematurely when receiving a new line character + */ +TEST_CASE("read does not return on new line character", "[usb serial jtag vfs]") { // Send a string with length less than the read requested length const char in_buffer[] = "!(@*#&(!*@&#((SDasdkjhad\nce"; // read should not early return on \n @@ -133,8 +135,6 @@ TEST_CASE("read with usj driver (non-blocking)", "[usb serial jtag vfs]") const size_t out_buffer_len = in_buffer_len - 1; // don't compare the null character at the end of in_buffer string char out_buffer[out_buffer_len] = {}; - // flush_stdin_stdout(); - usb_serial_jtag_driver_config_t usj_config = USB_SERIAL_JTAG_DRIVER_CONFIG_DEFAULT(); ESP_ERROR_CHECK(usb_serial_jtag_driver_install(&usj_config)); usb_serial_jtag_vfs_use_driver(); @@ -175,3 +175,34 @@ TEST_CASE("read with usj driver (non-blocking)", "[usb serial jtag vfs]") ESP_ERROR_CHECK(usb_serial_jtag_driver_uninstall()); vTaskDelay(2); // wait for tasks to exit } + +TEST_CASE("blocking read returns with available data", "[usb serial jtag vfs]") +{ + const size_t out_buffer_len = 32; + char out_buffer[out_buffer_len] = {}; + + usb_serial_jtag_driver_config_t usj_config = USB_SERIAL_JTAG_DRIVER_CONFIG_DEFAULT(); + ESP_ERROR_CHECK(usb_serial_jtag_driver_install(&usj_config)); + usb_serial_jtag_vfs_use_driver(); + + usb_serial_jtag_vfs_set_rx_line_endings(ESP_LINE_ENDINGS_LF); + usb_serial_jtag_vfs_set_tx_line_endings(ESP_LINE_ENDINGS_LF); + + int flags = fcntl(STDIN_FILENO, F_GETFL, 0); + fcntl(STDIN_FILENO, F_SETFL, flags & (~O_NONBLOCK)); + + // trigger the test environment to send the test message + char ready_msg[] = "ready to receive\n"; + write(fileno(stdout), ready_msg, sizeof(ready_msg)); + + const int nread = read(STDIN_FILENO, out_buffer, out_buffer_len); + TEST_ASSERT(nread > 0); + TEST_ASSERT(nread < out_buffer_len); + + usb_serial_jtag_vfs_use_nonblocking(); + fcntl(STDIN_FILENO, F_SETFL, flags); + usb_serial_jtag_vfs_set_rx_line_endings(ESP_LINE_ENDINGS_CRLF); + usb_serial_jtag_vfs_set_tx_line_endings(ESP_LINE_ENDINGS_CRLF); + ESP_ERROR_CHECK(usb_serial_jtag_driver_uninstall()); + vTaskDelay(2); // wait for tasks to exit +} diff --git a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py index c8a7f07e44..a66afd6b44 100644 --- a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py +++ b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py @@ -28,9 +28,9 @@ def test_usj_vfs_select(dut: Dut, test_message: list) -> None: @pytest.mark.usb_serial_jtag @pytest.mark.parametrize( - 'port, config', + 'port, flash_port, config', [ - pytest.param('/dev/ttyACM1', 'release'), + pytest.param('/dev/serial_ports/ttyACM-esp32', '/dev/serial_ports/ttyUSB-esp32', 'release'), ], indirect=True, ) @@ -38,7 +38,25 @@ def test_usj_vfs_select(dut: Dut, test_message: list) -> None: @idf_parametrize('target', ['esp32s3', 'esp32c3', 'esp32c6', 'esp32h2'], indirect=['target']) def test_usj_vfs_read(dut: Dut, test_message: list) -> None: dut.expect_exact('Press ENTER to see the list of tests') - dut.write('"read with usj driver (non-blocking)"') + dut.write('"read does not return on new line character"') + dut.expect_exact('ready to receive', timeout=2) + dut.write(test_message) + dut.expect(r'\d{1} Tests 0 Failures 0 Ignored', timeout=10) + + +@pytest.mark.usb_serial_jtag +@pytest.mark.parametrize( + 'port, flash_port, config', + [ + pytest.param('/dev/serial_ports/ttyACM-esp32', '/dev/serial_ports/ttyUSB-esp32', 'release'), + ], + indirect=True, +) +@pytest.mark.parametrize('test_message', ['testdata']) +@idf_parametrize('target', ['esp32s3', 'esp32c3', 'esp32c6', 'esp32h2'], indirect=['target']) +def test_usj_vfs_read(dut: Dut, test_message: list) -> None: + dut.expect_exact('Press ENTER to see the list of tests') + dut.write('"blocking read returns with available data"') dut.expect_exact('ready to receive', timeout=2) dut.write(test_message) dut.expect(r'\d{1} Tests 0 Failures 0 Ignored', timeout=10) 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 e4372bb38d..92441a0834 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 @@ -40,6 +40,63 @@ static bool wait_for_read_ready(FILE *stream) } } +static void test_usb_cdc_select(void) +{ + test_setup(__func__, sizeof(__func__)); + + struct timeval tv; + tv.tv_sec = 1; + tv.tv_usec = 0; + char out_buffer[32]; + memset(out_buffer, 0, sizeof(out_buffer)); + size_t out_buffer_len = sizeof(out_buffer); + + // stdin needs to be non blocking to properly call read after select returns + // with read ready on stdin. + int fd = fileno(stdin); + int flags = fcntl(fd, F_GETFL); + flags |= O_NONBLOCK; + int res = fcntl(fd, F_SETFL, flags); + TEST_ASSERT(res == 0); + + // init driver + // ESP_ERROR_CHECK(esp_usb_console_init()); + // esp_vfs_dev_cdcacm_register(); + + // send the message from pytest environment and make sure it can be read + 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); + if (nread > 0) { + char_read += nread; + if (out_buffer[char_read - 1] == '\n') { + message_received = true; + } + } else if (nread == -2) { + // time out occurred, send the expected message back to the testing + // environment to trigger the testing environment into sending the + // test message. don't update this message without updating the pytest + // 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; + } + } + + // 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 +} + +/** + * @brief Test that the non blocking read is compliant with POSIX standards + */ static void test_usb_cdc_read_non_blocking(void) { test_setup(__func__, sizeof(__func__)); @@ -86,6 +143,44 @@ static void test_usb_cdc_read_non_blocking(void) TEST_ASSERT(errno != EWOULDBLOCK); } +/** + * @brief Test that the blocking read will return with the available + * data even if the size is less than the requested size. + */ +static void test_usb_cdc_read_blocking(void) +{ + test_setup(__func__, sizeof(__func__)); + + const size_t out_buffer_len = 32; + char out_buffer[out_buffer_len] = {}; + + ESP_ERROR_CHECK(esp_vfs_dev_cdcacm_register()); + + esp_vfs_dev_cdcacm_set_rx_line_endings(ESP_LINE_ENDINGS_LF); + esp_vfs_dev_cdcacm_set_tx_line_endings(ESP_LINE_ENDINGS_LF); + + /* make sure blocking mode is enabled */ + int flags = fcntl(STDIN_FILENO, F_GETFL, 0); + fcntl(STDIN_FILENO, F_SETFL, flags & (~O_NONBLOCK)); + + // trigger the test environment to send the test message + char ready_msg[] = "ready to receive\n"; + write(fileno(stdout), ready_msg, sizeof(ready_msg)); + + const int nread = read(STDIN_FILENO, out_buffer, out_buffer_len); + TEST_ASSERT(nread > 0); + TEST_ASSERT(nread < out_buffer_len); + + fcntl(STDIN_FILENO, F_SETFL, flags); + esp_vfs_dev_cdcacm_set_rx_line_endings(ESP_LINE_ENDINGS_CRLF); + esp_vfs_dev_cdcacm_set_tx_line_endings(ESP_LINE_ENDINGS_CRLF); + vTaskDelay(2); // wait for tasks to exit +} + +/** + * @brief Test that the read function does not prematurely return + * on reception of new line character. + */ static void test_usb_cdc_read_no_exit_on_newline_reception(void) { test_setup(__func__, sizeof(__func__)); @@ -133,8 +228,12 @@ static void test_usb_cdc_read_no_exit_on_newline_reception(void) vTaskDelay(2); // wait for tasks to exit } +/* Always make sure that the function calling sequence in the main + * function matches the expected order in the pytest function. + */ void app_main(void) { test_usb_cdc_read_non_blocking(); + test_usb_cdc_read_blocking(); test_usb_cdc_read_no_exit_on_newline_reception(); } 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 0e1cde5cbc..0403a5e8fd 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 @@ -19,6 +19,11 @@ def test_usb_cdc_vfs_default(dut: Dut, test_message: str) -> None: dut.expect_exact('send_bytes', timeout=2) dut.write('abcdefgh') + # test run: test_usb_cdc_read_blocking + dut.expect_exact('test_usb_cdc_read_blocking', timeout=2) + dut.expect_exact('ready to receive', timeout=2) + dut.write('testdata') + # test run: test_usb_cdc_read_no_exit_on_newline_reception dut.expect_exact('test_usb_cdc_read_no_exit_on_newline_reception', timeout=2) dut.expect_exact('ready to receive', timeout=2) diff --git a/components/esp_vfs_console/vfs_cdcacm.c b/components/esp_vfs_console/vfs_cdcacm.c index 65dca2aa6e..b210b035b3 100644 --- a/components/esp_vfs_console/vfs_cdcacm.c +++ b/components/esp_vfs_console/vfs_cdcacm.c @@ -146,13 +146,16 @@ static void cdcacm_return_char(int c) static ssize_t cdcacm_read(int fd, void *data, size_t size) { - assert(fd == 0); + assert(fd == USB_CDC_LOCAL_FD); char *data_c = (char *) data; ssize_t received = 0; _lock_acquire_recursive(&s_read_lock); if (s_blocking) { - while (cdcacm_data_length_in_buffer() < size) { + /* in blocking mode, wait for data. by the POSIX standards, + * the amount of available bytes does not need to match the + * requested size to return */ + while (cdcacm_data_length_in_buffer() <= 0) { xSemaphoreTake(s_rx_semaphore, portMAX_DELAY); } } else { @@ -161,21 +164,12 @@ static ssize_t cdcacm_read(int fd, void *data, size_t size) 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); - if (s_rx_mode == ESP_LINE_ENDINGS_CR) { - /* Change CRs to newlines */ - for (ssize_t i = 0; i < received; i++) { - if (data_c[i] == '\r') { - data_c[i] = '\n'; - } - } - } - } else { - while (received < size) { - int c = cdcacm_read_char(); - if (c == '\r') { + while (received < size) { + int c = cdcacm_read_char(); + if (c == '\r') { + if (s_rx_mode == ESP_LINE_ENDINGS_CR) { + c = '\n'; + } else if (s_rx_mode == ESP_LINE_ENDINGS_CRLF) { /* look ahead */ int c2 = cdcacm_read_char(); if (c2 == NONE) { @@ -192,11 +186,12 @@ static ssize_t cdcacm_read(int fd, void *data, size_t size) */ cdcacm_return_char(c2); } - } else if (c == NONE) { - break; } - data_c[received++] = (char) c; + + } else if (c == NONE) { + break; } + data_c[received++] = (char) c; } _lock_release_recursive(&s_read_lock); if (received > 0) { From 8182f20774e3a742922e353d0b9c0bb1eefdc513 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Mon, 9 Jun 2025 09:03:40 +0200 Subject: [PATCH 4/4] feat(usb_serial_jtag): Update vfs read to be POSIX compliant The function now returns with available data in blocking mode instead of waiting for the requested size to be available before returning. --- .../include/driver/usb_serial_jtag_select.h | 9 +- .../src/usb_serial_jtag.c | 19 ++++- .../src/usb_serial_jtag_vfs.c | 84 +++++++++++++------ .../test_apps/usb_serial_jtag_vfs/README.md | 4 +- .../pytest_usb_serial_jtag_vfs.py | 5 +- 5 files changed, 85 insertions(+), 36 deletions(-) diff --git a/components/esp_driver_usb_serial_jtag/include/driver/usb_serial_jtag_select.h b/components/esp_driver_usb_serial_jtag/include/driver/usb_serial_jtag_select.h index 3fb185ed3d..4c3e36d9ce 100644 --- a/components/esp_driver_usb_serial_jtag/include/driver/usb_serial_jtag_select.h +++ b/components/esp_driver_usb_serial_jtag/include/driver/usb_serial_jtag_select.h @@ -1,6 +1,6 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -27,6 +27,13 @@ typedef void (*usj_select_notif_callback_t)(usj_select_notif_t usb_serial_jtag_s */ void usb_serial_jtag_set_select_notif_callback(usj_select_notif_callback_t usb_serial_jtag_select_notif_callback); +/** + * @brief Return the number of bytes available for reading + * + * @return the number of bytes available for reading in the buffer + */ +size_t usb_serial_jtag_get_read_bytes_available(void); + /** * @brief Return the readiness status of the driver for read operation * diff --git a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag.c b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag.c index a2ae108f1a..631829de9e 100644 --- a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag.c +++ b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag.c @@ -366,12 +366,23 @@ void usb_serial_jtag_set_select_notif_callback(usj_select_notif_callback_t usj_s } } -bool usb_serial_jtag_read_ready(void) +size_t usb_serial_jtag_get_read_bytes_available(void) { // sign the the driver is read ready is that data is waiting in the RX ringbuffer - UBaseType_t items_waiting = 0; - vRingbufferGetInfo(p_usb_serial_jtag_obj->rx_ring_buf, NULL, NULL, NULL, NULL, &items_waiting); - return items_waiting != 0; + UBaseType_t bytes_available = 0; + if (usb_serial_jtag_is_driver_installed()) { + vRingbufferGetInfo(p_usb_serial_jtag_obj->rx_ring_buf, NULL, NULL, NULL, NULL, &bytes_available); + if (bytes_available <= 0) { + return 0; + } + } + + return (size_t)bytes_available; +} + +bool usb_serial_jtag_read_ready(void) +{ + return usb_serial_jtag_get_read_bytes_available() != 0; } bool usb_serial_jtag_write_ready(void) diff --git a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c index 9bbcbc3e88..90d8addacf 100644 --- a/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c +++ b/components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.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 */ @@ -229,36 +229,66 @@ static ssize_t usb_serial_jtag_read(int fd, void* data, size_t size) assert(fd == USJ_LOCAL_FD); 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.read_lock); - while (received < size) { - int c = usb_serial_jtag_read_char(fd); - if (c == '\r') { - if (s_ctx.rx_mode == ESP_LINE_ENDINGS_CR) { - c = '\n'; - } else if (s_ctx.rx_mode == ESP_LINE_ENDINGS_CRLF) { - /* look ahead */ - int c2 = usb_serial_jtag_read_char(fd); - if (c2 == NONE) { - /* could not look ahead, put the current character back */ - usb_serial_jtag_return_char(fd, c); - break; - } - if (c2 == '\n') { - /* this was \r\n sequence. discard \r, return \n */ + // if blocking read, wait for data to be available + if (!s_ctx.non_blocking) { + c = usb_serial_jtag_read_char(fd); + } + + // find the actual fetch size + available_size += usb_serial_jtag_get_read_bytes_available(); + if (c != NONE) { + available_size++; + } + if (s_ctx.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 = usb_serial_jtag_read_char(fd); + } + assert(c != NONE); + + if (c == '\r') { + if (s_ctx.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. - */ - usb_serial_jtag_return_char(fd, c2); + } else if (s_ctx.rx_mode == ESP_LINE_ENDINGS_CRLF) { + /* look ahead */ + int c2 = usb_serial_jtag_read_char(fd); + fetch_size--; + if (c2 == NONE) { + /* could not look ahead, put the current character back */ + usb_serial_jtag_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. + */ + usb_serial_jtag_return_char(fd, c2); + fetch_size++; + } } } - } else if (c == NONE) { - break; - } - data_c[received] = (char) c; - ++received; + + data_c[received] = (char) c; + ++received; + c = NONE; + } while (received < fetch_size); + } + + if (c != NONE) { // fetched, but not used + usb_serial_jtag_return_char(fd, c); } _lock_release_recursive(&s_ctx.read_lock); if (received > 0) { @@ -454,7 +484,7 @@ static esp_err_t usb_serial_jtag_start_select(int nfds, fd_set *readfds, fd_set bool trigger_select = false; // check if the select should return instantly if the bus is read ready - if (FD_ISSET(USJ_LOCAL_FD, &args->readfds_orig) && usb_serial_jtag_read_ready()) { + if (FD_ISSET(USJ_LOCAL_FD, &args->readfds_orig) && (usb_serial_jtag_get_read_bytes_available() > 0)) { // signal immediately when data is buffered FD_SET(USJ_LOCAL_FD, readfds); trigger_select = true; diff --git a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/README.md b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/README.md index fa96566bda..ad95e41133 100644 --- a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/README.md +++ b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/README.md @@ -1,2 +1,2 @@ -| Supported Targets | ESP32-C3 | ESP32-C6 | ESP32-C61 | ESP32-H2 | ESP32-P4 | ESP32-S3 | -| ----------------- | -------- | -------- | --------- | -------- | -------- | -------- | +| Supported Targets | ESP32-C3 | ESP32-C6 | ESP32-H2 | ESP32-P4 | ESP32-S3 | +| ----------------- | -------- | -------- | -------- | -------- | -------- | diff --git a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py index a66afd6b44..fad239963a 100644 --- a/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py +++ b/components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: CC0-1.0 import pytest from pytest_embedded import Dut +from pytest_embedded_idf.utils import idf_parametrize @pytest.mark.esp32s3 @@ -36,7 +37,7 @@ def test_usj_vfs_select(dut: Dut, test_message: list) -> None: ) @pytest.mark.parametrize('test_message', ['!(@*#&(!*@&#((SDasdkjhad\nce']) @idf_parametrize('target', ['esp32s3', 'esp32c3', 'esp32c6', 'esp32h2'], indirect=['target']) -def test_usj_vfs_read(dut: Dut, test_message: list) -> None: +def test_usj_vfs_read_return(dut: Dut, test_message: list) -> None: dut.expect_exact('Press ENTER to see the list of tests') dut.write('"read does not return on new line character"') dut.expect_exact('ready to receive', timeout=2) @@ -54,7 +55,7 @@ def test_usj_vfs_read(dut: Dut, test_message: list) -> None: ) @pytest.mark.parametrize('test_message', ['testdata']) @idf_parametrize('target', ['esp32s3', 'esp32c3', 'esp32c6', 'esp32h2'], indirect=['target']) -def test_usj_vfs_read(dut: Dut, test_message: list) -> None: +def test_usj_vfs_read_blocking(dut: Dut, test_message: list) -> None: dut.expect_exact('Press ENTER to see the list of tests') dut.write('"blocking read returns with available data"') dut.expect_exact('ready to receive', timeout=2)