canalystii: Replace binary library with python driver

[projectgus]

Replaces the binary ControlCAN.dll/controlcan.so library with a Python implementation of the driver.

Python driver is created by me, found at https://pypi.org/project/canalystii/

Improvements

• When bus is configured with multiple channels, messages can be received from all of them (previous implementation of recv() would always receive from the first-configured channel).
• Fixes the issue with send() on a busy bus that was also noted in canalystii: Fix transmitting onto a busy bus #1114
• Possible to configure the software RX message queue size.
• Send timeout now relates to the message being sent on the bus, not the USB layer timeout (within constraints of what the USB protocol allows us to detect).
• Adds MacOS support (currently untested, I've verified that Linux and Windows work but haven't tested MacOS. Of course as there is currently no MacOS support, it's not going to become broken either!)
• Adds support for non-x86 Linux such as ARM (also untested, but assuming it will work same as on x86).

Performance

Replacing native code with Python code creates some performance impact. On my Linux machine (i7-6500U, approx 5 years old) current 'develop' branch uses 27% CPU to run can.logger at maximum message rate (~7800 messages/sec at 1Mbps bitrate). On this branch, it increases to 38% CPU. Logging two channels simultaneously at maximum rate is not possible on 'develop', but on this branch CPU usage is 44%.

My method to measure this is pretty unscientific: time timeout 20 python -m can.logger -i canalystii -c 0 -b 1000000 -f spammer.txt and then use this script to check no messages were dropped in the log file. Sender is this Arduino Sketch.

[codecov]

Codecov Report

Merging #1127 (2902d09) into develop (bb78312) will increase coverage by 0.44%.
The diff coverage is 77.27%.

❗ Current head 2902d09 differs from pull request most recent head 69b70db. Consider uploading reports for the commit 69b70db to get more accurate results

@@             Coverage Diff             @@
##           develop    #1127      +/-   ##
===========================================
+ Coverage    70.48%   70.93%   +0.44%     
===========================================
  Files           79       79              
  Lines         7684     7675       -9     
===========================================
+ Hits          5416     5444      +28     
+ Misses        2268     2231      -37     

[projectgus]

@hardbyte I added some tests for the canalystii interface (mocking at the driver interface layer), when you have a minute could you please trigger another workflow run?

(Alternatively, if you're amenable to it, you could merge my approved smaller PR #1114 and I'll rebase this branch onto it - then I won't be a first-time contributor and workflows should trigger automatically. 🙂.)

[hardbyte]

Have squashed your other PR. Appreciated it if if you wouldn't mind rebasing this one

[projectgus]

@hardbyte Thanks! Have rebased and squashed the commits in this branch, too.

[projectgus]

@hardbyte @felixdivo I know you're both probably busy and that's perfectly understandable, but is there anything else I can do from my side to assist with this PR?

[felixdivo]

Hey @projectgus, thanks for bringing this up so politely, really! I kinda slipped away from this project over the last months but I will definitely do a review of this PR within a week, maybe this weekend.

Maybe we could also finally manage a 4.0.0 release soon, so you could install your changes via the stable channel of the library too. Basically only #1046 is missing but it was much more work than anticipated.

[projectgus]

I kinda slipped away from this project over the last months but I will definitely do a review of this PR within a week, maybe this weekend.

@felixdivo Hey, no worries, I know how this can happen. I'm not in a particular rush to get this landed, but I know it's also possible to get in the situation where everyone is waiting on something else to happen and then it slips off the radar. :)

Basically only 1046 is missing but it was much more work than anticipated.

I've pushed a commit here to change the one instance of CanError in canalystii.py to CanTimeoutError, in line with that issue. 👍

[projectgus]

Hmm, I should have this PR alone. 😆

https://github.com/hardbyte/python-can/runs/3986308723?check_suite_focus=true

Run actions/setup-python@v2
Error: PyPy 3.6 not found

This looks like a change in the macos runner image...?

EDIT: Fixed here #1143

[felixdivo]

Replacing native code with Python code creates some performance impact

I suppose this is absolutely fine.

[felixdivo]

Can't we add #726 to the list of issues fixed by this PR?

Overall, a nice piece of code!

[projectgus]

@felixdivo Thanks for the review! Have made all the suggested changes apart from not raising ValueError for invalid arguments, see above.

[felixdivo]

We should be good after this. You also tested the interface locally?

[felixdivo]

We don't strictly require typing annotations, but if your want, you are welcome to add them. If not, we can also just merge this.

[felixdivo]

Ah, and thank you for completeing #1088 for canalystii.

[hardbyte]

Let me also add my thanks for your patience and work preparing this.

The changes look good to me and unless we hear from anyone who would be put out by the performance hit I agree its okay. If you can take a look at using the BitTiming class that would help to bring the Bus.__init__ method more inline with other interfaces.

[projectgus]

@hardbyte @felixdivo Thanks for the thoughtful review comments. I think this is up to date now!

You also tested the interface locally?

Yes, I've been using my Canalyst-II to do some RE work on a 2019 model car (so lots of CAN buses, lots of CAN traffic). Mostly logging but also some interactions using the isotp package. Also, in the lower level driver layer I have some tests that use a real interface (loopback tests, and also one that captures specific messages sent from an Arduino Due).

I did an on-car smoke test just now with the updated patch and everything seems to work as expected. Feel free to tag me in if anyone raises an issue related to this interface in the future, will try to help.