vfs_uart: fix write operation blocked by a read
vfs_uart used same locks for read and write operations on a single UART. If read operation was blocking (i.e. carried out via UART driver), the lock was held by reading task until it received a line. During this time, other tasks trying to write to the same UART would get blocked. This change introduces separate read/write locks, and adds a test. Another vfs_uart test if fixed (it was disabled since the CONFIG_NEWLIB_STDOUT_ADDCR option was removed).
This commit is contained in:
parent
3161854efb
commit
13ef3938a6
2 changed files with 90 additions and 12 deletions
|
@ -20,6 +20,9 @@
|
||||||
#include "soc/uart_struct.h"
|
#include "soc/uart_struct.h"
|
||||||
#include "freertos/FreeRTOS.h"
|
#include "freertos/FreeRTOS.h"
|
||||||
#include "freertos/task.h"
|
#include "freertos/task.h"
|
||||||
|
#include "freertos/semphr.h"
|
||||||
|
#include "driver/uart.h"
|
||||||
|
#include "esp_vfs_dev.h"
|
||||||
#include "sdkconfig.h"
|
#include "sdkconfig.h"
|
||||||
|
|
||||||
static void fwrite_str_loopback(const char* str, size_t size)
|
static void fwrite_str_loopback(const char* str, size_t size)
|
||||||
|
@ -68,10 +71,11 @@ TEST_CASE("can read from stdin", "[vfs]")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
#if CONFIG_NEWLIB_STDOUT_ADDCR
|
|
||||||
|
|
||||||
TEST_CASE("CRs are removed from the stdin correctly", "[vfs]")
|
TEST_CASE("CRs are removed from the stdin correctly", "[vfs]")
|
||||||
{
|
{
|
||||||
|
esp_vfs_dev_uart_set_rx_line_endings(ESP_LINE_ENDINGS_CRLF);
|
||||||
|
esp_vfs_dev_uart_set_tx_line_endings(ESP_LINE_ENDINGS_CRLF);
|
||||||
|
|
||||||
flush_stdin_stdout();
|
flush_stdin_stdout();
|
||||||
const char* send_str = "1234567890\n\r123\r\n4\n";
|
const char* send_str = "1234567890\n\r123\r\n4\n";
|
||||||
/* with CONFIG_NEWLIB_STDOUT_ADDCR, the following will be sent on the wire.
|
/* with CONFIG_NEWLIB_STDOUT_ADDCR, the following will be sent on the wire.
|
||||||
|
@ -123,5 +127,74 @@ TEST_CASE("CRs are removed from the stdin correctly", "[vfs]")
|
||||||
TEST_ASSERT_EQUAL_UINT8_ARRAY("4\n", dst, 2);
|
TEST_ASSERT_EQUAL_UINT8_ARRAY("4\n", dst, 2);
|
||||||
}
|
}
|
||||||
|
|
||||||
#endif //CONFIG_NEWLIB_STDOUT_ADDCR
|
TEST_CASE("can write to UART while another task is reading", "[vfs]")
|
||||||
|
{
|
||||||
|
struct read_task_arg_t {
|
||||||
|
char* out_buffer;
|
||||||
|
size_t out_buffer_len;
|
||||||
|
SemaphoreHandle_t ready;
|
||||||
|
SemaphoreHandle_t done;
|
||||||
|
};
|
||||||
|
|
||||||
|
struct write_task_arg_t {
|
||||||
|
const char* str;
|
||||||
|
SemaphoreHandle_t done;
|
||||||
|
};
|
||||||
|
|
||||||
|
void read_task_fn(void* varg)
|
||||||
|
{
|
||||||
|
struct read_task_arg_t* parg = (struct read_task_arg_t*) varg;
|
||||||
|
parg->out_buffer[0] = 0;
|
||||||
|
|
||||||
|
fgets(parg->out_buffer, parg->out_buffer_len, stdin);
|
||||||
|
xSemaphoreGive(parg->done);
|
||||||
|
vTaskDelete(NULL);
|
||||||
|
}
|
||||||
|
|
||||||
|
void write_task_fn(void* varg)
|
||||||
|
{
|
||||||
|
struct write_task_arg_t* parg = (struct write_task_arg_t*) varg;
|
||||||
|
fwrite_str_loopback(parg->str, strlen(parg->str));
|
||||||
|
xSemaphoreGive(parg->done);
|
||||||
|
vTaskDelete(NULL);
|
||||||
|
}
|
||||||
|
|
||||||
|
char out_buffer[32];
|
||||||
|
size_t out_buffer_len = sizeof(out_buffer);
|
||||||
|
|
||||||
|
struct read_task_arg_t read_arg = {
|
||||||
|
.out_buffer = out_buffer,
|
||||||
|
.out_buffer_len = out_buffer_len,
|
||||||
|
.done = xSemaphoreCreateBinary()
|
||||||
|
};
|
||||||
|
|
||||||
|
struct write_task_arg_t write_arg = {
|
||||||
|
.str = "!(@*#&(!*@&#((SDasdkjhadsl\n",
|
||||||
|
.done = xSemaphoreCreateBinary()
|
||||||
|
};
|
||||||
|
|
||||||
|
flush_stdin_stdout();
|
||||||
|
|
||||||
|
ESP_ERROR_CHECK( uart_driver_install(CONFIG_CONSOLE_UART_NUM,
|
||||||
|
256, 0, 0, NULL, 0) );
|
||||||
|
esp_vfs_dev_uart_use_driver(CONFIG_CONSOLE_UART_NUM);
|
||||||
|
|
||||||
|
|
||||||
|
xTaskCreate(&read_task_fn, "vfs_read", 4096, &read_arg, 5, NULL);
|
||||||
|
vTaskDelay(10);
|
||||||
|
xTaskCreate(&write_task_fn, "vfs_write", 4096, &write_arg, 6, NULL);
|
||||||
|
|
||||||
|
|
||||||
|
int res = xSemaphoreTake(write_arg.done, 100 / portTICK_PERIOD_MS);
|
||||||
|
TEST_ASSERT(res);
|
||||||
|
|
||||||
|
res = xSemaphoreTake(read_arg.done, 100 / portTICK_PERIOD_MS);
|
||||||
|
TEST_ASSERT(res);
|
||||||
|
|
||||||
|
TEST_ASSERT_EQUAL(0, strcmp(write_arg.str, read_arg.out_buffer));
|
||||||
|
|
||||||
|
esp_vfs_dev_uart_use_nonblocking(CONFIG_CONSOLE_UART_NUM);
|
||||||
|
uart_driver_delete(CONFIG_CONSOLE_UART_NUM);
|
||||||
|
vSemaphoreDelete(read_arg.done);
|
||||||
|
vSemaphoreDelete(write_arg.done);
|
||||||
|
}
|
||||||
|
|
|
@ -47,7 +47,8 @@ static int uart_rx_char_via_driver(int fd);
|
||||||
// Pointers to UART peripherals
|
// Pointers to UART peripherals
|
||||||
static uart_dev_t* s_uarts[UART_NUM] = {&UART0, &UART1, &UART2};
|
static uart_dev_t* s_uarts[UART_NUM] = {&UART0, &UART1, &UART2};
|
||||||
// per-UART locks, lazily initialized
|
// per-UART locks, lazily initialized
|
||||||
static _lock_t s_uart_locks[UART_NUM];
|
static _lock_t s_uart_read_locks[UART_NUM];
|
||||||
|
static _lock_t s_uart_write_locks[UART_NUM];
|
||||||
// One-character buffer used for newline conversion code, per UART
|
// One-character buffer used for newline conversion code, per UART
|
||||||
static int s_peek_char[UART_NUM] = { NONE, NONE, NONE };
|
static int s_peek_char[UART_NUM] = { NONE, NONE, NONE };
|
||||||
// Per-UART non-blocking flag. Note: default implementation does not honor this
|
// Per-UART non-blocking flag. Note: default implementation does not honor this
|
||||||
|
@ -144,7 +145,7 @@ static ssize_t uart_write(int fd, const void * data, size_t size)
|
||||||
* a dedicated UART lock if two streams (stdout and stderr) point to the
|
* a dedicated UART lock if two streams (stdout and stderr) point to the
|
||||||
* same UART.
|
* same UART.
|
||||||
*/
|
*/
|
||||||
_lock_acquire_recursive(&s_uart_locks[fd]);
|
_lock_acquire_recursive(&s_uart_write_locks[fd]);
|
||||||
for (size_t i = 0; i < size; i++) {
|
for (size_t i = 0; i < size; i++) {
|
||||||
int c = data_c[i];
|
int c = data_c[i];
|
||||||
if (c == '\n' && s_tx_mode != ESP_LINE_ENDINGS_LF) {
|
if (c == '\n' && s_tx_mode != ESP_LINE_ENDINGS_LF) {
|
||||||
|
@ -155,7 +156,7 @@ static ssize_t uart_write(int fd, const void * data, size_t size)
|
||||||
}
|
}
|
||||||
s_uart_tx_func[fd](fd, c);
|
s_uart_tx_func[fd](fd, c);
|
||||||
}
|
}
|
||||||
_lock_release_recursive(&s_uart_locks[fd]);
|
_lock_release_recursive(&s_uart_write_locks[fd]);
|
||||||
return size;
|
return size;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -186,7 +187,7 @@ static ssize_t uart_read(int fd, void* data, size_t size)
|
||||||
assert(fd >=0 && fd < 3);
|
assert(fd >=0 && fd < 3);
|
||||||
char *data_c = (char *) data;
|
char *data_c = (char *) data;
|
||||||
size_t received = 0;
|
size_t received = 0;
|
||||||
_lock_acquire_recursive(&s_uart_locks[fd]);
|
_lock_acquire_recursive(&s_uart_read_locks[fd]);
|
||||||
while (received < size) {
|
while (received < size) {
|
||||||
int c = uart_read_char(fd);
|
int c = uart_read_char(fd);
|
||||||
if (c == '\r') {
|
if (c == '\r') {
|
||||||
|
@ -219,7 +220,7 @@ static ssize_t uart_read(int fd, void* data, size_t size)
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
_lock_release_recursive(&s_uart_locks[fd]);
|
_lock_release_recursive(&s_uart_read_locks[fd]);
|
||||||
if (received > 0) {
|
if (received > 0) {
|
||||||
return received;
|
return received;
|
||||||
}
|
}
|
||||||
|
@ -286,16 +287,20 @@ void esp_vfs_dev_uart_set_tx_line_endings(esp_line_endings_t mode)
|
||||||
|
|
||||||
void esp_vfs_dev_uart_use_nonblocking(int fd)
|
void esp_vfs_dev_uart_use_nonblocking(int fd)
|
||||||
{
|
{
|
||||||
_lock_acquire_recursive(&s_uart_locks[fd]);
|
_lock_acquire_recursive(&s_uart_read_locks[fd]);
|
||||||
|
_lock_acquire_recursive(&s_uart_write_locks[fd]);
|
||||||
s_uart_tx_func[fd] = uart_tx_char;
|
s_uart_tx_func[fd] = uart_tx_char;
|
||||||
s_uart_rx_func[fd] = uart_rx_char;
|
s_uart_rx_func[fd] = uart_rx_char;
|
||||||
_lock_release_recursive(&s_uart_locks[fd]);
|
_lock_release_recursive(&s_uart_write_locks[fd]);
|
||||||
|
_lock_release_recursive(&s_uart_read_locks[fd]);
|
||||||
}
|
}
|
||||||
|
|
||||||
void esp_vfs_dev_uart_use_driver(int fd)
|
void esp_vfs_dev_uart_use_driver(int fd)
|
||||||
{
|
{
|
||||||
_lock_acquire_recursive(&s_uart_locks[fd]);
|
_lock_acquire_recursive(&s_uart_read_locks[fd]);
|
||||||
|
_lock_acquire_recursive(&s_uart_write_locks[fd]);
|
||||||
s_uart_tx_func[fd] = uart_tx_char_via_driver;
|
s_uart_tx_func[fd] = uart_tx_char_via_driver;
|
||||||
s_uart_rx_func[fd] = uart_rx_char_via_driver;
|
s_uart_rx_func[fd] = uart_rx_char_via_driver;
|
||||||
_lock_release_recursive(&s_uart_locks[fd]);
|
_lock_release_recursive(&s_uart_write_locks[fd]);
|
||||||
|
_lock_release_recursive(&s_uart_read_locks[fd]);
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue