From 95f79ea07eab750d8a556304352abc4f36845653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Luc=20Tessier=20Gagn=C3=A9?= Date: Tue, 11 Jun 2019 08:14:07 -0400 Subject: [PATCH 1/9] Use inter-process mutex to prevent concurrent neoVI device open When neoVI server is enabled, there is an issue with concurrent device open. --- can/interfaces/ics_neovi/neovi_bus.py | 12 +++++++++++- setup.py | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/can/interfaces/ics_neovi/neovi_bus.py b/can/interfaces/ics_neovi/neovi_bus.py index 530d7c524..b67dc1258 100644 --- a/can/interfaces/ics_neovi/neovi_bus.py +++ b/can/interfaces/ics_neovi/neovi_bus.py @@ -11,9 +11,12 @@ """ import logging +import os +import tempfile from collections import deque from can import Message, CanError, BusABC +from filelock import FileLock logger = logging.getLogger(__name__) @@ -118,7 +121,14 @@ def __init__(self, channel, can_filters=None, **kwargs): type_filter = kwargs.get('type_filter') serial = kwargs.get('serial') self.dev = self._find_device(type_filter, serial) - ics.open_device(self.dev) + + # Use inter-process mutex to prevent concurrent device open. + # When neoVI server is enabled, there is an issue with concurrent + # device open. + open_lock = FileLock( + os.path.join(tempfile.gettempdir(), 'neovi.lock')) + with open_lock: + ics.open_device(self.dev) if 'bitrate' in kwargs: for channel in self.channels: diff --git a/setup.py b/setup.py index e5b05e8dc..a7680693f 100644 --- a/setup.py +++ b/setup.py @@ -115,6 +115,7 @@ 'aenum', 'typing;python_version<"3.5"', 'windows-curses;platform_system=="Windows"', + 'filelock' ], setup_requires=pytest_runner, extras_require=extras_require, From f68de2db1a03f0f2234b8779250c373ff9ef997a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Luc=20Tessier=20Gagn=C3=A9?= Date: Tue, 11 Jun 2019 08:51:25 -0400 Subject: [PATCH 2/9] Move neoVI open lock to module level --- can/interfaces/ics_neovi/neovi_bus.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/can/interfaces/ics_neovi/neovi_bus.py b/can/interfaces/ics_neovi/neovi_bus.py index b67dc1258..0fef13465 100644 --- a/can/interfaces/ics_neovi/neovi_bus.py +++ b/can/interfaces/ics_neovi/neovi_bus.py @@ -15,9 +15,10 @@ import tempfile from collections import deque -from can import Message, CanError, BusABC from filelock import FileLock +from can import Message, CanError, BusABC + logger = logging.getLogger(__name__) try: @@ -30,6 +31,11 @@ ics = None +# Use inter-process mutex to prevent concurrent device open. +# When neoVI server is enabled, there is an issue with concurrent device open. +open_lock = FileLock(os.path.join(tempfile.gettempdir(), "neovi.lock")) + + class ICSApiError(CanError): """ Indicates an error with the ICS API. @@ -122,11 +128,6 @@ def __init__(self, channel, can_filters=None, **kwargs): serial = kwargs.get('serial') self.dev = self._find_device(type_filter, serial) - # Use inter-process mutex to prevent concurrent device open. - # When neoVI server is enabled, there is an issue with concurrent - # device open. - open_lock = FileLock( - os.path.join(tempfile.gettempdir(), 'neovi.lock')) with open_lock: ics.open_device(self.dev) From 01fbc69280807683c27694338df204fdebf405ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Luc=20Tessier=20Gagn=C3=A9?= Date: Tue, 11 Jun 2019 13:37:41 -0400 Subject: [PATCH 3/9] Adding dummy file lock when importing FileLock fails --- can/interfaces/ics_neovi/neovi_bus.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/can/interfaces/ics_neovi/neovi_bus.py b/can/interfaces/ics_neovi/neovi_bus.py index 0fef13465..a46d4d103 100644 --- a/can/interfaces/ics_neovi/neovi_bus.py +++ b/can/interfaces/ics_neovi/neovi_bus.py @@ -15,8 +15,6 @@ import tempfile from collections import deque -from filelock import FileLock - from can import Message, CanError, BusABC logger = logging.getLogger(__name__) @@ -31,6 +29,30 @@ ics = None +try: + from filelock import FileLock +except ImportError as ie: + + logger.warning( + "Using ICS NeoVi can backend without the " + "filelock module installed may cause some issues!: %s", + ie, + ) + + class FileLock: + """Dummy file lock that do not actually do anything""" + + def __init__(self, lock_file, timeout=-1): + self._lock_file = lock_file + self.timeout = timeout + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + return None + + # Use inter-process mutex to prevent concurrent device open. # When neoVI server is enabled, there is an issue with concurrent device open. open_lock = FileLock(os.path.join(tempfile.gettempdir(), "neovi.lock")) From 328027ac43e697423ae668156e040b4571623769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Luc=20Tessier=20Gagn=C3=A9?= Date: Wed, 12 Jun 2019 07:44:36 -0400 Subject: [PATCH 4/9] Grammar fix --- can/interfaces/ics_neovi/neovi_bus.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/interfaces/ics_neovi/neovi_bus.py b/can/interfaces/ics_neovi/neovi_bus.py index a46d4d103..4ca8cde91 100644 --- a/can/interfaces/ics_neovi/neovi_bus.py +++ b/can/interfaces/ics_neovi/neovi_bus.py @@ -40,7 +40,7 @@ ) class FileLock: - """Dummy file lock that do not actually do anything""" + """Dummy file lock that does not actually do anything""" def __init__(self, lock_file, timeout=-1): self._lock_file = lock_file From b1097944b0dbc329155642b59d29e898f0bef6eb Mon Sep 17 00:00:00 2001 From: Pierre-Luc Date: Thu, 9 Jul 2020 09:42:52 -0400 Subject: [PATCH 5/9] Raising more precise API error when set bitrate fails Also calling shutdown before raising exception from the init if the device is opened --- can/interfaces/ics_neovi/neovi_bus.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/can/interfaces/ics_neovi/neovi_bus.py b/can/interfaces/ics_neovi/neovi_bus.py index 4ca8cde91..3ad9d6426 100644 --- a/can/interfaces/ics_neovi/neovi_bus.py +++ b/can/interfaces/ics_neovi/neovi_bus.py @@ -153,16 +153,25 @@ def __init__(self, channel, can_filters=None, **kwargs): with open_lock: ics.open_device(self.dev) - if 'bitrate' in kwargs: - for channel in self.channels: - ics.set_bit_rate(self.dev, kwargs.get('bitrate'), channel) - - fd = kwargs.get('fd', False) - if fd: - if 'data_bitrate' in kwargs: + try: + if "bitrate" in kwargs: for channel in self.channels: - ics.set_fd_bit_rate( - self.dev, kwargs.get('data_bitrate'), channel) + ics.set_bit_rate(self.dev, kwargs.get("bitrate"), channel) + + fd = kwargs.get("fd", False) + if fd: + if "data_bitrate" in kwargs: + for channel in self.channels: + ics.set_fd_bit_rate( + self.dev, kwargs.get("data_bitrate"), channel + ) + except ics.RuntimeError as re: + logger.error(re) + err = ICSApiError(*ics.get_last_api_error(self.dev)) + try: + self.shutdown() + finally: + raise err self._use_system_timestamp = bool( kwargs.get('use_system_timestamp', False) From 7cdd7a0a0b02797f0821d89b7df44d32c33f861a Mon Sep 17 00:00:00 2001 From: Pierre-Luc Date: Thu, 9 Jul 2020 10:56:12 -0400 Subject: [PATCH 6/9] Simplified if fd condition in init --- can/interfaces/ics_neovi/neovi_bus.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/can/interfaces/ics_neovi/neovi_bus.py b/can/interfaces/ics_neovi/neovi_bus.py index 3ad9d6426..314967708 100644 --- a/can/interfaces/ics_neovi/neovi_bus.py +++ b/can/interfaces/ics_neovi/neovi_bus.py @@ -158,8 +158,7 @@ def __init__(self, channel, can_filters=None, **kwargs): for channel in self.channels: ics.set_bit_rate(self.dev, kwargs.get("bitrate"), channel) - fd = kwargs.get("fd", False) - if fd: + if kwargs.get("fd", False): if "data_bitrate" in kwargs: for channel in self.channels: ics.set_fd_bit_rate( From 8fde5251f7562f1f60f03a9bff9990d78d105fcd Mon Sep 17 00:00:00 2001 From: pierreluctg Date: Thu, 27 Aug 2020 14:50:26 -0400 Subject: [PATCH 7/9] Moving filelock to neovi extras_require --- setup.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index a7680693f..e83d63347 100644 --- a/setup.py +++ b/setup.py @@ -26,7 +26,7 @@ # Dependencies extras_require = { 'serial': ['pyserial~=3.0'], - 'neovi': ['python-ics>=2.12'] + 'neovi': ['python-ics>=2.12', 'filelock'] } tests_require = [ @@ -114,8 +114,7 @@ 'wrapt~=1.10', 'aenum', 'typing;python_version<"3.5"', - 'windows-curses;platform_system=="Windows"', - 'filelock' + 'windows-curses;platform_system=="Windows"' ], setup_requires=pytest_runner, extras_require=extras_require, From 0a776fb1f7106b647f86f4705198155970c2701a Mon Sep 17 00:00:00 2001 From: pierreluctg Date: Thu, 27 Aug 2020 14:50:53 -0400 Subject: [PATCH 8/9] Adding comma back --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index e83d63347..c3d4d6bfa 100644 --- a/setup.py +++ b/setup.py @@ -114,7 +114,7 @@ 'wrapt~=1.10', 'aenum', 'typing;python_version<"3.5"', - 'windows-curses;platform_system=="Windows"' + 'windows-curses;platform_system=="Windows"', ], setup_requires=pytest_runner, extras_require=extras_require, From ed3b816cb2a251ffa4811a920b138a1dfe1f9e2b Mon Sep 17 00:00:00 2001 From: pierreluctg Date: Fri, 28 Aug 2020 09:14:35 -0400 Subject: [PATCH 9/9] Update CHANGELOG.txt --- CHANGELOG.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 1584ef235..e75a3cd91 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -11,6 +11,7 @@ Version 3.3.3 Backported fixes from 4.x development branch which targets Python 3. +* #846 Use inter-process mutex to prevent concurrent neoVI device open. * #798 Backport caching msg.data value in neovi interface. * #796 Fix Vector CANlib treatment of empty app name. * #771 Handle empty CSV file.