Skip to content

Implement manual bus off recovery - #129

Open
anmelle-slamcore wants to merge 1 commit into
CANopenNode:masterfrom
anmelle-slamcore:issue-128-manual-bus-off-recovery
Open

Implement manual bus off recovery#129
anmelle-slamcore wants to merge 1 commit into
CANopenNode:masterfrom
anmelle-slamcore:issue-128-manual-bus-off-recovery

Conversation

@anmelle-slamcore

Copy link
Copy Markdown

@MaJerle

MaJerle commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

For me it looks ok.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new callback should ensure Bus-Off is currently asserted (not just “status changed”) before clearing CCCR.INIT, to avoid unintended behavior and improve robustness.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds an FDCAN HAL error-status callback to perform manual Bus-Off recovery on STM32 targets, addressing the inability to recover from Bus-Off automatically as discussed in issue #128.

Changes:

  • Implement HAL_FDCAN_ErrorStatusCallback to clear FDCAN_CCCR_INIT when Bus-Off is signaled.
  • Add brief inline documentation pointing to ST’s recommended recovery approach.
File summaries
File Description
CANopenNode_STM32/CO_driver_STM32.c Adds an FDCAN error-status callback intended to manually recover from Bus-Off.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +636 to +641
HAL_FDCAN_ErrorStatusCallback(FDCAN_HandleTypeDef* hfdcan, uint32_t ErrorStatusITs) {
if ((ErrorStatusITs & FDCAN_IT_BUS_OFF) != 0) // If Bus-Off error occurred
{
CLEAR_BIT(hfdcan->Instance->CCCR, FDCAN_CCCR_INIT); // Clear INIT bit to recover from Bus-Off
}
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this suggestion make sense @MaJerle ? This is not what STM32 article recommends

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, I'd rather use that we use the HAL_FDCAN_procotolStatus (or something like that) function to read all the statuses and if we have busoff, we start init.

I use a function like this:

/**
 * \brief           Check for BUS-OFF and reinitialize the init
 * 
 *                  IP will then wait for recessive bits before resuming the operation
 * 
 * \param           hfdcan 
 */
static void
prv_fdcan_bus_off_check_reset(FDCAN_HandleTypeDef* hfdcan) {
    FDCAN_ProtocolStatusTypeDef protocolStatus = {0};

    HAL_FDCAN_GetProtocolStatus(hfdcan, &protocolStatus);
    if (protocolStatus.BusOff) {
        CLEAR_BIT(hfdcan->Instance->CCCR, FDCAN_CCCR_INIT);
    }
}

And I call it on:

  • HAL_FDCAN_ErrorCallback()
  • HAL_FDCAN_ErrorStatusCallback()
  • Each time I try to send the packet

In practice, I'll do some reworking today of the drivers, so if @HamedJafarzadeh can agree to merge this today, it will be good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants