From 8182f20774e3a742922e353d0b9c0bb1eefdc513 Mon Sep 17 00:00:00 2001 From: Guillaume Souchere Date: Mon, 9 Jun 2025 09:03:40 +0200 Subject: [PATCH] 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)