From 560295c983d6def24021ac94c40d7bbc8912c1ea Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 18 Apr 2013 21:12:01 +0200 Subject: Improve HardwareSerial::flush() The flush() method blocks until all characters in the serial buffer have been written to the uart _and_ transmitted. This is checked by waiting until the "TXC" (TX Complete) bit is set by the UART, signalling completion. This bit is cleared by write() when adding a new byte to the buffer and set by the hardware after tranmission ends, so it is always guaranteed to be zero from the moment the first byte in a sequence is queued until the moment the last byte is transmitted, and it is one from the moment the last byte in the buffer is transmitted until the first byte in the next sequence is queued. However, the TXC bit is also zero from initialization to the moment the first byte ever is queued (and then continues to be zero until the first sequence of bytes completes transmission). Unfortunately we cannot manually set the TXC bit during initialization, we can only clear it. To make sure that flush() would not (indefinitely) block when it is called _before_ anything was written to the serial device, the "transmitting" variable was introduced. This variable suggests that it is only true when something is transmitting, which isn't currently the case (it remains true after transmission is complete until flush() is called, for example). Furthermore, there is no need to keep the status of transmission, the only thing needed is to remember if anything has ever been written, so the corner case described above can be detected. This commit improves the code by: - Renaming the "transmitting" variable to _written (making it more clear and following the leading underscore naming convention). - Not resetting the value of _written at the end of flush(), there is no point to this. - Only checking the "_written" value once in flush(), since it can never be toggled off anyway. - Initializing the value of _written in both versions of _begin (though it probably gets initialized to 0 by default anyway, better to be explicit). --- cores/arduino/HardwareSerial.cpp | 20 +++++++++++++++----- cores/arduino/HardwareSerial.h | 3 ++- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/cores/arduino/HardwareSerial.cpp b/cores/arduino/HardwareSerial.cpp index 806fe5f..d78c205 100644 --- a/cores/arduino/HardwareSerial.cpp +++ b/cores/arduino/HardwareSerial.cpp @@ -276,6 +276,8 @@ void HardwareSerial::begin(unsigned long baud, byte config) *_ubrrh = baud_setting >> 8; *_ubrrl = baud_setting; + _written = false; + //set the data bits, parity, and stop bits #if defined(__AVR_ATmega8__) config |= 0x80; // select UCSRC register (shared with UBRRH) @@ -331,9 +333,15 @@ int HardwareSerial::read(void) void HardwareSerial::flush() { - // UDR is kept full while the buffer is not empty, so TXC triggers when EMPTY && SENT - while (transmitting && bit_is_clear(*_ucsra, TXC0)); - transmitting = false; + // If we have never written a byte, no need to flush. This special + // case is needed since there is no way to force the TXC (transmit + // complete) bit to 1 during initialization + if (!_written) + return; + + // UDR is kept full while the buffer is not empty, so TXC triggers + // when EMPTY && SENT + while (bit_is_clear(*_ucsra, TXC0)); } size_t HardwareSerial::write(uint8_t c) @@ -350,8 +358,10 @@ size_t HardwareSerial::write(uint8_t c) _tx_buffer_head = i; sbi(*_ucsrb, UDRIE0); - // clear the TXC bit -- "can be cleared by writing a one to its bit location" - transmitting = true; + _written = true; + // clear the TXC bit -- "can be cleared by writing a one to its bit + // location". This makes sure flush() won't return until the bytes + // actually got written sbi(*_ucsra, TXC0); return 1; diff --git a/cores/arduino/HardwareSerial.h b/cores/arduino/HardwareSerial.h index cf3f6bd..0ae51f4 100644 --- a/cores/arduino/HardwareSerial.h +++ b/cores/arduino/HardwareSerial.h @@ -72,7 +72,8 @@ class HardwareSerial : public Stream volatile uint8_t *_ucsrb; volatile uint8_t *_ucsrc; volatile uint8_t *_udr; - bool transmitting; + // Has any byte been written to the UART since begin() + bool _written; volatile uint8_t _rx_buffer_head; volatile uint8_t _rx_buffer_tail; -- cgit v1.2.3-18-g5258