From f85ea523f6dbda67967fd76c3b91c1d3ced26521 Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 19 Feb 2026 14:15:43 +0100 Subject: [PATCH 01/15] Outline of Systec implementation for Windows add systec.cmake --- CMakeLists.txt | 18 +- cmake/systec.cmake | 40 ++++ src/include/CanDiagnostics.h | 2 +- src/include/CanVendorSystec.h | 55 +++++ src/main/CanDevice.cpp | 7 + src/main/CanVendorSystec.cpp | 438 ++++++++++++++++++++++++++++++++++ 6 files changed, 557 insertions(+), 3 deletions(-) create mode 100644 cmake/systec.cmake create mode 100644 src/include/CanVendorSystec.h create mode 100644 src/main/CanVendorSystec.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 045c05cc..a80feb3a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,6 +36,13 @@ if (UNIX) src/main/CanVendorSocketCan.cpp src/main/CanVendorSocketCanSystec.cpp ) +elseif ("${CANMODULE_BUILD_SYSTEC_WINDOWS}" STREQUAL ON) + include(cmake/systec.cmake) + include_directories(${SYSTEC_PATH_HEADERS}) + list(APPEND VENDOR_SOURCES + src/main/CanVendorSystec.cpp + ) + add_compile_definitions(CANMODULE_BUILD_SYSTEC_WINDOWS) endif() if (NOT DEFINED CAN_MODULE_MAIN_ONLY) @@ -63,11 +70,18 @@ if (UNIX) libsocketcan ) else() + target_include_directories(CanModuleMain PUBLIC ${systec_BINARY_DIR}/Examples/Include) target_link_libraries(CanModuleMain PUBLIC - ${anagate_SOURCE_DIR}/Win64/AnaGateCanDll64.lib - ) + ${anagate_SOURCE_DIR}/Win64/AnaGateCanDll64.lib) file(COPY "${anagate_SOURCE_DIR}/Win64/AnaGateCan64.dll" DESTINATION "${CMAKE_BINARY_DIR}/Release") + + if ("${CANMODULE_BUILD_SYSTEC_WINDOWS}" STREQUAL ON) + target_link_libraries(CanModuleMain PUBLIC + ${systec_BINARY_DIR}/Examples/lib/USBCAN64.lib) + file(COPY "${systec_BINARY_DIR}/Examples/lib/USBCAN64.dll" + DESTINATION "${CMAKE_BINARY_DIR}/Release") + endif() endif() if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") diff --git a/cmake/systec.cmake b/cmake/systec.cmake new file mode 100644 index 00000000..451d83a5 --- /dev/null +++ b/cmake/systec.cmake @@ -0,0 +1,40 @@ +include(FetchContent) + +if(NOT DEFINED SYSTEC_LIBRARY) + set(SYSTEC_LIBRARY "https://www.systec-electronic.com/media/default/Redakteur/produkte/Interfaces_Gateways/sysWORXX_USB_CANmodul_Series/Downloads/SO-387.zip") +endif() + +set(SYSTEC_FETCHCONTENT_ARGS + URL "${SYSTEC_LIBRARY}" + DOWNLOAD_EXTRACT_TIMESTAMP True +) + +if(EXISTS "${SYSTEC_LIBRARY}") + message(STATUS "Using local Systec archive: ${SYSTEC_LIBRARY}") +elseif(SYSTEC_LIBRARY MATCHES "^https?://") + message(STATUS "Downloading Systec archive: ${SYSTEC_LIBRARY}") +else() + message(FATAL_ERROR "SYSTEC_LIBRARY must be an existing local archive or an http(s) URL. Got: ${SYSTEC_LIBRARY}") +endif() + +FetchContent_Declare( + Systec + ${SYSTEC_FETCHCONTENT_ARGS} +) + +FetchContent_MakeAvailable(Systec) + +execute_process( + COMMAND ${systec_SOURCE_DIR}/SO-387.exe /SP- /VERYSILENT /DIR=${systec_BINARY_DIR} /LOG=${systec_SOURCE_DIR}/build.log + WORKING_DIRECTORY ${systec_SOURCE_DIR} + RESULT_VARIABLE systec_build_result + OUTPUT_VARIABLE systec_build_output +) + +if(NOT systec_build_result EQUAL 0) + message(FATAL_ERROR "Error installing USB-CANmodul Utility Disk: ${systec_build_output}") +endif() + +if (WIN32) + add_compile_definitions(WIN32) +endif() diff --git a/src/include/CanDiagnostics.h b/src/include/CanDiagnostics.h index a0218620..5c45f502 100644 --- a/src/include/CanDiagnostics.h +++ b/src/include/CanDiagnostics.h @@ -47,7 +47,7 @@ struct CanDiagnostics { std::optional temperature; ///< Optional temperature reading for Anagate devices. - std::optional uptime; ///< Optional uptime for Anagate devices. + std::optional uptime; ///< Optional uptime in seconds. std::optional tcp_rx; ///< Optional TCP Received counter for ///< both SocketCAN and Anagate devices. diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h new file mode 100644 index 00000000..8be61f06 --- /dev/null +++ b/src/include/CanVendorSystec.h @@ -0,0 +1,55 @@ +#ifndef SRC_INCLUDE_CANVENDORSYSTEC_H_ +#define SRC_INCLUDE_CANVENDORSYSTEC_H_ + +#include +#include +#include "tchar.h" +#include "Winsock2.h" +#include "windows.h" +#include +#include "usbcan32.h" +#include +#include +#include //NOLINT +#include "CanDiagnostics.h" +#include "CanVendorLoopback.h" +#include "CanDevice.h" +#include + +/** + * @struct CanVendorSystec + * @brief Represents a specific implementation of a CanDevice for Systec devices + * on Windows utilising libraries from USB-CANmodul Utility Disk. + * + * This struct provides methods for opening, closing, sending, and receiving CAN + * frames using the Systec CAN-over-USB interface. It also provides diagnostics + * information. + */ +struct CanVendorSystec : CanDevice { + explicit CanVendorSystec(const CanDeviceArguments& args); + ~CanVendorSystec() { vendor_close(); } + static DWORD WINAPI SystecRxThread(LPVOID pCanVendorSystec); + + private: + std::atomic m_receive_thread_flag = true; + tUcanHandle m_UcanHandle; + int m_module_number; + int m_channel_number; + HANDLE m_receive_thread_handle; + DWORD m_receive_thread_id; + CanReturnCode vendor_open() noexcept override; + CanReturnCode vendor_close() noexcept override; + CanReturnCode vendor_send(const CanFrame& frame) noexcept override; + CanDiagnostics vendor_diagnostics() noexcept override; + + CanReturnCode init_can_port(); + static std::mutex m_handles_lock; + static std::unordered_map m_handle_map; + + inline void map_module_to_handle(int module, tUcanHandle handle) { m_handle_map[module] = handle; } + inline int erase_module_handle(int module) { return m_handle_map.erase(module); } + + std::string UsbCanGetErrorText( long err_code ); +}; + +#endif // SRC_INCLUDE_CANVENDORSYSTEC_H_ diff --git a/src/main/CanDevice.cpp b/src/main/CanDevice.cpp index 204b70ea..34d6d816 100644 --- a/src/main/CanDevice.cpp +++ b/src/main/CanDevice.cpp @@ -13,6 +13,8 @@ #ifndef _WIN32 #include "CanVendorSocketCan.h" #include "CanVendorSocketCanSystec.h" +#elif defined(CANMODULE_BUILD_SYSTEC_WINDOWS) +#include "CanVendorSystec.h" #endif /** @@ -163,6 +165,11 @@ std::unique_ptr CanDevice::create( LOG(Log::DBG, CanLogIt::h()) << "Creating SocketCAN Systec CAN device"; return std::make_unique(configuration); } +#elif defined(CANMODULE_BUILD_SYSTEC_WINDOWS) + if (vendor == "systec") { + LOG(Log::DBG, CanLogIt::h()) << "Creating Systec CAN device for Windows"; + return std::make_unique(configuration); + } #endif if (vendor == "anagate") { diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp new file mode 100644 index 00000000..cbc103be --- /dev/null +++ b/src/main/CanVendorSystec.cpp @@ -0,0 +1,438 @@ +#include "CanVendorSystec.h" + +#include +#include +#include + +std::mutex CanVendorSystec::m_handles_lock; +std::unordered_map CanVendorSystec::m_handle_map; + +CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args) + : CanDevice("systec", args) { + if (!args.config.bus_name.has_value()) { + throw std::invalid_argument("Missing required configuration parameters"); + } + + // TODO trim possible can prefix use hardcoded value + int handle_number = std::stoi(args.config.bus_name.value()); + m_module_number = handle_number / 2; + m_channel_number = handle_number % 2; +} + +// TODO should we make this noexcept? how can we guarantee that? +CanReturnCode CanVendorSystec::init_can_port() { + BYTE systec_call_return = USBCAN_SUCCESSFUL; + tUcanHandle can_module_handle; + + unsigned int baud_rate = USBCAN_BAUD_125kBit; + switch (args().config.bitrate.value_or(0)) { + case 50000: baud_rate = USBCAN_BAUD_50kBit; break; + case 100000: baud_rate = USBCAN_BAUD_100kBit; break; + case 125000: baud_rate = USBCAN_BAUD_125kBit; break; + case 250000: baud_rate = USBCAN_BAUD_250kBit; break; + case 500000: baud_rate = USBCAN_BAUD_500kBit; break; + case 1000000: baud_rate = USBCAN_BAUD_1MBit; break; + default: { + LOG(Log::WRN, CanLogIt::h()) << "baud rate illegal, taking default 125000 [" << baud_rate << "]"; + } + } + + tUcanInitCanParam initialization_parameters; + initialization_parameters.m_dwSize = sizeof(initialization_parameters); // size of this struct + initialization_parameters.m_bMode = kUcanModeNormal; // normal operation mode + initialization_parameters.m_bBTR0 = HIBYTE( baud_rate ); // baudrate + initialization_parameters.m_bBTR1 = LOBYTE( baud_rate ); + initialization_parameters.m_bOCR = 0x1A; // standard output + initialization_parameters.m_dwAMR = USBCAN_AMR_ALL; // receive all CAN messages + initialization_parameters.m_dwACR = USBCAN_ACR_ALL; + initialization_parameters.m_dwBaudrate = USBCAN_BAUDEX_USE_BTR01; + initialization_parameters.m_wNrOfRxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES; + initialization_parameters.m_wNrOfTxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES; + + // check if USB-CANmodul already is initialized + std::lock_guard guard(CanVendorSystec::m_handles_lock); + auto pos = m_handle_map.find(m_module_number); + if (pos == m_handle_map.end()) { // module not in use + systec_call_return = UcanInitHardwareEx(&can_module_handle, m_module_number, 0, 0); + if (systec_call_return != USBCAN_SUCCESSFUL ) { + LOG(Log::ERR, CanLogIt::h()) << "UcanInitHardwareEx, return code = [ 0x" << std::hex << (int) systec_call_return << std::dec << "]"; + UcanDeinitHardware(can_module_handle); + return CanReturnCode::unknown_open_error; + } + map_module_to_handle(m_module_number, can_module_handle); + } else { // find existing handle of module + can_module_handle = pos->second; + LOG(Log::WRN, CanLogIt::h()) << "trying to open a can port which is in use, reuse handle, skipping UCanDeinitHardware"; + } + + systec_call_return = UcanInitCanEx2(can_module_handle, m_channel_number, &initialization_parameters); + if ( systec_call_return != USBCAN_SUCCESSFUL ) { + LOG(Log::ERR, CanLogIt::h()) << "UcanInitCanEx2, return code = [ 0x" << std::hex << (int) systec_call_return << std::dec << "]"; + UcanDeinitCanEx(can_module_handle, m_channel_number); + return CanReturnCode::unknown_open_error; + } + + m_UcanHandle = can_module_handle; + LOG(Log::INF, CanLogIt::h()) << "Successfully opened CAN port on module " << m_module_number << ", channel " << m_channel_number; + return CanReturnCode::success; +} + +CanReturnCode CanVendorSystec::vendor_open() noexcept { + + auto returnCode = init_can_port(); + if (returnCode != CanReturnCode::success) return returnCode; + + // TODO set time since opened equivalent... + // m_statistics.setTimeSinceOpened(); + + // After the canboard is configured and started, we start the scan control thread + m_receive_thread_flag = true; + m_receive_thread_handle = CreateThread(NULL, 0, SystecRxThread, this, 0, &m_receive_thread_id); + + if (NULL == m_receive_thread_handle) { + LOG(Log::ERR, CanLogIt::h()) << "Error creating the canScanControl thread."; + return CanReturnCode::internal_api_error; + } + + return returnCode; +} + +CanReturnCode CanVendorSystec::vendor_close() noexcept { + // TODO what if the return code is not success? + std::lock_guard guard(CanVendorSystec::m_handles_lock); + erase_module_handle(m_module_number); + m_receive_thread_flag = false; + DWORD result = WaitForSingleObject(m_receive_thread_handle, INFINITE); //Shut down can scan thread + UcanDeinitCanEx (m_UcanHandle, (BYTE)m_channel_number); + LOG(Log::DBG, CanLogIt::h()) << __FUNCTION__ << " closed successfully"; + return CanReturnCode::success; +}; + +CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { + // bool CanVendorSystec::sendMessage(short cobID, unsigned char len, unsigned char *message, bool rtr) + bool rtr = frame.is_remote_request(); + uint32_t len = frame.length(); + char *message = frame.message().data(); + short cobID = frame.id(); + + LOG(Log::DBG, CanLogIt::h()) << "Sending message: [" << ( message == 0 ? "" : (const char *) message) << "], cobID: [" << cobID << "], Message Length: [" << static_cast(len) << "]"; + + tCanMsgStruct can_msg_to_send; + BYTE Status; + + can_msg_to_send.m_dwID = cobID; + can_msg_to_send.m_bDLC = len; + can_msg_to_send.m_bFF = 0; + if (rtr) { + can_msg_to_send.m_bFF = USBCAN_MSG_FF_RTR; + } + int message_length_to_process; + //If there is more than 8 characters to process, we process 8 of them in this iteration of the loop + if (len > 8) { + message_length_to_process = 8; + LOG(Log::DBG, CanLogIt::h()) << "The length is more then 8 bytes, adjust to 8, ignore >8. len= " << len; + } else { + //Otherwise if there is less than 8 characters to process, we process all of them in this iteration of the loop + message_length_to_process = len; + if (len < 8) { + LOG(Log::DBG, CanLogIt::h())<< "The length is less then 8 bytes, process only. len= " << len; + } + } + can_msg_to_send.m_bDLC = message_length_to_process; + memcpy(can_msg_to_send.m_bData, message, message_length_to_process); + // MLOG(TRC,this) << "Channel Number: [" << m_channel_number << "], cobID: [" << can_msg_to_send.m_dwID << "], Message Length: [" << static_cast(can_msg_to_send.m_bDLC) << "]"; + Status = UcanWriteCanMsgEx(m_UcanHandle, m_channel_number, &can_msg_to_send, NULL); + if (Status != USBCAN_SUCCESSFUL) { + LOG(Log::ERR, CanLogIt::h()) << "There was a problem when sending a message: " + << UsbCanGetErrorText(Status); + + // for now, just always reconnect on a failed send. + vendor_close(); // TODO maybe we just call close instead of vendor_close + // see how CanVendorSocketCanSystec does reconnects, it intercepts the receiver function and wraps it + vendor_open(); + + switch (Status) { + case USBCAN_ERR_CANNOTINIT: + case USBCAN_ERR_ILLHANDLE: return CanReturnCode::disconnected; + case USBCAN_ERR_DLL_TXFULL: return CanReturnCode::tx_buffer_overflow; + case USBCAN_ERR_MAXINSTANCES: return CanReturnCode::too_many_connections; + case USBCAN_ERR_ILLPARAM: + case USBCAN_ERR_ILLHW: + case USBCAN_ERR_ILLCHANNEL: + case USBCAN_WARN_TXLIMIT: + case USBCAN_WARN_FW_TXOVERRUN: + default: return CanReturnCode::unknown_send_error; + } + // m_statistics.onTransmit( can_msg_to_send.m_bDLC ); + // m_statistics.setTimeSinceTransmitted(); + } + return CanReturnCode::success; +}; + +CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { + + // TODO we can read the operating mode, either kUcanModeNormal, ListenOnly or TxEcho + CanDiagnostics diagnostics{}; + tStatusStruct status; + // TODO check return code of these functions... + UcanGetStatusEx(m_UcanHandle, m_channel_number, &status); + WORD can_status = status.m_wCanStatus; + switch (can_status) { + case USBCAN_CANERR_OK: + diagnostics.state = "USBCAN_CANERR_OK"; break; + case USBCAN_CANERR_XMTFULL: + diagnostics.state = "USBCAN_CANERR_XMTFULL"; break; + case USBCAN_CANERR_OVERRUN: + diagnostics.state = "USBCAN_CANERR_OVERRUN"; break; + case USBCAN_CANERR_BUSLIGHT: + diagnostics.state = "USBCAN_CANERR_BUSLIGHT"; break; + case USBCAN_CANERR_BUSHEAVY: + diagnostics.state = "USBCAN_CANERR_BUSHEAVY"; break; + case USBCAN_CANERR_BUSOFF: + diagnostics.state = "USBCAN_CANERR_BUSOFF"; break; + case USBCAN_CANERR_QOVERRUN: + diagnostics.state = "USBCAN_CANERR_QOVERRUN"; break; + case USBCAN_CANERR_QXMTFULL: + diagnostics.state = "USBCAN_CANERR_QXMTFULL"; break; + case USBCAN_CANERR_REGTEST: + diagnostics.state = "USBCAN_CANERR_REGTEST"; break; + case USBCAN_CANERR_TXMSGLOST: + diagnostics.state = "USBCAN_CANERR_TXMSGLOST"; break; + } + + tUcanMsgCountInfo msg_count_info; + UcanGetMsgCountInfoEx(m_UcanHandle, m_channel_number, &msg_count_info); + diagnostics.tx = msg_count_info.m_wSentMsgCount; + diagnostics.rx = msg_count_info.m_wRecvdMsgCount; + + DWORD tx_error, rx_error; + UcanGetCanErrorCounter(m_UcanHandle, m_channel_number, &tx_error, &rx_error); + diagnostics.tx_error = tx_error; + diagnostics.rx_error = rx_error; + + tUcanHardwareInfo hw_info; + if (UcanGetHardwareInfo(m_UcanHandle, &hw_info) != USBCAN_SUCCESSFUL) + diagnostics.mode = "OFFLINE"; + else switch (hw_info.m_bMode) { + case kUcanModeNormal: + diagnostics.mode = "NORMAL"; break; + case kUcanModeListenOnly: + diagnostics.mode = "LISTEN_ONLY"; break; + case kUcanModeTxEcho: + diagnostics.mode = "LOOPBACK"; break; + } + + DWORD module_time; // in ms + UcanGetModuleTime(m_UcanHandle, &module_time); + diagnostics.uptime = (uint32_t) module_time / 1000; + + return diagnostics; +}; + + +/** + * thread to handle reception of Can messages from the systec device + */ +DWORD WINAPI CanVendorSystec::SystecRxThread(LPVOID pCanVendorSystec) +{ + BYTE status; + tCanMsgStruct read_can_message; + CanVendorSystec *vendor_pointer = reinterpret_cast(pCanVendorSystec); + LOG(Log::DBG, CanLogIt::h()) << "SystecRxThread Started. m_receive_thread_flag = [" << vendor_pointer->m_receive_thread_flag <<"]"; + while (vendor_pointer->m_receive_thread_flag) { + status = UcanReadCanMsgEx(vendor_pointer->m_UcanHandle, (BYTE *)&vendor_pointer->m_channel_number, &read_can_message, NULL); + switch (status) { + case USBCAN_WARN_SYS_RXOVERRUN: + case USBCAN_WARN_DLL_RXOVERRUN: + case USBCAN_WARN_FW_RXOVERRUN: + LOG(Log::WRN, CanLogIt::h()) << UsbCanGetErrorText(status); + [[ fallthrough ]]; + case USBCAN_SUCCESSFUL: { + if (read_can_message.m_bFF & USBCAN_MSG_FF_RTR) break; + // can_msg_copy.c_time = convertTimepointToTimeval(currentTimeTimeval()); + std::vector data(read_can_message.m_bData, read_can_message.m_bData + read_can_message.m_bDLC); + // id, data, flags + CanFrame can_msg_copy(read_can_message.m_dwID, data, read_can_message.m_bFF); + // TODO the read_can_message contains a DWORD m_dwTime "receipt time in ms" + vendor_pointer->received(can_msg_copy); + // vendor_pointer->m_statistics.onReceive( read_can_message.m_bDLC ); + // vendor_pointer->m_statistics.setTimeSinceReceived(); + + // we can reset the reconnectionTimeout here, since we have received a message + // vendor_pointer->resetTimeoutOnReception(); + break; + } + case USBCAN_WARN_NODATA: + LOG(Log::TRC, CanLogIt::h()) << UsbCanGetErrorText(status); + // TODO is it correct to sleep here? + // Sleep(100); // ms + break; + default: // errors + // USBCAN_ERR_MAXINSTANCES, USBCAN_ERR_ILLHANDLE, USBCAN_ERR_CANNOTINIT, + // USBCAN_ERR_ILLPARAM, USBCAN_ERR_ILLHW, USBCAN_ERR_ILLCHANNEL + // TODO should we raise some error state here? + LOG(Log::ERR, CanLogIt::h()) << UsbCanGetErrorText(status); + break; + } + } + + ExitThread(0); + return 0; +} + +std::string CanVendorSystec::UsbCanGetErrorText( long err_code ) { + switch( err_code ){ + case USBCAN_SUCCESSFUL: return("success"); + + case USBCAN_ERR_RESOURCE: return ("This error code returns if one resource could not be generated. In this " + "case the term resource means memory and handles provided by the Windows OS"); + + case USBCAN_ERR_MAXMODULES: return("An application has tried to open more than 64 USB-CANmodul devices. " + "The standard version of the DLL only supports up to 64 USB-CANmodul " + "devices at the same time. This error also appears if several applications " + "try to access more than 64 USB-CANmodul devices. For example, " + "application 1 has opened 60 modules, application 2 has opened 4 " + "modules and application 3 wants to open a module. Application 3 " + "receives this error code."); + + case USBCAN_ERR_HWINUSE: return("An application tries to initialize an USB-CANmodul with the given device " + "number. If this module has already been initialized by its own or by " + "another application, this error code is returned."); + + case USBCAN_ERR_ILLVERSION: return("This error code returns if the firmware version of the USB-CANmodul is " + "not compatible to the software version of the DLL. In this case, install " + "the latest driver for the USB-CANmodul. Furthermore make sure that " + "the latest firmware version is programmed to the USB-CANmodul."); + + case USBCAN_ERR_ILLHW: return("This error code returns if an USB-CANmodul with the given device " + "number is not found. If the function UcanInitHardware() or " + "UcanInitHardwareEx() has been called with the device number " + "USBCAN_ANY_MODULE, and the error code appears, it indicates that " + "no module is connected to the PC or all connected modules are already " + "in use."); + + case USBCAN_ERR_ILLHANDLE: return("This error code returns if a function received an incorrect USBCAN " + "handle. The function first checks which USB-CANmodul is related to this " + "handle. This error occurs if no device belongs this handle."); + + case USBCAN_ERR_ILLPARAM: return("This error code returns if a wrong parameter is passed to the function. " + "For example, the value NULL has been passed to a pointer variable " + "instead of a valid address."); + + case USBCAN_ERR_BUSY: return("This error code occurs if several threads are accessing an " + "USB-CANmodul within a single application. After the other threads have " + "finished their tasks, the function may be called again."); + + case USBCAN_ERR_TIMEOUT: return("This error code occurs if the function transmits a command to the " + "USB-CANmodul but no reply is returned. To solve this problem, close " + "the application, disconnect the USB-CANmodul, and connect it again."); + + case USBCAN_ERR_IOFAILED: return("This error code occurs if the communication to the kernel driver was " + "interrupted. This happens, for example, if the USB-CANmodul is " + "disconnected during transferring data or commands to the " + "USB-CANmodul."); + + case USBCAN_ERR_DLL_TXFULL: return("The function UcanWriteCanMsg() or UcanWriteCanMsgEx() first checks " + "if the transmit buffer within the DLL has enough capacity to store new " + "CAN messages. If the buffer is full, this error code returns. The CAN " + "message passed to these functions will not be written into the transmit " + "buffer in order to protect other CAN messages against overwriting. The " + "size of the transmit buffer is configurable (refer to function " + "UcanInitCanEx() and structure tUcanInitCanParam)."); + + case USBCAN_ERR_MAXINSTANCES: return("A maximum amount of 64 applications are able to have access to the " + "DLL. If more applications attempting to access to the DLL, this error " + "code is returned. In this case, it is not possible to use an " + "USB-CANmodul by this application."); + + case USBCAN_ERR_CANNOTINIT: return("This error code returns if an application tries to call an API function " + "which only can be called in software state CAN_INIT but the current " + "software is still in state HW_INIT. Refer to section 4.3.1 and Table 11 for " + "detailed information."); + + case USBCAN_ERR_DISCONNECT: return("This error code occurs if an API function was called for an " + "USB-CANmodul that was plugged-off from the computer recently."); + + case USBCAN_ERR_ILLCHANNEL: return("This error code is returned if an extended function of the DLL is called " + "with parameter bChannel_p = USBCAN_CHANNEL_CH1, but a single-channel USB-CANmodul was used."); + + case USBCAN_ERR_ILLHWTYPE: return("This error code occurs if an extended function of the DLL was called for " + "a hardware which does not support the feature."); + + case USBCAN_ERRCMD_NOTEQU: return("This error code occurs during communication between the PC and an " + "USB-CANmodul. The PC sends a command to the USB-CANmodul, " + "then the module executes the command and returns a response to the " + "PC. This error code returns if the reply does not correspond to the command."); + + case USBCAN_ERRCMD_REGTST: return("The software tests the CAN controller on the USB-CANmodul when the " + "CAN interface is initialized. Several registers of the CAN controller are " + "checked. This error code returns if an error appears during this register test."); + + case USBCAN_ERRCMD_ILLCMD: return("This error code returns if the USB-CANmodul receives a non-defined " + "command. This error represents a version conflict between the firmware in the USB-CANmodul and the DLL."); + + case USBCAN_ERRCMD_EEPROM: return("The USB-CANmodul has a built-in EEPROM. This EEPROM contains " + "several configurations, e.g. the device number and the serial number. If " + "an error occurs while reading these values, this error code is returned."); + + case USBCAN_ERRCMD_ILLBDR: return("The USB-CANmodul has been initialized with an invalid baud rate (refer " + "to section 4.3.4)."); + + case USBCAN_ERRCMD_NOTINIT: return("It was tried to access a CAN-channel of a multi-channel " + "USB-CANmodul that was not initialized."); + + case USBCAN_ERRCMD_ALREADYINIT: return("The accessed CAN-channel of a multi-channel USB-CANmodul was " + "already initialized"); + + case USBCAN_ERRCMD_ILLSUBCMD: return("An internal error occurred within the DLL. In this case an unknown sub- " + "command was called instead of a main command (e.g. for the cyclic CAN message-feature)."); + + case USBCAN_ERRCMD_ILLIDX: return("An internal error occurred within the DLL. In this case an invalid index " + "for a list was delivered to the firmware (e.g. for the cyclic CAN message-feature)."); + + case USBCAN_ERRCMD_RUNNING: return("The caller tries to define a new list of cyclic CAN messages but this " + "feature was already started. For defining a new list, it is necessary to stop the feature beforehand."); + + case USBCAN_WARN_NODATA: return("If the function UcanReadCanMsg() or UcanReadCanMsgEx() returns " + "with this warning, it is an indication that the receive buffer contains no CAN messages."); + + case USBCAN_WARN_SYS_RXOVERRUN: return("This is returned by UcanReadCanMsg() or UcanReadCanMsgEx() if the " + "receive buffer within the kernel driver runs over. The function " + "nevertheless returns a valid CAN message. It also indicates that at least " + "one CAN message are lost. However, it does not indicate the position of the lost CAN messages."); + + case USBCAN_WARN_DLL_RXOVERRUN: return("The DLL automatically requests CAN messages from the " + "USB-CANmodul and stores the messages into a buffer of the DLL. If " + "more CAN messages are received than the DLL buffer size allows, this " + "error code returns and CAN messages are lost. However, it does not " + "indicate the position of the lost CAN messages. The size of the receive " + "buffer is configurable (refer to function UcanInitCanEx() and structure " + "tUcanInitCanParam)."); + + case USBCAN_WARN_FW_TXOVERRUN: return("This warning is returned by function UcanWriteCanMsg() or " + "UcanWriteCanMsgEx() if flag USBCAN_CANERR_QXMTFULL is set in " + "the CAN driver status. However, the transmit CAN message could be " + "stored to the DLL transmit buffer. This warning indicates that at least " + "one transmit CAN message got lost in the device firmware layer. This " + "warning does not indicate the position of the lost CAN message."); + + case USBCAN_WARN_FW_RXOVERRUN: return("This warning is returned by function UcanWriteCanMsg() or " + "UcanWriteCanMsgEx() if flag USBCAN_CANERR_QOVERRUN or flag " + "USBCAN_CANERR_OVERRUN are set in the CAN driver status. The " + "function has returned with a valid CAN message. This warning indicates " + "that at least one received CAN message got lost in the firmware layer. " + "This warning does not indicate the position of the lost CAN message."); + + case USBCAN_WARN_NULL_PTR: return("This warning is returned by functions UcanInitHwConnectControl() or " + "UcanInitHwConnectControlEx() if a NULL pointer was passed as callback function address."); + + case USBCAN_WARN_TXLIMIT: return("This warning is returned by the function UcanWriteCanMsgEx() if it was " + "called to transmit more than one CAN message, but a part of them " + "could not be stored to the transmit buffer within the DLL (because the " + "buffer is full). The returned variable addressed by the parameter " + "pdwCount_p indicates the number of CAN messages which are stored " + "successfully to the transmit buffer."); + + default: return("unknown error code"); + } +} From 4ec6c6073bc098acafbfbfc9860e16dc5c967ca2 Mon Sep 17 00:00:00 2001 From: James Souter Date: Fri, 27 Feb 2026 17:02:00 +0100 Subject: [PATCH 02/15] replace Windows specific thread calls with std library calls replace broken memcpy with std::copy, some reformatting remove check on message length in systec vendor_send --- src/include/CanVendorSystec.h | 6 +-- src/main/CanVendorSystec.cpp | 70 ++++++++++++----------------------- 2 files changed, 26 insertions(+), 50 deletions(-) diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h index 8be61f06..d8e7d947 100644 --- a/src/include/CanVendorSystec.h +++ b/src/include/CanVendorSystec.h @@ -15,6 +15,7 @@ #include "CanVendorLoopback.h" #include "CanDevice.h" #include +#include /** * @struct CanVendorSystec @@ -28,15 +29,14 @@ struct CanVendorSystec : CanDevice { explicit CanVendorSystec(const CanDeviceArguments& args); ~CanVendorSystec() { vendor_close(); } - static DWORD WINAPI SystecRxThread(LPVOID pCanVendorSystec); + int SystecRxThread(); private: std::atomic m_receive_thread_flag = true; tUcanHandle m_UcanHandle; int m_module_number; int m_channel_number; - HANDLE m_receive_thread_handle; - DWORD m_receive_thread_id; + std::thread m_SystecRxThread; CanReturnCode vendor_open() noexcept override; CanReturnCode vendor_close() noexcept override; CanReturnCode vendor_send(const CanFrame& frame) noexcept override; diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index cbc103be..f3b876c7 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -1,8 +1,10 @@ #include "CanVendorSystec.h" +#include #include #include #include +#include std::mutex CanVendorSystec::m_handles_lock; std::unordered_map CanVendorSystec::m_handle_map; @@ -87,12 +89,13 @@ CanReturnCode CanVendorSystec::vendor_open() noexcept { // After the canboard is configured and started, we start the scan control thread m_receive_thread_flag = true; - m_receive_thread_handle = CreateThread(NULL, 0, SystecRxThread, this, 0, &m_receive_thread_id); + m_SystecRxThread = std::thread(&CanVendorSystec::SystecRxThread, this); - if (NULL == m_receive_thread_handle) { - LOG(Log::ERR, CanLogIt::h()) << "Error creating the canScanControl thread."; - return CanReturnCode::internal_api_error; - } + // todo reintroduce check here... + // if (NULL == m_receive_thread_handle) { + // LOG(Log::ERR, CanLogIt::h()) << "Error creating the canScanControl thread."; + // return CanReturnCode::internal_api_error; + // } return returnCode; } @@ -102,45 +105,27 @@ CanReturnCode CanVendorSystec::vendor_close() noexcept { std::lock_guard guard(CanVendorSystec::m_handles_lock); erase_module_handle(m_module_number); m_receive_thread_flag = false; - DWORD result = WaitForSingleObject(m_receive_thread_handle, INFINITE); //Shut down can scan thread - UcanDeinitCanEx (m_UcanHandle, (BYTE)m_channel_number); + if (m_SystecRxThread.joinable()) m_SystecRxThread.join(); + UcanDeinitCanEx (m_UcanHandle, (BYTE) m_channel_number); LOG(Log::DBG, CanLogIt::h()) << __FUNCTION__ << " closed successfully"; return CanReturnCode::success; }; CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { - // bool CanVendorSystec::sendMessage(short cobID, unsigned char len, unsigned char *message, bool rtr) - bool rtr = frame.is_remote_request(); - uint32_t len = frame.length(); - char *message = frame.message().data(); - short cobID = frame.id(); - - LOG(Log::DBG, CanLogIt::h()) << "Sending message: [" << ( message == 0 ? "" : (const char *) message) << "], cobID: [" << cobID << "], Message Length: [" << static_cast(len) << "]"; + std::vector message = frame.message(); tCanMsgStruct can_msg_to_send; BYTE Status; - can_msg_to_send.m_dwID = cobID; - can_msg_to_send.m_bDLC = len; + can_msg_to_send.m_dwID = frame.id(); + can_msg_to_send.m_bDLC = frame.length(); can_msg_to_send.m_bFF = 0; - if (rtr) { - can_msg_to_send.m_bFF = USBCAN_MSG_FF_RTR; - } - int message_length_to_process; - //If there is more than 8 characters to process, we process 8 of them in this iteration of the loop - if (len > 8) { - message_length_to_process = 8; - LOG(Log::DBG, CanLogIt::h()) << "The length is more then 8 bytes, adjust to 8, ignore >8. len= " << len; - } else { - //Otherwise if there is less than 8 characters to process, we process all of them in this iteration of the loop - message_length_to_process = len; - if (len < 8) { - LOG(Log::DBG, CanLogIt::h())<< "The length is less then 8 bytes, process only. len= " << len; - } + if (frame.is_remote_request()) { + can_msg_to_send.m_bFF = USBCAN_MSG_FF_RTR; } - can_msg_to_send.m_bDLC = message_length_to_process; - memcpy(can_msg_to_send.m_bData, message, message_length_to_process); - // MLOG(TRC,this) << "Channel Number: [" << m_channel_number << "], cobID: [" << can_msg_to_send.m_dwID << "], Message Length: [" << static_cast(can_msg_to_send.m_bDLC) << "]"; + + std::copy(message.begin(), message.begin() + can_msg_to_send.m_bDLC, can_msg_to_send.m_bData); + Status = UcanWriteCanMsgEx(m_UcanHandle, m_channel_number, &can_msg_to_send, NULL); if (Status != USBCAN_SUCCESSFUL) { LOG(Log::ERR, CanLogIt::h()) << "There was a problem when sending a message: " @@ -233,14 +218,13 @@ CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { /** * thread to handle reception of Can messages from the systec device */ -DWORD WINAPI CanVendorSystec::SystecRxThread(LPVOID pCanVendorSystec) +int CanVendorSystec::SystecRxThread() { BYTE status; tCanMsgStruct read_can_message; - CanVendorSystec *vendor_pointer = reinterpret_cast(pCanVendorSystec); - LOG(Log::DBG, CanLogIt::h()) << "SystecRxThread Started. m_receive_thread_flag = [" << vendor_pointer->m_receive_thread_flag <<"]"; - while (vendor_pointer->m_receive_thread_flag) { - status = UcanReadCanMsgEx(vendor_pointer->m_UcanHandle, (BYTE *)&vendor_pointer->m_channel_number, &read_can_message, NULL); + LOG(Log::DBG, CanLogIt::h()) << "SystecRxThread Started. m_receive_thread_flag = [" << m_receive_thread_flag <<"]"; + while (m_receive_thread_flag) { + status = UcanReadCanMsgEx(m_UcanHandle, (BYTE *) &m_channel_number, &read_can_message, NULL); switch (status) { case USBCAN_WARN_SYS_RXOVERRUN: case USBCAN_WARN_DLL_RXOVERRUN: @@ -249,17 +233,10 @@ DWORD WINAPI CanVendorSystec::SystecRxThread(LPVOID pCanVendorSystec) [[ fallthrough ]]; case USBCAN_SUCCESSFUL: { if (read_can_message.m_bFF & USBCAN_MSG_FF_RTR) break; - // can_msg_copy.c_time = convertTimepointToTimeval(currentTimeTimeval()); std::vector data(read_can_message.m_bData, read_can_message.m_bData + read_can_message.m_bDLC); // id, data, flags CanFrame can_msg_copy(read_can_message.m_dwID, data, read_can_message.m_bFF); - // TODO the read_can_message contains a DWORD m_dwTime "receipt time in ms" - vendor_pointer->received(can_msg_copy); - // vendor_pointer->m_statistics.onReceive( read_can_message.m_bDLC ); - // vendor_pointer->m_statistics.setTimeSinceReceived(); - - // we can reset the reconnectionTimeout here, since we have received a message - // vendor_pointer->resetTimeoutOnReception(); + received(can_msg_copy); break; } case USBCAN_WARN_NODATA: @@ -276,7 +253,6 @@ DWORD WINAPI CanVendorSystec::SystecRxThread(LPVOID pCanVendorSystec) } } - ExitThread(0); return 0; } From 08827b0c015cf6e34d7bfed852e9242699fa54b7 Mon Sep 17 00:00:00 2001 From: James Souter Date: Tue, 3 Mar 2026 15:33:10 +0100 Subject: [PATCH 03/15] check return codes of open and close on systec reconnect small cleanups --- src/main/CanVendorSystec.cpp | 71 +++++++++++++++++------------------- 1 file changed, 33 insertions(+), 38 deletions(-) diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index f3b876c7..094f691e 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -21,19 +21,18 @@ CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args) m_channel_number = handle_number % 2; } -// TODO should we make this noexcept? how can we guarantee that? CanReturnCode CanVendorSystec::init_can_port() { BYTE systec_call_return = USBCAN_SUCCESSFUL; tUcanHandle can_module_handle; unsigned int baud_rate = USBCAN_BAUD_125kBit; switch (args().config.bitrate.value_or(0)) { - case 50000: baud_rate = USBCAN_BAUD_50kBit; break; - case 100000: baud_rate = USBCAN_BAUD_100kBit; break; - case 125000: baud_rate = USBCAN_BAUD_125kBit; break; - case 250000: baud_rate = USBCAN_BAUD_250kBit; break; - case 500000: baud_rate = USBCAN_BAUD_500kBit; break; - case 1000000: baud_rate = USBCAN_BAUD_1MBit; break; + case 50000: baud_rate = USBCAN_BAUD_50kBit; break; + case 100000: baud_rate = USBCAN_BAUD_100kBit; break; + case 125000: baud_rate = USBCAN_BAUD_125kBit; break; + case 250000: baud_rate = USBCAN_BAUD_250kBit; break; + case 500000: baud_rate = USBCAN_BAUD_500kBit; break; + case 1000000: baud_rate = USBCAN_BAUD_1MBit; break; default: { LOG(Log::WRN, CanLogIt::h()) << "baud rate illegal, taking default 125000 [" << baud_rate << "]"; } @@ -51,7 +50,7 @@ CanReturnCode CanVendorSystec::init_can_port() { initialization_parameters.m_wNrOfRxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES; initialization_parameters.m_wNrOfTxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES; - // check if USB-CANmodul already is initialized + // check if USB-CANmodul is already initialized std::lock_guard guard(CanVendorSystec::m_handles_lock); auto pos = m_handle_map.find(m_module_number); if (pos == m_handle_map.end()) { // module not in use @@ -80,34 +79,31 @@ CanReturnCode CanVendorSystec::init_can_port() { } CanReturnCode CanVendorSystec::vendor_open() noexcept { + CanReturnCode return_code = CanReturnCode::unknown_open_error; + + try { + return_code = init_can_port(); + if (return_code != CanReturnCode::success) return return_code; + m_receive_thread_flag = true; + m_SystecRxThread = std::thread(&CanVendorSystec::SystecRxThread, this); + } catch(...) { + return_code = CanReturnCode::internal_api_error; + } - auto returnCode = init_can_port(); - if (returnCode != CanReturnCode::success) return returnCode; - - // TODO set time since opened equivalent... - // m_statistics.setTimeSinceOpened(); - - // After the canboard is configured and started, we start the scan control thread - m_receive_thread_flag = true; - m_SystecRxThread = std::thread(&CanVendorSystec::SystecRxThread, this); - - // todo reintroduce check here... - // if (NULL == m_receive_thread_handle) { - // LOG(Log::ERR, CanLogIt::h()) << "Error creating the canScanControl thread."; - // return CanReturnCode::internal_api_error; - // } - - return returnCode; + return return_code; } CanReturnCode CanVendorSystec::vendor_close() noexcept { - // TODO what if the return code is not success? - std::lock_guard guard(CanVendorSystec::m_handles_lock); - erase_module_handle(m_module_number); - m_receive_thread_flag = false; - if (m_SystecRxThread.joinable()) m_SystecRxThread.join(); - UcanDeinitCanEx (m_UcanHandle, (BYTE) m_channel_number); - LOG(Log::DBG, CanLogIt::h()) << __FUNCTION__ << " closed successfully"; + try { + m_receive_thread_flag = false; + std::lock_guard guard(CanVendorSystec::m_handles_lock); + erase_module_handle(m_module_number); + if (m_SystecRxThread.joinable()) m_SystecRxThread.join(); + auto return_code = UcanDeinitCanEx (m_UcanHandle, (BYTE) m_channel_number); + if (return_code != USBCAN_SUCCESSFUL) return CanReturnCode::unknown_close_error; + } catch (...) { + return CanReturnCode::internal_api_error; + } return CanReturnCode::success; }; @@ -132,9 +128,11 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { << UsbCanGetErrorText(Status); // for now, just always reconnect on a failed send. - vendor_close(); // TODO maybe we just call close instead of vendor_close - // see how CanVendorSocketCanSystec does reconnects, it intercepts the receiver function and wraps it - vendor_open(); + auto close_code = close(); + if (close_code != CanReturnCode::success) return close_code; + + auto open_code = open(); + if (open_code != CanReturnCode::success) return open_code; switch (Status) { case USBCAN_ERR_CANNOTINIT: @@ -148,15 +146,12 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { case USBCAN_WARN_FW_TXOVERRUN: default: return CanReturnCode::unknown_send_error; } - // m_statistics.onTransmit( can_msg_to_send.m_bDLC ); - // m_statistics.setTimeSinceTransmitted(); } return CanReturnCode::success; }; CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { - // TODO we can read the operating mode, either kUcanModeNormal, ListenOnly or TxEcho CanDiagnostics diagnostics{}; tStatusStruct status; // TODO check return code of these functions... From b5838f4a210d3e035d646cfaaa30bbd85367c30a Mon Sep 17 00:00:00 2001 From: James Souter Date: Wed, 4 Mar 2026 16:54:44 +0100 Subject: [PATCH 04/15] add get_module_handle() method --- src/include/CanVendorSystec.h | 15 +++--- src/main/CanVendorSystec.cpp | 98 ++++++++++++++++++++++++----------- 2 files changed, 77 insertions(+), 36 deletions(-) diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h index d8e7d947..86144f50 100644 --- a/src/include/CanVendorSystec.h +++ b/src/include/CanVendorSystec.h @@ -33,22 +33,25 @@ struct CanVendorSystec : CanDevice { private: std::atomic m_receive_thread_flag = true; - tUcanHandle m_UcanHandle; + std::atomic m_queued_reads; int m_module_number; int m_channel_number; + int m_port_number; std::thread m_SystecRxThread; + + tUcanHandle get_module_handle() { return m_module_to_handle_map[m_module_number]; } + CanReturnCode vendor_open() noexcept override; CanReturnCode vendor_close() noexcept override; CanReturnCode vendor_send(const CanFrame& frame) noexcept override; CanDiagnostics vendor_diagnostics() noexcept override; - + CanReturnCode init_can_port(); static std::mutex m_handles_lock; - static std::unordered_map m_handle_map; + static std::unordered_map m_module_to_handle_map; + static std::unordered_map m_port_to_vendor_map; - inline void map_module_to_handle(int module, tUcanHandle handle) { m_handle_map[module] = handle; } - inline int erase_module_handle(int module) { return m_handle_map.erase(module); } - + friend void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p); std::string UsbCanGetErrorText( long err_code ); }; diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index 094f691e..abf050ee 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -7,18 +7,29 @@ #include std::mutex CanVendorSystec::m_handles_lock; -std::unordered_map CanVendorSystec::m_handle_map; +std::unordered_map CanVendorSystec::m_module_to_handle_map; +std::unordered_map CanVendorSystec::m_port_to_vendor_map; + +// Callback registered per-module to handle receive events +void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p) { + if (bEvent_p == USBCAN_EVENT_RECEIVE) { + int module_number = *(reinterpret_cast(pArg_p)); + int port_number = 2 * module_number + bChannel_p; + CanVendorSystec *vendorPtr = CanVendorSystec::m_port_to_vendor_map[port_number]; + if (vendorPtr) ++(vendorPtr->m_queued_reads); // [] returns nullptr if not found in map; + } +} CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args) - : CanDevice("systec", args) { + : CanDevice("systec", args), m_queued_reads{0} { if (!args.config.bus_name.has_value()) { throw std::invalid_argument("Missing required configuration parameters"); } - // TODO trim possible can prefix use hardcoded value - int handle_number = std::stoi(args.config.bus_name.value()); - m_module_number = handle_number / 2; - m_channel_number = handle_number % 2; + // TODO trim possible can prefix + m_port_number = std::stoi(args.config.bus_name.value()); + m_module_number = m_port_number / 2; + m_channel_number = m_port_number % 2; } CanReturnCode CanVendorSystec::init_can_port() { @@ -52,20 +63,26 @@ CanReturnCode CanVendorSystec::init_can_port() { // check if USB-CANmodul is already initialized std::lock_guard guard(CanVendorSystec::m_handles_lock); - auto pos = m_handle_map.find(m_module_number); - if (pos == m_handle_map.end()) { // module not in use - systec_call_return = UcanInitHardwareEx(&can_module_handle, m_module_number, 0, 0); + auto mapping = m_module_to_handle_map.find(m_module_number); + if (mapping == m_module_to_handle_map.end()) { // module not in use + systec_call_return = UcanInitHardwareEx(&can_module_handle, m_module_number, systec_receive, (void*) &m_module_number); if (systec_call_return != USBCAN_SUCCESSFUL ) { - LOG(Log::ERR, CanLogIt::h()) << "UcanInitHardwareEx, return code = [ 0x" << std::hex << (int) systec_call_return << std::dec << "]"; + LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanInitHardwareEx: " << UsbCanGetErrorText(systec_call_return); UcanDeinitHardware(can_module_handle); return CanReturnCode::unknown_open_error; } - map_module_to_handle(m_module_number, can_module_handle); + LOG(Log::INF, CanLogIt::h()) << "Initialised hardware for Systec module " << m_module_number << " with handle " << (int) can_module_handle; + m_module_to_handle_map[m_module_number] = can_module_handle; } else { // find existing handle of module - can_module_handle = pos->second; - LOG(Log::WRN, CanLogIt::h()) << "trying to open a can port which is in use, reuse handle, skipping UCanDeinitHardware"; + can_module_handle = mapping->second; + LOG(Log::WRN, CanLogIt::h()) << "trying to open a can port which is in use, reuse handle, skipping UCanInitHardware"; } + // TODO handle error code for reset... + // also investigate the minimum amount of things to reset to restore good state + UcanResetCanEx(can_module_handle, (BYTE) m_channel_number, (DWORD) 0); + m_port_to_vendor_map[m_port_number] = this; + systec_call_return = UcanInitCanEx2(can_module_handle, m_channel_number, &initialization_parameters); if ( systec_call_return != USBCAN_SUCCESSFUL ) { LOG(Log::ERR, CanLogIt::h()) << "UcanInitCanEx2, return code = [ 0x" << std::hex << (int) systec_call_return << std::dec << "]"; @@ -73,7 +90,6 @@ CanReturnCode CanVendorSystec::init_can_port() { return CanReturnCode::unknown_open_error; } - m_UcanHandle = can_module_handle; LOG(Log::INF, CanLogIt::h()) << "Successfully opened CAN port on module " << m_module_number << ", channel " << m_channel_number; return CanReturnCode::success; } @@ -97,10 +113,29 @@ CanReturnCode CanVendorSystec::vendor_close() noexcept { try { m_receive_thread_flag = false; std::lock_guard guard(CanVendorSystec::m_handles_lock); - erase_module_handle(m_module_number); + m_port_to_vendor_map.erase(m_port_number); if (m_SystecRxThread.joinable()) m_SystecRxThread.join(); - auto return_code = UcanDeinitCanEx (m_UcanHandle, (BYTE) m_channel_number); - if (return_code != USBCAN_SUCCESSFUL) return CanReturnCode::unknown_close_error; + + auto handle = get_module_handle(); + + auto return_code = UcanDeinitCanEx(handle, (BYTE) m_channel_number); + if (return_code != USBCAN_SUCCESSFUL) { + LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitCanEx: " << UsbCanGetErrorText(return_code); + return CanReturnCode::unknown_close_error; + } + + int opposite_channel = 2 * m_module_number + (1 - m_channel_number); + if (!m_port_to_vendor_map[opposite_channel]) { + // de init hardware if neither channel on the module are in use + // e.g. if channel 0, we need to check if channel 1 is in use and vice versa + // TODO how does this work if UcanDeinitCanEx above fails? + auto return_code_hw = UcanDeinitHardware(handle); + m_module_to_handle_map.erase(m_module_number); + if (return_code_hw != USBCAN_SUCCESSFUL) { + LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitHardware: " << UsbCanGetErrorText(return_code_hw); + return CanReturnCode::unknown_close_error; + } + } } catch (...) { return CanReturnCode::internal_api_error; } @@ -122,7 +157,7 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { std::copy(message.begin(), message.begin() + can_msg_to_send.m_bDLC, can_msg_to_send.m_bData); - Status = UcanWriteCanMsgEx(m_UcanHandle, m_channel_number, &can_msg_to_send, NULL); + Status = UcanWriteCanMsgEx(get_module_handle(), m_channel_number, &can_msg_to_send, NULL); if (Status != USBCAN_SUCCESSFUL) { LOG(Log::ERR, CanLogIt::h()) << "There was a problem when sending a message: " << UsbCanGetErrorText(Status); @@ -155,7 +190,8 @@ CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { CanDiagnostics diagnostics{}; tStatusStruct status; // TODO check return code of these functions... - UcanGetStatusEx(m_UcanHandle, m_channel_number, &status); + auto handle = get_module_handle(); + UcanGetStatusEx(handle, m_channel_number, &status); WORD can_status = status.m_wCanStatus; switch (can_status) { case USBCAN_CANERR_OK: @@ -181,17 +217,17 @@ CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { } tUcanMsgCountInfo msg_count_info; - UcanGetMsgCountInfoEx(m_UcanHandle, m_channel_number, &msg_count_info); + UcanGetMsgCountInfoEx(handle, m_channel_number, &msg_count_info); diagnostics.tx = msg_count_info.m_wSentMsgCount; diagnostics.rx = msg_count_info.m_wRecvdMsgCount; DWORD tx_error, rx_error; - UcanGetCanErrorCounter(m_UcanHandle, m_channel_number, &tx_error, &rx_error); + UcanGetCanErrorCounter(handle, m_channel_number, &tx_error, &rx_error); diagnostics.tx_error = tx_error; diagnostics.rx_error = rx_error; tUcanHardwareInfo hw_info; - if (UcanGetHardwareInfo(m_UcanHandle, &hw_info) != USBCAN_SUCCESSFUL) + if (UcanGetHardwareInfo(handle, &hw_info) != USBCAN_SUCCESSFUL) diagnostics.mode = "OFFLINE"; else switch (hw_info.m_bMode) { case kUcanModeNormal: @@ -203,7 +239,7 @@ CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { } DWORD module_time; // in ms - UcanGetModuleTime(m_UcanHandle, &module_time); + UcanGetModuleTime(handle, &module_time); diagnostics.uptime = (uint32_t) module_time / 1000; return diagnostics; @@ -218,8 +254,11 @@ int CanVendorSystec::SystecRxThread() BYTE status; tCanMsgStruct read_can_message; LOG(Log::DBG, CanLogIt::h()) << "SystecRxThread Started. m_receive_thread_flag = [" << m_receive_thread_flag <<"]"; + size_t to_read; while (m_receive_thread_flag) { - status = UcanReadCanMsgEx(m_UcanHandle, (BYTE *) &m_channel_number, &read_can_message, NULL); + to_read = m_queued_reads; + if (to_read < 1) continue; + status = UcanReadCanMsgEx(get_module_handle(), (BYTE *) &m_channel_number, &read_can_message, NULL); switch (status) { case USBCAN_WARN_SYS_RXOVERRUN: case USBCAN_WARN_DLL_RXOVERRUN: @@ -227,17 +266,16 @@ int CanVendorSystec::SystecRxThread() LOG(Log::WRN, CanLogIt::h()) << UsbCanGetErrorText(status); [[ fallthrough ]]; case USBCAN_SUCCESSFUL: { + --m_queued_reads; if (read_can_message.m_bFF & USBCAN_MSG_FF_RTR) break; std::vector data(read_can_message.m_bData, read_can_message.m_bData + read_can_message.m_bDLC); - // id, data, flags - CanFrame can_msg_copy(read_can_message.m_dwID, data, read_can_message.m_bFF); - received(can_msg_copy); + CanFrame can_frame(read_can_message.m_dwID, data, read_can_message.m_bFF); + received(can_frame); break; } case USBCAN_WARN_NODATA: - LOG(Log::TRC, CanLogIt::h()) << UsbCanGetErrorText(status); - // TODO is it correct to sleep here? - // Sleep(100); // ms + m_queued_reads -= to_read; + LOG(Log::WRN, CanLogIt::h()) << UsbCanGetErrorText(status); break; default: // errors // USBCAN_ERR_MAXINSTANCES, USBCAN_ERR_ILLHANDLE, USBCAN_ERR_CANNOTINIT, From f9c5dbe2447f134ed8ff37408f0a57ab1961d1a9 Mon Sep 17 00:00:00 2001 From: James Souter Date: Tue, 17 Mar 2026 16:35:32 +0100 Subject: [PATCH 05/15] split up systec close logic, deinit hardware on failed open move lock_guard out of deinit_channel to prevent double lock acquisition --- src/include/CanVendorSystec.h | 2 ++ src/main/CanVendorSystec.cpp | 56 +++++++++++++++++++---------------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h index 86144f50..22fd4a7f 100644 --- a/src/include/CanVendorSystec.h +++ b/src/include/CanVendorSystec.h @@ -53,6 +53,8 @@ struct CanVendorSystec : CanDevice { friend void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p); std::string UsbCanGetErrorText( long err_code ); + + CanReturnCode deinit_channel() noexcept; }; #endif // SRC_INCLUDE_CANVENDORSYSTEC_H_ diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index abf050ee..2eed9a4e 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -86,7 +86,7 @@ CanReturnCode CanVendorSystec::init_can_port() { systec_call_return = UcanInitCanEx2(can_module_handle, m_channel_number, &initialization_parameters); if ( systec_call_return != USBCAN_SUCCESSFUL ) { LOG(Log::ERR, CanLogIt::h()) << "UcanInitCanEx2, return code = [ 0x" << std::hex << (int) systec_call_return << std::dec << "]"; - UcanDeinitCanEx(can_module_handle, m_channel_number); + deinit_channel(); return CanReturnCode::unknown_open_error; } @@ -110,38 +110,42 @@ CanReturnCode CanVendorSystec::vendor_open() noexcept { } CanReturnCode CanVendorSystec::vendor_close() noexcept { + auto return_code = CanReturnCode::success; try { m_receive_thread_flag = false; - std::lock_guard guard(CanVendorSystec::m_handles_lock); - m_port_to_vendor_map.erase(m_port_number); if (m_SystecRxThread.joinable()) m_SystecRxThread.join(); - - auto handle = get_module_handle(); - - auto return_code = UcanDeinitCanEx(handle, (BYTE) m_channel_number); - if (return_code != USBCAN_SUCCESSFUL) { - LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitCanEx: " << UsbCanGetErrorText(return_code); - return CanReturnCode::unknown_close_error; - } - - int opposite_channel = 2 * m_module_number + (1 - m_channel_number); - if (!m_port_to_vendor_map[opposite_channel]) { - // de init hardware if neither channel on the module are in use - // e.g. if channel 0, we need to check if channel 1 is in use and vice versa - // TODO how does this work if UcanDeinitCanEx above fails? - auto return_code_hw = UcanDeinitHardware(handle); - m_module_to_handle_map.erase(m_module_number); - if (return_code_hw != USBCAN_SUCCESSFUL) { - LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitHardware: " << UsbCanGetErrorText(return_code_hw); - return CanReturnCode::unknown_close_error; - } - } + std::lock_guard guard(CanVendorSystec::m_handles_lock); + return_code = deinit_channel(); } catch (...) { - return CanReturnCode::internal_api_error; + return_code = CanReturnCode::internal_api_error; } - return CanReturnCode::success; + return return_code; }; +CanReturnCode CanVendorSystec::deinit_channel() noexcept { + auto internal_return_code = CanReturnCode::success; + m_port_to_vendor_map.erase(m_port_number); + auto handle = get_module_handle(); + auto return_code = UcanDeinitCanEx(handle, m_channel_number); + if (return_code != USBCAN_SUCCESSFUL) { + LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitCanEx: " << UsbCanGetErrorText(return_code); + internal_return_code = CanReturnCode::unknown_close_error; + } // still attempt to deinit hardware if channel deinit fails + + if (!m_port_to_vendor_map[m_port_number ^ 1]) { + // de init hardware if neither channel on the module are in use + // toggle last bit to get the other port on the same module + // e.g. if channel 0, we need to check if channel 1 is in use and vice versa + auto return_code_hw = UcanDeinitHardware(handle); + m_module_to_handle_map.erase(m_module_number); + if (return_code_hw != USBCAN_SUCCESSFUL) { + LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitHardware: " << UsbCanGetErrorText(return_code_hw); + internal_return_code = CanReturnCode::unknown_close_error; + } + } + return internal_return_code; +} + CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { std::vector message = frame.message(); From e5ea0a6ff06fdad98dd3fec2eef0cd1ac5dccec4 Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 5 Mar 2026 09:59:00 +0100 Subject: [PATCH 06/15] Implement comments from review * use bus_number in systec * update uptime comment * use string_view for systec error message * Use descriptive error messages for systec status in diagnostics.state * throw in systec if invalid bitrate specified --- src/include/CanDiagnostics.h | 3 +- src/include/CanVendorSystec.h | 6 ++- src/main/CanVendorSystec.cpp | 77 ++++++++++++++++------------------- 3 files changed, 41 insertions(+), 45 deletions(-) diff --git a/src/include/CanDiagnostics.h b/src/include/CanDiagnostics.h index 5c45f502..4b44fc18 100644 --- a/src/include/CanDiagnostics.h +++ b/src/include/CanDiagnostics.h @@ -47,7 +47,8 @@ struct CanDiagnostics { std::optional temperature; ///< Optional temperature reading for Anagate devices. - std::optional uptime; ///< Optional uptime in seconds. + std::optional uptime; ///< Optional uptime in seconds for Anagate + ///< and Systec for Windows. std::optional tcp_rx; ///< Optional TCP Received counter for ///< both SocketCAN and Anagate devices. diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h index 22fd4a7f..21c15a49 100644 --- a/src/include/CanVendorSystec.h +++ b/src/include/CanVendorSystec.h @@ -32,11 +32,12 @@ struct CanVendorSystec : CanDevice { int SystecRxThread(); private: - std::atomic m_receive_thread_flag = true; + std::atomic m_receive_thread_flag {true}; std::atomic m_queued_reads; int m_module_number; int m_channel_number; int m_port_number; + DWORD m_baud_rate; std::thread m_SystecRxThread; tUcanHandle get_module_handle() { return m_module_to_handle_map[m_module_number]; } @@ -52,7 +53,8 @@ struct CanVendorSystec : CanDevice { static std::unordered_map m_port_to_vendor_map; friend void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p); - std::string UsbCanGetErrorText( long err_code ); + std::string_view UsbCanGetErrorText( long err_code ); + std::string UsbCanGetStatusText( long err_code ); CanReturnCode deinit_channel() noexcept; }; diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index 2eed9a4e..6e997832 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -22,12 +22,23 @@ void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, v CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args) : CanDevice("systec", args), m_queued_reads{0} { - if (!args.config.bus_name.has_value()) { + if (!args.config.bus_number.has_value() || !args.config.bitrate.has_value()) { throw std::invalid_argument("Missing required configuration parameters"); } - // TODO trim possible can prefix - m_port_number = std::stoi(args.config.bus_name.value()); + switch (args.config.bitrate.value()) { + case 50000: m_baud_rate = USBCAN_BAUD_50kBit; break; + case 100000: m_baud_rate = USBCAN_BAUD_100kBit; break; + case 125000: m_baud_rate = USBCAN_BAUD_125kBit; break; + case 250000: m_baud_rate = USBCAN_BAUD_250kBit; break; + case 500000: m_baud_rate = USBCAN_BAUD_500kBit; break; + case 1000000: m_baud_rate = USBCAN_BAUD_1MBit; break; + default: { + throw std::invalid_argument("Invalid bitrate provided"); + } + } + + m_port_number = args.config.bus_number.value(); m_module_number = m_port_number / 2; m_channel_number = m_port_number % 2; } @@ -36,24 +47,11 @@ CanReturnCode CanVendorSystec::init_can_port() { BYTE systec_call_return = USBCAN_SUCCESSFUL; tUcanHandle can_module_handle; - unsigned int baud_rate = USBCAN_BAUD_125kBit; - switch (args().config.bitrate.value_or(0)) { - case 50000: baud_rate = USBCAN_BAUD_50kBit; break; - case 100000: baud_rate = USBCAN_BAUD_100kBit; break; - case 125000: baud_rate = USBCAN_BAUD_125kBit; break; - case 250000: baud_rate = USBCAN_BAUD_250kBit; break; - case 500000: baud_rate = USBCAN_BAUD_500kBit; break; - case 1000000: baud_rate = USBCAN_BAUD_1MBit; break; - default: { - LOG(Log::WRN, CanLogIt::h()) << "baud rate illegal, taking default 125000 [" << baud_rate << "]"; - } - } - tUcanInitCanParam initialization_parameters; initialization_parameters.m_dwSize = sizeof(initialization_parameters); // size of this struct initialization_parameters.m_bMode = kUcanModeNormal; // normal operation mode - initialization_parameters.m_bBTR0 = HIBYTE( baud_rate ); // baudrate - initialization_parameters.m_bBTR1 = LOBYTE( baud_rate ); + initialization_parameters.m_bBTR0 = HIBYTE( m_baud_rate ); // baudrate + initialization_parameters.m_bBTR1 = LOBYTE( m_baud_rate ); initialization_parameters.m_bOCR = 0x1A; // standard output initialization_parameters.m_dwAMR = USBCAN_AMR_ALL; // receive all CAN messages initialization_parameters.m_dwACR = USBCAN_ACR_ALL; @@ -197,29 +195,7 @@ CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { auto handle = get_module_handle(); UcanGetStatusEx(handle, m_channel_number, &status); WORD can_status = status.m_wCanStatus; - switch (can_status) { - case USBCAN_CANERR_OK: - diagnostics.state = "USBCAN_CANERR_OK"; break; - case USBCAN_CANERR_XMTFULL: - diagnostics.state = "USBCAN_CANERR_XMTFULL"; break; - case USBCAN_CANERR_OVERRUN: - diagnostics.state = "USBCAN_CANERR_OVERRUN"; break; - case USBCAN_CANERR_BUSLIGHT: - diagnostics.state = "USBCAN_CANERR_BUSLIGHT"; break; - case USBCAN_CANERR_BUSHEAVY: - diagnostics.state = "USBCAN_CANERR_BUSHEAVY"; break; - case USBCAN_CANERR_BUSOFF: - diagnostics.state = "USBCAN_CANERR_BUSOFF"; break; - case USBCAN_CANERR_QOVERRUN: - diagnostics.state = "USBCAN_CANERR_QOVERRUN"; break; - case USBCAN_CANERR_QXMTFULL: - diagnostics.state = "USBCAN_CANERR_QXMTFULL"; break; - case USBCAN_CANERR_REGTEST: - diagnostics.state = "USBCAN_CANERR_REGTEST"; break; - case USBCAN_CANERR_TXMSGLOST: - diagnostics.state = "USBCAN_CANERR_TXMSGLOST"; break; - } - + diagnostics.state = UsbCanGetStatusText(can_status); tUcanMsgCountInfo msg_count_info; UcanGetMsgCountInfoEx(handle, m_channel_number, &msg_count_info); diagnostics.tx = msg_count_info.m_wSentMsgCount; @@ -293,7 +269,7 @@ int CanVendorSystec::SystecRxThread() return 0; } -std::string CanVendorSystec::UsbCanGetErrorText( long err_code ) { +std::string_view CanVendorSystec::UsbCanGetErrorText( long err_code ) { switch( err_code ){ case USBCAN_SUCCESSFUL: return("success"); @@ -447,5 +423,22 @@ std::string CanVendorSystec::UsbCanGetErrorText( long err_code ) { "successfully to the transmit buffer."); default: return("unknown error code"); +std::string CanVendorSystec::UsbCanGetStatusText(long err_code) { + switch(err_code) { + case USBCAN_CANERR_OK: return "No error."; + case USBCAN_CANERR_XMTFULL: return "Transmit buffer in CAN controller is overrun."; + case USBCAN_CANERR_OVERRUN: return "Receive buffer in CAN controller is overrun."; + case USBCAN_CANERR_BUSLIGHT: return " Error limit 1 in CAN controller exceeded, CAN controller " + "is in state “Warning limit” now."; + case USBCAN_CANERR_BUSHEAVY: return "Error limit 2 in CAN controller exceeded, CAN controller " + "is in state “Error Passive” now"; + case USBCAN_CANERR_BUSOFF: return "CAN controller is in BUSOFF state."; + case USBCAN_CANERR_QOVERRUN: return "Receive buffer in module is overrun."; + case USBCAN_CANERR_QXMTFULL: return "Transmit buffer in module is overrun."; + case USBCAN_CANERR_REGTEST: return "CAN controller not found (hardware error)."; + case USBCAN_CANERR_TXMSGLOST: return "A transmit CAN message was deleted automatically by the " + "firmware because transmission timeout run over (refer to " + "function UcanSetTxTimeout() )."; + default: return "unknown error code"; } } From e3c061f74b18dfe3fefa88698f41fd1173f1bf8e Mon Sep 17 00:00:00 2001 From: James Souter Date: Tue, 24 Mar 2026 10:45:34 +0100 Subject: [PATCH 07/15] Use std::optional for systec handles, avoid use of [] accessor Only call deinit on nonzero handles, clean up some logging messages rename m_receive_thread_flag to m_module_in_use --- src/include/CanVendorSystec.h | 12 ++++-- src/main/CanVendorSystec.cpp | 80 ++++++++++++++++++++++------------- 2 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h index 21c15a49..687b6daa 100644 --- a/src/include/CanVendorSystec.h +++ b/src/include/CanVendorSystec.h @@ -15,6 +15,7 @@ #include "CanVendorLoopback.h" #include "CanDevice.h" #include +#include #include /** @@ -30,9 +31,9 @@ struct CanVendorSystec : CanDevice { explicit CanVendorSystec(const CanDeviceArguments& args); ~CanVendorSystec() { vendor_close(); } int SystecRxThread(); - + private: - std::atomic m_receive_thread_flag {true}; + std::atomic m_module_in_use {false}; std::atomic m_queued_reads; int m_module_number; int m_channel_number; @@ -40,7 +41,12 @@ struct CanVendorSystec : CanDevice { DWORD m_baud_rate; std::thread m_SystecRxThread; - tUcanHandle get_module_handle() { return m_module_to_handle_map[m_module_number]; } + std::optional get_module_handle() { + if (auto mapping = m_module_to_handle_map.find(m_module_number); mapping != m_module_to_handle_map.end()) { + return mapping->second; + } + return std::nullopt; + } CanReturnCode vendor_open() noexcept override; CanReturnCode vendor_close() noexcept override; diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index 6e997832..cdf8343e 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -61,19 +61,19 @@ CanReturnCode CanVendorSystec::init_can_port() { // check if USB-CANmodul is already initialized std::lock_guard guard(CanVendorSystec::m_handles_lock); - auto mapping = m_module_to_handle_map.find(m_module_number); - if (mapping == m_module_to_handle_map.end()) { // module not in use + auto handle = get_module_handle(); + if (!handle.has_value()) { // module not in use systec_call_return = UcanInitHardwareEx(&can_module_handle, m_module_number, systec_receive, (void*) &m_module_number); - if (systec_call_return != USBCAN_SUCCESSFUL ) { + if (systec_call_return != USBCAN_SUCCESSFUL ) { LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanInitHardwareEx: " << UsbCanGetErrorText(systec_call_return); UcanDeinitHardware(can_module_handle); return CanReturnCode::unknown_open_error; } - LOG(Log::INF, CanLogIt::h()) << "Initialised hardware for Systec module " << m_module_number << " with handle " << (int) can_module_handle; + LOG(Log::INF, CanLogIt::h()) << "Initialised hardware for Systec module " << m_module_number; m_module_to_handle_map[m_module_number] = can_module_handle; } else { // find existing handle of module can_module_handle = mapping->second; - LOG(Log::WRN, CanLogIt::h()) << "trying to open a can port which is in use, reuse handle, skipping UCanInitHardware"; + LOG(Log::INF, CanLogIt::h()) << "Reuing handle for module " << m_module_number << " already in use, skipping UCanInitHardwareEx"; } // TODO handle error code for reset... @@ -98,7 +98,7 @@ CanReturnCode CanVendorSystec::vendor_open() noexcept { try { return_code = init_can_port(); if (return_code != CanReturnCode::success) return return_code; - m_receive_thread_flag = true; + m_module_in_use = true; m_SystecRxThread = std::thread(&CanVendorSystec::SystecRxThread, this); } catch(...) { return_code = CanReturnCode::internal_api_error; @@ -110,7 +110,7 @@ CanReturnCode CanVendorSystec::vendor_open() noexcept { CanReturnCode CanVendorSystec::vendor_close() noexcept { auto return_code = CanReturnCode::success; try { - m_receive_thread_flag = false; + m_module_in_use = false; if (m_SystecRxThread.joinable()) m_SystecRxThread.join(); std::lock_guard guard(CanVendorSystec::m_handles_lock); return_code = deinit_channel(); @@ -124,22 +124,26 @@ CanReturnCode CanVendorSystec::deinit_channel() noexcept { auto internal_return_code = CanReturnCode::success; m_port_to_vendor_map.erase(m_port_number); auto handle = get_module_handle(); - auto return_code = UcanDeinitCanEx(handle, m_channel_number); - if (return_code != USBCAN_SUCCESSFUL) { - LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitCanEx: " << UsbCanGetErrorText(return_code); - internal_return_code = CanReturnCode::unknown_close_error; - } // still attempt to deinit hardware if channel deinit fails - - if (!m_port_to_vendor_map[m_port_number ^ 1]) { - // de init hardware if neither channel on the module are in use - // toggle last bit to get the other port on the same module - // e.g. if channel 0, we need to check if channel 1 is in use and vice versa - auto return_code_hw = UcanDeinitHardware(handle); - m_module_to_handle_map.erase(m_module_number); - if (return_code_hw != USBCAN_SUCCESSFUL) { - LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitHardware: " << UsbCanGetErrorText(return_code_hw); + if (handle.has_value()) { + auto return_code = UcanDeinitCanEx(handle.value(), m_channel_number); + if (return_code != USBCAN_SUCCESSFUL) { + LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitCanEx: " << UsbCanGetErrorText(return_code); internal_return_code = CanReturnCode::unknown_close_error; + } // still attempt to deinit hardware if channel deinit fails + + if (!m_port_to_vendor_map[m_port_number ^ 1]) { + // de init hardware if neither channel on the module are in use + // toggle last bit to get the other port on the same module + // e.g. if channel 0, we need to check if channel 1 is in use and vice versa + auto return_code_hw = UcanDeinitHardware(handle.value()); + m_module_to_handle_map.erase(m_module_number); + if (return_code_hw != USBCAN_SUCCESSFUL) { + LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitHardware: " << UsbCanGetErrorText(return_code_hw); + internal_return_code = CanReturnCode::unknown_close_error; + } } + } else { + LOG(Log::WRN, CanLogIt::h()) << "No handle found for module, close() may have already been called"; } return internal_return_code; } @@ -159,7 +163,12 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { std::copy(message.begin(), message.begin() + can_msg_to_send.m_bDLC, can_msg_to_send.m_bData); - Status = UcanWriteCanMsgEx(get_module_handle(), m_channel_number, &can_msg_to_send, NULL); + auto handle = get_module_handle(); + if (!handle.has_value()) { + LOG(Log::ERR, CanLogIt::h()) << "Could not send message, no handle found for module " << m_module_number; + return CanReturnCode::disconnected; + } + Status = UcanWriteCanMsgEx(handle.value(), m_channel_number, &can_msg_to_send, NULL); if (Status != USBCAN_SUCCESSFUL) { LOG(Log::ERR, CanLogIt::h()) << "There was a problem when sending a message: " << UsbCanGetErrorText(Status); @@ -193,21 +202,26 @@ CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { tStatusStruct status; // TODO check return code of these functions... auto handle = get_module_handle(); - UcanGetStatusEx(handle, m_channel_number, &status); + if (!handle.has_value()) { + LOG(Log::ERR, CanLogIt::h()) << "TODO figure out what to do here, no handle found"; + return CanReturnCode::disconnected; + } + auto handle_value = handle.value(); + UcanGetStatusEx(handle_value, m_channel_number, &status); WORD can_status = status.m_wCanStatus; diagnostics.state = UsbCanGetStatusText(can_status); tUcanMsgCountInfo msg_count_info; - UcanGetMsgCountInfoEx(handle, m_channel_number, &msg_count_info); + UcanGetMsgCountInfoEx(handle_value, m_channel_number, &msg_count_info); diagnostics.tx = msg_count_info.m_wSentMsgCount; diagnostics.rx = msg_count_info.m_wRecvdMsgCount; DWORD tx_error, rx_error; - UcanGetCanErrorCounter(handle, m_channel_number, &tx_error, &rx_error); + UcanGetCanErrorCounter(handle_value, m_channel_number, &tx_error, &rx_error); diagnostics.tx_error = tx_error; diagnostics.rx_error = rx_error; tUcanHardwareInfo hw_info; - if (UcanGetHardwareInfo(handle, &hw_info) != USBCAN_SUCCESSFUL) + if (UcanGetHardwareInfo(handle_value, &hw_info) != USBCAN_SUCCESSFUL) diagnostics.mode = "OFFLINE"; else switch (hw_info.m_bMode) { case kUcanModeNormal: @@ -219,7 +233,7 @@ CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { } DWORD module_time; // in ms - UcanGetModuleTime(handle, &module_time); + UcanGetModuleTime(handle_value, &module_time); diagnostics.uptime = (uint32_t) module_time / 1000; return diagnostics; @@ -233,12 +247,18 @@ int CanVendorSystec::SystecRxThread() { BYTE status; tCanMsgStruct read_can_message; - LOG(Log::DBG, CanLogIt::h()) << "SystecRxThread Started. m_receive_thread_flag = [" << m_receive_thread_flag <<"]"; + LOG(Log::DBG, CanLogIt::h()) << "SystecRxThread Started. m_module_in_use = [" << m_module_in_use <<"]"; size_t to_read; - while (m_receive_thread_flag) { + + auto handle = get_module_handle(); + if (!handle.has_value()) { + LOG(Log::ERR, CanLogIt::h()) << "Could not start rx thread without valid handle for module"; + return -1; // TODO more useful error code + } + while (m_module_in_use) { to_read = m_queued_reads; if (to_read < 1) continue; - status = UcanReadCanMsgEx(get_module_handle(), (BYTE *) &m_channel_number, &read_can_message, NULL); + status = UcanReadCanMsgEx(handle.value(), (BYTE *) &m_channel_number, &read_can_message, NULL); switch (status) { case USBCAN_WARN_SYS_RXOVERRUN: case USBCAN_WARN_DLL_RXOVERRUN: From 4b45c86b7e3c8dbac00c72255fff94be3d80edba Mon Sep 17 00:00:00 2001 From: James Souter Date: Mon, 13 Apr 2026 16:36:12 +0200 Subject: [PATCH 08/15] Improve logging for error messages Register callback to log systec connection events --- src/include/CanVendorSystec.h | 4 +- src/main/CanVendorSystec.cpp | 163 ++++++++++++++++++---------------- 2 files changed, 89 insertions(+), 78 deletions(-) diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h index 687b6daa..65201ec3 100644 --- a/src/include/CanVendorSystec.h +++ b/src/include/CanVendorSystec.h @@ -31,6 +31,8 @@ struct CanVendorSystec : CanDevice { explicit CanVendorSystec(const CanDeviceArguments& args); ~CanVendorSystec() { vendor_close(); } int SystecRxThread(); + static std::string_view UsbCanGetErrorText( long err_code ); + static std::string UsbCanGetStatusText( long err_code ); private: std::atomic m_module_in_use {false}; @@ -59,8 +61,6 @@ struct CanVendorSystec : CanDevice { static std::unordered_map m_port_to_vendor_map; friend void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p); - std::string_view UsbCanGetErrorText( long err_code ); - std::string UsbCanGetStatusText( long err_code ); CanReturnCode deinit_channel() noexcept; }; diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index cdf8343e..fd8fa726 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -9,6 +9,26 @@ std::mutex CanVendorSystec::m_handles_lock; std::unordered_map CanVendorSystec::m_module_to_handle_map; std::unordered_map CanVendorSystec::m_port_to_vendor_map; +static bool module_control_callback_registered = false; + +template +long CallAndLog(T f, const char *name, Args... args) { + long code = f(args...); + if (code != USBCAN_SUCCESSFUL) + LOG(Log::ERR, CanLogIt::h()) << "Got error code calling " << name << ": " << CanVendorSystec::UsbCanGetErrorText(code); + return code; +} + +void connect_control_callback(BYTE bEvent_p, DWORD dwParam_p) { + switch(bEvent_p) { + case USBCAN_EVENT_CONNECT: + LOG(Log::DBG) << "USB CAN module connected"; break; + case USBCAN_EVENT_DISCONNECT: + LOG(Log::WRN) << "USB CAN module disconnected"; break; + case USBCAN_EVENT_FATALDISCON: + LOG(Log::ERR) << "USB CAN module with handle " << (int) dwParam_p << "fatally disconnected"; break; + } +} // Callback registered per-module to handle receive events void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p) { @@ -63,27 +83,27 @@ CanReturnCode CanVendorSystec::init_can_port() { std::lock_guard guard(CanVendorSystec::m_handles_lock); auto handle = get_module_handle(); if (!handle.has_value()) { // module not in use - systec_call_return = UcanInitHardwareEx(&can_module_handle, m_module_number, systec_receive, (void*) &m_module_number); - if (systec_call_return != USBCAN_SUCCESSFUL ) { - LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanInitHardwareEx: " << UsbCanGetErrorText(systec_call_return); - UcanDeinitHardware(can_module_handle); + if (!module_control_callback_registered) { + if (!CallAndLog(UcanInitHwConnectControl, "hw connect control callback", connect_control_callback)) + module_control_callback_registered = true; + } + if (auto systec_code = CallAndLog(UcanInitHardwareEx, "init hardware", &can_module_handle, m_module_number, systec_receive, (void*) &m_module_number); systec_code != 0) { + CallAndLog(UcanDeinitHardware, "deinit hardware", can_module_handle); return CanReturnCode::unknown_open_error; } LOG(Log::INF, CanLogIt::h()) << "Initialised hardware for Systec module " << m_module_number; m_module_to_handle_map[m_module_number] = can_module_handle; } else { // find existing handle of module - can_module_handle = mapping->second; - LOG(Log::INF, CanLogIt::h()) << "Reuing handle for module " << m_module_number << " already in use, skipping UCanInitHardwareEx"; + can_module_handle = handle.value(); + LOG(Log::INF, CanLogIt::h()) << "Reusing handle (" << (size_t) can_module_handle << ") for module " << m_module_number << " already in use, skipping UCanInitHardwareEx"; } // TODO handle error code for reset... // also investigate the minimum amount of things to reset to restore good state - UcanResetCanEx(can_module_handle, (BYTE) m_channel_number, (DWORD) 0); + CallAndLog(UcanResetCanEx, "reset channel", can_module_handle, (BYTE) m_channel_number, (DWORD) 0); m_port_to_vendor_map[m_port_number] = this; - systec_call_return = UcanInitCanEx2(can_module_handle, m_channel_number, &initialization_parameters); - if ( systec_call_return != USBCAN_SUCCESSFUL ) { - LOG(Log::ERR, CanLogIt::h()) << "UcanInitCanEx2, return code = [ 0x" << std::hex << (int) systec_call_return << std::dec << "]"; + if (CallAndLog(UcanInitCanEx2, "init channel", can_module_handle, m_channel_number, &initialization_parameters)) { deinit_channel(); return CanReturnCode::unknown_open_error; } @@ -125,22 +145,16 @@ CanReturnCode CanVendorSystec::deinit_channel() noexcept { m_port_to_vendor_map.erase(m_port_number); auto handle = get_module_handle(); if (handle.has_value()) { - auto return_code = UcanDeinitCanEx(handle.value(), m_channel_number); - if (return_code != USBCAN_SUCCESSFUL) { - LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitCanEx: " << UsbCanGetErrorText(return_code); + if (CallAndLog(UcanDeinitCanEx, "deinit channel", handle.value(), m_channel_number)) internal_return_code = CanReturnCode::unknown_close_error; - } // still attempt to deinit hardware if channel deinit fails if (!m_port_to_vendor_map[m_port_number ^ 1]) { // de init hardware if neither channel on the module are in use // toggle last bit to get the other port on the same module // e.g. if channel 0, we need to check if channel 1 is in use and vice versa - auto return_code_hw = UcanDeinitHardware(handle.value()); - m_module_to_handle_map.erase(m_module_number); - if (return_code_hw != USBCAN_SUCCESSFUL) { - LOG(Log::ERR, CanLogIt::h()) << "Error calling UcanDeinitHardware: " << UsbCanGetErrorText(return_code_hw); + if (CallAndLog(UcanDeinitHardware, "deinit hw", handle.value())) internal_return_code = CanReturnCode::unknown_close_error; - } + m_module_to_handle_map.erase(m_module_number); } } else { LOG(Log::WRN, CanLogIt::h()) << "No handle found for module, close() may have already been called"; @@ -152,7 +166,6 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { std::vector message = frame.message(); tCanMsgStruct can_msg_to_send; - BYTE Status; can_msg_to_send.m_dwID = frame.id(); can_msg_to_send.m_bDLC = frame.length(); @@ -168,30 +181,18 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { LOG(Log::ERR, CanLogIt::h()) << "Could not send message, no handle found for module " << m_module_number; return CanReturnCode::disconnected; } - Status = UcanWriteCanMsgEx(handle.value(), m_channel_number, &can_msg_to_send, NULL); - if (Status != USBCAN_SUCCESSFUL) { - LOG(Log::ERR, CanLogIt::h()) << "There was a problem when sending a message: " - << UsbCanGetErrorText(Status); - - // for now, just always reconnect on a failed send. - auto close_code = close(); - if (close_code != CanReturnCode::success) return close_code; - - auto open_code = open(); - if (open_code != CanReturnCode::success) return open_code; - - switch (Status) { - case USBCAN_ERR_CANNOTINIT: - case USBCAN_ERR_ILLHANDLE: return CanReturnCode::disconnected; - case USBCAN_ERR_DLL_TXFULL: return CanReturnCode::tx_buffer_overflow; - case USBCAN_ERR_MAXINSTANCES: return CanReturnCode::too_many_connections; - case USBCAN_ERR_ILLPARAM: - case USBCAN_ERR_ILLHW: - case USBCAN_ERR_ILLCHANNEL: - case USBCAN_WARN_TXLIMIT: - case USBCAN_WARN_FW_TXOVERRUN: - default: return CanReturnCode::unknown_send_error; - } + switch(CallAndLog(UcanWriteCanMsgEx, "write", handle.value(), m_channel_number, &can_msg_to_send, nullptr)) { + case USBCAN_SUCCESSFUL: break; + case USBCAN_ERR_CANNOTINIT: + case USBCAN_ERR_ILLHANDLE: return CanReturnCode::disconnected; + case USBCAN_ERR_DLL_TXFULL: return CanReturnCode::tx_buffer_overflow; + case USBCAN_ERR_MAXINSTANCES: return CanReturnCode::too_many_connections; + case USBCAN_ERR_ILLPARAM: + case USBCAN_ERR_ILLHW: + case USBCAN_ERR_ILLCHANNEL: + case USBCAN_WARN_TXLIMIT: + case USBCAN_WARN_FW_TXOVERRUN: + default: return CanReturnCode::unknown_send_error; } return CanReturnCode::success; }; @@ -200,42 +201,49 @@ CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { CanDiagnostics diagnostics{}; tStatusStruct status; - // TODO check return code of these functions... auto handle = get_module_handle(); + diagnostics.log_entries = std::vector(); if (!handle.has_value()) { - LOG(Log::ERR, CanLogIt::h()) << "TODO figure out what to do here, no handle found"; - return CanReturnCode::disconnected; - } - auto handle_value = handle.value(); - UcanGetStatusEx(handle_value, m_channel_number, &status); - WORD can_status = status.m_wCanStatus; - diagnostics.state = UsbCanGetStatusText(can_status); - tUcanMsgCountInfo msg_count_info; - UcanGetMsgCountInfoEx(handle_value, m_channel_number, &msg_count_info); - diagnostics.tx = msg_count_info.m_wSentMsgCount; - diagnostics.rx = msg_count_info.m_wRecvdMsgCount; - - DWORD tx_error, rx_error; - UcanGetCanErrorCounter(handle_value, m_channel_number, &tx_error, &rx_error); - diagnostics.tx_error = tx_error; - diagnostics.rx_error = rx_error; - - tUcanHardwareInfo hw_info; - if (UcanGetHardwareInfo(handle_value, &hw_info) != USBCAN_SUCCESSFUL) + LOG(Log::ERR, CanLogIt::h()) << "Could not get diagnostics as no handle found for module " << m_module_number; diagnostics.mode = "OFFLINE"; - else switch (hw_info.m_bMode) { - case kUcanModeNormal: - diagnostics.mode = "NORMAL"; break; - case kUcanModeListenOnly: - diagnostics.mode = "LISTEN_ONLY"; break; - case kUcanModeTxEcho: - diagnostics.mode = "LOOPBACK"; break; - } + } else { + auto handle_value = handle.value(); + CallAndLog(UcanGetStatusEx, "get status", handle_value, m_channel_number, &status); + WORD can_status = status.m_wCanStatus; + diagnostics.state = UsbCanGetStatusText(can_status); + if (can_status) { + diagnostics.log_entries.value().push_back(diagnostics.state.value()); + } + - DWORD module_time; // in ms - UcanGetModuleTime(handle_value, &module_time); - diagnostics.uptime = (uint32_t) module_time / 1000; + tUcanMsgCountInfo msg_count_info; + long err_code; + if (err_code = CallAndLog(UcanGetMsgCountInfoEx, "get msg counts", handle_value, m_channel_number, &msg_count_info); err_code == 0) { + diagnostics.tx = msg_count_info.m_wSentMsgCount; + diagnostics.rx = msg_count_info.m_wRecvdMsgCount; + } else diagnostics.log_entries.value().push_back(std::string(UsbCanGetErrorText(err_code))); + + DWORD tx_error, rx_error; + if (err_code = CallAndLog(UcanGetCanErrorCounter, "get errors", handle_value, m_channel_number, &tx_error, &rx_error); err_code == 0) { + diagnostics.tx_error = tx_error; + diagnostics.rx_error = rx_error; + } else diagnostics.log_entries.value().push_back(std::string(UsbCanGetErrorText(err_code))); + + tUcanHardwareInfo hw_info; + if (err_code = CallAndLog(UcanGetHardwareInfo, "get hw info", handle_value, &hw_info); err_code != 0) { + diagnostics.mode = "OFFLINE"; + diagnostics.log_entries.value().push_back(std::string(UsbCanGetErrorText(err_code))); + } else switch (hw_info.m_bMode) { + case kUcanModeNormal: diagnostics.mode = "NORMAL"; break; + case kUcanModeListenOnly: diagnostics.mode = "LISTEN_ONLY"; break; + case kUcanModeTxEcho: diagnostics.mode = "LOOPBACK"; break; + } + DWORD module_time; // in ms + if (err_code = CallAndLog(UcanGetModuleTime, "get module time", handle_value, &module_time); err_code == 0) + diagnostics.uptime = (uint32_t) module_time / 1000; + else diagnostics.log_entries.value().push_back(std::string(UsbCanGetErrorText(err_code))); + } return diagnostics; }; @@ -459,6 +467,9 @@ std::string CanVendorSystec::UsbCanGetStatusText(long err_code) { case USBCAN_CANERR_TXMSGLOST: return "A transmit CAN message was deleted automatically by the " "firmware because transmission timeout run over (refer to " "function UcanSetTxTimeout() )."; - default: return "unknown error code"; + default: + std::stringstream ss; + ss << "Unknown error code: 0x" << std::hex << err_code; + return ss.str(); } } From 614851ff41efe2a7d4b97a8feccfa374cec238dd Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 26 Mar 2026 09:57:52 +0100 Subject: [PATCH 09/15] add config option to deinit both channels when vendor_close called on systec register callback for connect control add error messages to log_entries in diagnostics --- src/include/CanDeviceConfiguration.h | 10 ++++ src/include/CanVendorSystec.h | 3 +- src/main/CanVendorSystec.cpp | 75 ++++++++++++++++++---------- 3 files changed, 61 insertions(+), 27 deletions(-) diff --git a/src/include/CanDeviceConfiguration.h b/src/include/CanDeviceConfiguration.h index 93731a99..a727d6ec 100644 --- a/src/include/CanDeviceConfiguration.h +++ b/src/include/CanDeviceConfiguration.h @@ -95,6 +95,16 @@ struct CanDeviceConfiguration { */ std::optional sent_acknowledgement; + /** + * @brief Enable or disable closing both channels on systec module when one is closed. + * + * This parameter is optional for Systec on Windows and defaults to false. + * If turned on, calling close on a Systec CanDevice will deintialise both channels + * of a module when one is closed. + * + */ + std::optional close_both_channels; + std::string to_string() const noexcept; }; diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h index 65201ec3..b1e5ae90 100644 --- a/src/include/CanVendorSystec.h +++ b/src/include/CanVendorSystec.h @@ -62,7 +62,8 @@ struct CanVendorSystec : CanDevice { friend void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p); - CanReturnCode deinit_channel() noexcept; + CanReturnCode deinit_channel(tUcanHandle handle) noexcept; + CanReturnCode deinit_other_channel(tUcanHandle handle, CanVendorSystec *other) noexcept; }; #endif // SRC_INCLUDE_CANVENDORSYSTEC_H_ diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index fd8fa726..d6cd82df 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -61,6 +61,7 @@ CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args) m_port_number = args.config.bus_number.value(); m_module_number = m_port_number / 2; m_channel_number = m_port_number % 2; + m_port_to_vendor_map[m_port_number] = this; } CanReturnCode CanVendorSystec::init_can_port() { @@ -98,16 +99,14 @@ CanReturnCode CanVendorSystec::init_can_port() { LOG(Log::INF, CanLogIt::h()) << "Reusing handle (" << (size_t) can_module_handle << ") for module " << m_module_number << " already in use, skipping UCanInitHardwareEx"; } - // TODO handle error code for reset... - // also investigate the minimum amount of things to reset to restore good state - CallAndLog(UcanResetCanEx, "reset channel", can_module_handle, (BYTE) m_channel_number, (DWORD) 0); - m_port_to_vendor_map[m_port_number] = this; - if (CallAndLog(UcanInitCanEx2, "init channel", can_module_handle, m_channel_number, &initialization_parameters)) { - deinit_channel(); + deinit_channel(can_module_handle); return CanReturnCode::unknown_open_error; } + // investigate the minimum amount of things to reset to restore good state + CallAndLog(UcanResetCanEx, "reset channel", can_module_handle, (BYTE) m_channel_number, (DWORD) 0); + LOG(Log::INF, CanLogIt::h()) << "Successfully opened CAN port on module " << m_module_number << ", channel " << m_channel_number; return CanReturnCode::success; } @@ -129,36 +128,60 @@ CanReturnCode CanVendorSystec::vendor_open() noexcept { CanReturnCode CanVendorSystec::vendor_close() noexcept { auto return_code = CanReturnCode::success; + std::lock_guard guard(CanVendorSystec::m_handles_lock); + + bool other_in_use = false; + CanVendorSystec *other = nullptr; + // toggle last bit to get the other port on the same module + // e.g. if channel 0, we need to check if channel 1 is in use and vice versa + if (auto mapping = m_port_to_vendor_map.find(m_port_number ^ 1); mapping != m_port_to_vendor_map.end()) { + other = mapping->second; + other_in_use = other->m_module_in_use; + } + + bool close_both_channels = args().config.close_both_channels.value_or(false); + try { m_module_in_use = false; if (m_SystecRxThread.joinable()) m_SystecRxThread.join(); - std::lock_guard guard(CanVendorSystec::m_handles_lock); - return_code = deinit_channel(); + + auto handle = get_module_handle(); + if (!handle.has_value()) { + LOG(Log::WRN, CanLogIt::h()) << "No handle found for module, close() may have already been called"; + return CanReturnCode::success; // is success correct? + } + + if (close_both_channels && other_in_use) { + LOG(Log::WRN, CanLogIt::h()) << "Deinitialising other channel " << other->m_channel_number << " on module."; + return_code = deinit_other_channel(handle.value(), other); + } + return_code = deinit_channel(handle.value()); + + // if there are no channels still using the handle, deinit hardware + // and erase handle from map + if (!other_in_use || close_both_channels) { + if (auto systec_code = CallAndLog(UcanDeinitHardware, "deinit hw", handle.value()); systec_code != 0) + return_code = CanReturnCode::unknown_close_error; + m_module_to_handle_map.erase(m_module_number); + } } catch (...) { return_code = CanReturnCode::internal_api_error; } return return_code; }; -CanReturnCode CanVendorSystec::deinit_channel() noexcept { +CanReturnCode CanVendorSystec::deinit_channel(tUcanHandle handle) noexcept { auto internal_return_code = CanReturnCode::success; - m_port_to_vendor_map.erase(m_port_number); - auto handle = get_module_handle(); - if (handle.has_value()) { - if (CallAndLog(UcanDeinitCanEx, "deinit channel", handle.value(), m_channel_number)) - internal_return_code = CanReturnCode::unknown_close_error; - - if (!m_port_to_vendor_map[m_port_number ^ 1]) { - // de init hardware if neither channel on the module are in use - // toggle last bit to get the other port on the same module - // e.g. if channel 0, we need to check if channel 1 is in use and vice versa - if (CallAndLog(UcanDeinitHardware, "deinit hw", handle.value())) - internal_return_code = CanReturnCode::unknown_close_error; - m_module_to_handle_map.erase(m_module_number); - } - } else { - LOG(Log::WRN, CanLogIt::h()) << "No handle found for module, close() may have already been called"; - } + if (CallAndLog(UcanDeinitCanEx, "deinit channel", handle, m_channel_number)) + internal_return_code = CanReturnCode::unknown_close_error; + return internal_return_code; +} + +CanReturnCode CanVendorSystec::deinit_other_channel(tUcanHandle handle, CanVendorSystec *other) noexcept { + auto internal_return_code = CanReturnCode::success; + if (CallAndLog(UcanDeinitCanEx, "deinit channel", handle, other->m_channel_number)) + internal_return_code = CanReturnCode::unknown_close_error; + other->m_module_in_use = false; return internal_return_code; } From 406a59bf31e4c8f3a61b8d6cf1dbd66147d9214c Mon Sep 17 00:00:00 2001 From: James Souter Date: Fri, 20 Mar 2026 11:12:54 +0100 Subject: [PATCH 10/15] Add WIP systec test for pcaticswin11 --- docs/CANMODULE-UTILS.md | 2 ++ test/python/test_systec.py | 73 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 test/python/test_systec.py diff --git a/docs/CANMODULE-UTILS.md b/docs/CANMODULE-UTILS.md index 5201ad81..695bd87e 100644 --- a/docs/CANMODULE-UTILS.md +++ b/docs/CANMODULE-UTILS.md @@ -23,6 +23,8 @@ The compatibility matrix is available here: ## Installation To use this tool, ensure you have a recent version of Python installed (tested with version 3.9.18). +Additionally, you need to have `libsocketcan` installed on your Linux system, or `USB-CANmodul Utility Disk` +on Windows to communicate with Systec devices. You will also require the `canmodule.cpython*` file and all Anagate-related libraries (such as `.so` files on Linux or `.dll` files on Windows). These files can be found in the build artifacts, located in the `/build` directory on Linux and the `/build/Release` directory on Windows. diff --git a/test/python/test_systec.py b/test/python/test_systec.py new file mode 100644 index 00000000..7c7e6fb9 --- /dev/null +++ b/test/python/test_systec.py @@ -0,0 +1,73 @@ +from time import sleep, time +import pytest +from common import * +import os +import socket + +pytestmark = pytest.mark.skipif( + socket.gethostname() != "pcaticswin11", + reason="Tests currently only work when run on the pcaticswin11 development server" +) + +ELMB_ID = 19 + +@pytest.fixture +def device_and_frames(): + config = CanDeviceConfiguration() + config.bitrate = 125_000 + config.enable_termination = True + config.high_speed = True + config.bus_number = 0 + received = [] + device = CanDevice.create( + "systec", CanDeviceArguments(config, received.append) + ) + o1 = device.open() + assert o1 == CanReturnCode.success + received.clear() + yield device, received + c1 = device.close() + assert c1 == CanReturnCode.success + + +def test_sync_messages_elmb(device_and_frames): + device, received = device_and_frames + # on new connect, all statistics reset (for now) + diag = device.diagnostics() + assert diag.mode == "NORMAL" + assert diag.state == "No error." + assert isinstance(diag.uptime, int) + rx = diag.rx + tx = diag.tx + assert diag.rx_error == 0 + assert diag.tx_error == 0 + + sleep(1) + received.clear() # hopefully clear any previously buffered frames + r = device.send(CanFrame(0x80)) # send sync message + assert r == CanReturnCode.success + + start = time() + while (time() - start < 15): + if len(received) == 65: + break + else: + raise RuntimeError(f"Did not receive expected frames before timeout: {len(received)}/65") + diff = time() - start + + diag = device.diagnostics() + assert diag.state == "No error." + assert diag.rx - rx == 65 + assert diag.tx - tx == 1 + assert diag.rx_per_second == pytest.approx(65/diff, rel=0.3) + assert diag.tx_per_second == pytest.approx(1/diff, rel=0.3) + + digital_input_frame = received[0] + # see page 16 https://www.nikhef.nl/pub/departments/ct/po/html/ELMB128/ELMB24.pdf + assert digital_input_frame.id() == 0x180 + ELMB_ID + assert ord(digital_input_frame.message()[0]) == 255 + for idx, frame in enumerate(received[1:]): + assert frame.id() == 0x380 + ELMB_ID + assert ord(frame.message()[0]) == idx + + From 6b8b55a1994c5febd5cdd80ba9fe251fda96403a Mon Sep 17 00:00:00 2001 From: James Souter Date: Mon, 13 Apr 2026 16:37:42 +0200 Subject: [PATCH 11/15] Lint fixes for CI black format python files cpplint fixes clang-format on CanVendorSystec --- src/include/CanDeviceConfiguration.h | 15 +- src/include/CanVendorSystec.h | 39 +- src/main/CanVendorSystec.cpp | 673 ++++++++++++++++----------- test/python/test_systec.py | 21 +- 4 files changed, 454 insertions(+), 294 deletions(-) diff --git a/src/include/CanDeviceConfiguration.h b/src/include/CanDeviceConfiguration.h index a727d6ec..3fb60acf 100644 --- a/src/include/CanDeviceConfiguration.h +++ b/src/include/CanDeviceConfiguration.h @@ -96,13 +96,14 @@ struct CanDeviceConfiguration { std::optional sent_acknowledgement; /** - * @brief Enable or disable closing both channels on systec module when one is closed. - * - * This parameter is optional for Systec on Windows and defaults to false. - * If turned on, calling close on a Systec CanDevice will deintialise both channels - * of a module when one is closed. - * - */ + * @brief Enable or disable closing both channels on systec module when one is + * closed. + * + * This parameter is optional for Systec on Windows and defaults to false. + * If turned on, calling close on a Systec CanDevice will deintialise both + * channels of a module when one is closed. + * + */ std::optional close_both_channels; std::string to_string() const noexcept; diff --git a/src/include/CanVendorSystec.h b/src/include/CanVendorSystec.h index b1e5ae90..b8e9b22b 100644 --- a/src/include/CanVendorSystec.h +++ b/src/include/CanVendorSystec.h @@ -1,22 +1,24 @@ #ifndef SRC_INCLUDE_CANVENDORSYSTEC_H_ #define SRC_INCLUDE_CANVENDORSYSTEC_H_ +#include +#include +#include +#include + #include -#include -#include "tchar.h" -#include "Winsock2.h" -#include "windows.h" -#include -#include "usbcan32.h" #include #include +#include #include //NOLINT -#include "CanDiagnostics.h" -#include "CanVendorLoopback.h" -#include "CanDevice.h" -#include #include +#include #include +#include + +#include "CanDevice.h" +#include "CanDiagnostics.h" +#include "CanVendorLoopback.h" /** * @struct CanVendorSystec @@ -31,11 +33,11 @@ struct CanVendorSystec : CanDevice { explicit CanVendorSystec(const CanDeviceArguments& args); ~CanVendorSystec() { vendor_close(); } int SystecRxThread(); - static std::string_view UsbCanGetErrorText( long err_code ); - static std::string UsbCanGetStatusText( long err_code ); + static std::string_view UsbCanGetErrorText(uint16_t err_code); + static std::string UsbCanGetStatusText(uint16_t err_code); - private: - std::atomic m_module_in_use {false}; + private: + std::atomic m_module_in_use{false}; std::atomic m_queued_reads; int m_module_number; int m_channel_number; @@ -44,7 +46,8 @@ struct CanVendorSystec : CanDevice { std::thread m_SystecRxThread; std::optional get_module_handle() { - if (auto mapping = m_module_to_handle_map.find(m_module_number); mapping != m_module_to_handle_map.end()) { + if (auto mapping = m_module_to_handle_map.find(m_module_number); + mapping != m_module_to_handle_map.end()) { return mapping->second; } return std::nullopt; @@ -60,10 +63,12 @@ struct CanVendorSystec : CanDevice { static std::unordered_map m_module_to_handle_map; static std::unordered_map m_port_to_vendor_map; - friend void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p); + friend void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, + BYTE bChannel_p, void* pArg_p); CanReturnCode deinit_channel(tUcanHandle handle) noexcept; - CanReturnCode deinit_other_channel(tUcanHandle handle, CanVendorSystec *other) noexcept; + CanReturnCode deinit_other_channel(tUcanHandle handle, + CanVendorSystec* other) noexcept; }; #endif // SRC_INCLUDE_CANVENDORSYSTEC_H_ diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index d6cd82df..2129f6ff 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -1,9 +1,12 @@ #include "CanVendorSystec.h" -#include -#include #include +#include + +#include #include +#include +#include #include std::mutex CanVendorSystec::m_handles_lock; @@ -12,31 +15,39 @@ std::unordered_map CanVendorSystec::m_port_to_vendor_map; static bool module_control_callback_registered = false; template -long CallAndLog(T f, const char *name, Args... args) { - long code = f(args...); +uint16_t CallAndLog(T f, const char* name, Args... args) { + uint16_t code = f(args...); if (code != USBCAN_SUCCESSFUL) - LOG(Log::ERR, CanLogIt::h()) << "Got error code calling " << name << ": " << CanVendorSystec::UsbCanGetErrorText(code); + LOG(Log::ERR, CanLogIt::h()) << "Got error code calling " << name << ": " + << CanVendorSystec::UsbCanGetErrorText(code); return code; } void connect_control_callback(BYTE bEvent_p, DWORD dwParam_p) { - switch(bEvent_p) { + switch (bEvent_p) { case USBCAN_EVENT_CONNECT: - LOG(Log::DBG) << "USB CAN module connected"; break; + LOG(Log::DBG) << "USB CAN module connected"; + break; case USBCAN_EVENT_DISCONNECT: - LOG(Log::WRN) << "USB CAN module disconnected"; break; + LOG(Log::WRN) << "USB CAN module disconnected"; + break; case USBCAN_EVENT_FATALDISCON: - LOG(Log::ERR) << "USB CAN module with handle " << (int) dwParam_p << "fatally disconnected"; break; + LOG(Log::ERR) << "USB CAN module with handle " + << static_cast(dwParam_p) << "fatally disconnected"; + break; } } // Callback registered per-module to handle receive events -void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, void* pArg_p) { +void systec_receive(tUcanHandle UcanHandle_p, DWORD bEvent_p, BYTE bChannel_p, + void* pArg_p) { if (bEvent_p == USBCAN_EVENT_RECEIVE) { int module_number = *(reinterpret_cast(pArg_p)); int port_number = 2 * module_number + bChannel_p; - CanVendorSystec *vendorPtr = CanVendorSystec::m_port_to_vendor_map[port_number]; - if (vendorPtr) ++(vendorPtr->m_queued_reads); // [] returns nullptr if not found in map; + CanVendorSystec* vendorPtr = + CanVendorSystec::m_port_to_vendor_map[port_number]; + if (vendorPtr) + ++(vendorPtr->m_queued_reads); // [] returns nullptr if not found in map; } } @@ -47,12 +58,24 @@ CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args) } switch (args.config.bitrate.value()) { - case 50000: m_baud_rate = USBCAN_BAUD_50kBit; break; - case 100000: m_baud_rate = USBCAN_BAUD_100kBit; break; - case 125000: m_baud_rate = USBCAN_BAUD_125kBit; break; - case 250000: m_baud_rate = USBCAN_BAUD_250kBit; break; - case 500000: m_baud_rate = USBCAN_BAUD_500kBit; break; - case 1000000: m_baud_rate = USBCAN_BAUD_1MBit; break; + case 50000: + m_baud_rate = USBCAN_BAUD_50kBit; + break; + case 100000: + m_baud_rate = USBCAN_BAUD_100kBit; + break; + case 125000: + m_baud_rate = USBCAN_BAUD_125kBit; + break; + case 250000: + m_baud_rate = USBCAN_BAUD_250kBit; + break; + case 500000: + m_baud_rate = USBCAN_BAUD_500kBit; + break; + case 1000000: + m_baud_rate = USBCAN_BAUD_1MBit; + break; default: { throw std::invalid_argument("Invalid bitrate provided"); } @@ -66,48 +89,61 @@ CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args) CanReturnCode CanVendorSystec::init_can_port() { BYTE systec_call_return = USBCAN_SUCCESSFUL; - tUcanHandle can_module_handle; - - tUcanInitCanParam initialization_parameters; - initialization_parameters.m_dwSize = sizeof(initialization_parameters); // size of this struct - initialization_parameters.m_bMode = kUcanModeNormal; // normal operation mode - initialization_parameters.m_bBTR0 = HIBYTE( m_baud_rate ); // baudrate - initialization_parameters.m_bBTR1 = LOBYTE( m_baud_rate ); - initialization_parameters.m_bOCR = 0x1A; // standard output - initialization_parameters.m_dwAMR = USBCAN_AMR_ALL; // receive all CAN messages - initialization_parameters.m_dwACR = USBCAN_ACR_ALL; - initialization_parameters.m_dwBaudrate = USBCAN_BAUDEX_USE_BTR01; - initialization_parameters.m_wNrOfRxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES; - initialization_parameters.m_wNrOfTxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES; + tUcanHandle can_module_handle; + + tUcanInitCanParam init_params; + init_params.m_dwSize = sizeof(init_params); // size of this struct + init_params.m_bMode = kUcanModeNormal; // normal operation mode + init_params.m_bBTR0 = HIBYTE(m_baud_rate); // baudrate + init_params.m_bBTR1 = LOBYTE(m_baud_rate); + init_params.m_bOCR = 0x1A; // standard output + init_params.m_dwAMR = USBCAN_AMR_ALL; // receive all CAN messages + init_params.m_dwACR = USBCAN_ACR_ALL; + init_params.m_dwBaudrate = USBCAN_BAUDEX_USE_BTR01; + init_params.m_wNrOfRxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES; + init_params.m_wNrOfTxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES; // check if USB-CANmodul is already initialized std::lock_guard guard(CanVendorSystec::m_handles_lock); auto handle = get_module_handle(); - if (!handle.has_value()) { // module not in use + if (!handle.has_value()) { // module not in use if (!module_control_callback_registered) { - if (!CallAndLog(UcanInitHwConnectControl, "hw connect control callback", connect_control_callback)) + if (!CallAndLog(UcanInitHwConnectControl, "hw connect control callback", + connect_control_callback)) module_control_callback_registered = true; } - if (auto systec_code = CallAndLog(UcanInitHardwareEx, "init hardware", &can_module_handle, m_module_number, systec_receive, (void*) &m_module_number); systec_code != 0) { + if (auto systec_code = + CallAndLog(UcanInitHardwareEx, "init hardware", &can_module_handle, + m_module_number, systec_receive, + reinterpret_cast(&m_module_number)); + systec_code != 0) { CallAndLog(UcanDeinitHardware, "deinit hardware", can_module_handle); return CanReturnCode::unknown_open_error; } - LOG(Log::INF, CanLogIt::h()) << "Initialised hardware for Systec module " << m_module_number; + LOG(Log::INF, CanLogIt::h()) + << "Initialised hardware for Systec module " << m_module_number; m_module_to_handle_map[m_module_number] = can_module_handle; - } else { // find existing handle of module + } else { // find existing handle of module can_module_handle = handle.value(); - LOG(Log::INF, CanLogIt::h()) << "Reusing handle (" << (size_t) can_module_handle << ") for module " << m_module_number << " already in use, skipping UCanInitHardwareEx"; + LOG(Log::INF, CanLogIt::h()) + << "Reusing handle (" << static_cast(can_module_handle) + << ") for module " << m_module_number + << " already in use, skipping UCanInitHardwareEx"; } - if (CallAndLog(UcanInitCanEx2, "init channel", can_module_handle, m_channel_number, &initialization_parameters)) { + if (CallAndLog(UcanInitCanEx2, "init channel", can_module_handle, + m_channel_number, &init_params)) { deinit_channel(can_module_handle); return CanReturnCode::unknown_open_error; } // investigate the minimum amount of things to reset to restore good state - CallAndLog(UcanResetCanEx, "reset channel", can_module_handle, (BYTE) m_channel_number, (DWORD) 0); + CallAndLog(UcanResetCanEx, "reset channel", can_module_handle, + (BYTE)m_channel_number, (DWORD)0); - LOG(Log::INF, CanLogIt::h()) << "Successfully opened CAN port on module " << m_module_number << ", channel " << m_channel_number; + LOG(Log::INF, CanLogIt::h()) + << "Successfully opened CAN port on module " << m_module_number + << ", channel " << m_channel_number; return CanReturnCode::success; } @@ -119,7 +155,7 @@ CanReturnCode CanVendorSystec::vendor_open() noexcept { if (return_code != CanReturnCode::success) return return_code; m_module_in_use = true; m_SystecRxThread = std::thread(&CanVendorSystec::SystecRxThread, this); - } catch(...) { + } catch (...) { return_code = CanReturnCode::internal_api_error; } @@ -131,10 +167,11 @@ CanReturnCode CanVendorSystec::vendor_close() noexcept { std::lock_guard guard(CanVendorSystec::m_handles_lock); bool other_in_use = false; - CanVendorSystec *other = nullptr; + CanVendorSystec* other = nullptr; // toggle last bit to get the other port on the same module // e.g. if channel 0, we need to check if channel 1 is in use and vice versa - if (auto mapping = m_port_to_vendor_map.find(m_port_number ^ 1); mapping != m_port_to_vendor_map.end()) { + if (auto mapping = m_port_to_vendor_map.find(m_port_number ^ 1); + mapping != m_port_to_vendor_map.end()) { other = mapping->second; other_in_use = other->m_module_in_use; } @@ -147,12 +184,14 @@ CanReturnCode CanVendorSystec::vendor_close() noexcept { auto handle = get_module_handle(); if (!handle.has_value()) { - LOG(Log::WRN, CanLogIt::h()) << "No handle found for module, close() may have already been called"; - return CanReturnCode::success; // is success correct? + LOG(Log::WRN, CanLogIt::h()) + << "No handle found for module, close() may have already been called"; + return CanReturnCode::success; // is success correct? } if (close_both_channels && other_in_use) { - LOG(Log::WRN, CanLogIt::h()) << "Deinitialising other channel " << other->m_channel_number << " on module."; + LOG(Log::WRN, CanLogIt::h()) << "Deinitialising other channel " + << other->m_channel_number << " on module."; return_code = deinit_other_channel(handle.value(), other); } return_code = deinit_channel(handle.value()); @@ -160,7 +199,9 @@ CanReturnCode CanVendorSystec::vendor_close() noexcept { // if there are no channels still using the handle, deinit hardware // and erase handle from map if (!other_in_use || close_both_channels) { - if (auto systec_code = CallAndLog(UcanDeinitHardware, "deinit hw", handle.value()); systec_code != 0) + if (auto systec_code = + CallAndLog(UcanDeinitHardware, "deinit hw", handle.value()); + systec_code != 0) return_code = CanReturnCode::unknown_close_error; m_module_to_handle_map.erase(m_module_number); } @@ -177,9 +218,11 @@ CanReturnCode CanVendorSystec::deinit_channel(tUcanHandle handle) noexcept { return internal_return_code; } -CanReturnCode CanVendorSystec::deinit_other_channel(tUcanHandle handle, CanVendorSystec *other) noexcept { +CanReturnCode CanVendorSystec::deinit_other_channel( + tUcanHandle handle, CanVendorSystec* other) noexcept { auto internal_return_code = CanReturnCode::success; - if (CallAndLog(UcanDeinitCanEx, "deinit channel", handle, other->m_channel_number)) + if (CallAndLog(UcanDeinitCanEx, "deinit channel", handle, + other->m_channel_number)) internal_return_code = CanReturnCode::unknown_close_error; other->m_module_in_use = false; return internal_return_code; @@ -194,113 +237,154 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept { can_msg_to_send.m_bDLC = frame.length(); can_msg_to_send.m_bFF = 0; if (frame.is_remote_request()) { - can_msg_to_send.m_bFF = USBCAN_MSG_FF_RTR; + can_msg_to_send.m_bFF = USBCAN_MSG_FF_RTR; } - std::copy(message.begin(), message.begin() + can_msg_to_send.m_bDLC, can_msg_to_send.m_bData); + std::copy(message.begin(), message.begin() + can_msg_to_send.m_bDLC, + can_msg_to_send.m_bData); auto handle = get_module_handle(); if (!handle.has_value()) { - LOG(Log::ERR, CanLogIt::h()) << "Could not send message, no handle found for module " << m_module_number; + LOG(Log::ERR, CanLogIt::h()) + << "Could not send message, no handle found for module " + << m_module_number; return CanReturnCode::disconnected; } - switch(CallAndLog(UcanWriteCanMsgEx, "write", handle.value(), m_channel_number, &can_msg_to_send, nullptr)) { - case USBCAN_SUCCESSFUL: break; + switch (CallAndLog(UcanWriteCanMsgEx, "write", handle.value(), + m_channel_number, &can_msg_to_send, nullptr)) { + case USBCAN_SUCCESSFUL: + break; case USBCAN_ERR_CANNOTINIT: - case USBCAN_ERR_ILLHANDLE: return CanReturnCode::disconnected; - case USBCAN_ERR_DLL_TXFULL: return CanReturnCode::tx_buffer_overflow; - case USBCAN_ERR_MAXINSTANCES: return CanReturnCode::too_many_connections; + case USBCAN_ERR_ILLHANDLE: + return CanReturnCode::disconnected; + case USBCAN_ERR_DLL_TXFULL: + return CanReturnCode::tx_buffer_overflow; + case USBCAN_ERR_MAXINSTANCES: + return CanReturnCode::too_many_connections; case USBCAN_ERR_ILLPARAM: case USBCAN_ERR_ILLHW: case USBCAN_ERR_ILLCHANNEL: case USBCAN_WARN_TXLIMIT: case USBCAN_WARN_FW_TXOVERRUN: - default: return CanReturnCode::unknown_send_error; + default: + return CanReturnCode::unknown_send_error; } return CanReturnCode::success; }; CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept { - CanDiagnostics diagnostics{}; tStatusStruct status; auto handle = get_module_handle(); diagnostics.log_entries = std::vector(); if (!handle.has_value()) { - LOG(Log::ERR, CanLogIt::h()) << "Could not get diagnostics as no handle found for module " << m_module_number; + LOG(Log::ERR, CanLogIt::h()) + << "Could not get diagnostics as no handle found for module " + << m_module_number; diagnostics.mode = "OFFLINE"; } else { auto handle_value = handle.value(); - CallAndLog(UcanGetStatusEx, "get status", handle_value, m_channel_number, &status); + CallAndLog(UcanGetStatusEx, "get status", handle_value, m_channel_number, + &status); WORD can_status = status.m_wCanStatus; diagnostics.state = UsbCanGetStatusText(can_status); if (can_status) { diagnostics.log_entries.value().push_back(diagnostics.state.value()); } - tUcanMsgCountInfo msg_count_info; - long err_code; - if (err_code = CallAndLog(UcanGetMsgCountInfoEx, "get msg counts", handle_value, m_channel_number, &msg_count_info); err_code == 0) { + uint16_t err_code; + if (err_code = CallAndLog(UcanGetMsgCountInfoEx, "get msg counts", + handle_value, m_channel_number, &msg_count_info); + err_code == 0) { diagnostics.tx = msg_count_info.m_wSentMsgCount; diagnostics.rx = msg_count_info.m_wRecvdMsgCount; - } else diagnostics.log_entries.value().push_back(std::string(UsbCanGetErrorText(err_code))); + } else { + diagnostics.log_entries.value().push_back( + std::string(UsbCanGetErrorText(err_code))); + } DWORD tx_error, rx_error; - if (err_code = CallAndLog(UcanGetCanErrorCounter, "get errors", handle_value, m_channel_number, &tx_error, &rx_error); err_code == 0) { + if (err_code = + CallAndLog(UcanGetCanErrorCounter, "get errors", handle_value, + m_channel_number, &tx_error, &rx_error); + err_code == 0) { diagnostics.tx_error = tx_error; diagnostics.rx_error = rx_error; - } else diagnostics.log_entries.value().push_back(std::string(UsbCanGetErrorText(err_code))); + } else { + diagnostics.log_entries.value().push_back( + std::string(UsbCanGetErrorText(err_code))); + } tUcanHardwareInfo hw_info; - if (err_code = CallAndLog(UcanGetHardwareInfo, "get hw info", handle_value, &hw_info); err_code != 0) { + if (err_code = CallAndLog(UcanGetHardwareInfo, "get hw info", handle_value, + &hw_info); + err_code != 0) { diagnostics.mode = "OFFLINE"; - diagnostics.log_entries.value().push_back(std::string(UsbCanGetErrorText(err_code))); - } else switch (hw_info.m_bMode) { - case kUcanModeNormal: diagnostics.mode = "NORMAL"; break; - case kUcanModeListenOnly: diagnostics.mode = "LISTEN_ONLY"; break; - case kUcanModeTxEcho: diagnostics.mode = "LOOPBACK"; break; + diagnostics.log_entries.value().push_back( + std::string(UsbCanGetErrorText(err_code))); + } else { + switch (hw_info.m_bMode) { + case kUcanModeNormal: + diagnostics.mode = "NORMAL"; + break; + case kUcanModeListenOnly: + diagnostics.mode = "LISTEN_ONLY"; + break; + case kUcanModeTxEcho: + diagnostics.mode = "LOOPBACK"; + break; + } } - DWORD module_time; // in ms - if (err_code = CallAndLog(UcanGetModuleTime, "get module time", handle_value, &module_time); err_code == 0) - diagnostics.uptime = (uint32_t) module_time / 1000; - else diagnostics.log_entries.value().push_back(std::string(UsbCanGetErrorText(err_code))); + DWORD module_time; // in ms + if (err_code = CallAndLog(UcanGetModuleTime, "get module time", + handle_value, &module_time); + err_code == 0) + diagnostics.uptime = static_cast(module_time) / 1000; + else + diagnostics.log_entries.value().push_back( + std::string(UsbCanGetErrorText(err_code))); } return diagnostics; }; - /** * thread to handle reception of Can messages from the systec device */ -int CanVendorSystec::SystecRxThread() -{ +int CanVendorSystec::SystecRxThread() { BYTE status; tCanMsgStruct read_can_message; - LOG(Log::DBG, CanLogIt::h()) << "SystecRxThread Started. m_module_in_use = [" << m_module_in_use <<"]"; + LOG(Log::DBG, CanLogIt::h()) << "SystecRxThread Started. m_module_in_use = [" + << m_module_in_use << "]"; size_t to_read; auto handle = get_module_handle(); if (!handle.has_value()) { - LOG(Log::ERR, CanLogIt::h()) << "Could not start rx thread without valid handle for module"; - return -1; // TODO more useful error code + LOG(Log::ERR, CanLogIt::h()) + << "Could not start rx thread without valid handle for module"; + return -1; // TODO(jsouter): more useful error code } while (m_module_in_use) { to_read = m_queued_reads; if (to_read < 1) continue; - status = UcanReadCanMsgEx(handle.value(), (BYTE *) &m_channel_number, &read_can_message, NULL); + status = UcanReadCanMsgEx(handle.value(), + reinterpret_cast(&m_channel_number), + &read_can_message, NULL); switch (status) { case USBCAN_WARN_SYS_RXOVERRUN: case USBCAN_WARN_DLL_RXOVERRUN: case USBCAN_WARN_FW_RXOVERRUN: LOG(Log::WRN, CanLogIt::h()) << UsbCanGetErrorText(status); - [[ fallthrough ]]; + [[fallthrough]]; case USBCAN_SUCCESSFUL: { --m_queued_reads; if (read_can_message.m_bFF & USBCAN_MSG_FF_RTR) break; - std::vector data(read_can_message.m_bData, read_can_message.m_bData + read_can_message.m_bDLC); - CanFrame can_frame(read_can_message.m_dwID, data, read_can_message.m_bFF); + std::vector data( + read_can_message.m_bData, + read_can_message.m_bData + read_can_message.m_bDLC); + CanFrame can_frame(read_can_message.m_dwID, data, + read_can_message.m_bFF); received(can_frame); break; } @@ -308,10 +392,9 @@ int CanVendorSystec::SystecRxThread() m_queued_reads -= to_read; LOG(Log::WRN, CanLogIt::h()) << UsbCanGetErrorText(status); break; - default: // errors + default: // errors // USBCAN_ERR_MAXINSTANCES, USBCAN_ERR_ILLHANDLE, USBCAN_ERR_CANNOTINIT, // USBCAN_ERR_ILLPARAM, USBCAN_ERR_ILLHW, USBCAN_ERR_ILLCHANNEL - // TODO should we raise some error state here? LOG(Log::ERR, CanLogIt::h()) << UsbCanGetErrorText(status); break; } @@ -320,176 +403,248 @@ int CanVendorSystec::SystecRxThread() return 0; } -std::string_view CanVendorSystec::UsbCanGetErrorText( long err_code ) { - switch( err_code ){ - case USBCAN_SUCCESSFUL: return("success"); - - case USBCAN_ERR_RESOURCE: return ("This error code returns if one resource could not be generated. In this " - "case the term resource means memory and handles provided by the Windows OS"); - - case USBCAN_ERR_MAXMODULES: return("An application has tried to open more than 64 USB-CANmodul devices. " - "The standard version of the DLL only supports up to 64 USB-CANmodul " - "devices at the same time. This error also appears if several applications " - "try to access more than 64 USB-CANmodul devices. For example, " - "application 1 has opened 60 modules, application 2 has opened 4 " - "modules and application 3 wants to open a module. Application 3 " - "receives this error code."); - - case USBCAN_ERR_HWINUSE: return("An application tries to initialize an USB-CANmodul with the given device " - "number. If this module has already been initialized by its own or by " - "another application, this error code is returned."); - - case USBCAN_ERR_ILLVERSION: return("This error code returns if the firmware version of the USB-CANmodul is " - "not compatible to the software version of the DLL. In this case, install " - "the latest driver for the USB-CANmodul. Furthermore make sure that " - "the latest firmware version is programmed to the USB-CANmodul."); - - case USBCAN_ERR_ILLHW: return("This error code returns if an USB-CANmodul with the given device " - "number is not found. If the function UcanInitHardware() or " - "UcanInitHardwareEx() has been called with the device number " - "USBCAN_ANY_MODULE, and the error code appears, it indicates that " - "no module is connected to the PC or all connected modules are already " - "in use."); - - case USBCAN_ERR_ILLHANDLE: return("This error code returns if a function received an incorrect USBCAN " - "handle. The function first checks which USB-CANmodul is related to this " - "handle. This error occurs if no device belongs this handle."); - - case USBCAN_ERR_ILLPARAM: return("This error code returns if a wrong parameter is passed to the function. " - "For example, the value NULL has been passed to a pointer variable " - "instead of a valid address."); - - case USBCAN_ERR_BUSY: return("This error code occurs if several threads are accessing an " - "USB-CANmodul within a single application. After the other threads have " - "finished their tasks, the function may be called again."); - - case USBCAN_ERR_TIMEOUT: return("This error code occurs if the function transmits a command to the " - "USB-CANmodul but no reply is returned. To solve this problem, close " - "the application, disconnect the USB-CANmodul, and connect it again."); - - case USBCAN_ERR_IOFAILED: return("This error code occurs if the communication to the kernel driver was " - "interrupted. This happens, for example, if the USB-CANmodul is " - "disconnected during transferring data or commands to the " - "USB-CANmodul."); - - case USBCAN_ERR_DLL_TXFULL: return("The function UcanWriteCanMsg() or UcanWriteCanMsgEx() first checks " - "if the transmit buffer within the DLL has enough capacity to store new " - "CAN messages. If the buffer is full, this error code returns. The CAN " - "message passed to these functions will not be written into the transmit " - "buffer in order to protect other CAN messages against overwriting. The " - "size of the transmit buffer is configurable (refer to function " - "UcanInitCanEx() and structure tUcanInitCanParam)."); - - case USBCAN_ERR_MAXINSTANCES: return("A maximum amount of 64 applications are able to have access to the " - "DLL. If more applications attempting to access to the DLL, this error " - "code is returned. In this case, it is not possible to use an " - "USB-CANmodul by this application."); - - case USBCAN_ERR_CANNOTINIT: return("This error code returns if an application tries to call an API function " - "which only can be called in software state CAN_INIT but the current " - "software is still in state HW_INIT. Refer to section 4.3.1 and Table 11 for " - "detailed information."); - - case USBCAN_ERR_DISCONNECT: return("This error code occurs if an API function was called for an " - "USB-CANmodul that was plugged-off from the computer recently."); - - case USBCAN_ERR_ILLCHANNEL: return("This error code is returned if an extended function of the DLL is called " - "with parameter bChannel_p = USBCAN_CHANNEL_CH1, but a single-channel USB-CANmodul was used."); - - case USBCAN_ERR_ILLHWTYPE: return("This error code occurs if an extended function of the DLL was called for " - "a hardware which does not support the feature."); - - case USBCAN_ERRCMD_NOTEQU: return("This error code occurs during communication between the PC and an " - "USB-CANmodul. The PC sends a command to the USB-CANmodul, " - "then the module executes the command and returns a response to the " - "PC. This error code returns if the reply does not correspond to the command."); - - case USBCAN_ERRCMD_REGTST: return("The software tests the CAN controller on the USB-CANmodul when the " - "CAN interface is initialized. Several registers of the CAN controller are " - "checked. This error code returns if an error appears during this register test."); - - case USBCAN_ERRCMD_ILLCMD: return("This error code returns if the USB-CANmodul receives a non-defined " - "command. This error represents a version conflict between the firmware in the USB-CANmodul and the DLL."); - - case USBCAN_ERRCMD_EEPROM: return("The USB-CANmodul has a built-in EEPROM. This EEPROM contains " - "several configurations, e.g. the device number and the serial number. If " - "an error occurs while reading these values, this error code is returned."); - - case USBCAN_ERRCMD_ILLBDR: return("The USB-CANmodul has been initialized with an invalid baud rate (refer " - "to section 4.3.4)."); - - case USBCAN_ERRCMD_NOTINIT: return("It was tried to access a CAN-channel of a multi-channel " - "USB-CANmodul that was not initialized."); - - case USBCAN_ERRCMD_ALREADYINIT: return("The accessed CAN-channel of a multi-channel USB-CANmodul was " - "already initialized"); - - case USBCAN_ERRCMD_ILLSUBCMD: return("An internal error occurred within the DLL. In this case an unknown sub- " - "command was called instead of a main command (e.g. for the cyclic CAN message-feature)."); - - case USBCAN_ERRCMD_ILLIDX: return("An internal error occurred within the DLL. In this case an invalid index " - "for a list was delivered to the firmware (e.g. for the cyclic CAN message-feature)."); - - case USBCAN_ERRCMD_RUNNING: return("The caller tries to define a new list of cyclic CAN messages but this " - "feature was already started. For defining a new list, it is necessary to stop the feature beforehand."); - - case USBCAN_WARN_NODATA: return("If the function UcanReadCanMsg() or UcanReadCanMsgEx() returns " - "with this warning, it is an indication that the receive buffer contains no CAN messages."); - - case USBCAN_WARN_SYS_RXOVERRUN: return("This is returned by UcanReadCanMsg() or UcanReadCanMsgEx() if the " - "receive buffer within the kernel driver runs over. The function " - "nevertheless returns a valid CAN message. It also indicates that at least " - "one CAN message are lost. However, it does not indicate the position of the lost CAN messages."); - - case USBCAN_WARN_DLL_RXOVERRUN: return("The DLL automatically requests CAN messages from the " - "USB-CANmodul and stores the messages into a buffer of the DLL. If " - "more CAN messages are received than the DLL buffer size allows, this " - "error code returns and CAN messages are lost. However, it does not " - "indicate the position of the lost CAN messages. The size of the receive " - "buffer is configurable (refer to function UcanInitCanEx() and structure " - "tUcanInitCanParam)."); - - case USBCAN_WARN_FW_TXOVERRUN: return("This warning is returned by function UcanWriteCanMsg() or " - "UcanWriteCanMsgEx() if flag USBCAN_CANERR_QXMTFULL is set in " - "the CAN driver status. However, the transmit CAN message could be " - "stored to the DLL transmit buffer. This warning indicates that at least " - "one transmit CAN message got lost in the device firmware layer. This " - "warning does not indicate the position of the lost CAN message."); - - case USBCAN_WARN_FW_RXOVERRUN: return("This warning is returned by function UcanWriteCanMsg() or " - "UcanWriteCanMsgEx() if flag USBCAN_CANERR_QOVERRUN or flag " - "USBCAN_CANERR_OVERRUN are set in the CAN driver status. The " - "function has returned with a valid CAN message. This warning indicates " - "that at least one received CAN message got lost in the firmware layer. " - "This warning does not indicate the position of the lost CAN message."); - - case USBCAN_WARN_NULL_PTR: return("This warning is returned by functions UcanInitHwConnectControl() or " - "UcanInitHwConnectControlEx() if a NULL pointer was passed as callback function address."); - - case USBCAN_WARN_TXLIMIT: return("This warning is returned by the function UcanWriteCanMsgEx() if it was " - "called to transmit more than one CAN message, but a part of them " - "could not be stored to the transmit buffer within the DLL (because the " - "buffer is full). The returned variable addressed by the parameter " - "pdwCount_p indicates the number of CAN messages which are stored " - "successfully to the transmit buffer."); - - default: return("unknown error code"); -std::string CanVendorSystec::UsbCanGetStatusText(long err_code) { - switch(err_code) { - case USBCAN_CANERR_OK: return "No error."; - case USBCAN_CANERR_XMTFULL: return "Transmit buffer in CAN controller is overrun."; - case USBCAN_CANERR_OVERRUN: return "Receive buffer in CAN controller is overrun."; - case USBCAN_CANERR_BUSLIGHT: return " Error limit 1 in CAN controller exceeded, CAN controller " - "is in state “Warning limit” now."; - case USBCAN_CANERR_BUSHEAVY: return "Error limit 2 in CAN controller exceeded, CAN controller " - "is in state “Error Passive” now"; - case USBCAN_CANERR_BUSOFF: return "CAN controller is in BUSOFF state."; - case USBCAN_CANERR_QOVERRUN: return "Receive buffer in module is overrun."; - case USBCAN_CANERR_QXMTFULL: return "Transmit buffer in module is overrun."; - case USBCAN_CANERR_REGTEST: return "CAN controller not found (hardware error)."; - case USBCAN_CANERR_TXMSGLOST: return "A transmit CAN message was deleted automatically by the " - "firmware because transmission timeout run over (refer to " - "function UcanSetTxTimeout() )."; +std::string_view CanVendorSystec::UsbCanGetErrorText(uint16_t err_code) { + switch (err_code) { + case USBCAN_SUCCESSFUL: + return "success"; + + case USBCAN_ERR_RESOURCE: + return "This error code returns if one resource could not be generated. " + "In this case the term resource means memory and handles provided " + "by the Windows OS"; + + case USBCAN_ERR_MAXMODULES: + return "An application has tried to open more than 64 USB-CANmodul " + "devices. The standard version of the DLL only supports up to 64 " + "USB-CANmodul devices at the same time. This error also appears " + "if several applications try to access more than 64 USB-CANmodul " + "devices. For example, application 1 has opened 60 modules, " + "application 2 has opened 4 modules and application 3 wants to " + "open a module. Application 3 receives this error code."; + + case USBCAN_ERR_HWINUSE: + return "An application tries to initialize an USB-CANmodul with the " + "given device number. If this module has already been initialized " + "by its own or by another application, this error code is " + "returned."; + + case USBCAN_ERR_ILLVERSION: + return "This error code returns if the firmware version of the " + "USB-CANmodul is not compatible to the software version of the " + "DLL. " + "In this case, install the latest driver for the USB-CANmodul. " + "Furthermore make sure that the latest firmware version is " + "programmed to the USB-CANmodul."; + + case USBCAN_ERR_ILLHW: + return "This error code returns if an USB-CANmodul with the given device " + "number is not found. If the function UcanInitHardware() or " + "UcanInitHardwareEx() has been called with the device number " + "USBCAN_ANY_MODULE, and the error code appears, it indicates that " + "no module is connected to the PC or all connected modules are " + "already in use."; + + case USBCAN_ERR_ILLHANDLE: + return "This error code returns if a function received an incorrect " + "USBCAN handle. The function first checks which USB-CANmodul is " + "related to this handle. This error occurs if no device belongs " + "this handle."; + + case USBCAN_ERR_ILLPARAM: + return "This error code returns if a wrong parameter is passed to the " + "function. For example, the value NULL has been passed to a " + "pointer variable instead of a valid address."; + + case USBCAN_ERR_BUSY: + return "This error code occurs if several threads are accessing an " + "USB-CANmodul within a single application. After the other " + "threads have finished their tasks, the function may be called " + "again."; + + case USBCAN_ERR_TIMEOUT: + return "This error code occurs if the function transmits a command to " + "the USB-CANmodul but no reply is returned. To solve this " + "problem, close the application, disconnect the USB-CANmodul, " + "and connect it again."; + + case USBCAN_ERR_IOFAILED: + return "This error code occurs if the communication to the kernel driver " + "was interrupted. This happens, for example, if the USB-CANmodul " + "is disconnected during transferring data or commands to the " + "USB-CANmodul."; + + case USBCAN_ERR_DLL_TXFULL: + return "The function UcanWriteCanMsg() or UcanWriteCanMsgEx() first " + "checks if the transmit buffer within the DLL has enough capacity " + "to store new CAN messages. If the buffer is full, this error " + "code returns. " + "The CAN message passed to these functions will not be written " + "into the transmit buffer in order to protect other CAN messages " + "against overwriting. The size of the transmit buffer is " + "configurable (refer to function UcanInitCanEx() and structure " + "tUcanInitCanParam)."; + + case USBCAN_ERR_MAXINSTANCES: + return "A maximum amount of 64 applications are able to have access to " + "the DLL. If more applications attempting to access to the DLL, " + "this error code is returned. In this case, it is not possible to " + "use an USB-CANmodul by this application."; + + case USBCAN_ERR_CANNOTINIT: + return "This error code returns if an application tries to call an API " + "function which only can be called in software state CAN_INIT but " + "the current software is still in state HW_INIT. Refer to section " + "4.3.1 and Table 11 for detailed information."; + + case USBCAN_ERR_DISCONNECT: + return "This error code occurs if an API function was called for an " + "USB-CANmodul that was plugged-off from the computer recently."; + + case USBCAN_ERR_ILLCHANNEL: + return "This error code is returned if an extended function of the DLL " + "is called with parameter bChannel_p = USBCAN_CHANNEL_CH1, but a " + "single-channel USB-CANmodul was used."; + + case USBCAN_ERR_ILLHWTYPE: + return "This error code occurs if an extended function of the DLL was " + "called for a hardware which does not support the feature."; + + case USBCAN_ERRCMD_NOTEQU: + return "This error code occurs during communication between the PC and " + "an USB-CANmodul. The PC sends a command to the USB-CANmodul, " + "then the module executes the command and returns a response to " + "the PC. This error code returns if the reply does not correspond " + "to the command."; + + case USBCAN_ERRCMD_REGTST: + return "The software tests the CAN controller on the USB-CANmodul when " + "the CAN interface is initialized. Several registers of the CAN " + "controller are checked. This error code returns if an error " + "appears during this register test."; + + case USBCAN_ERRCMD_ILLCMD: + return "This error code returns if the USB-CANmodul receives a " + "non-defined command. This error represents a version conflict " + "between the firmware in the USB-CANmodul and the DLL."; + + case USBCAN_ERRCMD_EEPROM: + return "The USB-CANmodul has a built-in EEPROM. This EEPROM contains " + "several configurations, e.g. the device number and the serial " + "number. If an error occurs while reading these values, this " + "error code is returned."; + + case USBCAN_ERRCMD_ILLBDR: + return "The USB-CANmodul has been initialized with an invalid baud rate " + "(refer to section 4.3.4)."; + + case USBCAN_ERRCMD_NOTINIT: + return "It was tried to access a CAN-channel of a multi-channel " + "USB-CANmodul that was not initialized."; + + case USBCAN_ERRCMD_ALREADYINIT: + return "The accessed CAN-channel of a multi-channel USB-CANmodul was " + "already initialized"; + + case USBCAN_ERRCMD_ILLSUBCMD: + return "An internal error occurred within the DLL. In this case an " + "unknown sub-command was called instead of a main command (e.g. " + "for the cyclic CAN message-feature)."; + + case USBCAN_ERRCMD_ILLIDX: + return "An internal error occurred within the DLL. In this case an " + "invalid index for a list was delivered to the firmware (e.g. for " + "the cyclic CAN message-feature)."; + + case USBCAN_ERRCMD_RUNNING: + return "The caller tries to define a new list of cyclic CAN messages but " + "this feature was already started. For defining a new list, it is " + "necessary to stop the feature beforehand."; + + case USBCAN_WARN_NODATA: + return "If the function UcanReadCanMsg() or UcanReadCanMsgEx() returns " + "with this warning, it is an indication that the receive buffer " + "contains no CAN messages."; + + case USBCAN_WARN_SYS_RXOVERRUN: + return "This is returned by UcanReadCanMsg() or UcanReadCanMsgEx() if " + "the receive buffer within the kernel driver runs over. The " + "function nevertheless returns a valid CAN message. It also " + "indicates that at least one CAN message are lost. However, it " + "does not indicate the position of the lost CAN messages."; + + case USBCAN_WARN_DLL_RXOVERRUN: + return "The DLL automatically requests CAN messages from the " + "USB-CANmodul and stores the messages into a buffer of the DLL. " + "If more CAN messages are received than the DLL buffer size " + "allows, this error code returns and CAN messages are lost. " + "However, it does not indicate the position of the lost CAN " + "messages. The size of the receive buffer is configurable (refer " + "to function UcanInitCanEx() and structure tUcanInitCanParam)."; + + case USBCAN_WARN_FW_TXOVERRUN: + return "This warning is returned by function UcanWriteCanMsg() or " + "UcanWriteCanMsgEx() if flag USBCAN_CANERR_QXMTFULL is set in " + "the CAN driver status. However, the transmit CAN message could " + "be stored to the DLL transmit buffer. This warning indicates " + "that at least one transmit CAN message got lost in the device " + "firmware layer. This warning does not indicate the position of " + "the lost CAN message."; + + case USBCAN_WARN_FW_RXOVERRUN: + return "This warning is returned by function UcanWriteCanMsg() or " + "UcanWriteCanMsgEx() if flag USBCAN_CANERR_QOVERRUN or flag " + "USBCAN_CANERR_OVERRUN are set in the CAN driver status. The " + "function has returned with a valid CAN message. This warning " + "indicates that at least one received CAN message got lost in the " + "firmware layer. This warning does not indicate the position of " + "the lost CAN message."; + + case USBCAN_WARN_NULL_PTR: + return "This warning is returned by functions UcanInitHwConnectControl() " + "or UcanInitHwConnectControlEx() if a NULL pointer was passed as " + "callback function address."; + + case USBCAN_WARN_TXLIMIT: + return "This warning is returned by the function UcanWriteCanMsgEx() if " + "it was called to transmit more than one CAN message, but a part " + "of them could not be stored to the transmit buffer within the " + "DLL because the buffer is full). The returned variable addressed " + "by the parameter pdwCount_p indicates the number of CAN messages " + "which are stored successfully to the transmit buffer."; + + default: + std::stringstream ss; + ss << "Unknown error code: 0x" << std::hex << err_code; + return ss.str(); + } +} + +std::string CanVendorSystec::UsbCanGetStatusText(uint16_t err_code) { + switch (err_code) { + case USBCAN_CANERR_OK: + return "No error."; + case USBCAN_CANERR_XMTFULL: + return "Transmit buffer in CAN controller is overrun."; + case USBCAN_CANERR_OVERRUN: + return "Receive buffer in CAN controller is overrun."; + case USBCAN_CANERR_BUSLIGHT: + return " Error limit 1 in CAN controller exceeded, CAN controller " + "is in state “Warning limit” now."; + case USBCAN_CANERR_BUSHEAVY: + return "Error limit 2 in CAN controller exceeded, CAN controller " + "is in state “Error Passive” now"; + case USBCAN_CANERR_BUSOFF: + return "CAN controller is in BUSOFF state."; + case USBCAN_CANERR_QOVERRUN: + return "Receive buffer in module is overrun."; + case USBCAN_CANERR_QXMTFULL: + return "Transmit buffer in module is overrun."; + case USBCAN_CANERR_REGTEST: + return "CAN controller not found (hardware error)."; + case USBCAN_CANERR_TXMSGLOST: + return "A transmit CAN message was deleted automatically by the " + "firmware because transmission timeout run over (refer to " + "function UcanSetTxTimeout() )."; default: std::stringstream ss; ss << "Unknown error code: 0x" << std::hex << err_code; diff --git a/test/python/test_systec.py b/test/python/test_systec.py index 7c7e6fb9..496b0535 100644 --- a/test/python/test_systec.py +++ b/test/python/test_systec.py @@ -6,11 +6,12 @@ pytestmark = pytest.mark.skipif( socket.gethostname() != "pcaticswin11", - reason="Tests currently only work when run on the pcaticswin11 development server" + reason="Tests currently only work when run on the pcaticswin11 development server", ) ELMB_ID = 19 + @pytest.fixture def device_and_frames(): config = CanDeviceConfiguration() @@ -19,9 +20,7 @@ def device_and_frames(): config.high_speed = True config.bus_number = 0 received = [] - device = CanDevice.create( - "systec", CanDeviceArguments(config, received.append) - ) + device = CanDevice.create("systec", CanDeviceArguments(config, received.append)) o1 = device.open() assert o1 == CanReturnCode.success received.clear() @@ -43,24 +42,26 @@ def test_sync_messages_elmb(device_and_frames): assert diag.tx_error == 0 sleep(1) - received.clear() # hopefully clear any previously buffered frames + received.clear() # hopefully clear any previously buffered frames r = device.send(CanFrame(0x80)) # send sync message assert r == CanReturnCode.success start = time() - while (time() - start < 15): + while time() - start < 15: if len(received) == 65: break else: - raise RuntimeError(f"Did not receive expected frames before timeout: {len(received)}/65") + raise RuntimeError( + f"Did not receive expected frames before timeout: {len(received)}/65" + ) diff = time() - start diag = device.diagnostics() assert diag.state == "No error." assert diag.rx - rx == 65 assert diag.tx - tx == 1 - assert diag.rx_per_second == pytest.approx(65/diff, rel=0.3) - assert diag.tx_per_second == pytest.approx(1/diff, rel=0.3) + assert diag.rx_per_second == pytest.approx(65 / diff, rel=0.3) + assert diag.tx_per_second == pytest.approx(1 / diff, rel=0.3) digital_input_frame = received[0] # see page 16 https://www.nikhef.nl/pub/departments/ct/po/html/ELMB128/ELMB24.pdf @@ -69,5 +70,3 @@ def test_sync_messages_elmb(device_and_frames): for idx, frame in enumerate(received[1:]): assert frame.id() == 0x380 + ELMB_ID assert ord(frame.message()[0]) == idx - - From f0c97b6cd89f90252fb1218fb75c61d00e7d73ee Mon Sep 17 00:00:00 2001 From: James Souter Date: Wed, 22 Apr 2026 11:14:06 +0200 Subject: [PATCH 12/15] build systec on windows by default --- CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index a80feb3a..f41faec3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -29,6 +29,10 @@ set(VENDOR_SOURCES src/main/CanVendorAnagate.cpp ) +if (NOT DEFINED CANMODULE_BUILD_SYSTEC_WINDOWS AND WIN32) + set(CANMODULE_BUILD_SYSTEC_WINDOWS ON) +endif() + if (UNIX) include(cmake/libsocketcan.cmake) include_directories(${LIBSOCKETCAN_INCLUDE_DIR}) From 9f101b7a9179d3cb427fb0e463f1a022e6eada2e Mon Sep 17 00:00:00 2001 From: James Souter Date: Wed, 29 Jul 2026 10:07:14 +0200 Subject: [PATCH 13/15] remove close-both-channels option --- src/include/CanDeviceConfiguration.h | 11 ----------- src/main/CanVendorSystec.cpp | 9 +-------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/include/CanDeviceConfiguration.h b/src/include/CanDeviceConfiguration.h index 3fb60acf..93731a99 100644 --- a/src/include/CanDeviceConfiguration.h +++ b/src/include/CanDeviceConfiguration.h @@ -95,17 +95,6 @@ struct CanDeviceConfiguration { */ std::optional sent_acknowledgement; - /** - * @brief Enable or disable closing both channels on systec module when one is - * closed. - * - * This parameter is optional for Systec on Windows and defaults to false. - * If turned on, calling close on a Systec CanDevice will deintialise both - * channels of a module when one is closed. - * - */ - std::optional close_both_channels; - std::string to_string() const noexcept; }; diff --git a/src/main/CanVendorSystec.cpp b/src/main/CanVendorSystec.cpp index 2129f6ff..28446b86 100644 --- a/src/main/CanVendorSystec.cpp +++ b/src/main/CanVendorSystec.cpp @@ -176,8 +176,6 @@ CanReturnCode CanVendorSystec::vendor_close() noexcept { other_in_use = other->m_module_in_use; } - bool close_both_channels = args().config.close_both_channels.value_or(false); - try { m_module_in_use = false; if (m_SystecRxThread.joinable()) m_SystecRxThread.join(); @@ -189,16 +187,11 @@ CanReturnCode CanVendorSystec::vendor_close() noexcept { return CanReturnCode::success; // is success correct? } - if (close_both_channels && other_in_use) { - LOG(Log::WRN, CanLogIt::h()) << "Deinitialising other channel " - << other->m_channel_number << " on module."; - return_code = deinit_other_channel(handle.value(), other); - } return_code = deinit_channel(handle.value()); // if there are no channels still using the handle, deinit hardware // and erase handle from map - if (!other_in_use || close_both_channels) { + if (!other_in_use) { if (auto systec_code = CallAndLog(UcanDeinitHardware, "deinit hw", handle.value()); systec_code != 0) From af0ce71bb468754c4480149d6fd198ae83c31ceb Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 30 Jul 2026 10:29:41 +0100 Subject: [PATCH 14/15] Add systec module to canmodule-utils --- docs/CANMODULE-UTILS.md | 4 ++++ python/canmodule_utils/cli.py | 3 +++ 2 files changed, 7 insertions(+) diff --git a/docs/CANMODULE-UTILS.md b/docs/CANMODULE-UTILS.md index 695bd87e..e396fb6b 100644 --- a/docs/CANMODULE-UTILS.md +++ b/docs/CANMODULE-UTILS.md @@ -88,6 +88,7 @@ It opens a connection to the device and print all received frames. The syntax is ```bash python canmodule-utils.py anagate dump --set host=192.168.1.20 --set bus_number=0 python canmodule-utils.py socketcan dump --set bus_name=can0 +python canmodule-utils.py systec dump --set bus_number=0 --set bitrate=125000 ``` ### send @@ -97,6 +98,7 @@ It sends a single CAN frame. The syntax is: ```bash python canmodule-utils.py anagate send [can_frame] --config anagate0.json python canmodule-utils.py socketcan send [can_frame] --set bus_name=can0 +python canmodule-utils.py systec send [can_frame] --set bus_number=0 --set bitrate=125000 ``` Examples of CAN frames are: @@ -116,6 +118,7 @@ It opens a connection and send random frames. The syntax is: ```bash python canmodule-utils.py anagate gen --set host=192.168.1.20 --set bus_number=0 python canmodule-utils.py socketcan gen --set bus_name=can0 +python canmodule-utils.py systec gen --set bus_number=0 --set bitrate=125000 ``` ### diag @@ -125,4 +128,5 @@ It opens a connection and print the diagnostics. The syntax is: ```bash python canmodule-utils.py anagate diag --set host=192.168.1.20 --set bus_number=0 python canmodule-utils.py socketcan diag --set bus_name=can0 +python canmodule-utils.py systec diag --set bus_number=0 --set bitrate=125000 ``` diff --git a/python/canmodule_utils/cli.py b/python/canmodule_utils/cli.py index e64ece55..1dde620d 100644 --- a/python/canmodule_utils/cli.py +++ b/python/canmodule_utils/cli.py @@ -53,6 +53,9 @@ def build_parser(): ) add_action_parsers(socketcan_parser, "SocketCAN") + systec_parser = subparsers.add_parser("systec", help="Use the Systec module") + add_action_parsers(systec_parser, "Systec") + return parser From 299dd48da5dfffdc9243a1027949c4c024fae028 Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 30 Jul 2026 12:26:08 +0200 Subject: [PATCH 15/15] exclude windows-only CanVendorSystec.cpp from run-clang-analysis-linux.sh --- ci/run-clang-analysis-linux.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/run-clang-analysis-linux.sh b/ci/run-clang-analysis-linux.sh index 652fa7f7..605c70e8 100755 --- a/ci/run-clang-analysis-linux.sh +++ b/ci/run-clang-analysis-linux.sh @@ -110,7 +110,7 @@ fi log "Running clang-tidy" tidy_status=0 rm -f "$TIDY_LOG" -mapfile -t tidy_files < <(find src/main src/python -type f \( -name '*.c' -o -name '*.cc' -o -name '*.cpp' -o -name '*.cxx' \) | sort) +mapfile -t tidy_files < <(find src/main src/python -type f \( -name '*.c' -o -name '*.cc' -o -name '*.cpp' -o -name '*.cxx' \) | grep -v CanVendorSystec.cpp | sort) if [[ ${#tidy_files[@]} -eq 0 ]]; then log "ERROR: no project C/C++ source files found for clang-tidy" exit 1