Overflow flag in STM32L4xx

[dirkmb]

CanOpenSTM32/CANopenNode_STM32/CO_driver_STM32.c

Line 407 in c7fb1de

// I didn't find overflow error register in STM32, if you find it please let me know

I found this comment. The STM32L4xxb and STM32F4xx sets the FOVR flag in the CAN_RFxR Register when an overrun occurs. I think overrun in STM language is the same as overflow in CanOpen language. This flag exists for each receive fifo.

STM Reference manual manual says:

Overrun
Once the FIFO is in pending_3 state (i.e. the three mailboxes are full) the next valid
message reception will lead to an overrun and a message will be lost. The hardware
signals the overrun condition by setting the FOVR bit in the CAN_RFR register.

Not sure if the flag also exists when using the FDCAN driver.

[MaJerle]

Buffer overflow in canopen is not the same as can overflow on a hardqare level.

[dirkmb]

Two questions:

1. what does an overflow in canopen mean?
2. if canopen not interrested in the information when messages have been lost on the bus?

[MaJerle]

1. If it is at the specific buffer level, then it means that CPU didn't process CANopen stack fast enough. Problem if that this overflow is specific to CANopen ID (SDO, PDO, NMT, ...) while buffer overflow at HW level can only tell that there was data misread, and not which buffer is problematic.
2. I cannot answer this.

[HamedJafarzadeh]

2. if canopen not interrested in the information when messages have been lost on the bus?

It actually does care. Look here : https://github.com/CANopenNode/CANopenNode/blob/8c7d852902b2d307e8b91a43332c14e366641e00/301/CO_Emergency.c#L580

I will mark this as enhancement to consider for next release.

[Nadreoh]

I found an issue that looks related to this ticket.
In the latest version(b084ab2) of CO_driver_STM32.c, a hardware Tx overflow flag is set (line 344: CO_CAN_ERRTX_OVERFLOW), but it is never reset in the code.
This flag triggers a safety mechanism and switches to pre-operational mode. But the code remains stuck in this pre-operational mode, because this flag is never reset (I do activate the MACRO to allow the code to automatically jump back to operational mode).

Here's my use case where I detected this problem: I have a single ST device connected to my laptop via a CANbox (PCAN-USB). For some reason, I disconnect my CANbox. The device is left alone on the bus and can no longer communicate. The Tx overflow flag is raised.

I'll try to figure out how to reset this flag "correctly" and let you know if I succeed.

[ks1930]

@Nadreoh were you able to find a solution ?

[Nadreoh]

Sure, I deployed a fix that is working for my use case. I am not sure how far it is compliant with the original code design of this open source project nor the CANopen standard (maybe it requires some adjustments?).

Anyway, you must edit CO_app_STM32 with:

#define NMT_CONTROL                                                                                                    \
    CO_NMT_STARTUP_TO_OPERATIONAL                                                                                      \
    | CO_NMT_ERR_ON_ERR_REG | CO_ERR_REG_GENERIC_ERR | CO_ERR_REG_COMMUNICATION                                        \
    | CO_NMT_ERR_FREE_TO_OPERATIONAL

Then open and edit CO_driver_STM32 with:

CO_ReturnError_t
CO_CANsend(CO_CANmodule_t* CANmodule, CO_CANtx_t* buffer) {
    CO_ReturnError_t err = CO_ERROR_NO;

    /* Verify overflow */
    if (buffer->bufferFull) {
        if (!CANmodule->firstCANtxMessage) {
            /* don't set error, if bootup message is still on buffers */
            CANmodule->CANerrorStatus |= CO_CAN_ERRTX_OVERFLOW;
        }
        err = CO_ERROR_TX_OVERFLOW;
    }

    /*
     * Send message to CAN network
     *
     * Lock interrupts for atomic operation
     */
    CO_LOCK_CAN_SEND(CANmodule);
    if (prv_send_can_message(CANmodule, buffer)) {
        CANmodule->bufferInhibitFlag = buffer->syncFlag;
    } else {
        buffer->bufferFull = true;
        CANmodule->CANtxCount++;
#ifdef NADREOH
        static uint16_t prevCANerrorStatus = 0u;

        if(CANmodule->CANerrorStatus != prevCANerrorStatus)
        {
            uint16_t CANerrorStatusChanged = CANmodule->CANerrorStatus ^ prevCANerrorStatus;
            prevCANerrorStatus = CANmodule->CANerrorStatus;
        }
#endif
    }
    CO_UNLOCK_CAN_SEND(CANmodule);

    return err;
}

