Skip to content

Fix wrong API usage in disable() and beginAutoSpeed() #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sans-ltd
Copy link

@sans-ltd sans-ltd commented Apr 3, 2025

Hello,
we are using your CAN library in an automotive project on the ESP32-C6 where we have both CAN busses in use.
Thank you for restructuring your library, especially removing the static init of CAN busses and the MCP2517 stuff.
But we still have a crash when using your lib in our scenario.
This is the fix for it.
Feel free to contact us, maybe also for further collaboration.

@jeroenveer
Copy link

I have had the same issues and this seems to have fixed the crashes i had.

#74

#80

The issue i have now is that it somehow sees its own transmitted frames and then processes them as received frames.
I externally send a frame to CAN0 it is received and transmitted on CAN1.
CAN1 then sees its own send frame and sends it to CAN0 creating a constant loop.

	CAN_FRAME message;
	if (CAN0.available() && CAN0.read(message)) {
		CAN1.sendFrame(message);
	}

	CAN_FRAME message2;
	if (CAN1.available() && CAN1.read(message2)) {
		CAN0.sendFrame(message2);
	}

@sans-ltd
Copy link
Author

sans-ltd commented Apr 4, 2025

I have had the same issues and this seems to have fixed the crashes i had.

#74

#80

The issue i have now is that it somehow sees its own transmitted frames and then processes them as received frames. I externally send a frame to CAN0 it is received and transmitted on CAN1. CAN1 then sees its own send frame and sends it to CAN0 creating a constant loop.

	CAN_FRAME message;
	if (CAN0.available() && CAN0.read(message)) {
		CAN1.sendFrame(message);
	}

	CAN_FRAME message2;
	if (CAN1.available() && CAN1.read(message2)) {
		CAN0.sendFrame(message2);
	}

Yes it seems like I am doing the same thing as you. The trick is to remember the last forwarded frame and ignore it if it appears again. It seems to be a "feature" of the CAN transceiver. If you run candump can0 and cansend can0 some#thing on Linux in parallel, you will also see your sent frames in the candump.

@jeroenveer
Copy link

I see the same issue on the older ESP32 boards with single CAN as well. I believe it appeared with the new IDF update so do not think it's a feature of the transceiver.

@jeroenveer
Copy link

Which part of the frame would u use to ignore it? you have a example code?

@sans-ltd
Copy link
Author

sans-ltd commented Apr 4, 2025

I compare (==) id and length. And then memcmp the data

@sans-ltd
Copy link
Author

sans-ltd commented Apr 4, 2025

A full example would be a bit difficult to extract from my code.

@sans-ltd
Copy link
Author

sans-ltd commented Apr 4, 2025

I found out the core reason for this looping behavior:
from twai_types.h:

typedef struct {
    union {
        struct {
            //The order of these bits must match deprecated message flags for compatibility reasons
            uint32_t extd: 1;           /**< Extended Frame Format (29bit ID) */
            uint32_t rtr: 1;            /**< Message is a Remote Frame */
            uint32_t ss: 1;             /**< Transmit as a Single Shot Transmission. Unused for received. */
            uint32_t self: 1;           /**< Transmit as a Self Reception Request. Unused for received. */
            uint32_t dlc_non_comp: 1;   /**< Message's Data length code is larger than 8. This will break compliance with ISO 11898-1 */
            uint32_t reserved: 27;      /**< Reserved bits */
        };
        //Todo: Deprecate flags
        uint32_t flags;                 /**< Deprecated: Alternate way to set bits using message flags */
    };
    uint32_t identifier;                /**< 11 or 29 bit identifier */
    uint8_t data_length_code;           /**< Data length code */
    uint8_t data[TWAI_FRAME_MAX_DLC];    /**< Data bytes (not relevant in RTR frame) */
} twai_message_t;

In the example, they set it to 0, for a good reason, since we have a C struct here, it is pure random how it is initialized:

// Configure message to transmit
twai_message_t message = {
    // Message type and format settings
    .extd = 1,              // Standard vs extended format
    .rtr = 0,               // Data vs RTR frame
    .ss = 0,                // Whether the message is single shot (i.e., does not repeat on error)
    .self = 0,              // Whether the message is a self reception request (loopback)
    .dlc_non_comp = 0,      // DLC is less than 8
    // Message ID and payload
    .identifier = 0xAAAA,
    .data_length_code = 4,
    .data = {0, 1, 2, 3},
};

@TechOverflow
Copy link

TechOverflow commented Apr 4, 2025

Hi @sans-ltd,
Great find! However in my case, I believe that not only was it repeating the message on the bus that received the message, but I couldn't get the received message on CAN0 to be sent on CAN1. See my issue.

Edit: I now see @jeroenveer already mentioned the issue ;)

@sans-ltd
Copy link
Author

sans-ltd commented Apr 4, 2025

BTW: I do not use the synchronous loop code as seen here but a callback:

    CAN0.watchFor();                         
    CAN0.setCallback(0, &CANBusses::onOtherMessageCAN0);

void CANBusses::onOtherMessageCAN0(CAN_FRAME *msg)
{
   ...
   CAN1.sendFrame(*msg);
}

@jeroenveer
Copy link

@sans-ltd Awesome! That fixed it thank you!

It runs perfectly with synchronous loop
Tested with load of 90 different CAN frames for about 15 minutes and got 0 missed frames.

	CAN_FRAME message;
	if (CAN0.available() && CAN0.read(message)) {
		CAN1.sendFrame(message);
	}

	CAN_FRAME message2;
	if (CAN1.available() && CAN1.read(message2)) {
		CAN0.sendFrame(message2);
	}

@TechOverflow
Copy link

I can confirm that after applying the changes, I can forward thousands of messages back and forth to both buses without a hickup.

@nicolasf
Copy link

nicolasf commented Jul 3, 2025

This PR fixes issues with ESP32-C6 for me.
Without it I have crashes almost immediatelly on the CAN loop.

Thanks @sans-ltd .

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