fix(esp_vfs_console): USB CDC read when non blocking

In non blocking mode, the read function is expected
to return weather data is available for reading or not.

In case data are available but the size does not match
the expected size, the function read should return whatever
data is available.

Previously, the function was returning -1 with errno set
to EWOULDBLOCK even if the size of data in the buffer was
less than the requested size. It would only return the
available data if the size in the buffer was greater or equal
to the requested size.

The implementation of cdcacm_read is modified to return the avilable
data from the buffer even is the size is lesser than the requested
size.
This commit is contained in:
Guillaume Souchere 2025-01-17 07:45:12 +01:00
parent 6ac45b2559
commit ae7c8691d9
4 changed files with 122 additions and 18 deletions

View File

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

View File

@ -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();
}

View File

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

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
*/
@ -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) {