From ba5d727a7384057083710bc2df2b2ebfb4d7e83a Mon Sep 17 00:00:00 2001 From: Felix Divo <4403130+felixdivo@users.noreply.github.com> Date: Thu, 28 Oct 2021 00:34:59 +0200 Subject: [PATCH 1/4] Improve error handling with existing classes --- can/interfaces/systec/__init__.py | 3 - can/interfaces/systec/exceptions.py | 157 +++++++++++++++------------- test/test_systec.py | 1 - 3 files changed, 86 insertions(+), 75 deletions(-) diff --git a/can/interfaces/systec/__init__.py b/can/interfaces/systec/__init__.py index 4ecd39b4c..be7004b6a 100644 --- a/can/interfaces/systec/__init__.py +++ b/can/interfaces/systec/__init__.py @@ -1,4 +1 @@ -""" -""" - from can.interfaces.systec.ucanbus import UcanBus diff --git a/can/interfaces/systec/exceptions.py b/can/interfaces/systec/exceptions.py index 72ec92cfa..080337897 100644 --- a/can/interfaces/systec/exceptions.py +++ b/can/interfaces/systec/exceptions.py @@ -1,92 +1,107 @@ +from typing import Dict + +from abc import ABC, abstractmethod + from .constants import ReturnCode from can import CanError -class UcanException(CanError): - """ Base class for USB can errors. """ +class UcanException(CanError, ABC): + """Base class for USB can errors.""" def __init__(self, result, func, arguments): - self.result = result.value + self.result = result self.func = func self.arguments = arguments - self.return_msgs = {} - super().__init__() - def __str__(self): - message = self.return_msgs.get(self.result, "unknown") - return f"Function {self.func.__name__} returned {self.result}: {message}" + message = self._error_message_mapping.get(result, "unknown") + super().__init__( + message=f"Function {func.__name__} (called with {arguments}): {message}", + error_code=result.value, + ) + + @property + @abstractmethod + def _error_message_mapping(self) -> Dict[ReturnCode, str]: + ... class UcanError(UcanException): - """ Exception class for errors from USB-CAN-library. """ + """Exception class for errors from USB-CAN-library.""" - def __init__(self, result, func, arguments): - super().__init__(result, func, arguments) - self.return_msgs = { - ReturnCode.ERR_RESOURCE: "could not created a resource (memory, handle, ...)", - ReturnCode.ERR_MAXMODULES: "the maximum number of opened modules is reached", - ReturnCode.ERR_HWINUSE: "the specified module is already in use", - ReturnCode.ERR_ILLVERSION: "the software versions of the module and library are incompatible", - ReturnCode.ERR_ILLHW: "the module with the specified device number is not connected " - "(or used by an other application)", - ReturnCode.ERR_ILLHANDLE: "wrong USB-CAN-Handle handed over to the function", - ReturnCode.ERR_ILLPARAM: "wrong parameter handed over to the function", - ReturnCode.ERR_BUSY: "instruction can not be processed at this time", - ReturnCode.ERR_TIMEOUT: "no answer from module", - ReturnCode.ERR_IOFAILED: "a request to the driver failed", - ReturnCode.ERR_DLL_TXFULL: "a CAN message did not fit into the transmit buffer", - ReturnCode.ERR_MAXINSTANCES: "maximum number of applications is reached", - ReturnCode.ERR_CANNOTINIT: "CAN interface is not yet initialized", - ReturnCode.ERR_DISCONECT: "USB-CANmodul was disconnected", - ReturnCode.ERR_NOHWCLASS: "the needed device class does not exist", - ReturnCode.ERR_ILLCHANNEL: "illegal CAN channel", - ReturnCode.ERR_RESERVED1: "reserved", - ReturnCode.ERR_ILLHWTYPE: "the API function can not be used with this hardware", - } + _ERROR_MESSAGES = { + ReturnCode.ERR_RESOURCE: "could not created a resource (memory, handle, ...)", + ReturnCode.ERR_MAXMODULES: "the maximum number of opened modules is reached", + ReturnCode.ERR_HWINUSE: "the specified module is already in use", + ReturnCode.ERR_ILLVERSION: "the software versions of the module and library are incompatible", + ReturnCode.ERR_ILLHW: "the module with the specified device number is not connected " + "(or used by an other application)", + ReturnCode.ERR_ILLHANDLE: "wrong USB-CAN-Handle handed over to the function", + ReturnCode.ERR_ILLPARAM: "wrong parameter handed over to the function", + ReturnCode.ERR_BUSY: "instruction can not be processed at this time", + ReturnCode.ERR_TIMEOUT: "no answer from module", + ReturnCode.ERR_IOFAILED: "a request to the driver failed", + ReturnCode.ERR_DLL_TXFULL: "a CAN message did not fit into the transmit buffer", + ReturnCode.ERR_MAXINSTANCES: "maximum number of applications is reached", + ReturnCode.ERR_CANNOTINIT: "CAN interface is not yet initialized", + ReturnCode.ERR_DISCONECT: "USB-CANmodul was disconnected", + ReturnCode.ERR_NOHWCLASS: "the needed device class does not exist", + ReturnCode.ERR_ILLCHANNEL: "illegal CAN channel", + ReturnCode.ERR_RESERVED1: "reserved", + ReturnCode.ERR_ILLHWTYPE: "the API function can not be used with this hardware", + } + + @property + def _error_message_mapping(self) -> Dict[ReturnCode, str]: + return UcanError._ERROR_MESSAGES class UcanCmdError(UcanException): - """ Exception class for errors from firmware in USB-CANmodul.""" + """Exception class for errors from firmware in USB-CANmodul.""" - def __init__(self, result, func, arguments): - super().__init__(result, func, arguments) - self.return_msgs = { - ReturnCode.ERRCMD_NOTEQU: "the received response does not match to the transmitted command", - ReturnCode.ERRCMD_REGTST: "no access to the CAN controller", - ReturnCode.ERRCMD_ILLCMD: "the module could not interpret the command", - ReturnCode.ERRCMD_EEPROM: "error while reading the EEPROM", - ReturnCode.ERRCMD_RESERVED1: "reserved", - ReturnCode.ERRCMD_RESERVED2: "reserved", - ReturnCode.ERRCMD_RESERVED3: "reserved", - ReturnCode.ERRCMD_ILLBDR: "illegal baud rate value specified in BTR0/BTR1 for systec " - "USB-CANmoduls", - ReturnCode.ERRCMD_NOTINIT: "CAN channel is not initialized", - ReturnCode.ERRCMD_ALREADYINIT: "CAN channel is already initialized", - ReturnCode.ERRCMD_ILLSUBCMD: "illegal sub-command specified", - ReturnCode.ERRCMD_ILLIDX: "illegal index specified (e.g. index for cyclic CAN messages)", - ReturnCode.ERRCMD_RUNNING: "cyclic CAN message(s) can not be defined because transmission of " - "cyclic CAN messages is already running", - } + _ERROR_MESSAGES = { + ReturnCode.ERRCMD_NOTEQU: "the received response does not match to the transmitted command", + ReturnCode.ERRCMD_REGTST: "no access to the CAN controller", + ReturnCode.ERRCMD_ILLCMD: "the module could not interpret the command", + ReturnCode.ERRCMD_EEPROM: "error while reading the EEPROM", + ReturnCode.ERRCMD_RESERVED1: "reserved", + ReturnCode.ERRCMD_RESERVED2: "reserved", + ReturnCode.ERRCMD_RESERVED3: "reserved", + ReturnCode.ERRCMD_ILLBDR: "illegal baud rate value specified in BTR0/BTR1 for systec " + "USB-CANmoduls", + ReturnCode.ERRCMD_NOTINIT: "CAN channel is not initialized", + ReturnCode.ERRCMD_ALREADYINIT: "CAN channel is already initialized", + ReturnCode.ERRCMD_ILLSUBCMD: "illegal sub-command specified", + ReturnCode.ERRCMD_ILLIDX: "illegal index specified (e.g. index for cyclic CAN messages)", + ReturnCode.ERRCMD_RUNNING: "cyclic CAN message(s) can not be defined because transmission of " + "cyclic CAN messages is already running", + } + + @property + def _error_message_mapping(self) -> Dict[ReturnCode, str]: + return UcanCmdError._ERROR_MESSAGES class UcanWarning(UcanException): - """ Exception class for warnings, the function has been executed anyway. """ + """Exception class for warnings, the function has been executed anyway.""" - def __init__(self, result, func, arguments): - super().__init__(result, func, arguments) - self.return_msgs = { - ReturnCode.WARN_NODATA: "no CAN messages received", - ReturnCode.WARN_SYS_RXOVERRUN: "overrun in receive buffer of the kernel driver", - ReturnCode.WARN_DLL_RXOVERRUN: "overrun in receive buffer of the USB-CAN-library", - ReturnCode.WARN_RESERVED1: "reserved", - ReturnCode.WARN_RESERVED2: "reserved", - ReturnCode.WARN_FW_TXOVERRUN: "overrun in transmit buffer of the firmware (but this CAN message " - "was successfully stored in buffer of the ibrary)", - ReturnCode.WARN_FW_RXOVERRUN: "overrun in receive buffer of the firmware (but this CAN message " - "was successfully read)", - ReturnCode.WARN_FW_TXMSGLOST: "reserved", - ReturnCode.WARN_NULL_PTR: "pointer is NULL", - ReturnCode.WARN_TXLIMIT: "not all CAN messages could be stored to the transmit buffer in " - "USB-CAN-library", - ReturnCode.WARN_BUSY: "reserved", - } + _ERROR_MESSAGES = { + ReturnCode.WARN_NODATA: "no CAN messages received", + ReturnCode.WARN_SYS_RXOVERRUN: "overrun in receive buffer of the kernel driver", + ReturnCode.WARN_DLL_RXOVERRUN: "overrun in receive buffer of the USB-CAN-library", + ReturnCode.WARN_RESERVED1: "reserved", + ReturnCode.WARN_RESERVED2: "reserved", + ReturnCode.WARN_FW_TXOVERRUN: "overrun in transmit buffer of the firmware (but this CAN message " + "was successfully stored in buffer of the ibrary)", + ReturnCode.WARN_FW_RXOVERRUN: "overrun in receive buffer of the firmware (but this CAN message " + "was successfully read)", + ReturnCode.WARN_FW_TXMSGLOST: "reserved", + ReturnCode.WARN_NULL_PTR: "pointer is NULL", + ReturnCode.WARN_TXLIMIT: "not all CAN messages could be stored to the transmit buffer in " + "USB-CAN-library", + ReturnCode.WARN_BUSY: "reserved", + } + + @property + def _error_message_mapping(self) -> Dict[ReturnCode, str]: + return UcanWarning._ERROR_MESSAGES diff --git a/test/test_systec.py b/test/test_systec.py index 6910653fc..634023c10 100644 --- a/test/test_systec.py +++ b/test/test_systec.py @@ -1,5 +1,4 @@ #!/usr/bin/env python -# coding: utf-8 import unittest from unittest.mock import Mock, patch From f7d1d1c88ef5e3f850339be91b88487199d81bfa Mon Sep 17 00:00:00 2001 From: Felix Divo <4403130+felixdivo@users.noreply.github.com> Date: Wed, 27 Oct 2021 22:35:47 +0000 Subject: [PATCH 2/4] Format code with black --- can/interfaces/systec/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/interfaces/systec/exceptions.py b/can/interfaces/systec/exceptions.py index 080337897..733326194 100644 --- a/can/interfaces/systec/exceptions.py +++ b/can/interfaces/systec/exceptions.py @@ -35,7 +35,7 @@ class UcanError(UcanException): ReturnCode.ERR_HWINUSE: "the specified module is already in use", ReturnCode.ERR_ILLVERSION: "the software versions of the module and library are incompatible", ReturnCode.ERR_ILLHW: "the module with the specified device number is not connected " - "(or used by an other application)", + "(or used by an other application)", ReturnCode.ERR_ILLHANDLE: "wrong USB-CAN-Handle handed over to the function", ReturnCode.ERR_ILLPARAM: "wrong parameter handed over to the function", ReturnCode.ERR_BUSY: "instruction can not be processed at this time", From dcdd8fbbbdcc9dd23a516f4ca894b71af3b45a56 Mon Sep 17 00:00:00 2001 From: Felix Divo <4403130+felixdivo@users.noreply.github.com> Date: Thu, 28 Oct 2021 00:44:33 +0200 Subject: [PATCH 3/4] Correct exception handling at startup --- can/interfaces/systec/exceptions.py | 2 +- can/interfaces/systec/ucan.py | 10 ++++++++++ can/interfaces/systec/ucanbus.py | 9 +++++++-- test/test_systec.py | 1 + 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/can/interfaces/systec/exceptions.py b/can/interfaces/systec/exceptions.py index 080337897..733326194 100644 --- a/can/interfaces/systec/exceptions.py +++ b/can/interfaces/systec/exceptions.py @@ -35,7 +35,7 @@ class UcanError(UcanException): ReturnCode.ERR_HWINUSE: "the specified module is already in use", ReturnCode.ERR_ILLVERSION: "the software versions of the module and library are incompatible", ReturnCode.ERR_ILLHW: "the module with the specified device number is not connected " - "(or used by an other application)", + "(or used by an other application)", ReturnCode.ERR_ILLHANDLE: "wrong USB-CAN-Handle handed over to the function", ReturnCode.ERR_ILLPARAM: "wrong parameter handed over to the function", ReturnCode.ERR_BUSY: "instruction can not be processed at this time", diff --git a/can/interfaces/systec/ucan.py b/can/interfaces/systec/ucan.py index 3f90e6cc2..6e150f7b1 100644 --- a/can/interfaces/systec/ucan.py +++ b/can/interfaces/systec/ucan.py @@ -4,6 +4,8 @@ from ctypes import byref from ctypes import c_wchar_p as LPWSTR +from ...exceptions import CanInterfaceNotImplementedError + from .constants import * from .structures import * from .exceptions import * @@ -110,6 +112,7 @@ def check_result(result, func, arguments): return result +_UCAN_INITIALIZED = False if os.name != "nt": log.warning("SYSTEC ucan library does not work on %s platform.", sys.platform) else: @@ -310,6 +313,8 @@ def check_result(result, func, arguments): UcanEnableCyclicCanMsg.argtypes = [Handle, BYTE, DWORD] UcanEnableCyclicCanMsg.errcheck = check_result + _UCAN_INITIALIZED = True + except Exception as ex: log.warning("Cannot load SYSTEC ucan library: %s.", ex) @@ -323,6 +328,11 @@ class UcanServer: _connect_control_ref = None def __init__(self): + if not _UCAN_INITIALIZED: + raise CanInterfaceNotImplementedError( + "The interface could not be loaded on the current platform" + ) + self._handle = Handle(INVALID_HANDLE) self._is_initialized = False self._hw_is_initialized = False diff --git a/can/interfaces/systec/ucanbus.py b/can/interfaces/systec/ucanbus.py index 2d3a23777..9a6e6edd8 100644 --- a/can/interfaces/systec/ucanbus.py +++ b/can/interfaces/systec/ucanbus.py @@ -2,6 +2,7 @@ from threading import Event from can import BusABC, BusState, Message +from ...exceptions import CanError, CanInitializationError from .constants import * from .structures import * @@ -91,8 +92,12 @@ def __init__(self, channel, can_filters=None, **kwargs): """ try: self._ucan = Ucan() - except Exception: - raise ImportError("The SYSTEC ucan library has not been initialized.") + except CanError as error: + raise error + except Exception as exception: + raise CanInitializationError( + "The SYSTEC ucan library has not been initialized." + ) from exception self.channel = int(channel) device_number = int(kwargs.get("device_number", ANY_MODULE)) diff --git a/test/test_systec.py b/test/test_systec.py index 634023c10..5e8b30dcf 100644 --- a/test/test_systec.py +++ b/test/test_systec.py @@ -31,6 +31,7 @@ def setUp(self): ucan.UcanDeinitHardware = Mock() ucan.UcanWriteCanMsgEx = Mock() ucan.UcanResetCanEx = Mock() + ucan._UCAN_INITIALIZED = True # Fake this self.bus = can.Bus(bustype="systec", channel=0, bitrate=125000) def test_bus_creation(self): From 5cbb847faa09a164cc33caf190d7d45ef96161be Mon Sep 17 00:00:00 2001 From: Felix Divo <4403130+felixdivo@users.noreply.github.com> Date: Thu, 28 Oct 2021 00:52:20 +0200 Subject: [PATCH 4/4] Throw the correct exceptions --- can/interfaces/systec/ucanbus.py | 119 ++++++++++++++++++------------- 1 file changed, 71 insertions(+), 48 deletions(-) diff --git a/can/interfaces/systec/ucanbus.py b/can/interfaces/systec/ucanbus.py index 9a6e6edd8..c1068494f 100644 --- a/can/interfaces/systec/ucanbus.py +++ b/can/interfaces/systec/ucanbus.py @@ -2,10 +2,11 @@ from threading import Event from can import BusABC, BusState, Message -from ...exceptions import CanError, CanInitializationError +from ...exceptions import CanError, CanInitializationError, CanOperationError from .constants import * from .structures import * +from .exceptions import UcanException from .ucan import UcanServer log = logging.getLogger("can.systec") @@ -87,7 +88,10 @@ def __init__(self, channel, can_filters=None, **kwargs): :raises ValueError: If invalid input parameter were passed. - :raises can.CanError: + :raises can.CanInterfaceNotImplementedError: + If the platform is not supported. + + :raises can.CanInitializationError: If hardware or CAN interface initialization failed. """ try: @@ -105,7 +109,7 @@ def __init__(self, channel, can_filters=None, **kwargs): # configuration options bitrate = kwargs.get("bitrate", 500000) if bitrate not in self.BITRATES: - raise ValueError("Invalid bitrate {}".format(bitrate)) + raise ValueError(f"Invalid bitrate {bitrate}") state = kwargs.get("state", BusState.ACTIVE) if state is BusState.ACTIVE or state is BusState.PASSIVE: @@ -126,21 +130,29 @@ def __init__(self, channel, can_filters=None, **kwargs): if kwargs.get("tx_buffer_entries"): self._params["tx_buffer_entries"] = int(kwargs.get("tx_buffer_entries")) - self._ucan.init_hardware(device_number=device_number) - self._ucan.init_can(self.channel, **self._params) - hw_info_ex, _, _ = self._ucan.get_hardware_info() - self.channel_info = "%s, S/N %s, CH %s, BTR %s" % ( - self._ucan.get_product_code_message(hw_info_ex.product_code), - hw_info_ex.serial, - self.channel, - self._ucan.get_baudrate_message(self.BITRATES[bitrate]), - ) + try: + self._ucan.init_hardware(device_number=device_number) + self._ucan.init_can(self.channel, **self._params) + hw_info_ex, _, _ = self._ucan.get_hardware_info() + self.channel_info = "%s, S/N %s, CH %s, BTR %s" % ( + self._ucan.get_product_code_message(hw_info_ex.product_code), + hw_info_ex.serial, + self.channel, + self._ucan.get_baudrate_message(self.BITRATES[bitrate]), + ) + except UcanException as exception: + raise CanInitializationError() from exception + self._is_filtered = False super().__init__(channel=channel, can_filters=can_filters, **kwargs) def _recv_internal(self, timeout): - message, _ = self._ucan.read_can_msg(self.channel, 1, timeout) + try: + message, _ = self._ucan.read_can_msg(self.channel, 1, timeout) + except UcanException as exception: + raise CanOperationError() from exception + if not message: return None, False @@ -169,21 +181,23 @@ def send(self, msg, timeout=None): :param float timeout: Transmit timeout in seconds (value 0 switches off the "auto delete") - :raises can.CanError: + :raises can.CanOperationError: If the message could not be sent. - """ - if timeout is not None and timeout >= 0: - self._ucan.set_tx_timeout(self.channel, int(timeout * 1000)) - - message = CanMsg( - msg.arbitration_id, - MsgFrameFormat.MSG_FF_STD - | (MsgFrameFormat.MSG_FF_EXT if msg.is_extended_id else 0) - | (MsgFrameFormat.MSG_FF_RTR if msg.is_remote_frame else 0), - msg.data, - ) - self._ucan.write_can_msg(self.channel, [message]) + try: + if timeout is not None and timeout >= 0: + self._ucan.set_tx_timeout(self.channel, int(timeout * 1000)) + + message = CanMsg( + msg.arbitration_id, + MsgFrameFormat.MSG_FF_STD + | (MsgFrameFormat.MSG_FF_EXT if msg.is_extended_id else 0) + | (MsgFrameFormat.MSG_FF_RTR if msg.is_remote_frame else 0), + msg.data, + ) + self._ucan.write_can_msg(self.channel, [message]) + except UcanException as exception: + raise CanOperationError() from exception @staticmethod def _detect_available_configs(): @@ -211,16 +225,19 @@ def _detect_available_configs(): return configs def _apply_filters(self, filters): - if filters and len(filters) == 1: - can_id = filters[0]["can_id"] - can_mask = filters[0]["can_mask"] - self._ucan.set_acceptance(self.channel, can_mask, can_id) - self._is_filtered = True - log.info("Hardware filtering on ID 0x%X, mask 0x%X", can_id, can_mask) - else: - self._ucan.set_acceptance(self.channel) - self._is_filtered = False - log.info("Hardware filtering has been disabled") + try: + if filters and len(filters) == 1: + can_id = filters[0]["can_id"] + can_mask = filters[0]["can_mask"] + self._ucan.set_acceptance(self.channel, can_mask, can_id) + self._is_filtered = True + log.info("Hardware filtering on ID 0x%X, mask 0x%X", can_id, can_mask) + else: + self._ucan.set_acceptance(self.channel) + self._is_filtered = False + log.info("Hardware filtering has been disabled") + except UcanException as exception: + raise CanOperationError() from exception def flush_tx_buffer(self): """ @@ -230,7 +247,10 @@ def flush_tx_buffer(self): If flushing of the transmit buffer failed. """ log.info("Flushing transmit buffer") - self._ucan.reset_can(self.channel, ResetFlags.RESET_ONLY_TX_BUFF) + try: + self._ucan.reset_can(self.channel, ResetFlags.RESET_ONLY_TX_BUFF) + except UcanException as exception: + raise CanOperationError() from exception @staticmethod def create_filter(extended, from_id, to_id, rtr_only, rtr_too): @@ -275,15 +295,18 @@ def state(self, new_state): if self._state is not BusState.ERROR and ( new_state is BusState.ACTIVE or new_state is BusState.PASSIVE ): - # close the CAN channel - self._ucan.shutdown(self.channel, False) - # set mode - if new_state is BusState.ACTIVE: - self._params["mode"] &= ~Mode.MODE_LISTEN_ONLY - else: - self._params["mode"] |= Mode.MODE_LISTEN_ONLY - # reinitialize CAN channel - self._ucan.init_can(self.channel, **self._params) + try: + # close the CAN channel + self._ucan.shutdown(self.channel, False) + # set mode + if new_state is BusState.ACTIVE: + self._params["mode"] &= ~Mode.MODE_LISTEN_ONLY + else: + self._params["mode"] |= Mode.MODE_LISTEN_ONLY + # reinitialize CAN channel + self._ucan.init_can(self.channel, **self._params) + except UcanException as exception: + raise CanOperationError() from exception def shutdown(self): """ @@ -291,5 +314,5 @@ def shutdown(self): """ try: self._ucan.shutdown() - except Exception as ex: - log.error(ex) + except Exception as exception: + log.error(exception)