Skip to content
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

Flexcan fixes + Latest updates #5

Merged
merged 3 commits into from
Oct 12, 2023
Merged

Flexcan fixes + Latest updates #5

merged 3 commits into from
Oct 12, 2023

Conversation

ad3154
Copy link
Member

@ad3154 ad3154 commented Oct 12, 2023

What's New:

  • Improves reliability of transmitting large ETP sessions through the FlexCAN driver (VT example does not work properly #4)
  • Brings over bugfixes from the main repo for:
    • Element Number deserialization in device descriptor object pools
    • Fixes a crash when receiving some ETP sessions' final frame
    • Fixes a couple broken log statements
  • Brings over a new feature from the main repo: Address violation handling

A little about the FlexCAN issue...

When I was originally testing use of this library on Teensy with FlexCAN, I did my testing with the VT example using a terminal that only supports 16 ETP frames at a time, which hid the fact that large ETP sessions could become corrupted due to a few issues/gremlins which I will discuss below. When testing on a terminal that supported 255 frames, I could reproduce the issue in #4

Results of my investigating:

  1. I wasn't setting the seq flag when writing frames to FlexCAN, which could cause out of order messages to be sent.
  2. Despite setting the seq flag, there is definitely a bug either in the FlexCAN_t4 driver, or with the actual FlexCAN peripheral on the i.MX RT1060
    • Even will all mitigations enabled to ensure messages were sent using only 1 constant tx mailbox from FlexCAN_t4, I could not stop random out of order frames!
    • I went so far as to add little shims inside the driver itself to make sure our CAN stack was not at fault, and it wasn't. We are always sending frames perfectly sequentially.
    • Even editing the FlexCAN_t4 code directly to hack out all the funny business inside it with queues and mailboxes and interrupts could not fix it.
    • There's suspicious stuff in this chip's errata about FlexCAN...
    • There's some speculation on the FlexCAN_t4's GitHub page that there may be a race condition in their driver...

Long story short, there's gremlins in the CAN peripheral on the Teensy, which is very frustrating as a library developer. I have worked with many ARM devices with rock-solid CAN peripherals before, and this one seems... not great.

Anyways, rant over, I think this PR will essentially "fix" the issue for nearly every use-case for our library, because it ensures that an entire ETP sessions worth of frames will fit with plenty of overhead. I tested with a terminal that supported the max ETP frame count and several other devices on the bus, and it seemed much more reliable.

Side note

One thing I noticed the better terminal doing was if it received an ETP frame out of order, it would re-request part of the same session starting from the last good packet it received. Our library does not yet support this, but we should! I will open a ticket on the main repo to support that at some point.

Closes #4

Brought over the latest bugfixes and features from the main repo.
Fixes include: resolved a possible crash with ETP reception,
fixed some log statements,
fixed element number deserialization in DDOPs.
New features include: Address violation handling.
Asked the driver to use only 1 tx mailbox, and increased queue size
to allow nearly two full ETP EDPO windows with max size.
@ad3154 ad3154 added bug Something isn't working enhancement New feature or request labels Oct 12, 2023
@ad3154 ad3154 self-assigned this Oct 12, 2023
@ad3154 ad3154 merged commit dad214a into main Oct 12, 2023
2 checks passed
@ad3154 ad3154 deleted the flexcan-fixes branch October 12, 2023 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VT example does not work properly
1 participant