Ixxat bus state and hardware errors detection (#1141)

* Added comment to CAN_MSGFLAGS_* and CAN_MSGFLAGS2_* constants

* CANMSGINFO.bAddFlags has been renamed to bFlags2 in IXXAT VCI4

* Added a comment that CANMSGINFO.Bytes.bAddFlags is called bFlags2 in VCI v4.

* Implementation is now tested against VCI v4

* Dropped manual timeout handling as it is a job done by BusABC.read().
Better handling of CAN error messages:
  - In case of HW errors (overrun, warning limit exceeded, bus coupling error) raise VCIError
  - In case of error log IXXAT-specific error codes

* Fixed unbound variable usage.
Use log.warning instead of deprecated log.warn.

* Now wrapping IXXAT VCI v4

* Added CAN_OPMODE_AUTOBAUD controller operating mode.

* Mapped symbol canChannelGetStatus, used to implement the 'state' property.
Hardware error checking in _recv_internal() handles BUS OFF situation.

* Use CANMSGINFO.bAddFlags instead of CANMSGINFO.bFlags2

* Call canControlClose() AFTER canControlReset() or it will always fail

* Added CANMSG.__str__

* Removed conflict marker

* Renamed parameter 'msg' to 'msgs' in _send_periodic_internal(), consistent with BusABC

* Changed plain format() calls to f-strings as per review

* Removed binascii module dependency using memoryview

Co-authored-by: Giuseppe Corbelli <[email protected]>

Author: cowo78

can/interfaces/ixxat/__init__.py

@@ -1,7 +1,7 @@
 """
-Ctypes wrapper module for IXXAT Virtual CAN Interface V3 on win32 systems
+Ctypes wrapper module for IXXAT Virtual CAN Interface V4 on win32 systems
 
-Copyright (C) 2016 Giuseppe Corbelli <[email protected]>
+Copyright (C) 2016-2021 Giuseppe Corbelli <[email protected]>
 """
 
 from can.interfaces.ixxat.canlib import IXXATBus

can/interfaces/ixxat/canlib.py

@@ -2,6 +2,8 @@
 import can.interfaces.ixxat.canlib_vcinpl2 as vcinpl2
 
 from can import BusABC, Message
+from can.bus import BusState
+
 from typing import Optional
 
 
@@ -11,7 +13,8 @@ class IXXATBus(BusABC):
     Based on the C implementation of IXXAT, two different dlls are provided by IXXAT, one to work with CAN,
     the other with CAN-FD.
 
-    This class only delegates to related implementation (in calib_vcinpl or canlib_vcinpl2) class depending on fd user option.
+    This class only delegates to related implementation (in calib_vcinpl or canlib_vcinpl2)
+    class depending on fd user option.
     """
 
     def __init__(
@@ -140,8 +143,18 @@ def _recv_internal(self, timeout):
     def send(self, msg: Message, timeout: Optional[float] = None) -> None:
         return self.bus.send(msg, timeout)
 
-    def _send_periodic_internal(self, msg, period, duration=None):
-        return self.bus._send_periodic_internal(msg, period, duration)
+    def _send_periodic_internal(self, msgs, period, duration=None):
+        return self.bus._send_periodic_internal(msgs, period, duration)
 
     def shutdown(self):
         return self.bus.shutdown()
+
+    @property
+    def state(self) -> BusState:
+        """
+        Return the current state of the hardware
+        """
+        return self.bus.state
+
+
+# ~class IXXATBus(BusABC): ---------------------------------------------------

can/interfaces/ixxat/canlib_vcinpl.py

@@ -1,5 +1,5 @@
 """
-Ctypes wrapper module for IXXAT Virtual CAN Interface V3 on win32 systems
+Ctypes wrapper module for IXXAT Virtual CAN Interface V4 on win32 systems
 
 TODO: We could implement this interface such that setting other filters
       could work when the initial filters were set to zero using the
@@ -16,6 +16,7 @@
 from typing import Optional, Callable, Tuple
 
 from can import BusABC, Message
+from can.bus import BusState
 from can.exceptions import CanInterfaceNotImplementedError, CanInitializationError
 from can.broadcastmanager import (
     LimitedDurationCyclicSendTaskABC,
@@ -38,7 +39,6 @@
 
 log = logging.getLogger("can.ixxat")
 
-from time import perf_counter
 
 # Hack to have vciFormatError as a free function, see below
 vciFormatError = None
@@ -225,6 +225,13 @@ def __check_status(result, function, args):
         (HANDLE, ctypes.c_uint32, structures.PCANMSG),
         __check_status,
     )
+    # HRESULT canChannelGetStatus (HANDLE hCanChn, PCANCHANSTATUS pStatus );
+    _canlib.map_symbol(
+        "canChannelGetStatus",
+        ctypes.c_long,
+        (HANDLE, structures.PCANCHANSTATUS),
+        __check_status,
+    )
 
     # EXTERN_C HRESULT VCIAPI canControlOpen( IN  HANDLE  hDevice, IN  UINT32  dwCanNo, OUT PHANDLE phCanCtl );
     _canlib.map_symbol(
@@ -509,11 +516,11 @@ def __init__(
                     == bytes(unique_hardware_id, "ascii")
                 ):
                     break
-                else:
-                    log.debug(
-                        "Ignoring IXXAT with hardware id '%s'.",
-                        self._device_info.UniqueHardwareId.AsChar.decode("ascii"),
-                    )
+
+                log.debug(
+                    "Ignoring IXXAT with hardware id '%s'.",
+                    self._device_info.UniqueHardwareId.AsChar.decode("ascii"),
+                )
         _canlib.vciEnumDeviceClose(self._device_handle)
 
         try:
@@ -522,7 +529,9 @@ def __init__(
                 ctypes.byref(self._device_handle),
             )
         except Exception as exception:
-            raise CanInitializationError(f"Could not open device: {exception}")
+            raise CanInitializationError(
+                f"Could not open device: {exception}"
+            ) from exception
 
         log.info("Using unique HW ID %s", self._device_info.UniqueHardwareId.AsChar)
 
@@ -543,7 +552,7 @@ def __init__(
         except Exception as exception:
             raise CanInitializationError(
                 f"Could not open and initialize channel: {exception}"
-            )
+            ) from exception
 
         # Signal TX/RX events when at least one frame has been handled
         _canlib.canChannelInitialize(
@@ -637,85 +646,88 @@ def flush_tx_buffer(self):
 
     def _recv_internal(self, timeout):
         """Read a message from IXXAT device."""
-
-        # TODO: handling CAN error messages?
         data_received = False
 
-        if timeout == 0:
+        if self._inWaiting() or timeout == 0:
             # Peek without waiting
-            try:
-                _canlib.canChannelPeekMessage(
-                    self._channel_handle, ctypes.byref(self._message)
-                )
-            except (VCITimeout, VCIRxQueueEmptyError):
-                return None, True
-            else:
-                if self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_DATA:
-                    data_received = True
+            recv_function = functools.partial(
+                _canlib.canChannelPeekMessage,
+                self._channel_handle,
+                ctypes.byref(self._message),
+            )
         else:
             # Wait if no message available
-            if timeout is None or timeout < 0:
-                remaining_ms = constants.INFINITE
-                t0 = None
-            else:
-                timeout_ms = int(timeout * 1000)
-                remaining_ms = timeout_ms
-                t0 = perf_counter()
-
-            while True:
-                try:
-                    _canlib.canChannelReadMessage(
-                        self._channel_handle, remaining_ms, ctypes.byref(self._message)
+            timeout = (
+                constants.INFINITE
+                if (timeout is None or timeout < 0)
+                else int(timeout * 1000)
+            )
+            recv_function = functools.partial(
+                _canlib.canChannelReadMessage,
+                self._channel_handle,
+                timeout,
+                ctypes.byref(self._message),
+            )
+
+        try:
+            recv_function()
+        except (VCITimeout, VCIRxQueueEmptyError):
+            # Ignore the 2 errors, overall timeout is handled by BusABC.recv
+            pass
+        else:
+            # See if we got a data or info/error messages
+            if self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_DATA:
+                data_received = True
+            elif self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_INFO:
+                log.info(
+                    CAN_INFO_MESSAGES.get(
+                        self._message.abData[0],
+                        f"Unknown CAN info message code {self._message.abData[0]}",
                     )
-                except (VCITimeout, VCIRxQueueEmptyError):
-                    # Ignore the 2 errors, the timeout is handled manually with the perf_counter()
-                    pass
+                )
+            elif self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_ERROR:
+                if self._message.uMsgInfo.Bytes.bFlags & constants.CAN_MSGFLAGS_OVR:
+                    log.warning("CAN error: data overrun")
                 else:
-                    # See if we got a data or info/error messages
-                    if self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_DATA:
-                        data_received = True
-                        break
-                    elif self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_INFO:
-                        log.info(
-                            CAN_INFO_MESSAGES.get(
-                                self._message.abData[0],
-                                "Unknown CAN info message code {}".format(
-                                    self._message.abData[0]
-                                ),
-                            )
-                        )
-
-                    elif (
-                        self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_ERROR
-                    ):
-                        log.warning(
-                            CAN_ERROR_MESSAGES.get(
-                                self._message.abData[0],
-                                "Unknown CAN error message code {}".format(
-                                    self._message.abData[0]
-                                ),
-                            )
+                    log.warning(
+                        CAN_ERROR_MESSAGES.get(
+                            self._message.abData[0],
+                            f"Unknown CAN error message code {self._message.abData[0]}",
                         )
-
-                    elif (
-                        self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_STATUS
-                    ):
-                        log.info(_format_can_status(self._message.abData[0]))
-                        if self._message.abData[0] & constants.CAN_STATUS_BUSOFF:
-                            raise VCIBusOffError()
-
-                    elif (
-                        self._message.uMsgInfo.Bits.type
-                        == constants.CAN_MSGTYPE_TIMEOVR
-                    ):
-                        pass
-                    else:
-                        log.warning("Unexpected message info type")
-
-                if t0 is not None:
-                    remaining_ms = timeout_ms - int((perf_counter() - t0) * 1000)
-                    if remaining_ms < 0:
-                        break
+                    )
+                    log.warning(
+                        "CAN message flags bAddFlags/bFlags2 0x%02X bflags 0x%02X",
+                        self._message.uMsgInfo.Bytes.bAddFlags,
+                        self._message.uMsgInfo.Bytes.bFlags,
+                    )
+            elif self._message.uMsgInfo.Bits.type == constants.CAN_MSGTYPE_TIMEOVR:
+                pass
+            else:
+                log.warning(
+                    "Unexpected message info type 0x%X",
+                    self._message.uMsgInfo.Bits.type,
+                )
+        finally:
+            if not data_received:
+                # Check hard errors
+                status = structures.CANLINESTATUS()
+                _canlib.canControlGetStatus(self._control_handle, ctypes.byref(status))
+                error_byte_1 = status.dwStatus & 0x0F
+                error_byte_2 = status.dwStatus & 0xF0
+                if error_byte_1 > constants.CAN_STATUS_TXPEND:
+                    # CAN_STATUS_OVRRUN   = 0x02  # data overrun occurred
+                    # CAN_STATUS_ERRLIM   = 0x04  # error warning limit exceeded
+                    # CAN_STATUS_BUSOFF = 0x08  # bus off status
+                    if error_byte_1 & constants.CAN_STATUS_OVRRUN:
+                        raise VCIError("Data overrun occurred")
+                    elif error_byte_1 & constants.CAN_STATUS_ERRLIM:
+                        raise VCIError("Error warning limit exceeded")
+                    elif error_byte_1 & constants.CAN_STATUS_BUSOFF:
+                        raise VCIError("Bus off status")
+                elif error_byte_2 > constants.CAN_STATUS_ININIT:
+                    # CAN_STATUS_BUSCERR  = 0x20  # bus coupling error
+                    if error_byte_2 & constants.CAN_STATUS_BUSCERR:
+                        raise VCIError("Bus coupling error")
 
         if not data_received:
             # Timed out / can message type is not DATA
@@ -764,11 +776,12 @@ def send(self, msg: Message, timeout: Optional[float] = None) -> None:
             _canlib.canChannelSendMessage(
                 self._channel_handle, int(timeout * 1000), message
             )
-
         else:
             _canlib.canChannelPostMessage(self._channel_handle, message)
+        # Want to log outgoing messages?
+        # log.log(self.RECV_LOGGING_LEVEL, "Sent: %s", message)
 
-    def _send_periodic_internal(self, msg, period, duration=None):
+    def _send_periodic_internal(self, msgs, period, duration=None):
         """Send a message using built-in cyclic transmit list functionality."""
         if self._scheduler is None:
             self._scheduler = HANDLE()
@@ -778,17 +791,43 @@ def _send_periodic_internal(self, msg, period, duration=None):
             self._scheduler_resolution = caps.dwClockFreq / caps.dwCmsDivisor
             _canlib.canSchedulerActivate(self._scheduler, constants.TRUE)
         return CyclicSendTask(
-            self._scheduler, msg, period, duration, self._scheduler_resolution
+            self._scheduler, msgs, period, duration, self._scheduler_resolution
         )
 
     def shutdown(self):
         if self._scheduler is not None:
             _canlib.canSchedulerClose(self._scheduler)
         _canlib.canChannelClose(self._channel_handle)
         _canlib.canControlStart(self._control_handle, constants.FALSE)
+        _canlib.canControlReset(self._control_handle)
         _canlib.canControlClose(self._control_handle)
         _canlib.vciDeviceClose(self._device_handle)
 
+    @property
+    def state(self) -> BusState:
+        """
+        Return the current state of the hardware
+        """
+        status = structures.CANLINESTATUS()
+        _canlib.canControlGetStatus(self._control_handle, ctypes.byref(status))
+        if status.bOpMode == constants.CAN_OPMODE_LISTONLY:
+            return BusState.PASSIVE
+
+        error_byte_1 = status.dwStatus & 0x0F
+        # CAN_STATUS_BUSOFF = 0x08  # bus off status
+        if error_byte_1 & constants.CAN_STATUS_BUSOFF:
+            return BusState.ERROR
+
+        error_byte_2 = status.dwStatus & 0xF0
+        # CAN_STATUS_BUSCERR  = 0x20  # bus coupling error
+        if error_byte_2 & constants.CAN_STATUS_BUSCERR:
+            raise BusState.ERROR
+
+        return BusState.ACTIVE
+
+
+# ~class IXXATBus(BusABC): ---------------------------------------------------
+
 
 class CyclicSendTask(LimitedDurationCyclicSendTaskABC, RestartableCyclicTaskABC):
     """A message in the cyclic transmit list."""

can/interfaces/ixxat/canlib_vcinpl2.py

@@ -825,9 +825,7 @@ def _recv_internal(self, timeout):
                         log.info(
                             CAN_INFO_MESSAGES.get(
                                 self._message.abData[0],
-                                "Unknown CAN info message code {}".format(
-                                    self._message.abData[0]
-                                ),
+                                f"Unknown CAN info message code {self._message.abData[0]}",
                             )
                         )
 
@@ -837,9 +835,7 @@ def _recv_internal(self, timeout):
                         log.warning(
                             CAN_ERROR_MESSAGES.get(
                                 self._message.abData[0],
-                                "Unknown CAN error message code {}".format(
-                                    self._message.abData[0]
-                                ),
+                                f"Unknown CAN error message code {self._message.abData[0]}",
                             )
                         )
 
@@ -933,7 +929,7 @@ def send(self, msg: Message, timeout: Optional[float] = None) -> None:
         else:
             _canlib.canChannelPostMessage(self._channel_handle, message)
 
-    def _send_periodic_internal(self, msg, period, duration=None):
+    def _send_periodic_internal(self, msgs, period, duration=None):
         """Send a message using built-in cyclic transmit list functionality."""
         if self._scheduler is None:
             self._scheduler = HANDLE()
@@ -945,7 +941,7 @@ def _send_periodic_internal(self, msg, period, duration=None):
             )  # TODO: confirm
             _canlib.canSchedulerActivate(self._scheduler, constants.TRUE)
         return CyclicSendTask(
-            self._scheduler, msg, period, duration, self._scheduler_resolution
+            self._scheduler, msgs, period, duration, self._scheduler_resolution
         )
 
     def shutdown(self):