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

Enable CanFD support in "serial" interface #1566

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jacky309
Copy link

  • Remove (DLC <= 8) limitation

* Remove (DLC <= 8) limitation
@jacky309
Copy link
Author

jacky309 commented May 9, 2023

Any feedback regarding this pull-request ?

@lumagi
Copy link
Collaborator

lumagi commented May 26, 2023

Hey @jacky309,

your PR does not take into consideration the fact that a regular CAN frame shouldn't have a payload larger than 8 Byte. If you want to send CAN-FD frames over the interface, I recommend adding proper CAN-FD support to the interface.
This would entail the following points:

  • Adding a flag field to the transmission frame
  • sending up to 8 byte if frame.is_fd == False, sending and receiving up to 64 byte if frame.is_fd is True

@jacky309
Copy link
Author

Do you mean that the serial frame format described at https://python-can.readthedocs.io/en/master/interfaces/serial.html should be changed to introduce a new field to identify if a frame is CanFD or not ? This would be a breaking change which is likely to cause some trouble with existing software, right ? IMO, this is not acceptable.
This PR, as it is now, introduces the improvement that CanFD frames can be transmitted over a serial connection, without requiring any change to the serial frame format. IMO, this benefit outweights the fact that the length check is gone, so I would consider merging this PR for now.

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

Successfully merging this pull request may close these issues.

3 participants