#ifdef NADREOH /* Fix CO_CAN_ERRTX_OVERFLOW flag; was never reset. */
void HAL_FDCAN_TxFifoEmptyCallback(FDCAN_HandleTypeDef *hfdcan)
{
    CO_CANtx_t* buffer = &CANModule_local->txArray[0]; /* Start with first buffer handle */
    uint16_t i;
    for (i = CANModule_local->txSize; i > 0U; --i, ++buffer) {
        if (buffer->bufferFull) {
            buffer->bufferFull = false;
            CANModule_local->CANerrorStatus &= ~CO_CAN_ERRTX_OVERFLOW;
        }
    }
}
#endif

/**
 * \brief           TX buffer has been well transmitted callback
 * \param[in]       hfdcan: pointer to an FDCAN_HandleTypeDef structure that contains
 *                      the configuration information for the specified FDCAN.
 * \param[in]       BufferIndexes: Bits of successfully sent TX buffers
 */
void
HAL_FDCAN_TxBufferCompleteCallback(FDCAN_HandleTypeDef* hfdcan, uint32_t BufferIndexes) {
    CANModule_local->firstCANtxMessage = false;            /* First CAN message (bootup) was sent successfully */
    CANModule_local->bufferInhibitFlag = false;            /* Clear flag from previous message */
    if (CANModule_local->CANtxCount > 0U) {                /* Are there any new messages waiting to be send */
        CO_CANtx_t* buffer = &CANModule_local->txArray[0]; /* Start with first buffer handle */
        uint16_t i;

        /*
         * Try to send more buffers, process all empty ones
         *
         * This function is always called from interrupt,
         * however to make sure no preemption can happen, interrupts are anyway locked
         * (unless you can guarantee no higher priority interrupt will try to access to FDCAN instance and send data,
         *  then no need to lock interrupts..)
         */
        CO_LOCK_CAN_SEND(CANModule_local);
        for (i = CANModule_local->txSize; i > 0U; --i, ++buffer) {
            /* Try to send message */
            if (buffer->bufferFull) {
                if (prv_send_can_message(CANModule_local, buffer)) {
                    buffer->bufferFull = false;

#ifdef NADREOH /* Fix issue where that flag was never reset. */
                    CANModule_local->CANerrorStatus &= ~CO_CAN_ERRTX_OVERFLOW;
#endif
                    CANModule_local->CANtxCount--;
                    CANModule_local->bufferInhibitFlag = buffer->syncFlag;
                }
            }
        }
        /* Clear counter if no more messages */
        if (i == 0U) {
            CANModule_local->CANtxCount = 0U;
        }
        CO_UNLOCK_CAN_SEND(CANModule_local);
    }
}

Hope something will help you there?

[Frab012]

@Nadreoh Is the section

#ifdef NADREOH
        static uint16_t prevCANerrorStatus = 0u;

        if(CANmodule->CANerrorStatus != prevCANerrorStatus)
        {
            uint16_t CANerrorStatusChanged = CANmodule->CANerrorStatus ^ prevCANerrorStatus;
            prevCANerrorStatus = CANmodule->CANerrorStatus;
        }
#endif

in your code above just for debugging, or does it serve a deeper purpose?

[Frab012]

@Nadreoh

It seems that your workaround is working, but I see some issues, particularly regarding the use of the callback HAL_FDCAN_TxFifoEmptyCallback(). The callback is executed as soon as the hardware TX FIFO becomes empty, which is often the case after transmitting each individual frame.

Message loss:
bufferFull is cleared without calling prv_send_can_message(). As a result, the software queue is discarded and pending messages are lost.

CANtxCount is not decremented:
The counter remains unchanged. HAL_FDCAN_TxBufferCompleteCallback() checks whether CANtxCount > 0, but since no bufferFull entries exist anymore, no messages can be processed and the counter remains non-zero indefinitely.

"Hardware FIFO empty" does not mean the overflow condition has been resolved:
Software buffers may still contain queued messages. These buffered messages are discarded at this point, even though they should be transmitted once hardware buffer space becomes available again.

[Nadreoh]

@Nadreoh Is the section

