From 51569b0fba75352f02203f9a0875df9d86095dd4 Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 8 Jun 2021 17:10:43 +0200 Subject: [PATCH 1/6] improve error handling in neousys; fix bugs --- can/interfaces/neousys/neousys.py | 112 +++++++++++++----------------- 1 file changed, 48 insertions(+), 64 deletions(-) diff --git a/can/interfaces/neousys/neousys.py b/can/interfaces/neousys/neousys.py index a0037981b..1950bb1d0 100644 --- a/can/interfaces/neousys/neousys.py +++ b/can/interfaces/neousys/neousys.py @@ -13,10 +13,7 @@ # pylint: disable=too-few-public-methods # pylint: disable=too-many-instance-attributes # pylint: disable=wrong-import-position -# pylint: disable=method-hidden -# pylint: disable=unused-import -import warnings import queue import logging import platform @@ -33,11 +30,13 @@ Structure, ) -if platform.system() == "Windows": +try: from ctypes import WinDLL -else: +except ImportError: from ctypes import CDLL + from can import BusABC, Message +from ...exceptions import CanInitializationError, CanOperationError, CanInterfaceNotImplementedError logger = logging.getLogger(__name__) @@ -126,14 +125,13 @@ class NeousysCanBitClk(Structure): NEOUSYS_CANLIB = WinDLL("./WDT_DIO.dll") else: NEOUSYS_CANLIB = CDLL("libwdt_dio.so") - logger.info("Loaded Neousys WDT_DIO Can driver") + logger.info("Loaded Neousys WDT_DIO Can driver") except OSError as error: - logger.info("Cannot Neousys CAN bus dll or share object: %d", format(error)) - # NEOUSYS_CANLIB = None + logger.info("Cannot load Neousys CAN bus dll or shared object: %d", format(error)) class NeousysBus(BusABC): - """ Neousys CAN bus Class""" + """Neousys CAN bus Class""" def __init__(self, channel, device=0, bitrate=500000, **kwargs): """ @@ -143,43 +141,41 @@ def __init__(self, channel, device=0, bitrate=500000, **kwargs): """ super().__init__(channel, **kwargs) - if NEOUSYS_CANLIB is not None: - self.channel = channel - - self.device = device + if NEOUSYS_CANLIB is None: + raise CanInterfaceNotImplementedError("Neousys WDT_DIO Can driver missing") - self.channel_info = "Neousys Can: device {}, channel {}".format( - self.device, self.channel - ) + self.channel = channel + self.device = device + self.channel_info = f"Neousys Can: device {self.device}, channel {self.channel}" - self.queue = queue.Queue() + self.queue = queue.Queue() - # Init with accept all and wanted bitrate - self.init_config = NeousysCanSetup( - bitrate, NEOUSYS_CAN_MSG_USE_ID_FILTER, 0, 0 - ) + # Init with accept all and wanted bitrate + self.init_config = NeousysCanSetup( + bitrate, NEOUSYS_CAN_MSG_USE_ID_FILTER, 0, 0 + ) - self._neousys_recv_cb = NEOUSYS_CAN_MSG_CALLBACK(self._neousys_recv_cb) - self._neousys_status_cb = NEOUSYS_CAN_STATUS_CALLBACK( - self._neousys_status_cb - ) + self._neousys_recv_cb = NEOUSYS_CAN_MSG_CALLBACK(self._neousys_recv_cb) + self._neousys_status_cb = NEOUSYS_CAN_STATUS_CALLBACK( + self._neousys_status_cb + ) - if NEOUSYS_CANLIB.CAN_RegisterReceived(0, self._neousys_recv_cb) == 0: - logger.error("Neousys CAN bus Setup receive callback") + if NEOUSYS_CANLIB.CAN_RegisterReceived(0, self._neousys_recv_cb) == 0: + raise CanInitializationError("Neousys CAN bus Setup receive callback") - if NEOUSYS_CANLIB.CAN_RegisterStatus(0, self._neousys_status_cb) == 0: - logger.error("Neousys CAN bus Setup status callback") + if NEOUSYS_CANLIB.CAN_RegisterStatus(0, self._neousys_status_cb) == 0: + raise CanInitializationError("Neousys CAN bus Setup status callback") - if ( - NEOUSYS_CANLIB.CAN_Setup( - channel, byref(self.init_config), sizeof(self.init_config) - ) - == 0 - ): - logger.error("Neousys CAN bus Setup Error") + if ( + NEOUSYS_CANLIB.CAN_Setup( + channel, byref(self.init_config), sizeof(self.init_config) + ) + == 0 + ): + raise CanInitializationError("Neousys CAN bus Setup Error") - if NEOUSYS_CANLIB.CAN_Start(channel) == 0: - logger.error("Neousys CAN bus Start Error") + if NEOUSYS_CANLIB.CAN_Start(channel) == 0: + raise CanInitializationError("Neousys CAN bus Start Error") def send(self, msg, timeout=None): """ @@ -188,26 +184,18 @@ def send(self, msg, timeout=None): :return: """ - if NEOUSYS_CANLIB is None: - logger.error("Can't send msg as Neousys DLL/SO is not loaded") - else: - tx_msg = NeousysCanMsg( - msg.arbitration_id, 0, 0, msg.dlc, (c_ubyte * 8)(*msg.data) - ) + tx_msg = NeousysCanMsg( + msg.arbitration_id, 0, 0, msg.dlc, (c_ubyte * 8)(*msg.data) + ) - if ( - NEOUSYS_CANLIB.CAN_Send(self.channel, byref(tx_msg), sizeof(tx_msg)) - == 0 - ): - logger.error("Neousys Can can't send message") + if NEOUSYS_CANLIB.CAN_Send(self.channel, byref(tx_msg), sizeof(tx_msg)) == 0: + raise CanOperationError("Neousys Can can't send message") def _recv_internal(self, timeout): - msg = None - - if not self.queue.empty(): - msg = self.queue.get() - - return msg, False + try: + return self.queue.get(), False + except queue.Empty: + return None, False def _neousys_recv_cb(self, msg, sizeof_msg): """ @@ -242,10 +230,10 @@ def _neousys_recv_cb(self, msg, sizeof_msg): # Reading happens in Callback function and # with Python-CAN it happens polling # so cache stuff in array to for poll - if not self.queue.full(): + try: self.queue.put(msg) - else: - logger.error("Neousys message Queue is full") + except queue.Full: + raise CanOperationError("Neousys message Queue is full") from None def _neousys_status_cb(self, status): """ @@ -255,8 +243,7 @@ def _neousys_status_cb(self, status): logger.info("%s _neousys_status_cb: %d", self.init_config, status) def shutdown(self): - if NEOUSYS_CANLIB is not None: - NEOUSYS_CANLIB.CAN_Stop(self.channel) + NEOUSYS_CANLIB.CAN_Stop(self.channel) def fileno(self): # Return an invalid file descriptor as not used @@ -264,8 +251,5 @@ def fileno(self): @staticmethod def _detect_available_configs(): - channels = [] - # There is only one channel - channels.append({"interface": "neousys", "channel": 0}) - return channels + return [{"interface": "neousys", "channel": 0}] From d94bad0cceb534f5926b7ca758fe730307f38c86 Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 8 Jun 2021 17:11:33 +0200 Subject: [PATCH 2/6] remove unused code from neousys --- can/interfaces/neousys/neousys.py | 34 +++++++++---------------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/can/interfaces/neousys/neousys.py b/can/interfaces/neousys/neousys.py index 1950bb1d0..d358a6cc3 100644 --- a/can/interfaces/neousys/neousys.py +++ b/can/interfaces/neousys/neousys.py @@ -17,7 +17,7 @@ import queue import logging import platform -import time +from time import time from ctypes import ( byref, @@ -177,11 +177,10 @@ def __init__(self, channel, device=0, bitrate=500000, **kwargs): if NEOUSYS_CANLIB.CAN_Start(channel) == 0: raise CanInitializationError("Neousys CAN bus Start Error") - def send(self, msg, timeout=None): + def send(self, msg, timeout=None) -> None: """ :param msg: message to send :param timeout: timeout is not used here - :return: """ tx_msg = NeousysCanMsg( @@ -197,28 +196,20 @@ def _recv_internal(self, timeout): except queue.Empty: return None, False - def _neousys_recv_cb(self, msg, sizeof_msg): + def _neousys_recv_cb(self, msg, sizeof_msg) -> None: """ - :param msg struct CAN_MSG - :param sizeof_msg message number - :return: + :param msg: struct CAN_MSG + :param sizeof_msg: message number """ - remote_frame = False - extended_frame = False - msg_bytes = bytearray(msg.contents.data) - - if msg.contents.flags & NEOUSYS_CAN_MSG_REMOTE_FRAME: - remote_frame = True - - if msg.contents.flags & NEOUSYS_CAN_MSG_EXTENDED_ID: - extended_frame = True + remote_frame = bool(msg.contents.flags & NEOUSYS_CAN_MSG_REMOTE_FRAME) + extended_frame = bool(msg.contents.flags & NEOUSYS_CAN_MSG_EXTENDED_ID) if msg.contents.flags & NEOUSYS_CAN_MSG_DATA_LOST: logger.error("_neousys_recv_cb flag CAN_MSG_DATA_LOST") msg = Message( - timestamp=time.time(), + timestamp=time(), arbitration_id=msg.contents.id, is_remote_frame=remote_frame, is_extended_id=extended_frame, @@ -235,20 +226,15 @@ def _neousys_recv_cb(self, msg, sizeof_msg): except queue.Full: raise CanOperationError("Neousys message Queue is full") from None - def _neousys_status_cb(self, status): + def _neousys_status_cb(self, status) -> None: """ - :param status BUS Status - :return: + :param status: BUS Status """ logger.info("%s _neousys_status_cb: %d", self.init_config, status) def shutdown(self): NEOUSYS_CANLIB.CAN_Stop(self.channel) - def fileno(self): - # Return an invalid file descriptor as not used - return -1 - @staticmethod def _detect_available_configs(): # There is only one channel From 1f5b03a4b54e91a43fee6aafdc77ebbbaba2bf13 Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 8 Jun 2021 15:19:44 +0000 Subject: [PATCH 3/6] Format code with black --- can/interfaces/neousys/neousys.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/can/interfaces/neousys/neousys.py b/can/interfaces/neousys/neousys.py index d358a6cc3..3b8e2737d 100644 --- a/can/interfaces/neousys/neousys.py +++ b/can/interfaces/neousys/neousys.py @@ -36,7 +36,11 @@ from ctypes import CDLL from can import BusABC, Message -from ...exceptions import CanInitializationError, CanOperationError, CanInterfaceNotImplementedError +from ...exceptions import ( + CanInitializationError, + CanOperationError, + CanInterfaceNotImplementedError, +) logger = logging.getLogger(__name__) @@ -151,14 +155,10 @@ def __init__(self, channel, device=0, bitrate=500000, **kwargs): self.queue = queue.Queue() # Init with accept all and wanted bitrate - self.init_config = NeousysCanSetup( - bitrate, NEOUSYS_CAN_MSG_USE_ID_FILTER, 0, 0 - ) + self.init_config = NeousysCanSetup(bitrate, NEOUSYS_CAN_MSG_USE_ID_FILTER, 0, 0) self._neousys_recv_cb = NEOUSYS_CAN_MSG_CALLBACK(self._neousys_recv_cb) - self._neousys_status_cb = NEOUSYS_CAN_STATUS_CALLBACK( - self._neousys_status_cb - ) + self._neousys_status_cb = NEOUSYS_CAN_STATUS_CALLBACK(self._neousys_status_cb) if NEOUSYS_CANLIB.CAN_RegisterReceived(0, self._neousys_recv_cb) == 0: raise CanInitializationError("Neousys CAN bus Setup receive callback") From 8c669336bc854f19bffc5efd0fe1fda8f90df484 Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 8 Jun 2021 17:56:50 +0200 Subject: [PATCH 4/6] remove unused test flag --- test/test_neousys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_neousys.py b/test/test_neousys.py index d5a59ed0e..c2ae535f5 100644 --- a/test/test_neousys.py +++ b/test/test_neousys.py @@ -34,7 +34,7 @@ def setUp(self) -> None: can.interfaces.neousys.neousys.NEOUSYS_CANLIB.CAN_Start = Mock(return_value=1) can.interfaces.neousys.neousys.NEOUSYS_CANLIB.CAN_Send = Mock(return_value=1) can.interfaces.neousys.neousys.NEOUSYS_CANLIB.CAN_Stop = Mock(return_value=1) - self.bus = can.Bus(channel=0, bustype="neousys", _testing=True) + self.bus = can.Bus(channel=0, bustype="neousys") def tearDown(self) -> None: if self.bus: @@ -67,7 +67,7 @@ def test_bus_creation(self) -> None: ) def test_bus_creation_bitrate(self) -> None: - self.bus = can.Bus(channel=0, bustype="neousys", bitrate=200000, _testing=True) + self.bus = can.Bus(channel=0, bustype="neousys", bitrate=200000) self.assertIsInstance(self.bus, neousys.NeousysBus) CAN_Start_args = ( can.interfaces.neousys.neousys.NEOUSYS_CANLIB.CAN_Setup.call_args[0] From 34488616ef7ea69901aeca7aef2f4b84238fdc00 Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 8 Jun 2021 17:57:18 +0200 Subject: [PATCH 5/6] support timeouts while receiving --- can/interfaces/neousys/neousys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/interfaces/neousys/neousys.py b/can/interfaces/neousys/neousys.py index 3b8e2737d..d79a0a572 100644 --- a/can/interfaces/neousys/neousys.py +++ b/can/interfaces/neousys/neousys.py @@ -192,7 +192,7 @@ def send(self, msg, timeout=None) -> None: def _recv_internal(self, timeout): try: - return self.queue.get(), False + return self.queue.get(block=True, timeout=timeout), False except queue.Empty: return None, False From 16edfad90bc5632f5f49991c0e32be42b3d32e76 Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 8 Jun 2021 17:58:54 +0200 Subject: [PATCH 6/6] remove confusing parameter documentation --- can/interfaces/neousys/neousys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/interfaces/neousys/neousys.py b/can/interfaces/neousys/neousys.py index d79a0a572..c996c8298 100644 --- a/can/interfaces/neousys/neousys.py +++ b/can/interfaces/neousys/neousys.py @@ -141,7 +141,7 @@ def __init__(self, channel, device=0, bitrate=500000, **kwargs): """ :param channel: channel number :param device: device number - :param bitrate: bit rate. Renamed to bitrate in next release. + :param bitrate: bit rate. """ super().__init__(channel, **kwargs)