Skip to content

Commit ae7c869

Browse files
committed
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.
1 parent 6ac45b2 commit ae7c869

File tree

4 files changed

+122
-18
lines changed

4 files changed

+122
-18
lines changed

components/esp_system/include/esp_private/usb_console.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11

22
/*
3-
* SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD
3+
* SPDX-FileCopyrightText: 2019-2025 Espressif Systems (Shanghai) CO LTD
44
*
55
* SPDX-License-Identifier: Apache-2.0
66
*/
@@ -101,6 +101,14 @@ esp_err_t esp_usb_console_set_cb(esp_usb_console_cb_t rx_cb, esp_usb_console_cb_
101101
*/
102102
bool esp_usb_console_is_installed(void);
103103

104+
/**
105+
* @brief Call the USB interrupt handler while any interrupts
106+
* are pending, but not more than a few times. Non-static to
107+
* allow placement into IRAM by ldgen.
108+
*
109+
*/
110+
void esp_usb_console_poll_interrupts(void); // [refactor-todo] Remove when implementing IDF-12175
111+
104112
#ifdef __cplusplus
105113
}
106114
#endif

components/esp_vfs_console/test_apps/usb_cdc_vfs/main/test_app_main.c

Lines changed: 94 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
2+
* SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD
33
*
44
* SPDX-License-Identifier: Unlicense OR CC0-1.0
55
*/
@@ -11,23 +11,36 @@
1111
#include "esp_private/usb_console.h"
1212
#include "esp_vfs_cdcacm.h"
1313

