Add new SYSTEC interface with docu#466
Conversation
|
Thanks for the PR! As a heads up it can take a while to get newly contributed interfaces reviewed and merged in - although after a brief look this looks great. Not often docs come from the start! Any how I'll try spend a few hours this weekend or next but if anyone else wants to do a code review please jump in. |
|
I have checked the failed checks. The issue is that the driver library was not installed as described in the documentation. My question is, should there be an ImportError thrown when configuration detection is executed and the library was not found? Or an empy configuration should be returned? Or? |
There was a problem hiding this comment.
Overall this looks good to merge once the C.I system approves.
| Initializes the device with the corresponding serial or device number. | ||
|
|
||
| :param int or None serial: Serial number of the USB-CANmodul. | ||
| :param int device_number: Device number (0 – 254, or :const:`ANY_MODULE` for the first device). |
There was a problem hiding this comment.
There is an encoding error here.
ucanbus.py 35 WARNING Cannot load SYSTEC ucan library: Non-ASCII character '\xe2' in file C:\projects\python-can\can\interfaces\systec\ucan.py on line 350, but no encoding declared; see http://python.org/dev/peps/pep-0263/ for details (ucan.py, line 349)
can/interfaces/systec/ucanbus.py
8000
Outdated
| @staticmethod | ||
| def _detect_available_configs(): | ||
| configs = {} | ||
| for index, is_used, hw_info_ex, init_info in Ucan.enumerate_hardware(): |
There was a problem hiding this comment.
This static method has to work even if Ucan isn't defined
|
Personally I like the approach in |
Codecov Report
@@ Coverage Diff @@
## develop #466 +/- ##
===========================================
+ Coverage 63.29% 63.88% +0.58%
===========================================
Files 57 63 +6
Lines 4784 5615 +831
===========================================
+ Hits 3028 3587 +559
- Misses 1756 2028 +272 |
- The function needs to succeeded even if the driver was not found or not loaded.
|
Hi, what is the "Codecov Report"? It has something to do with unittesting? |
|
Yeah codecov measures how much code was executed by the ci system (travis-ci). For this PR travis ran these unit tests and pushed the code coverage to https://codecov.io/gh/hardbyte/python-can/pull/466/diff In this case as the interface has no unittests it just shows the lines with definitions as green - i.e. what lines were imported before the It isn't a required check before merging, just applies a gentle pressure to encourage tested code! We ensure the core of python-can is well tested, but as the interfaces are all individual contributions they are tested at different levels. If you're willing it certainly would be better to add a few tests that don't require hardware (or the |
|
I have tried to add tests for the interface. The interface is only for Windows platform, it fails on the CI, cause the structures use wintypes and some of them also WINFUNCTYPEs which do not exist under Linux. Any ideas how to fix this? |
|
Ok, I have added a workaround to allow the Unix based testing system use the interface for testing purposes. |
|
Thanks for the contribution! |
No description provided.