#ifdef NADREOH
static uint16_t prevCANerrorStatus = 0u;

    if(CANmodule->CANerrorStatus != prevCANerrorStatus)
    {
        uint16_t CANerrorStatusChanged = CANmodule->CANerrorStatus ^ prevCANerrorStatus;
        prevCANerrorStatus = CANmodule->CANerrorStatus;
    }

#endif
in your code above just for debugging, or does it serve a deeper purpose?

Good point. I apologize for this issue tracking, it has been an old fix for me and I did not have access to my hardware anymore.

It looks like I copy paste this pièce of code for debugging propose. But this débug is relates to the issue here, I just did not fully remember my meaning in détails...

[Nadreoh]

@Nadreoh

It seems that your workaround is working, but I see some issues, particularly regarding the use of the callback HAL_FDCAN_TxFifoEmptyCallback(). The callback is executed as soon as the hardware TX FIFO becomes empty, which is often the case after transmitting each individual frame.

Message loss: bufferFull is cleared without calling prv_send_can_message(). As a result, the software queue is discarded and pending messages are lost.

CANtxCount is not decremented: The counter remains unchanged. HAL_FDCAN_TxBufferCompleteCallback() checks whether CANtxCount > 0, but since no bufferFull entries exist anymore, no messages can be processed and the counter remains non-zero indefinitely.

"Hardware FIFO empty" does not mean the overflow condition has been resolved: Software buffers may still contain queued messages. These buffered messages are discarded at this point, even though they should be transmitted once hardware buffer space becomes available again.

This is correct, my proposal lost the pending messages. Once again I must clarified that I only raise a possible bug and provide some clues for investigation. I stopped my fix where my projet needs were met.
For my use case, it was acceptable to empty the queue and lost all my pending Tx.

That is good that somebody else takes the lead to provide an even better fix to be deployed for the community.

[Frab012]

I have adapted the function CO_CANmodule_process() accordingly, based on the CO_driver_blank.c template. The error detection is based on the TX/RX error counter. I removed the evaluation of flags FDCAN_PSR_EW and FDCAN_PSR_EP, as they are unsuitable for separately analyzing sending and receiving problems.

I tested this function with a C092- and a L476-Nucleo board. The overflow flag CO_CAN_ERRTX_OVERFLOW is reset as soon as the TX status is no longer passive.

The function was analyzed using AI, and two points were criticized:

• Destructive reading of FDCAN PSR: Every call reads Instance->PSR. According to the FDCAN/M_CAN specification, a read operation resets LEC, DLEC, and PXE. While this currently has no impact on the current project, it interferes with subsequent diagnostics performed via HAL_FDCAN_GetProtocolStatus().
• No bus-off recovery, see Manual Bus Off recovery needed #128

@MaJerle @HamedJafarzadeh
Regarding the destructive reading of the PSR: I would handle the detection of the "bus-off" state differently than via the PSR. For example, by setting a flag CANmodule->busOff in the function HAL_FDCAN_ErrorStatusCallback(), see #128.

I want to run a few more tests before submitting a PR.

