arduino / arduino/ArduinoCore-sam

UARTClass::write May lose characters if write occurs around the same time as a previous byte is finished transmitted

Open
#64 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
HTML
Stars
91
Forks
112
PR merge metrics
No merged PRs in 30d

Description

In this example some of the 'C' and '\n' characters is lost!
```
void setup()
{
Serial.begin(115200);
}
void loop()
{
while (true)
{
Serial.write('A');
Serial.write('B');
delayMicroseconds(random(150));

Serial.write('C');
Serial.write('\n');
delayMicroseconds(1000);
}
}
```

This is due to the fact that interrupts could cause the following evaluation UARTClass:write to wrongly be evaluated to false:
if (((_pUart->UART_SR & UART_SR_TXRDY) != UART_SR_TXRDY) |
(_tx_buffer->_iTail != _tx_buffer->_iHead))
{...

1. The TXRDY could be set and read before the IrqHandler is executed ==> first part of if is false.
2. IrqHandler is executed and moves the last byte in tx_buffer into serial hardware register.
3. Second if part "(_tx_buffer->_iTail != _tx_buffer->_iHead))" will evaluate to false since buffer is now empty.
4. The new byte is wrongly placed into serial hardware register and is lost!
5. Note: Strange that bit wise OR "|" is used instead of logical OR "||" but this has no effect in this case

Changes made in UARTClass::write to rectify the problem:
```
size_t UARTClass::write( const uint8_t uc_data )
{
// Using static since it is faster, no stack operation
static bool put_in_buffer;

// Make sure that UART_SR_TXRDY and _tx_buffer is not changed by IrqHandler during evaluation
__ASM volatile ("cpsid i");
put_in_buffer = (((_pUart->UART_SR & UART_SR_TXRDY) != UART_SR_TXRDY) | (_tx_buffer->_iTail != _tx_buffer->_iHead));
__ASM volatile ("cpsie i");

// Is the hardware currently busy?
if (put_in_buffer)
{
// If busy we buffer
int nextWrite = (_tx_buffer->_iHead + 1) % SERIAL_BUFFER_SIZE;
while (_tx_buffer->_iTail == nextWrite)
; // Spin locks if we're about to overwrite the buffer. This continues once the data is sent

_tx_buffer->_aucBuffer[_tx_buffer->_iHead] = uc_data;
_tx_buffer->_iHead = nextWrite;
// Make sure TX interrupt is enabled
_pUart->UART_IER = UART_IER_TXRDY;
}
else
{
// Bypass buffering and send character directly
_pUart->UART_THR = uc_data;
}
return 1;
}

```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at UARTClass::write and inspect how IrqHandler moves bytes from _tx_buffer to the UART hardware register. Reproduce the provided sketch with the UART timing delays, then verify that the C and newline characters are no longer lost when transmission completion overlaps a write.

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
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.