Avoid data loss with serial interrupt used at high baudrates #4734
Conversation
LMESTM
changed the title
LMESTM
changed the title
There was a problem hiding this comment.
Seems correct to me. Only thing that makes me nervous is the question "what if the IRQ handler doesn't transfer data"?
Previously (at least on this platform) they could have gotten away with just not reading - it would have meant the data being thrown away. Or they could have gotten away with not writing - the transmit empty interrupt would clear and not retrigger. The UARTSerial IRQ handlers take great care to try to clear the interrupt condition, but other users might not.
So could break some ST-specific applications, but any portable applications should be correctly clearing already.
|
I agree that possible application that were not enabling / disabling interrupts on a need basis may encounter issues and will need update, not sure how to avoid this risk :-( |
|
/morph test |
Result: FAILUREYour command has finished executing! Here's what you wrote!
|
|
@LMESTM Can you please resolve the conflicts with a rebase? |
The RXNE flag is getting cleared when reading Data Register so it should not be cleared here. Especially in case of high data rate, another byte of data could have received during irq_handler call and clearing the flag would read and discard this data which would be lost for application.
TXE indicates that a byte can be written to UART register for sending, while TC indicates that last byte was completely sent. So the TXE flag can be used in case of interrupt based Serial communication, to allow faster and efficient application buffer emptying. Also TXE flag will be erased from the interrupt when writing to register. In case there is nothing to write in the register, the application is expected to disable the interrupt.
|
@0xc0170 - rebased |
|
/morph test |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputTest failed! |
|
tests-mbedmicro-rtos-mbed-signals,tests-mbedmicro-rtos-mbed-mail failed in connection process. /morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
|
@0xc0170 please don't merge - this needs rework as suggested in #4780 Waiting for confirmation from @betzw anyway |
|
OK for me! |
|
@betzw Thanks for your quick feedback ! |
|
@0xc0170 thanks for waiting - nothing has changed, so this can be moved back to ready_for_merge :-) |
Description
There was a hidden bug in the uart irq handler that was seen when reaching high baud rates as described in details in #4722 . The RXE flags shouldn't be erased in the uart_irq as erasing the flag is actually done when reading the data in the data register. At low baudrate, this has usually not effect, but at igh baud rates, this can actually erase a new newly arrived data, not yet read by application. This would cause data loss at application layer.
The 2nd commit of this PR consist in using TXE (room in byte FIFO for posting next data) flag instead of TC (transmission complete), in order to improve overall performance. Also this is now in line with TxIrq definition from SerialBAse.h : TxIrq for transmit buffer empty
Status
READY
Related PRs
Dependency to #4707
Testing
This has been tested by @RobMeades in the scope of Serial handling at high data rates with HW flow control on ST platforms #4722
Unfortunately I could not find mbed tests for checking serial interrupt - this would be good enhancement on test side.