Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Windows support for the Systec USB-CANmodul vendor to the CAN module library. It introduces a new vendor implementation that uses the Systec Windows DLL (USBCAN64.lib/dll) to communicate with Systec USB-CAN hardware on Windows platforms, complementing the existing Linux SocketCAN Systec implementation.
Changes:
- Added Windows-specific Systec vendor implementation with receive thread handling
- Integrated Systec vendor into device factory with Windows platform guards
- Configured CMake build system to include Systec headers and libraries on Windows
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/CanVendorSystec.cpp | Core implementation of Windows Systec vendor with USB-CAN API integration |
| src/include/CanVendorSystec.h | Header defining CanVendorSystec class with Windows threading primitives |
| src/main/CanDevice.cpp | Added conditional compilation to register Systec vendor on Windows |
| CMakeLists.txt | Added Systec source files to Windows build configuration |
| cmake/systec.cmake | CMake configuration for Systec USB-CANmodul library paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Dear @parasxos , Currently, the project no longer compiles. Besides all the noise from the copilot (with a significant amount of them being relevant), I see two big points (I did not enter into details of the implementation yet):
Thanks and regards, Luis |
I have some changes on my local environment which provide the necessary header and library paths to compile, but with the expectation that USBCAN-modul Utility Disk is already installed, I am looking into ways to install it through cmake though I haven't yet figured out how to do this non-graphically.
I agree and will investigate, largely this code is adapted from an older branch of CanModule
I'm not aware of a version of the utility that comes with a static build but I will look into it |
86b7d2a to
14dcce4
Compare
use actual systec error code names for diagnostics.state
small cleanups
14dcce4 to
8d1624b
Compare
luismiguensfernandez
left a comment
There was a problem hiding this comment.
Hi, thanks for your changes. I scan a bit, and I have some comments
| file(COPY "${anagate_SOURCE_DIR}/Win64/AnaGateCan64.dll" | ||
| DESTINATION "${CMAKE_BINARY_DIR}/Release") | ||
|
|
||
| if ("${CANMODULE_BUILD_SYSTEC_WINDOWS}" STREQUAL ON) |
There was a problem hiding this comment.
Is systec planned to be supported also in the power supplies and/or several OPC Servers? or is it only a single-case? The answer will tell us if having a variable to activate that part of the build is good or bad idea.
| # reason="This test module is only for Windows environments.", | ||
| # ) | ||
|
|
||
| pytestmark = pytest.mark.skipif( |
There was a problem hiding this comment.
If your activities are planned to be merged into the master, we will need to set up the machine and a 2-port system in our lab.
* parse bus_number in systec * update uptime comment * use string_view for systec error message
… systec register callback for connect control add error messages to log_entries in diagnostics
ab8e721 to
9362dba
Compare
4d93302 to
b53784c
Compare
No description provided.