The gs_usb interface support#905
Conversation
87930a0 to
52a2aab
Compare
Codecov Report
@@ Coverage Diff @@
## develop #905 +/- ##
===========================================
- Coverage 70.76% 70.34% -0.43%
===========================================
Files 71 72 +1
Lines 7061 7106 +45
===========================================
+ Hits 4997 4999 +2
- Misses 2064 2107 +43 |
hardbyte
left a comment
There was a problem hiding this comment.
Overall looks very clean, just minor comments mostly around documentation.
Bit of a shame to not have support for periodic messages or filtering.
|
@ericevenchick any chance you could provide your thoughts? |
|
@jxltom is right, CANtact is diverging a bit from gs_usb to add FD support. There's already a gs_usb_fd Linux kernel module for this: https://github.com/linklayer/gs_usb_fd The goal was to keep support with the original gs_usb, but that's not my standard to modify. The gs_usb_fd module supports normal gs_usb devices, and FD devices provide a flag to show they have support. |
Co-authored-by: Brian Thorne <brian@thorne.link>
3402873 to
676dd7c
Compare
|
Hey @hardbyte thanks for your time for reviewing. I've already updated the PR according to the comments. Just let me know if there are more concerns. Thanks! |
hardbyte
left a comment
There was a problem hiding this comment.
Thanks for making all those changes this looks good to merge.
The
gs_usbis the interface for Geschwister Schneider USB/CAN devices and bytewerk.org candleLight USB CAN devices such as candlelight, canable, cantact, etc. Refer to https://github.com/torvalds/linux/blob/master/drivers/net/can/usb/gs_usb.c and https://github.com/candle-usb/candleLight_fwThis PR implements the general driver for
gs_usbdevices based onpyusb, tested on Windows/Linux/Mac.It looks like one of the
gs_usbdevicecantactis supported in #853. However, that interface might diverge fromgs_usbsince it is going to add CAN FD support. This PR is more universal forgs_usbdevices. Since this interface is using pyusb as backend, it has better crossplatform compatibility.