diff --git a/.travis.yml b/.travis.yml index bcd18033a..cca71b50e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -25,6 +25,7 @@ env: install: - if [[ "$TEST_SOCKETCAN" ]]; then sudo bash test/open_vcan.sh ; fi - travis_retry pip install .[test] + - pip freeze script: - | @@ -52,6 +53,14 @@ jobs: # Unit Testing Stage + # testing socketcan on Trusty & Python 2.7, since it is not available on Xenial + - stage: test + name: Socketcan + os: linux + dist: trusty + python: "2.7" + sudo: required + env: TEST_SOCKETCAN=TRUE # testing socketcan on Trusty & Python 3.6, since it is not available on Xenial - stage: test name: Socketcan diff --git a/CHANGELOG.txt b/CHANGELOG.txt index c2a6ea38a..7899d50b6 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,17 @@ +Version 3.3.4 +==== + +Last call for Python2 support. + +* #916 Vector: Skip channels without CAN support +* #846 Use inter-process mutex to prevent concurrent neoVI device open. +* #901 Fix iteration in Bus.stop_all_periodic_tasks +* #850 Fix socket.error is a deprecated alias of OSError used on Python versions lower than 3.3. +* #879 Updating incorrect api documentation. +* #885 Fix recursion message in Message.__getattr__ +* #845 Fix socketcan issue + + Version 3.3.3 ==== diff --git a/can/__init__.py b/can/__init__.py index 481dc29e3..2704efc3d 100644 --- a/can/__init__.py +++ b/can/__init__.py @@ -8,7 +8,7 @@ import logging -__version__ = "3.3.3" +__version__ = "3.3.4" log = logging.getLogger('can') diff --git a/can/bus.py b/can/bus.py index 2b36b3c57..2b3044cf1 100644 --- a/can/bus.py +++ b/can/bus.py @@ -247,13 +247,21 @@ def _send_periodic_internal(self, msg, period, duration=None): return task def stop_all_periodic_tasks(self, remove_tasks=True): - """Stop sending any messages that were started using bus.send_periodic + """Stop sending any messages that were started using **bus.send_periodic**. + + .. note:: + The result is undefined if a single task throws an exception while being stopped. :param bool remove_tasks: Stop tracking the stopped tasks. """ for task in self._periodic_tasks: - task.stop(remove_task=remove_tasks) + # we cannot let `task.stop()` modify `self._periodic_tasks` while we are + # iterating over it (#634) + task.stop(remove_task=False) + + if remove_tasks: + self._periodic_tasks = [] def __iter__(self): """Allow iteration on messages as they are received. diff --git a/can/interfaces/ics_neovi/neovi_bus.py b/can/interfaces/ics_neovi/neovi_bus.py index 530d7c524..314967708 100644 --- a/can/interfaces/ics_neovi/neovi_bus.py +++ b/can/interfaces/ics_neovi/neovi_bus.py @@ -11,6 +11,8 @@ """ import logging +import os +import tempfile from collections import deque from can import Message, CanError, BusABC @@ -27,6 +29,35 @@ 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 does 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")) + + class ICSApiError(CanError): """ Indicates an error with the ICS API. @@ -118,18 +149,28 @@ 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) - if 'bitrate' in kwargs: - for channel in self.channels: - ics.set_bit_rate(self.dev, kwargs.get('bitrate'), channel) + with open_lock: + ics.open_device(self.dev) - 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) + + if kwargs.get("fd", False): + 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) diff --git a/can/interfaces/ixxat/canlib.py b/can/interfaces/ixxat/canlib.py index 84c8751c1..eac867344 100644 --- a/can/interfaces/ixxat/canlib.py +++ b/can/interfaces/ixxat/canlib.py @@ -328,10 +328,12 @@ def __init__(self, channel, can_filters=None, **kwargs): else: raise VCIDeviceNotFoundError("Unique HW ID {} not connected or not available.".format(UniqueHardwareId)) else: - if (UniqueHardwareId is None) or (self._device_info.UniqueHardwareId.AsChar == bytes(UniqueHardwareId, 'ascii')): + if (UniqueHardwareId is None) or ( + self._device_info.UniqueHardwareId.AsChar == UniqueHardwareId.encode("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) _canlib.vciDeviceOpen(ctypes.byref(self._device_info.VciObjectId), ctypes.byref(self._device_handle)) log.info("Using unique HW ID %s", self._device_info.UniqueHardwareId.AsChar) diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index b671f926a..4bb989dd7 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -363,7 +363,7 @@ def _tx_setup(self, message): ) try: self.bcm_socket.send(check_header) - except OSError as e: + except (socket.error, OSError) as e: if e.errno != errno.EINVAL: raise e else: diff --git a/can/interfaces/vector/canlib.py b/can/interfaces/vector/canlib.py index 65af0147b..0864ecefe 100644 --- a/can/interfaces/vector/canlib.py +++ b/can/interfaces/vector/canlib.py @@ -387,6 +387,8 @@ def _detect_available_configs(): channel_configs = get_channel_configs() LOG.info('Found %d channels', len(channel_configs)) for channel_config in channel_configs: + if not channel_config.channelBusCapabilities & vxlapi.XL_BUS_ACTIVE_CAP_CAN: + continue LOG.info('Channel index %d: %s', channel_config.channelIndex, channel_config.name.decode('ascii')) diff --git a/can/interfaces/vector/vxlapi.py b/can/interfaces/vector/vxlapi.py index ae87706c4..461a62197 100644 --- a/can/interfaces/vector/vxlapi.py +++ b/can/interfaces/vector/vxlapi.py @@ -63,6 +63,7 @@ XL_INTERFACE_VERSION = 3 XL_INTERFACE_VERSION_V4 = 4 +XL_BUS_ACTIVE_CAP_CAN = XL_BUS_TYPE_CAN << 16 XL_CHANNEL_FLAG_CANFD_ISO_SUPPORT = 0x80000000 # structure for XL_RECEIVE_MSG, XL_TRANSMIT_MSG diff --git a/can/message.py b/can/message.py index f85218fc0..84f675183 100644 --- a/can/message.py +++ b/can/message.py @@ -53,7 +53,9 @@ def __getattr__(self, key): # TODO keep this for a version, in order to not break old code # this entire method (as well as the _dict attribute in __slots__ and the __setattr__ method) # can be removed in 4.0 - # this method is only called if the attribute was not found elsewhere, like in __slots__ + # this method is only called if the attribute was not found elsewhere, like in __slots_ + if key not in self.__slots__: + raise AttributeError try: warnings.warn("Custom attributes of messages are deprecated and will be removed in 4.0", DeprecationWarning) return self._dict[key] @@ -110,6 +112,7 @@ def __init__(self, timestamp=0.0, arbitration_id=0, is_extended_id=None, if is_extended_id is not None: self.is_extended_id = is_extended_id else: + # Default behaviour is to create extended id messages self.is_extended_id = True if extended_id is None else extended_id self.is_remote_frame = is_remote_frame diff --git a/doc/configuration.rst b/doc/configuration.rst index 142e816da..230eec1bd 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -19,7 +19,7 @@ the **interface** and **channel** before importing from ``can.interfaces``. can.rc['interface'] = 'socketcan' can.rc['channel'] = 'vcan0' can.rc['bitrate'] = 500000 - from can.interfaces.interface import Bus + from can.interface import Bus bus = Bus() diff --git a/setup.py b/setup.py index e5b05e8dc..898f54c16 100644 --- a/setup.py +++ b/setup.py @@ -26,12 +26,12 @@ # Dependencies extras_require = { 'serial': ['pyserial~=3.0'], - 'neovi': ['python-ics>=2.12'] + 'neovi': ['python-ics>=2.12', 'filelock'] } tests_require = [ 'mock~=2.0', - 'pytest~=4.3', + 'pytest~=4.6', 'pytest-timeout~=1.3', 'pytest-cov~=2.8', # coveragepy==5.0 fails with `Safety level may not be changed inside a transaction` @@ -53,7 +53,6 @@ needs_pytest = {"pytest", "test", "ptr"}.intersection(sys.argv) pytest_runner = ["pytest-runner"] if needs_pytest else [] - setup( # Description name="python-can", @@ -67,6 +66,7 @@ "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", + "Programming Language :: Python :: 3.8", "Programming Language :: Python :: Implementation :: CPython", "Programming Language :: Python :: Implementation :: PyPy", "License :: OSI Approved :: GNU Lesser General Public License v3 (LGPLv3)", diff --git a/test/test_cyclic_socketcan.py b/test/test_cyclic_socketcan.py new file mode 100644 index 000000000..831264bf4 --- /dev/null +++ b/test/test_cyclic_socketcan.py @@ -0,0 +1,226 @@ +""" +This module tests multiple message cyclic send tasks. +""" +import unittest + +import time +import can + +from .config import TEST_INTERFACE_SOCKETCAN + + +@unittest.skipUnless(TEST_INTERFACE_SOCKETCAN, "skip testing of socketcan") +class CyclicSocketCan(unittest.TestCase): + BITRATE = 500000 + TIMEOUT = 0.1 + + INTERFACE_1 = "socketcan" + CHANNEL_1 = "vcan0" + INTERFACE_2 = "socketcan" + CHANNEL_2 = "vcan0" + + PERIOD = 1.0 + + DELTA = 0.01 + + def _find_start_index(self, tx_messages, message): + """ + :param tx_messages: + The list of messages that were passed to the periodic backend + :param message: + The message whose data we wish to match and align to + + :returns: start index in the tx_messages + """ + start_index = -1 + for index, tx_message in enumerate(tx_messages): + if tx_message.data == message.data: + start_index = index + break + return start_index + + def setUp(self): + self._send_bus = can.Bus( + interface=self.INTERFACE_1, channel=self.CHANNEL_1, bitrate=self.BITRATE + ) + self._recv_bus = can.Bus( + interface=self.INTERFACE_2, channel=self.CHANNEL_2, bitrate=self.BITRATE + ) + + def tearDown(self): + self._send_bus.shutdown() + self._recv_bus.shutdown() + + def test_cyclic_initializer_message(self): + message = can.Message( + arbitration_id=0x401, + data=[0x11, 0x11, 0x11, 0x11, 0x11, 0x11], + is_extended_id=False, + ) + + task = self._send_bus.send_periodic(message, self.PERIOD) + self.assertIsInstance(task, can.broadcastmanager.CyclicSendTaskABC) + + # Take advantage of kernel's queueing mechanisms + time.sleep(4 * self.PERIOD) + task.stop() + + for _ in range(4): + tx_message = message + rx_message = self._recv_bus.recv(self.TIMEOUT) + + self.assertIsNotNone(rx_message) + self.assertEqual(tx_message.arbitration_id, rx_message.arbitration_id) + self.assertEqual(tx_message.dlc, rx_message.dlc) + self.assertEqual(tx_message.data, rx_message.data) + self.assertEqual(tx_message.is_extended_id, rx_message.is_extended_id) + self.assertEqual(tx_message.is_remote_frame, rx_message.is_remote_frame) + self.assertEqual(tx_message.is_error_frame, rx_message.is_error_frame) + self.assertEqual(tx_message.is_fd, rx_message.is_fd) + + def test_create_same_id_raises_exception(self): + messages_a = can.Message( + arbitration_id=0x401, + data=[0x11, 0x11, 0x11, 0x11, 0x11, 0x11], + is_extended_id=False, + ) + + messages_b = can.Message( + arbitration_id=0x401, + data=[0x22, 0x22, 0x22, 0x22, 0x22, 0x22], + is_extended_id=False, + ) + + task_a = self._send_bus.send_periodic(messages_a, 1) + self.assertIsInstance(task_a, can.broadcastmanager.CyclicSendTaskABC) + + # The second one raises a ValueError when we attempt to create a new + # Task, since it has the same arbitration ID. + with self.assertRaises(ValueError): + task_b = self._send_bus.send_periodic(messages_b, 1) + + def test_modify_data_message(self): + message_odd = can.Message( + arbitration_id=0x401, + data=[0x11, 0x11, 0x11, 0x11, 0x11, 0x11], + is_extended_id=False, + ) + message_even = can.Message( + arbitration_id=0x401, + data=[0x22, 0x22, 0x22, 0x22, 0x22, 0x22], + is_extended_id=False, + ) + task = self._send_bus.send_periodic(message_odd, self.PERIOD) + self.assertIsInstance(task, can.broadcastmanager.ModifiableCyclicTaskABC) + + results_odd = [] + results_even = [] + for _ in range(1 * 4): + result = self._recv_bus.recv(self.PERIOD * 2) + if result: + results_odd.append(result) + + task.modify_data(message_even) + for _ in range(1 * 4): + result = self._recv_bus.recv(self.PERIOD * 2) + if result: + results_even.append(result) + + task.stop() + + # Now go through the partitioned results and assert that they're equal + for rx_index, rx_message in enumerate(results_even): + tx_message = message_even + + self.assertEqual(tx_message.arbitration_id, rx_message.arbitration_id) + self.assertEqual(tx_message.dlc, rx_message.dlc) + self.assertEqual(tx_message.data, rx_message.data) + self.assertEqual(tx_message.is_extended_id, rx_message.is_extended_id) + self.assertEqual(tx_message.is_remote_frame, rx_message.is_remote_frame) + self.assertEqual(tx_message.is_error_frame, rx_message.is_error_frame) + self.assertEqual(tx_message.is_fd, rx_message.is_fd) + + if rx_index != 0: + prev_rx_message = results_even[rx_index - 1] + # Assert timestamps are within the expected period + self.assertTrue( + abs( + (rx_message.timestamp - prev_rx_message.timestamp) - self.PERIOD + ) + <= self.DELTA + ) + + for rx_index, rx_message in enumerate(results_odd): + tx_message = message_odd + + self.assertEqual(tx_message.arbitration_id, rx_message.arbitration_id) + self.assertEqual(tx_message.dlc, rx_message.dlc) + self.assertEqual(tx_message.data, rx_message.data) + self.assertEqual(tx_message.is_extended_id, rx_message.is_extended_id) + self.assertEqual(tx_message.is_remote_frame, rx_message.is_remote_frame) + self.assertEqual(tx_message.is_error_frame, rx_message.is_error_frame) + self.assertEqual(tx_message.is_fd, rx_message.is_fd) + + if rx_index != 0: + prev_rx_message = results_odd[rx_index - 1] + # Assert timestamps are within the expected period + self.assertTrue( + abs( + (rx_message.timestamp - prev_rx_message.timestamp) - self.PERIOD + ) + <= self.DELTA + ) + + def test_stop_all_periodic_tasks_and_remove_task(self): + message_a = can.Message( + arbitration_id=0x401, + data=[0x11, 0x11, 0x11, 0x11, 0x11, 0x11], + is_extended_id=False, + ) + message_b = can.Message( + arbitration_id=0x402, + data=[0x22, 0x22, 0x22, 0x22, 0x22, 0x22], + is_extended_id=False, + ) + message_c = can.Message( + arbitration_id=0x403, + data=[0x33, 0x33, 0x33, 0x33, 0x33, 0x33], + is_extended_id=False, + ) + + # Start Tasks + task_a = self._send_bus.send_periodic(message_a, self.PERIOD) + task_b = self._send_bus.send_periodic(message_b, self.PERIOD) + task_c = self._send_bus.send_periodic(message_c, self.PERIOD) + + self.assertIsInstance(task_a, can.broadcastmanager.ModifiableCyclicTaskABC) + self.assertIsInstance(task_b, can.broadcastmanager.ModifiableCyclicTaskABC) + self.assertIsInstance(task_c, can.broadcastmanager.ModifiableCyclicTaskABC) + + for _ in range(6): + _ = self._recv_bus.recv(self.PERIOD) + + # Stop all tasks and delete + self._send_bus.stop_all_periodic_tasks(remove_tasks=True) + + # Now wait for a few periods, after which we should definitely not + # receive any CAN messages + time.sleep(4 * self.PERIOD) + + # If we successfully deleted everything, then we will eventually read + # 0 messages. + successfully_stopped = False + for _ in range(6): + rx_message = self._recv_bus.recv(self.PERIOD) + + if rx_message is None: + successfully_stopped = True + break + self.assertTrue(successfully_stopped, "Still received messages after stopping") + + # None of the tasks should still be associated with the bus + self.assertEqual(0, len(self._send_bus._periodic_tasks)) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_message_class.py b/test/test_message_class.py index 85dbe8560..05ed14b2a 100644 --- a/test/test_message_class.py +++ b/test/test_message_class.py @@ -5,12 +5,15 @@ import sys from math import isinf, isnan from copy import copy, deepcopy +import pickle from hypothesis import given, settings, reproduce_failure import hypothesis.strategies as st from can import Message +from .message_helper import ComparingMessagesTestCase + class TestMessageClass(unittest.TestCase): """ @@ -70,7 +73,7 @@ def test_methods(self, **kwargs): # check copies and equalities if is_valid: - self.assertEqual(message, message) + self.assertEqual(message, message) normal_copy = copy(message) deep_copy = deepcopy(message) for other in (normal_copy, deep_copy, message): @@ -79,5 +82,31 @@ def test_methods(self, **kwargs): self.assertTrue(message.equals(other, timestamp_delta=0)) -if __name__ == '__main__': +class MessageSerialization(unittest.TestCase, ComparingMessagesTestCase): + def __init__(self, *args, **kwargs): + unittest.TestCase.__init__(self, *args, **kwargs) + ComparingMessagesTestCase.__init__( + self, allowed_timestamp_delta=0.016, preserves_channel=True + ) + + def test_serialization(self): + message = Message( + timestamp=1.0, + arbitration_id=0x401, + is_extended_id=False, + is_remote_frame=False, + is_error_frame=False, + channel=1, + dlc=6, + data=bytearray([0x01, 0x02, 0x03, 0x04, 0x05, 0x06]), + is_fd=False, + ) + + serialized = pickle.dumps(message, -1) + deserialized = pickle.loads(serialized) + + self.assertMessageEqual(message, deserialized) + + +if __name__ == "__main__": unittest.main()