BUG: CAN race condition if HAL_CAN_Receive_IT is used

[vromanov]

Posted on April 15, 2015 at 22:27

If you use HAL_CAN_Receive_IT for receive and send messages at same time you can get race condition. Typical callback looks like:

void
HAL_CAN_RxCpltCallback(CAN_HandleTypeDef* CanHandle) {
if
((CanHandle->pRxMsg->StdId == 0x321) && (CanHandle->pRxMsg->IDE == CAN_ID_STD) && (CanHandle->pRxMsg->DLC == 2)) {
if
(CanHandle->pRxMsg->Data[0]) {
LED_GREEN_ON();
} 
else
{
LED_GREEN_OFF();
}
}
/* Receive */
if
(HAL_CAN_Receive_IT(CanHandle, CAN_FIFO0) != HAL_OK) {
/* Reception Error */
Error_Handler(1, 20);
}
}

If interrupt occurs when HAL_CAN_Transmit (or other) is called,

HAL_CAN_Receive_IT

will return HAL_BUSY because can handle is locked. I can reproduce this error in 100% if loopback mode is used. I attach sample code to reproduce this. my_main() must be called inside generated main.c #cubemx #bug #bxcan #stm32f2

[vromanov]

Posted on April 15, 2015 at 22:53

This bug is very critical for me. It make CAN part of cubemx useless :(.

[markb]

Posted on April 16, 2015 at 10:19 Yes, the CAN routines are terrible, what I have done to avoid this situation is put the following code where I otherwise would have called HAL_CAN_Receive_IT():

// ideally, we should be able to call HAL_CAN_Receive_IT() here to set up for another
// receive but the API is flawed because that function will fail if HAL_CAN_Transmit*()
// had already locked the handle when the receive interrupt occurred - so we do what
// HAL_CAN_Receive_IT() would do
if(hcan->State == HAL_CAN_STATE_BUSY_TX)
hcan->State = HAL_CAN_STATE_BUSY_TX_RX;
else {
hcan->State = HAL_CAN_STATE_BUSY_RX;
/* Set CAN error code to none */
hcan->ErrorCode = HAL_CAN_ERROR_NONE;
/* Enable Error warning Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_EWG);
/* Enable Error passive Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_EPV);
/* Enable Bus-off Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_BOF);
/* Enable Last error code Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_LEC);
/* Enable Error Interrupt */
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_ERR);
}
// Enable FIFO 0 message pending Interrupt
__HAL_CAN_ENABLE_IT(hcan, CAN_IT_FMP0);

[vromanov]

Posted on April 16, 2015 at 13:07

Many thnx!

This code fix problem!

[stst9193]

Posted on April 17, 2015 at 09:29

In fact I believe you still have a race condition in the modified code.

Let' s assume you have a transmit under interrupt in progress  when calling the modified receive_It.

If you happen to get and ''end of transmission flag'' the hcan->State will be modified.

In both (original and modified)  version of receive_It you have the following sequence:

    if(hcan->State == HAL_CAN_STATE_BUSY_TX)

    {

      /* Change CAN state */

      hcan->State = HAL_CAN_STATE_BUSY_TX_RX;

    }

    else

    {

      /* Change CAN state */

      hcan->State = HAL_CAN_STATE_BUSY_RX;

    }

In case the  previously mentionned interrupt happens after readind state for the ''if''  statement, but before writng the new value, then the state will be corrupted.

This is a very short time window but it may (and so it will one day or the other) happen.

I already mentionned that issue on the forum (for the UART Hal) but did not get any answer.

Gerard

[markb]

Posted on April 17, 2015 at 16:00

What you say makes sense but it's not a problem for me because I only do polled CAN transmits. There is not a lot of benefit of using the interrupt version because it doesn't let you have multiple outstanding TXs. Basically, the API is a heap of crap.

[Andrei Chichak]

Posted on September 19, 2015 at 00:17

September 18th, 2015.

I can confirm that this bug still exists in the latest F4 library (1.8.0).

The workaround for HAL_CAN_Receive_IT, as shown at the top of the discussion does work and should be a good clue to the ST development team.

Andrei from The Great White North