Skip to content

Conversation

@mickeprag
Copy link

  • Implement CAN FD frame
  • Implement a CAN FD interface

Copy link
Contributor

@liamkinne liamkinne left a comment

Choose a reason for hiding this comment

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

Just some thoughts on this implementation.

fn transmit_fd(&mut self, frame: &Self::FDFrame) -> Result<(), Self::Error>;

/// Blocks until a fd frame was received or an error occurred.
fn receive_fd(&mut self) -> Result<Self::FDFrame, Self::Error>;
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this work for receivers that want to receive both FD and non-FD frames? By your description this would only return FD frames.

Copy link
Author

Choose a reason for hiding this comment

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

Do you have a suggestion for a signature?
Should we perhaps return an enum that can handle both frame types?

fn dlc(&self) -> usize;

/// Returns the length of the frame data in bytes.
fn length(&self) -> usize {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this any different to .data().len()? I'm assuming it would be implemented that way anyway.

Copy link
Author

Choose a reason for hiding this comment

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

The length will be derived from the dlc. So the .data() function must read the dlc anyway to be able to return the correct length for the slice. Doing .data().len() needs to call dlc() internally anyway. I see no reason for the extra roundtrip creating a slice. I am providing a default implementation anyway so consumers of the trait does not need to implement it.

Copy link
Author

Choose a reason for hiding this comment

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

To be honest, I rather think the implementation will be the other way around. Implementations of data() will be calling the length() method, instead of length() calling data()...

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.

2 participants