arduino / arduino/ArduinoCore-renesas

UART Class write Method is Blocking and Makes no Use of the SafeRingBuffer

Open
#307 1 comment 0 reactions 0 assignees Claimed by @delta-G View on GitHub
topic: code type: enhancement
Dominant language
C
Stars
193
Forks
112
PR merge metrics
No merged PRs in 30d

Description

The UART class write method currently blocks waiting on a flag that will not be set until the transmission is complete. For low baud rates this can be a long time. There is currently a SafeRingBuffer instance allocated in the class, but no attempt has been made to make use of it. The code as written now submits a pointer to the HAL and then hangs until the transmission ends.

```
/* -------------------------------------------------------------------------- */
size_t UART::write(uint8_t c) {
/* -------------------------------------------------------------------------- */
if(init_ok) {
tx_done = false;
R_SCI_UART_Write(&uart_ctrl, &c, 1);
while (!tx_done) {}
return 1;
}
else {
return 0;
}
}

size_t UART::write(uint8_t* c, size_t len) {
if(init_ok) {
tx_done = false;
R_SCI_UART_Write(&uart_ctrl, c, len);
while (!tx_done) {}
return len;
}
else {
return 0;
}
}
```

The flag `tx_done` is only set in the interrupt handler after transmission ends.
```
/* -------------------------------------------------------------------------- */
void UART::WrapperCallback(uart_callback_args_t *p_args) {
/* -------------------------------------------------------------------------- */

uint32_t channel = p_args->channel;

UART *uart_ptr = UART::g_uarts[channel];

if(uart_ptr == nullptr) {
return;
}

switch (p_args->event){
case UART_EVENT_ERR_PARITY:
case UART_EVENT_ERR_FRAMING:
case UART_EVENT_ERR_OVERFLOW:
case UART_EVENT_RX_COMPLETE: // This is called when all the "expected" data are received
{
break;
}
case UART_EVENT_TX_COMPLETE:
case UART_EVENT_TX_DATA_EMPTY:
{
//uint8_t to_enqueue = uart_ptr->txBuffer.available() < uart_ptr->uart_ctrl.fifo_depth ? uart_ptr->txBuffer.available() : uart_ptr->uart_ctrl.fifo_depth;
//while (to_enqueue) {
uart_ptr->tx_done = true;
break;
}
case UART_EVENT_RX_CHAR:
{
if (uart_ptr->rxBuffer.availableForStore()) {
uart_ptr->rxBuffer.store_char(p_args->data);
}
break;
}
case UART_EVENT_BREAK_DETECT:
{
break;
}
}

}
```

Additionally, by not making use of the buffer, a pointer to the users data is passed into the HAL. This means that any changes made to the data behind that pointer while it's still being sent will corrupt the output. Using the txBuffer will solve that problem as well.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by locating the UART::write overloads and WrapperCallback shown in the issue, then inspect the existing SafeRingBuffer and txBuffer members. Trace the UART transmit callbacks and buffer availability logic. Done means writes no longer block on transmission, user data is not corrupted while being sent, and the relevant UART behavior is covered by tests or hardware validation.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
embedded-iot
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.