Race condition in UARTClass.cpp

[luke-lombardi]

Recently, I was working on a serial protocol running on an Arduino Due (using the Serial1 interface). I was noticing fairly infrequent data loss at 460800 baud. Periodically, it seems the second byte of an outgoing buffer would be dropped, while the rest of the buffer would be unchanged - for example:

61522123414411111111512461115112611171173250
->
6522123414411111111512461115112611171173250

The 1 in the second place would be dropped. I thought this was the way we were doing packet encoding, but I believe it is actually a race condition in UARTClass: https://github.com/arduino/ArduinoCore-sam/blob/master/cores/arduino/UARTClass.cpp#L148

The write function first checks if the hardware is busy, and if so, it buffers the outgoing byte. If not, it directly writes the character and bypasses the ringbuffer: https://github.com/arduino/ArduinoCore-sam/blob/master/cores/arduino/UARTClass.cpp#L164

If write is called and the hardware is busy, the byte is buffered in the TX ringbuffer. If write is called again and the hardware is not busy, and the UART interrupt was not triggered before that, I believe you would send the characters out of sequence, or overwrite the byte about to be sent from the ringbuffer. I think this is my issue. To fix it, I modified it to always buffer every write. Is there a performance concern with doing this / am I misunderstanding the way the driver is working?

Here's my modified write function:

size_t UARTClass::write( const uint8_t uc_data )
{
  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;

  return 1;
}

Thanks!