-
Notifications
You must be signed in to change notification settings - Fork 590
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
Vector CAN FD #311
Vector CAN FD #311
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see this being implemented!
can/interfaces/vector/canlib.py
Outdated
vxlapi.XL_INTERFACE_VERSION, vxlapi.XL_BUS_TYPE_CAN) | ||
if fd: | ||
vxlapi.xlOpenPort(self.port_handle, self._app_name, self.mask, | ||
permission_mask, 16000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 16000? Can this also be rx_queue_size
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, hardcoded value was taken over from a Vector example. Anyway the Vector documentation gives different ranges for rx_queue_size in case of CAN or CAN-FD. So I will change the default value to be in both ranges and use rx_queue_size in both cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello together,
did somebody test the CAN FD ? Is it working?
Thanks
David
can/interfaces/vector/canlib.py
Outdated
self.canFdConf.arbitrationBitRate = ctypes.c_uint(bitrate) | ||
else: | ||
self.canFdConf.arbitrationBitRate = ctypes.c_uint(500000) | ||
self.canFdConf.sjwAbr = ctypes.c_uint(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these need to be different for different bitrates? Should they be parameters that the user can set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way: Do you know whether ctypes.c_uint is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s not necessary. The types have already been defined for the struct or the function.
can/interfaces/vector/canlib.py
Outdated
extended_id=bool(msg_id & vxlapi.XL_CAN_EXT_MSG_ID), | ||
is_remote_frame=bool(flags & vxlapi.XL_CAN_RXMSG_FLAG_RTR), | ||
is_error_frame=bool(flags & vxlapi.XL_CAN_RXMSG_FLAG_EF), | ||
is_fd=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understood it, this should be bool(flags & vxlapi.XL_CAN_RXMSG_FLAG_EDL)
since EDL is set for all FD-frames to distinguish them from regular frames. Or what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I messed up fd for channel and fd per message. You're right!
can/interfaces/vector/canlib.py
Outdated
|
||
if self.fd: | ||
if msg.dlc > vxlapi.MAX_MSG_LEN: #extended data length (XL_CAN_RXMSG_FLAG_EDL) not in msg class |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, shouldn't EDL be set for all FD-frames?
can/interfaces/vector/canlib.py
Outdated
|
||
XLcanTxEvent.tagData.canMsg.canId = msg_id | ||
XLcanTxEvent.tagData.canMsg.msgFlags = flags | ||
XLcanTxEvent.tagData.canMsg.dlc = msg.dlc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use can.util.len2dlc()
since I believe this should be the code, not the number of bytes.
can/interfaces/vector/canlib.py
Outdated
else: | ||
if event.tag == vxlapi.XL_CAN_EV_TAG_RX_OK or event.tag == vxlapi.XL_CAN_EV_TAG_TX_OK: | ||
msg_id = event.tagData.canRxOkMsg.canId | ||
dlc = self.map_dlc(event.tagData.canRxOkMsg.dlc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use can.util.dlc2len()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove the own mapping and use the utils function.
Hi, thanks for the review - it's really appreciated! I'll commit the changes soon. I also plan some testing with CANoe and VN1610, but most likely it will take some time... |
Closes #300.