AnsweredAssumed Answered

HAL_UART_IRQHandler bug?

Question asked by butenko.vladimir on Apr 26, 2015
Latest reply on Jan 26, 2016 by _.Adri
I was struggling for quite a few hours to get the simplest UART input ring buffer working. As soon as an "overrun" error occurred, the thing stopped, while the code was prepared to handle that situation (or so I hoped).

After quite a struggle, the following things were observed in theHAL_UART_IRQHandler code:

a) when an Overrun error is detected by the hardware, the huart->ErrorCode field is set
b) if the hardware has received a data byte, the UART_Receive_IT routine is called;
that routine stores the received byte in the user-provided buffer, decreases the buffer counter, and if it becomes zero (this always happens when we are reading byte-by-byte, using a single-byte buffer), the route calls [user-supplied] HAL_UART_RxCpltCallback
Unless the buffer is filled and  HAL_UART_RxCpltCallback is called, the UART_Receive_IT code does not touch neither huart->Status nor huart->Error fields. 
c) finally, just before completing, HAL_UART_IRQHandler checks if the huart->Error  field contains some error. If it does, the IRQ routine calls the [user-supplied] HAL_UART_ErrorCallback routine. But before it does this, it  MODIFIES the State field:
    huart->State = HAL_UART_STATE_READY

Let us review the following situations. For simplicity sake we review only the Receive side of things, ignoring the fact that UART Transfers can take place at the same time:

I) an overrun error is detected, data is received, [remaining] buffer size is 1 byte.
huart->Error is set to Overrrun
UART_Receive_IT is called, and moves the input to the buffer: it's the last byte
UART_Receive_IT sets huart->State to HAL_UART_STATE_READY, callsHAL_UART_RxCpltCallback
HAL_UART_RxCpltCallback should check for and remember the huart->Error field, and use the new HAL_UART_RECEIVE_IT call to re-initiate next byte reading.
If the routine itself does not clear the huart->Error field, HAL_UART_RECEIVE_IT does it in any case;
huart->State is set to HAL_UART_STATE_BUSY_RX
the (c) check in HAL_UART_IRQHandler is not fired
all is fine and clean for now.

II) an overrun error is detected, data is NOT received
huart->Error is set to Overrrun
UART_Receive_IT is not called; huart->State remains at HAL_UART_STATE_BUSY_RX, hardware is reading data, operation continues
the (c) check in HAL_UART_IRQHandler is FIRED (Error field is not empty):
State is changed to HAL_UART_STATE_READY
[user-supplied] HAL_UART_ErrorCallback  is called.

Here we hit the problem:
a)  the user assumes that if their HAL_UART_ErrorCallback  is called, the transaction has been cancelled by HAL. The user tries to start a new one, using  UART_Receive_IT - and it results in an Overrun error appearing again immediately: while the "software" part thinks that the hardware is in the idle state, in reality the hardware is still reading data.
b) the user assumes that if their HAL_UART_ErrorCallback is called, they must record huart->Error code somewhere, but not do anything else, i.e. the operation continues. But now it will never get the next byte: when the byte is received, and the IRQ routine is called, the huart->State field is set to HAL_UART_STATE_READY, and the UART_Receive_IT will not be called to process the received byte.

III) an overrun error is detected, data is received, but the buffer is longer than 1 byte.
UART_Receive_IT is called, and moves the input to the buffer
it's NOT the last byte, UART_Receive_IT  simply returns
the (c) check in HAL_UART_IRQHandler is FIRED (Error field is not empty):
State is changed to HAL_UART_STATE_READY
[user-supplied] HAL_UART_ErrorCallback  is called.

The situation is the same as in case II.
Fortunately, in our code we know for sure if there is a reading and/or writing operation pending when our HAL_UART_ErrorCallback routine is called. So, we just restore the huart->Status value back to HAL_UART_STATE_BUSY_RX or AL_UART_STATE_BUSY_TX_RX in that routine, before returning to IRQ.

But this is obviously just a dirty fix, and if the analysis above is correct, it would be nice to have a fix in the HAL_UART_IRQHandler. It looks like huart->State should not be modified there.