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

Implement basic CAN functionality #258

Open
wants to merge 91 commits into
base: devel
from

Conversation

@Wetmelon
Copy link
Collaborator

commented Oct 7, 2018

No description provided.

Wetmelon added some commits Sep 22, 2018

Wetmelon added some commits Mar 17, 2019

@@ -124,6 +124,7 @@ bool ODriveCAN::read(CAN_message_t &rxmsg) {
rxmsg.isExt = header.IDE;
rxmsg.id = rxmsg.isExt ? header.ExtId : header.StdId; // If it's an extended message, pass the extended ID
rxmsg.len = header.DLC;
rxmsg.rtr = (header.RTR == CAN_RTR_REMOTE) ? true : false;

This comment has been minimized.

Copy link
@kalvdans

kalvdans Mar 17, 2019

Contributor

The result of == is already a boolean.

This comment has been minimized.

Copy link
@Wetmelon

Wetmelon Mar 17, 2019

Author Collaborator

Lol I noticed that after I typed it but I was too lazy to change it :p

txmsg.id = axis->config_.can_node_id << NUM_CMD_ID_BITS;
txmsg.id += MSG_GET_MOTOR_ERROR; // heartbeat ID
txmsg.isExt = false;
txmsg.len = 8;

This comment has been minimized.

Copy link
@samuelsadok

samuelsadok May 7, 2019

Collaborator

This should probably be txmsg.len = 4.

Or maybe even if (txmsg.len != 4) return;. Based on this explanation collisions can occur if remote frame and data frame use different length fields so it may be good to discourage this by not responding, even if it sometimes works.

This comment has been minimized.

Copy link
@Wetmelon

Wetmelon May 8, 2019

Author Collaborator

We can use 4 bytes in this message if we want, but right now all messages are 8 bytes. Common thing to do for whatever reason.

But why would I check for txmsg.len == 4 when I'm the one who is deciding how long the message is?

This comment has been minimized.

Copy link
@samuelsadok

samuelsadok May 8, 2019

Collaborator

Ah ok didn't know that was common.

The problem with remote frames is that if two nodes send the same remote frame but with a different length field, they can both win arbitration and then collide during the data field (I worded this a bit wrong before).

So it makes somewhat sense to encourage all client implementations to send the same data length field, by just ignoring any non-compliant remote frame. I mean once you received a non-compliant remote frame the collision was already evaded so you might as well just serve it, but in the long run the "ignore strategy" will kill off non-compliant implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.