void
CO_CANmodule_process(CO_CANmodule_t* CANmodule) {
    uint32_t err = 0;

    // CANOpen just care about Bus_off, Warning, Passive and Overflow

#ifdef CO_STM32_FDCAN_Driver
    FDCAN_HandleTypeDef *fdcanHandle = (FDCAN_HandleTypeDef*)((CANopenNodeSTM32*)CANmodule->CANptr)->CANHandle;
    uint8_t txErrorCnt = (uint8_t)((fdcanHandle->Instance->ECR & FDCAN_ECR_TEC) >> FDCAN_ECR_TEC_Pos);
    uint8_t rxErrorCnt = (uint8_t)((fdcanHandle->Instance->ECR & FDCAN_ECR_REC) >> FDCAN_ECR_REC_Pos);
    uint8_t rxErrorPassive = (uint8_t)((fdcanHandle->Instance->ECR & FDCAN_ECR_RP) >> FDCAN_ECR_RP_Pos);

    err = ((uint32_t)txErrorCnt << 24) | ((uint32_t)rxErrorCnt << 16) | ((uint32_t)rxErrorPassive << 8)
          | (fdcanHandle->Instance->PSR & FDCAN_PSR_BO);

    if (CANmodule->errOld != err) {

        uint16_t status = CANmodule->CANerrorStatus;

        CANmodule->errOld = err;

        if ((err & FDCAN_PSR_BO) != 0U) {
            status |= CO_CAN_ERRTX_BUS_OFF;
            // In this driver we expect that the controller is automatically handling the protocol exceptions.

        } else {
            /* recalculate CANerrorStatus, first clear some flags */
            status &= ~((uint16_t)(CO_CAN_ERRTX_BUS_OFF | CO_CAN_ERRRX_WARNING | CO_CAN_ERRRX_PASSIVE
                                   | CO_CAN_ERRTX_WARNING | CO_CAN_ERRTX_PASSIVE));

            /* rx bus warning or passive */
            if (rxErrorPassive != 0U) {
                status |= CO_CAN_ERRRX_WARNING | CO_CAN_ERRRX_PASSIVE;
            } else if (rxErrorCnt >= 96U) {
                status |= CO_CAN_ERRRX_WARNING;
            }

            /* tx bus warning or passive */
            if (txErrorCnt >= 128U) {
                status |= CO_CAN_ERRTX_WARNING | CO_CAN_ERRTX_PASSIVE;
            } else if (txErrorCnt >= 96U) {
                status |= CO_CAN_ERRTX_WARNING;
            }

            /* if not tx passive clear also overflow */
            if ((status & CO_CAN_ERRTX_PASSIVE) == 0U) {
                status &= ~((uint16_t)CO_CAN_ERRTX_OVERFLOW);
            }
        }

        CANmodule->CANerrorStatus = status;
    }
#else
    CAN_HandleTypeDef *canHandle = (CAN_HandleTypeDef*)((CANopenNodeSTM32*)CANmodule->CANptr)->CANHandle;
    uint8_t txErrorCnt = (uint8_t)((canHandle->Instance->ESR & CAN_ESR_TEC) >> CAN_ESR_TEC_Pos);
    uint8_t rxErrorCnt = (uint8_t)((canHandle->Instance->ESR & CAN_ESR_REC) >> CAN_ESR_REC_Pos);

    err = ((uint32_t)txErrorCnt << 24) | ((uint32_t)rxErrorCnt << 16)
          | (canHandle->Instance->ESR & CAN_ESR_BOFF);

    if (CANmodule->errOld != err) {

        uint16_t status = CANmodule->CANerrorStatus;

        CANmodule->errOld = err;

        if ((err & CAN_ESR_BOFF) != 0U) {
            status |= CO_CAN_ERRTX_BUS_OFF;
            // In this driver, we assume that auto bus recovery is activated ! so this error will eventually handled automatically.

        } else {
            /* recalculate CANerrorStatus, first clear some flags */
            status &= ~((uint16_t)(CO_CAN_ERRTX_BUS_OFF | CO_CAN_ERRRX_WARNING | CO_CAN_ERRRX_PASSIVE
                                   | CO_CAN_ERRTX_WARNING | CO_CAN_ERRTX_PASSIVE));

           /* rx bus warning or passive */
            if (rxErrorCnt >= 128U) {
                status |= CO_CAN_ERRRX_WARNING | CO_CAN_ERRRX_PASSIVE;
            } else if (rxErrorCnt >= 96U) {
                status |= CO_CAN_ERRRX_WARNING;
            }

            /* tx bus warning or passive */
            if (txErrorCnt >= 128U) {
                status |= CO_CAN_ERRTX_WARNING | CO_CAN_ERRTX_PASSIVE;
            } else if (txErrorCnt >= 96U) {
                status |= CO_CAN_ERRTX_WARNING;
            }

            /* if not tx passive clear also overflow */
            if ((status & CO_CAN_ERRTX_PASSIVE) == 0U) {
                status &= ~((uint16_t)CO_CAN_ERRTX_OVERFLOW);
            }
        }

        CANmodule->CANerrorStatus = status;
    }
#endif
}

[MaJerle]

Before we do the PR, we shall resolve the PR I've submitted to rework the driver generally, then we should accept further PR.

#132

[MaJerle]

@Frab012 just the idea. If the HAL drivers (didn't check) protocol status function does the destructive read, I would suggest we then only do it in the process function that is periodically called.

I agree we can set the flag in the status error, but not sure what's going to help us if we periodically call this in process.
And I think we should call it on every transmit, too.