espressif/esp-idf
1
0
Fork 1
mirror of https://github.com/espressif/esp-idf.git synced 2025-07-29 18:27:20 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'feat/remove-unecessray-condition-in-usj-read' into 'master'

Browse Source 
fix(driver): remove unecessary if conditions in the read function

Closes IDF-13166

See merge request espressif/esp-idf!39523
This commit is contained in:
Guillaume Souchere
2025-06-17 14:19:38 +02:00
parent bc34abb201 740762c997
commit 2f4c5d278e
8 changed files with 323 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
components/esp_driver_usb_serial_jtag/include/driver/usb_serial_jtag_select.h
View File

@ -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
*

19
components/esp_driver_usb_serial_jtag/src/usb_serial_jtag.c
View File

@ -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)

89
components/esp_driver_usb_serial_jtag/src/usb_serial_jtag_vfs.c
View File

@ -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
*/
@ -226,40 +226,69 @@ 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;
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;
if (c == '\n') {
break;
}
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) {
@ -455,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;

92
components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/main/test_vfs_usb_serial_jtag.c
View File

@ -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,86 @@ TEST_CASE("test select read, write and timeout", "[usb_serial_jtag]")
usb_serial_jtag_vfs_use_nonblocking();
usb_serial_jtag_driver_uninstall();
}
/* 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
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] = {};
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
}
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
}

38
components/esp_driver_usb_serial_jtag/test_apps/usb_serial_jtag_vfs/pytest_usb_serial_jtag_vfs.py
View File

@ -15,10 +15,46 @@ from pytest_embedded_idf.utils import idf_parametrize
)
@pytest.mark.parametrize('test_message', ['test123456789!@#%^&*'])
@idf_parametrize('target', ['esp32s3', 'esp32c3', 'esp32c6', 'esp32h2'], indirect=['target'])
def test_usj_vfs_release(dut: Dut, test_message: list) -> None:
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, flash_port, config',
[
pytest.param('/dev/serial_ports/ttyACM-esp32', '/dev/serial_ports/ttyUSB-esp32', 'release'),
],
indirect=True,
)
@pytest.mark.parametrize('test_message', ['!(@*#&(!*@&#((SDasdkjhad\nce'])
@idf_parametrize('target', ['esp32s3', 'esp32c3', 'esp32c6', 'esp32h2'], indirect=['target'])
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)
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_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)
dut.write(test_message)
dut.expect(r'\d{1} Tests 0 Failures 0 Ignored', timeout=10)

93
components/esp_vfs_console/test_apps/usb_cdc_vfs/main/test_app_main.c
View File

@ -129,6 +129,9 @@ static void test_usb_cdc_select(void)
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__));
@ -175,8 +178,98 @@ 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__));
// 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
}
/* 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_select();
test_usb_cdc_read_non_blocking();
test_usb_cdc_read_blocking();
test_usb_cdc_read_no_exit_on_newline_reception();
}

10
components/esp_vfs_console/test_apps/usb_cdc_vfs/pytest_usb_cdc_vfs.py
View File

@ -26,3 +26,13 @@ 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_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)
dut.write('!(@*#&(!*@&#((SDasdkjhad\nce')

39
components/esp_vfs_console/vfs_cdcacm.c
View File

@ -176,13 +176,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 {
@ -190,21 +193,13 @@ static ssize_t cdcacm_read(int fd, void *data, size_t size)
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);
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) {
@ -221,14 +216,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;
if (c == '\n') {
break;
}
} else if (c == NONE) {
break;
}
data_c[received++] = (char) c;
}
_lock_release_recursive(&s_read_lock);
if (received > 0) {