Fix HardwareSerial::flush() when interrupts are kept disabled for a while
It turns out there is an additional corner case. The analysis in the previous commit wrt to flush() assumes that the data register is always kept filled by the interrupt handler, so the TXC bit won't get set until all the queued bytes have been transmitted. But, when interrupts are disabled for a longer period (for example when an interrupt handler for another device is running for longer than 1-2 byte times), it could happen that the UART stops transmitting while there are still more bytes queued (but these are in the buffer, not in the UDR register, so the UART can't know about them). In this case, the TXC bit would get set, but the transmission is not complete yet. We can easily detect this case by looking at the head and tail pointers, but it seems easier to instead look at the UDRIE bit (the TX interrupt is enabled if and only if there are bytes in the queue). To fix this corner case, this commit: - Checks the UDRIE bit and only if it is unset, looks at the TXC bit. - Moves the clearing of TXC from write() to the tx interrupt handler. This (still) causes the TXC bit to be cleared whenever a byte is queued when the buffer is empty (in this case the tx interrupt will trigger directly after write() is called). It also causes the TXC bit to be cleared whenever transmission is resumed after it halted because interrupts have been disabled for too long. As a side effect, another race condition is prevented. This could occur at very high bitrates, where the transmission would be completed before the code got time to clear the TXC0 register, making the clear happen _after_ the transmission was already complete. With the new code, the clearing of TXC happens directly after writing to the UDR register, while interrupts are disabled, and we can be certain the data transmission needs more time than one instruction to complete. This fixes #1463 and replaces #1456.
This commit is contained in:
parent
560295c983
commit
fa8df58c93
|
@ -233,6 +233,11 @@ void HardwareSerial::_tx_udr_empty_irq(void)
|
|||
_tx_buffer_tail = (_tx_buffer_tail + 1) % SERIAL_BUFFER_SIZE;
|
||||
|
||||
*_udr = c;
|
||||
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -339,9 +344,9 @@ void HardwareSerial::flush()
|
|||
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));
|
||||
while (bit_is_set(*_ucsrb, UDRIE0) || bit_is_clear(*_ucsra, TXC0));
|
||||
// If we get here, nothing is queued anymore (DRIE is disabled) and
|
||||
// the hardware finished tranmission (TXC is set).
|
||||
}
|
||||
|
||||
size_t HardwareSerial::write(uint8_t c)
|
||||
|
@ -359,10 +364,6 @@ size_t HardwareSerial::write(uint8_t c)
|
|||
|
||||
sbi(*_ucsrb, UDRIE0);
|
||||
_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;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue