arduino/ArduinoCore-sam

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

magnus-hylten opened this issue · 0 comments

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;
}