14-
static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struct timeval tv)
14+
static void flush_write(void)
15+
{
16+
fflush(stdout);
17+
fsync(fileno(stdout));
18+
}
19+
20+
static void test_setup(const char* func_name, const size_t func_name_size)
21+
{
22+
/* write the name of the test back to the pytest environment to start the test */
23+
write(fileno(stdout), func_name, func_name_size);
24+
write(fileno(stdout), "\n", 1);
25+
flush_write();
26+
}
27+
28+
static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struct timeval* tv)
1529
{
1630
int fd = fileno(stream);
1731
fd_set read_fds;
1832
FD_ZERO(&read_fds);
1933
FD_SET(fd, &read_fds);
20-
21-
/* call select with to wait for either a read ready or timeout to happen */
22-
int nread = select(fd + 1, &read_fds, NULL, NULL, &tv);
34+
/* call select to wait for either a read ready or an except to happen */
35+
int nread = select(fd + 1, &read_fds, NULL, NULL, tv);
2336
if (nread < 0) {
2437
return -1;
2538
} else if (FD_ISSET(fd, &read_fds)) {
2639
int read_count = 0;
2740
int total_read = 0;
2841

2942
do {
30-
read_count = read(fileno(stream), buf + total_read, 1);
43+
read_count = read(fd, buf + total_read, buf_len - total_read);
3144
if (read_count < 0 && errno != EWOULDBLOCK) {
3245
return -1;
3346
} else if (read_count > 0) {
@@ -41,12 +54,31 @@ static int read_bytes_with_select(FILE *stream, void *buf, size_t buf_len, struc
4154

4255
return total_read;
4356
} else {
57+
/* select timed out */
4458
return -2;
4559
}
60+
return nread;
4661
}
4762

48-
void app_main(void)
63+
static bool wait_for_read_ready(FILE *stream)
64+
{
65+
int fd = fileno(stream);
66+
fd_set read_fds;
67+
FD_ZERO(&read_fds);
68+
FD_SET(fd, &read_fds);
69+
/* call select to wait for either a read ready or an except to happen */
70+
int nread = select(fd + 1, &read_fds, NULL, NULL, NULL);
71+
if ((nread >= 0) && (FD_ISSET(fd, &read_fds))) {
72+
return true;
73+
} else {
74+
return false;
75+
}
76+
}
77+
78+
static void test_usb_cdc_select(void)
4979
{
80+
test_setup(__func__, sizeof(__func__));
81+
5082
struct timeval tv;
5183
tv.tv_sec = 1;
5284
tv.tv_usec = 0;
@@ -70,7 +102,7 @@ void app_main(void)
70102
bool message_received = false;
71103
size_t char_read = 0;
72104
while (!message_received && out_buffer_len > char_read) {
73-
int nread = read_bytes_with_select(stdin, out_buffer + char_read, out_buffer_len - char_read, tv);
105+
int nread = read_bytes_with_select(stdin, out_buffer + char_read, out_buffer_len - char_read, &tv);
74106
if (nread > 0) {
75107
char_read += nread;
76108
if (out_buffer[char_read - 1] == '\n') {
@@ -83,6 +115,7 @@ void app_main(void)
83115
// function since the string is expected as is by the test environment
84116
char timeout_msg[] = "select timed out\n";
85117
write(fileno(stdout), timeout_msg, sizeof(timeout_msg));
118+
flush_write();
86119
} else {
87120
break;
88121
}
@@ -91,6 +124,59 @@ void app_main(void)
91124
// write the received message back to the test environment. The test
92125
// environment will check that the message received matches the one sent
93126
write(fileno(stdout), out_buffer, char_read);
127+
flush_write();
94128

95129
vTaskDelay(10); // wait for the string to send
96130
}
131+
132+
static void test_usb_cdc_read_non_blocking(void)
133+
{
134+
test_setup(__func__, sizeof(__func__));
135+
136+
const size_t max_read_bytes = 16;
137+
char read_data_buffer[max_read_bytes];
138+
139+
memset(read_data_buffer, 0x00, max_read_bytes);
140+
141+
/* reset errno to 0 for the purpose of the test to make sure it is no
142+
* longer set to EWOULDBLOCK later in the test. */
143+
errno = 0;
144+
145+
/* make sure to read in non blocking mode */
146+
int stdin_fd = fileno(stdin);
147+
int flags = fcntl(stdin_fd, F_GETFL);
148+
flags |= O_NONBLOCK;
149+
int res = fcntl(stdin_fd, F_SETFL, flags);
150+
TEST_ASSERT(res == 0);
151+
152+
/* call read when no data is available for reading.
153+
* expected result:
154+
* - read returns -1 and errno has been set to EWOULDBLOCK */
155+
int nread = read(stdin_fd, read_data_buffer, max_read_bytes);
156+
TEST_ASSERT(nread == -1);
157+
TEST_ASSERT(errno == EWOULDBLOCK);
158+
159+
/* reset errno to 0 for the purpose of the test to make sure it is no
160+
* longer set to EWOULDBLOCK later in the test. */
161+
errno = 0;
162+
163+
/* call read X number of bytes when less than X number of
164+
* bytes are available for reading (Y bytes).
165+
* expected result:
166+
* - read returns at least 1 bytes errno is not set to EWOULDBLOCK */
167+
char message_8[] = "send_bytes\n";
168+
write(fileno(stdout), message_8, sizeof(message_8));
169+
flush_write();
170+
171+
const bool is_ready = wait_for_read_ready(stdin);
172+
TEST_ASSERT(is_ready);
173+
nread = read(stdin_fd, read_data_buffer, max_read_bytes);
174+
TEST_ASSERT(nread >= 1);
175+
TEST_ASSERT(errno != EWOULDBLOCK);
176+
}
177+
178+
void app_main(void)
179+
{
180+
test_usb_cdc_select();
181+
test_usb_cdc_read_non_blocking();
182+
}

components/esp_vfs_console/test_apps/usb_cdc_vfs/pytest_usb_cdc_vfs.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
1+
# SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
22
# SPDX-License-Identifier: CC0-1.0
33
import pytest
44
from pytest_embedded import Dut
@@ -14,6 +14,13 @@
1414
indirect=True,)
1515
@pytest.mark.parametrize('test_message', ['test123456789!@#%^&*'])
1616
def test_usb_cdc_vfs_default(dut: Dut, test_message: str) -> None:
17+
# test run: test_usb_cdc_select
18+
dut.expect_exact('test_usb_cdc_select', timeout=2)
1719
dut.expect_exact('select timed out', timeout=2)
1820
dut.write(test_message)
1921
dut.expect_exact(test_message, timeout=2)
22+
23+
# test run: test_usb_cdc_read_non_blocking
24+
dut.expect_exact('test_usb_cdc_read_non_blocking', timeout=2)
25+
dut.expect_exact('send_bytes', timeout=2)
26+
dut.write('abcdefgh')

components/esp_vfs_console/vfs_cdcacm.c

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
2+
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
33
*
44
* SPDX-License-Identifier: Apache-2.0
55
*/
@@ -181,15 +181,15 @@ static ssize_t cdcacm_read(int fd, void *data, size_t size)
181181
ssize_t received = 0;
182182
_lock_acquire_recursive(&s_read_lock);
183183

184-
while (cdcacm_data_length_in_buffer() < size) {
185-
if (!s_blocking) {
186-
errno = EWOULDBLOCK;
187-
_lock_release_recursive(&s_read_lock);
188-
return -1;
184+
if (s_blocking) {
185+
while (cdcacm_data_length_in_buffer() < size) {
186+
xSemaphoreTake(s_rx_semaphore, portMAX_DELAY);
189187
}
190-
xSemaphoreTake(s_rx_semaphore, portMAX_DELAY);
188+
} else {
189+
/* process pending interrupts before requesting available data */
190+
esp_usb_console_poll_interrupts();
191+
size = MIN(size, cdcacm_data_length_in_buffer());
191192
}
192-
193193
if (s_rx_mode == ESP_LINE_ENDINGS_CR || s_rx_mode == ESP_LINE_ENDINGS_LF) {
194194
/* This is easy. Just receive, and if needed replace \r by \n. */
195195
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
443443
return ret;
444444
}
445445

446+
/* make sure the driver processes any pending interrupts before checking read or write
447+
* readiness */
448+
esp_usb_console_poll_interrupts();
446449
bool trigger_select = false;
447450
if (FD_ISSET(USB_CDC_LOCAL_FD, &args->readfds_orig) &&
448451
esp_usb_console_available_for_read() > 0) {

0 commit comments

Comments
 (0)