From 7c441017e0551882d6f5f9aa88712f65793b0f11 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Thu, 6 Sep 2018 23:02:01 +0200 Subject: [PATCH 01/32] use is_extended_id instead of id_type for message objects --- can/interfaces/ixxat/canlib.py | 4 +-- can/interfaces/kvaser/canlib.py | 2 +- can/interfaces/nican.py | 2 +- can/interfaces/pcan/pcan.py | 2 +- can/interfaces/usb2can/usb2canInterface.py | 2 +- can/interfaces/vector/canlib.py | 2 +- can/io/blf.py | 2 +- can/io/csv.py | 2 +- can/io/sqlite.py | 2 +- can/message.py | 29 +++++++++++++--------- test/back2back_test.py | 2 +- test/network_test.py | 2 +- test/test_kvaser.py | 6 ++--- 13 files changed, 32 insertions(+), 27 deletions(-) diff --git a/can/interfaces/ixxat/canlib.py b/can/interfaces/ixxat/canlib.py index b087fe41e..29d6ec59b 100644 --- a/can/interfaces/ixxat/canlib.py +++ b/can/interfaces/ixxat/canlib.py @@ -486,7 +486,7 @@ def send(self, msg, timeout=None): message = structures.CANMSG() message.uMsgInfo.Bits.type = constants.CAN_MSGTYPE_DATA message.uMsgInfo.Bits.rtr = 1 if msg.is_remote_frame else 0 - message.uMsgInfo.Bits.ext = 1 if msg.id_type else 0 + message.uMsgInfo.Bits.ext = 1 if msg.is_extended_id else 0 message.uMsgInfo.Bits.srr = 1 if self._receive_own_messages else 0 message.dwMsgId = msg.arbitration_id if msg.dlc: @@ -546,7 +546,7 @@ def __init__(self, scheduler, msg, period, duration, resolution): self._msg.wCycleTime = int(round(period * resolution)) self._msg.dwMsgId = msg.arbitration_id self._msg.uMsgInfo.Bits.type = constants.CAN_MSGTYPE_DATA - self._msg.uMsgInfo.Bits.ext = 1 if msg.id_type else 0 + self._msg.uMsgInfo.Bits.ext = 1 if msg.is_extended_id else 0 self._msg.uMsgInfo.Bits.rtr = 1 if msg.is_remote_frame else 0 self._msg.uMsgInfo.Bits.dlc = msg.dlc for i, b in enumerate(msg.data): diff --git a/can/interfaces/kvaser/canlib.py b/can/interfaces/kvaser/canlib.py index efe4cb92c..99850a2ec 100644 --- a/can/interfaces/kvaser/canlib.py +++ b/can/interfaces/kvaser/canlib.py @@ -527,7 +527,7 @@ def _recv_internal(self, timeout=None): def send(self, msg, timeout=None): #log.debug("Writing a message: {}".format(msg)) - flags = canstat.canMSG_EXT if msg.id_type else canstat.canMSG_STD + flags = canstat.canMSG_EXT if msg.is_extended_id else canstat.canMSG_STD if msg.is_remote_frame: flags |= canstat.canMSG_RTR if msg.is_error_frame: diff --git a/can/interfaces/nican.py b/can/interfaces/nican.py index 320ef6901..dd18263a6 100644 --- a/can/interfaces/nican.py +++ b/can/interfaces/nican.py @@ -264,7 +264,7 @@ def send(self, msg, timeout=None): It does not wait for message to be ACKed currently. """ arb_id = msg.arbitration_id - if msg.id_type: + if msg.is_extended_id: arb_id |= NC_FL_CAN_ARBID_XTD raw_msg = TxMessageStruct(arb_id, bool(msg.is_remote_frame), diff --git a/can/interfaces/pcan/pcan.py b/can/interfaces/pcan/pcan.py index 05aa533ff..60d96b02a 100644 --- a/can/interfaces/pcan/pcan.py +++ b/can/interfaces/pcan/pcan.py @@ -227,7 +227,7 @@ def _recv_internal(self, timeout): return rx_msg, False def send(self, msg, timeout=None): - if msg.id_type: + if msg.is_extended_id: msgType = PCAN_MESSAGE_EXTENDED else: msgType = PCAN_MESSAGE_STANDARD diff --git a/can/interfaces/usb2can/usb2canInterface.py b/can/interfaces/usb2can/usb2canInterface.py index 46bbda20e..ee05f93a3 100644 --- a/can/interfaces/usb2can/usb2canInterface.py +++ b/can/interfaces/usb2can/usb2canInterface.py @@ -52,7 +52,7 @@ def message_convert_tx(msg): if msg.is_remote_frame: messagetx.flags |= IS_REMOTE_FRAME - if msg.id_type: + if msg.is_extended_id: messagetx.flags |= IS_ID_TYPE return messagetx diff --git a/can/interfaces/vector/canlib.py b/can/interfaces/vector/canlib.py index bace816fc..aa6ad963f 100644 --- a/can/interfaces/vector/canlib.py +++ b/can/interfaces/vector/canlib.py @@ -320,7 +320,7 @@ def _recv_internal(self, timeout): def send(self, msg, timeout=None): msg_id = msg.arbitration_id - if msg.id_type: + if msg.is_extended_id: msg_id |= vxlapi.XL_CAN_EXT_MSG_ID flags = 0 diff --git a/can/io/blf.py b/can/io/blf.py index 9de76aec4..b9aa19438 100644 --- a/can/io/blf.py +++ b/can/io/blf.py @@ -294,7 +294,7 @@ def on_message_received(self, msg): channel += 1 arb_id = msg.arbitration_id - if msg.id_type: + if msg.is_extended_id: arb_id |= CAN_MSG_EXT flags = REMOTE_FLAG if msg.is_remote_frame else 0 data = bytes(msg.data) diff --git a/can/io/csv.py b/can/io/csv.py index e108679b8..8f7623d72 100644 --- a/can/io/csv.py +++ b/can/io/csv.py @@ -62,7 +62,7 @@ def on_message_received(self, msg): row = ','.join([ repr(msg.timestamp), # cannot use str() here because that is rounding hex(msg.arbitration_id), - '1' if msg.id_type else '0', + '1' if msg.is_extended_id else '0', '1' if msg.is_remote_frame else '0', '1' if msg.is_error_frame else '0', str(msg.dlc), diff --git a/can/io/sqlite.py b/can/io/sqlite.py index 23be2b8f5..941f58ed0 100644 --- a/can/io/sqlite.py +++ b/can/io/sqlite.py @@ -196,7 +196,7 @@ def _db_writer_thread(self): messages.append(( msg.timestamp, msg.arbitration_id, - msg.id_type, + msg.is_extended_id, msg.is_remote_frame, msg.is_error_frame, msg.dlc, diff --git a/can/message.py b/can/message.py index 9bbf82a0e..80ce5d61c 100644 --- a/can/message.py +++ b/can/message.py @@ -3,6 +3,10 @@ """ This module contains the implementation of :class:`can.Message`. + +.. note:: + Could also use `@dataclass `__ + starting with Python 3.7. """ import logging @@ -12,7 +16,7 @@ class Message(object): """ The :class:`~can.Message` object is used to represent CAN messages for - both sending and receiving. + sending, receiving and other purposes like converting between. Messages can use extended identifiers, be remote or error frames, contain data and can be associated to a channel. @@ -29,18 +33,18 @@ class Message(object): """ - def __init__(self, timestamp=0.0, is_remote_frame=False, extended_id=True, - is_error_frame=False, arbitration_id=0, dlc=None, data=None, - is_fd=False, bitrate_switch=False, error_state_indicator=False, - channel=None): + def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, + is_remote_frame=False, is_error_frame=False, channel=None, + dlc=None, data=None, + is_fd=False, bitrate_switch=False, error_state_indicator=False,): self.timestamp = timestamp - self.id_type = extended_id + self.arbitration_id = arbitration_id + self.id_type = extended_id # deprecated self.is_extended_id = extended_id self.is_remote_frame = is_remote_frame self.is_error_frame = is_error_frame - self.arbitration_id = arbitration_id self.channel = channel self.is_fd = is_fd @@ -63,6 +67,7 @@ def __init__(self, timestamp=0.0, is_remote_frame=False, extended_id=True, else: self.dlc = dlc + # TODO: this shall only be checked when asked for if is_fd and self.dlc > 64: logger.warning("data link count was %d but it should be less than or equal to 64", self.dlc) if not is_fd and self.dlc > 8: @@ -70,7 +75,7 @@ def __init__(self, timestamp=0.0, is_remote_frame=False, extended_id=True, def __str__(self): field_strings = ["Timestamp: {0:>15.6f}".format(self.timestamp)] - if self.id_type: + if self.is_extended_id: # Extended arbitrationID arbitration_id_string = "ID: {0:08x}".format(self.arbitration_id) else: @@ -78,7 +83,7 @@ def __str__(self): field_strings.append(arbitration_id_string.rjust(12, " ")) flag_string = " ".join([ - "X" if self.id_type else "S", + "X" if self.is_extended_id else "S", "E" if self.is_error_frame else " ", "R" if self.is_remote_frame else " ", "F" if self.is_fd else " ", @@ -117,7 +122,7 @@ def __repr__(self): data = ["{:#02x}".format(byte) for byte in self.data] args = ["timestamp={}".format(self.timestamp), "is_remote_frame={}".format(self.is_remote_frame), - "extended_id={}".format(self.id_type), + "extended_id={}".format(self.is_extended_id), "is_error_frame={}".format(self.is_error_frame), "arbitration_id={:#x}".format(self.arbitration_id), "dlc={}".format(self.dlc), @@ -135,7 +140,7 @@ def __eq__(self, other): return ( self.arbitration_id == other.arbitration_id and #self.timestamp == other.timestamp and # allow the timestamp to differ - self.id_type == other.id_type and + self.is_extended_id == other.is_extended_id and self.dlc == other.dlc and self.data == other.data and self.is_remote_frame == other.is_remote_frame and @@ -156,7 +161,7 @@ def __hash__(self): return hash(( self.arbitration_id, # self.timestamp # excluded, like in self.__eq__(self, other) - self.id_type, + self.is_extended_id, self.dlc, self.data, self.is_fd, diff --git a/test/back2back_test.py b/test/back2back_test.py index 5d0034330..25629bc0e 100644 --- a/test/back2back_test.py +++ b/test/back2back_test.py @@ -53,7 +53,7 @@ def _check_received_message(self, recv_msg, sent_msg): self.assertIsNotNone(recv_msg, "No message was received on %s" % self.INTERFACE_2) self.assertEqual(recv_msg.arbitration_id, sent_msg.arbitration_id) - self.assertEqual(recv_msg.id_type, sent_msg.id_type) + self.assertEqual(recv_msg.is_extended_id, sent_msg.is_extended_id) self.assertEqual(recv_msg.is_remote_frame, sent_msg.is_remote_frame) self.assertEqual(recv_msg.is_error_frame, sent_msg.is_error_frame) self.assertEqual(recv_msg.is_fd, sent_msg.is_fd) diff --git a/test/network_test.py b/test/network_test.py index 830adceca..cf5acca76 100644 --- a/test/network_test.py +++ b/test/network_test.py @@ -101,7 +101,7 @@ def testProducerConsumer(self): self.assertIsNotNone(msg, "Didn't receive a message") #logging.debug("Received message {} with data: {}".format(i, msg.data)) - self.assertEqual(msg.id_type, self.extended_flags[i]) + self.assertEqual(msg.is_extended_id, self.extended_flags[i]) if not msg.is_remote_frame: self.assertEqual(msg.data, self.data[i]) self.assertEqual(msg.arbitration_id, self.ids[i]) diff --git a/test/test_kvaser.py b/test/test_kvaser.py index 229381934..173b80d48 100644 --- a/test/test_kvaser.py +++ b/test/test_kvaser.py @@ -136,7 +136,7 @@ def test_recv_extended(self): msg = self.bus.recv() self.assertEqual(msg.arbitration_id, 0xc0ffef) self.assertEqual(msg.dlc, 8) - self.assertEqual(msg.id_type, True) + self.assertEqual(msg.is_extended_id, True) self.assertSequenceEqual(msg.data, self.msg_in_cue.data) self.assertTrue(now - 1 < msg.timestamp < now + 1) @@ -149,7 +149,7 @@ def test_recv_standard(self): msg = self.bus.recv() self.assertEqual(msg.arbitration_id, 0x123) self.assertEqual(msg.dlc, 2) - self.assertEqual(msg.id_type, False) + self.assertEqual(msg.is_extended_id, False) self.assertSequenceEqual(msg.data, [100, 101]) def test_available_configs(self): @@ -178,7 +178,7 @@ def canReadWait(self, handle, arb_id, data, dlc, flags, timestamp, timeout): dlc._obj.value = self.msg_in_cue.dlc data._obj.raw = self.msg_in_cue.data flags_temp = 0 - if self.msg_in_cue.id_type: + if self.msg_in_cue.is_extended_id: flags_temp |= constants.canMSG_EXT else: flags_temp |= constants.canMSG_STD From b3fd52f014c7e4fde2e8795e570fa7fd57a7cc19 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Thu, 6 Sep 2018 23:02:11 +0200 Subject: [PATCH 02/32] better documentation --- doc/listeners.rst | 11 +++++++++++ doc/message.rst | 5 +++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/doc/listeners.rst b/doc/listeners.rst index b807cc7a7..1d0ec1dd4 100644 --- a/doc/listeners.rst +++ b/doc/listeners.rst @@ -19,6 +19,17 @@ the CAN bus. .. autoclass:: can.Listener :members: +There are some listeners that already ship toghether with `python-can` +and are listed below. +Some of them allow messages to be written to files, and the corresponing file +readers are also documented here. + +.. warning :: + + Please note that writing and the reading a message might not always yield a + completely unchanged message agian, since some properties are not (yet) + supported by some file formats. + BufferedReader -------------- diff --git a/doc/message.rst b/doc/message.rst index cd350d9f1..a6ed6b46f 100644 --- a/doc/message.rst +++ b/doc/message.rst @@ -30,7 +30,7 @@ Message The frame identifier used for arbitration on the bus. The arbitration ID can take an int between 0 and the - maximum value allowed depending on the is_extended_id flag + maximum value allowed depending on the ``is_extended_id`` flag (either 2\ :sup:`11` - 1 for 11-bit IDs, or 2\ :sup:`29` - 1 for 29-bit identifiers). @@ -63,7 +63,7 @@ Message :type: int - The :abbr:`DLC (Data Link Count)` parameter of a CAN message is an integer + The :abbr:`DLC (Data Length Code)` parameter of a CAN message is an integer between 0 and 8 representing the frame payload length. In the case of a CAN FD message, this indicates the data length in @@ -96,6 +96,7 @@ Message Previously this was exposed as `id_type`. + Please use `is_extended_id` from now on. .. attribute:: is_error_frame From 00903f8fc98755dcdf174700651ad659f6ae6cd3 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Thu, 6 Sep 2018 23:40:31 +0200 Subject: [PATCH 03/32] document check option --- can/message.py | 59 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/can/message.py b/can/message.py index 80ce5d61c..7f9a1769e 100644 --- a/can/message.py +++ b/can/message.py @@ -5,38 +5,43 @@ This module contains the implementation of :class:`can.Message`. .. note:: - Could also use `@dataclass `__ + Could use `@dataclass `__ starting with Python 3.7. """ import logging -logger = logging.getLogger(__name__) class Message(object): """ The :class:`~can.Message` object is used to represent CAN messages for - sending, receiving and other purposes like converting between. + sending, receiving and other purposes like converting between different + logging formats. Messages can use extended identifiers, be remote or error frames, contain data and can be associated to a channel. - When testing for equality of the messages, the timestamp and the channel - is not used for comparing. - - .. note:: - - This class does not strictly check the input. Thus, the caller must - prevent the creation of invalid messages. Possible problems include - the `dlc` field not matching the length of `data` or creating a message - with both `is_remote_frame` and `is_error_frame` set to True. - + When testing for equality of messages, the timestamp and the channel + are not used for comparing. """ def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, is_remote_frame=False, is_error_frame=False, channel=None, dlc=None, data=None, - is_fd=False, bitrate_switch=False, error_state_indicator=False,): + is_fd=False, bitrate_switch=False, error_state_indicator=False, + check=False): + """ + To create a message object, simply provide any of the below attributes + toghether with additional parameters as keyword arguments to the constructor. + + :param bool check: By default, the constructor of this class does not strictly check the input. + Thus, the caller must prevent the creation of invalid messages or + set this parameter to `True`, to raise an Error on invalid inputs. + Possible problems include the `dlc` field not matching the length of `data` + or creating a message with both `is_remote_frame` and `is_error_frame` set to `True`. + + :raises ValueError: iff `check` is set to `True` and one or more arguments were invalid + """ self.timestamp = timestamp self.arbitration_id = arbitration_id @@ -67,11 +72,8 @@ def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, else: self.dlc = dlc - # TODO: this shall only be checked when asked for - if is_fd and self.dlc > 64: - logger.warning("data link count was %d but it should be less than or equal to 64", self.dlc) - if not is_fd and self.dlc > 8: - logger.warning("data link count was %d but it should be less than or equal to 8", self.dlc) + if check: + self._check() def __str__(self): field_strings = ["Timestamp: {0:>15.6f}".format(self.timestamp)] @@ -171,4 +173,21 @@ def __hash__(self): )) def __format__(self, format_spec): - return self.__str__() + if not format_spec: + return self.__str__() + else: + raise ValueError('non empty format_specs are not supported') + + def __bytes__(self): + return bytes(self.data) + + def _check(self): + """Checks if the message parameters are valid. + + :raises ValueError: iff one or more attributes are invalid + """ + # TODO add checks + if is_fd and self.dlc > 64: + logger.warning("data link count was %d but it should be less than or equal to 64", self.dlc) + if not is_fd and self.dlc > 8: + logger.warning("data link count was %d but it should be less than or equal to 8", self.dlc) From 6204cda07888e1235aae01f3d5579a0540437f31 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Fri, 7 Sep 2018 00:14:34 +0200 Subject: [PATCH 04/32] fixes for the Message class --- can/message.py | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/can/message.py b/can/message.py index 7f9a1769e..e05690275 100644 --- a/can/message.py +++ b/can/message.py @@ -78,7 +78,6 @@ def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, def __str__(self): field_strings = ["Timestamp: {0:>15.6f}".format(self.timestamp)] if self.is_extended_id: - # Extended arbitrationID arbitration_id_string = "ID: {0:08x}".format(self.arbitration_id) else: arbitration_id_string = "ID: {0:04x}".format(self.arbitration_id) @@ -89,6 +88,8 @@ def __str__(self): "E" if self.is_error_frame else " ", "R" if self.is_remote_frame else " ", "F" if self.is_fd else " ", + "BS" if self.bitrate_switch else " ", + "EI" if self.error_state_indicator else " " ]) field_strings.append(flag_string) @@ -109,6 +110,9 @@ def __str__(self): except UnicodeError: pass + if self.channel is not None: + field_strings.append("Channel: {}".format(self.channel)) + return " ".join(field_strings).strip() def __len__(self): @@ -121,34 +125,40 @@ def __nonzero__(self): return self.__bool__() def __repr__(self): - data = ["{:#02x}".format(byte) for byte in self.data] args = ["timestamp={}".format(self.timestamp), - "is_remote_frame={}".format(self.is_remote_frame), - "extended_id={}".format(self.is_extended_id), - "is_error_frame={}".format(self.is_error_frame), "arbitration_id={:#x}".format(self.arbitration_id), - "dlc={}".format(self.dlc), - "data=[{}]".format(", ".join(data))] + "extended_id={}".format(self.is_extended_id), + "is_remote_frame={}".format(self.is_remote_frame), + "is_error_frame={}".format(self.is_error_frame)] + if self.channel is not None: - args.append("channel={!r}".format(self.channel)) + args.append("channel={!r}".format(self.channel)) + + data = ["{:#02x}".format(byte) for byte in self.data] + args += ["dlc={}".format(self.dlc), + "data=[{}]".format(", ".join(data))] + if self.is_fd: args.append("is_fd=True") args.append("bitrate_switch={}".format(self.bitrate_switch)) args.append("error_state_indicator={}".format(self.error_state_indicator)) + return "can.Message({})".format(", ".join(args)) def __eq__(self, other): if isinstance(other, self.__class__): return ( - self.arbitration_id == other.arbitration_id and #self.timestamp == other.timestamp and # allow the timestamp to differ + self.arbitration_id == other.arbitration_id and self.is_extended_id == other.is_extended_id and - self.dlc == other.dlc and - self.data == other.data and self.is_remote_frame == other.is_remote_frame and self.is_error_frame == other.is_error_frame and + self.channel == other.channel and + self.dlc == other.dlc and + self.data == other.data and self.is_fd == other.is_fd and - self.bitrate_switch == other.bitrate_switch + self.bitrate_switch == other.bitrate_switch and + self.error_state_indicator == other.error_state_indicator ) else: return NotImplemented From 965bbbbb74dfd20b1257cde283374f3475c56040 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Fri, 7 Sep 2018 01:10:26 +0200 Subject: [PATCH 05/32] run test on CI for logformats_test.py --- test/logformats_test.py | 67 ++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 15 deletions(-) diff --git a/test/logformats_test.py b/test/logformats_test.py index 2a315352d..09f750ea8 100644 --- a/test/logformats_test.py +++ b/test/logformats_test.py @@ -59,22 +59,28 @@ def _setup_instance(self): def _setup_instance_helper(self, writer_constructor, reader_constructor, binary_file=False, - check_remote_frames=True, check_error_frames=True, check_comments=False, - test_append=False, round_timestamps=False): + check_remote_frames=True, check_error_frames=True, check_fd=True, + check_comments=False, test_append=False, + round_timestamps=False, preserves_timestamp_exact=True, preserves_channel=True): """ :param Callable writer_constructor: the constructor of the writer class :param Callable reader_constructor: the constructor of the reader class + :param bool binary_file: if True, opens files in binary and not in text mode :param bool check_remote_frames: if True, also tests remote frames :param bool check_error_frames: if True, also tests error frames + :param bool check_fd: if True, also tests CAN FD frames :param bool check_comments: if True, also inserts comments at some locations and checks if they are contained anywhere literally in the resulting file. The locations as selected randomly but deterministically, which makes the test reproducible. :param bool test_append: tests the writer in append mode as well - :param bool round_timestamps: if True, rounds timestamps using :meth:`~builtin.round` - before comparing the read messages/events + :param bool round_timestamps: if True, rounds timestamps using :meth:`~builtin.round` + to integers before comparing + :param bool preserves_timestamp_exact: if True, checks that timestamps match exactly + in this case, no rounding is performed + :param bool preserves_channel: if True, checks that the channel attribute is preserved """ # get all test messages self.original_messages = TEST_MESSAGES_BASE @@ -82,6 +88,8 @@ def _setup_instance_helper(self, self.original_messages += TEST_MESSAGES_REMOTE_FRAMES if check_error_frames: self.original_messages += TEST_MESSAGES_ERROR_FRAMES + if check_fd: + self.original_messages += [] # TODO: add TEST_MESSAGES_CAN_FD # sort them so that for example ASCWriter does not "fix" any messages with timestamp 0.0 self.original_messages = sort_messages(self.original_messages) @@ -101,7 +109,10 @@ def _setup_instance_helper(self, self.reader_constructor = reader_constructor self.binary_file = binary_file self.test_append_enabled = test_append + self.round_timestamps = round_timestamps + self.preserves_timestamp_exact = preserves_timestamp_exact + self.preserves_channel = preserves_channel def setUp(self): with tempfile.NamedTemporaryFile('w+', delete=False) as test_file: @@ -284,19 +295,39 @@ def assertMessagesEqual(self, read_messages): """ for index, (original, read) in enumerate(zip(self.original_messages, read_messages)): try: - # check everything except the timestamp - self.assertEqual(original, read, "messages are not equal at index #{}".format(index)) - # check the timestamp - if self.round_timestamps: - original.timestamp = round(original.timestamp) - read.timestamp = round(read.timestamp) - self.assertAlmostEqual(read.timestamp, original.timestamp, places=6, - msg="message timestamps are not almost_equal at index #{} ({!r} !~= {!r})" - .format(index, original.timestamp, read.timestamp)) - except: + self.assertMessageEqual(original, read) + except AssertionError as e: print("Comparing: original message: {!r}".format(original)) print(" read message: {!r}".format(read)) - raise + self.fail("messages are not equal at index #{}:\n{}".format(index, e)) + + def assertMessageEqual(self, message_1, message_2): + """ + Checks that two messages are equal, according to the current rules. + """ + # check the timestamp + if self.preserves_timestamp_exact: + self.assertEqual(message_1.timestamp, message_2.timestamp) + else: + t1 = round(message_1.timestamp) if self.round_timestamps else message_1.timestamp + t2 = round(message_2.timestamp) if self.round_timestamps else message_2.timestamp + self.assertAlmostEqual(message_2.timestamp, message_1.timestamp, places=6, + msg="message timestamps are not almost_equal ({!r} !~= {!r}{})" + .format(message_1.timestamp, message_2.timestamp, + "; rounded" if self.round_timestamps else "")) + + # check the rest + self.assertEqual(message_1.arbitration_id, message_2.arbitration_id) + self.assertEqual(message_1.is_extended_id, message_2.is_extended_id) + self.assertEqual(message_1.is_remote_frame, message_2.is_remote_frame) + self.assertEqual(message_1.is_error_frame, message_2.is_error_frame) + if self.preserves_channel: + self.assertEqual(message_1.channel, message_2.channel) + self.assertEqual(message_1.dlc, message_2.dlc) + self.assertEqual(message_1.data, message_2.data) + self.assertEqual(message_1.is_fd, message_2.is_fd) + self.assertEqual(message_1.bitrate_switch, message_2.bitrate_switch) + self.assertEqual(message_1.error_state_indicator, message_2.error_state_indicator) def assertIncludesComments(self, filename): """ @@ -321,6 +352,7 @@ class TestAscFileFormat(ReaderWriterTest): def _setup_instance(self): super(TestAscFileFormat, self)._setup_instance_helper( can.ASCWriter, can.ASCReader, + check_fd=False, check_comments=True, round_timestamps=True ) @@ -334,6 +366,7 @@ def _setup_instance(self): super(TestBlfFileFormat, self)._setup_instance_helper( can.BLFWriter, can.BLFReader, binary_file=True, + check_fd=False, check_comments=False ) @@ -364,6 +397,7 @@ class TestCanutilsFileFormat(ReaderWriterTest): def _setup_instance(self): super(TestCanutilsFileFormat, self)._setup_instance_helper( can.CanutilsLogWriter, can.CanutilsLogReader, + check_fd=False, test_append=True, check_comments=False ) @@ -376,6 +410,7 @@ class TestCsvFileFormat(ReaderWriterTest): def _setup_instance(self): super(TestCsvFileFormat, self)._setup_instance_helper( can.CSVWriter, can.CSVReader, + check_fd=False, test_append=True, check_comments=False ) @@ -388,6 +423,7 @@ class TestSqliteDatabaseFormat(ReaderWriterTest): def _setup_instance(self): super(TestSqliteDatabaseFormat, self)._setup_instance_helper( can.SqliteWriter, can.SqliteReader, + check_fd=False, test_append=True, check_comments=False ) @@ -423,6 +459,7 @@ def test_read_all(self): class TestPrinter(unittest.TestCase): """Tests that can.Printer does not crash""" + # TODO add CAN FD messages messages = TEST_MESSAGES_BASE + TEST_MESSAGES_REMOTE_FRAMES + TEST_MESSAGES_ERROR_FRAMES def test_not_crashes_with_stdout(self): From 1797e436bc657bb187d96b130e7826f21fdfce0f Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Fri, 7 Sep 2018 01:16:09 +0200 Subject: [PATCH 06/32] added channel to docs --- doc/message.rst | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/doc/message.rst b/doc/message.rst index a6ed6b46f..e7e9fb353 100644 --- a/doc/message.rst +++ b/doc/message.rst @@ -23,6 +23,15 @@ Message 2.0B) in length, and ``python-can`` exposes this difference with the :attr:`~can.Message.is_extended_id` attribute. + .. attribute:: timestamp + + :type: float + + The timestamp field in a CAN message is a floating point number representing when + the message was received since the epoch in seconds. Where possible this will be + timestamped in hardware. + + .. attribute:: arbitration_id :type: int @@ -82,6 +91,12 @@ Message represents the amount of data contained in the message, in remote frames it represents the amount of data being requested. + .. attribute:: channel + + :type: str or int or None + + This might store the channel from which the message came. + .. attribute:: is_extended_id @@ -142,15 +157,6 @@ Message If this is a CAN FD message, this indicates an error active state. - .. attribute:: timestamp - - :type: float - - The timestamp field in a CAN message is a floating point number representing when - the message was received since the epoch in seconds. Where possible this will be - timestamped in hardware. - - .. method:: __str__ A string representation of a CAN message: From e76f0173bb00035bd7280bfeb516538e25f84a1d Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Fri, 7 Sep 2018 01:16:46 +0200 Subject: [PATCH 07/32] add example messages with channel attribute set --- test/data/example_data.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/data/example_data.py b/test/data/example_data.py index e1a446384..1d36a1c8f 100644 --- a/test/data/example_data.py +++ b/test/data/example_data.py @@ -58,6 +58,14 @@ def sort_messages(messages): # empty data data=[0xFF, 0xFE, 0xFD], ), + Message( + # with channel as integer + channel=42, + ), + Message( + # with channel as string + channel="awesome_channel", + ), Message( arbitration_id=0xABCDEF, extended_id=True, timestamp=TEST_TIME, From 4e238083208cdfb9b410250f9c653c0fbdef46a6 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sat, 8 Sep 2018 15:35:22 +0200 Subject: [PATCH 08/32] test codecov --- .codecov.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.codecov.yml b/.codecov.yml index d533fd085..6e2bb4be2 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -7,7 +7,10 @@ coverage: round: down range: 50...100 status: - # pull-requests only + project: + default: + # coverage may fall by <1.0% and still be considered "passing" + threshold: 1.0% patch: default: # coverage may fall by <1.0% and still be considered "passing" From f71fd5696a12992c91b946916ebd2ac4488730ab Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 10 Sep 2018 15:39:19 +0200 Subject: [PATCH 09/32] change logformat tests --- test/data/example_data.py | 8 ++++++++ test/logformats_test.py | 29 +++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/test/data/example_data.py b/test/data/example_data.py index 1d36a1c8f..d4b1b877c 100644 --- a/test/data/example_data.py +++ b/test/data/example_data.py @@ -58,10 +58,18 @@ def sort_messages(messages): # empty data data=[0xFF, 0xFE, 0xFD], ), + Message( + # with channel as integer + channel=0, + ), Message( # with channel as integer channel=42, ), + Message( + # with channel as string + channel="vcan0", + ), Message( # with channel as string channel="awesome_channel", diff --git a/test/logformats_test.py b/test/logformats_test.py index 09f750ea8..03aad2c86 100644 --- a/test/logformats_test.py +++ b/test/logformats_test.py @@ -10,6 +10,7 @@ different writer/reader pairs - e.g., some don't handle error frames and comments. +TODO: correctly set preserves_channel and adds_default_channel TODO: implement CAN FD support testing """ @@ -61,7 +62,8 @@ def _setup_instance_helper(self, writer_constructor, reader_constructor, binary_file=False, check_remote_frames=True, check_error_frames=True, check_fd=True, check_comments=False, test_append=False, - round_timestamps=False, preserves_timestamp_exact=True, preserves_channel=True): + round_timestamps=False, preserves_timestamp_exact=True, + preserves_channel=True, adds_default_channel=None): """ :param Callable writer_constructor: the constructor of the writer class :param Callable reader_constructor: the constructor of the reader class @@ -81,6 +83,8 @@ def _setup_instance_helper(self, :param bool preserves_timestamp_exact: if True, checks that timestamps match exactly in this case, no rounding is performed :param bool preserves_channel: if True, checks that the channel attribute is preserved + :param any adds_default_channel: sets this as the channel when not other channel was given + ignored, if *preserves_channel* is True """ # get all test messages self.original_messages = TEST_MESSAGES_BASE @@ -113,6 +117,7 @@ def _setup_instance_helper(self, self.round_timestamps = round_timestamps self.preserves_timestamp_exact = preserves_timestamp_exact self.preserves_channel = preserves_channel + self.adds_default_channel = adds_default_channel def setUp(self): with tempfile.NamedTemporaryFile('w+', delete=False) as test_file: @@ -305,6 +310,10 @@ def assertMessageEqual(self, message_1, message_2): """ Checks that two messages are equal, according to the current rules. """ + # try conventional + if message_1 == message_2: + return + # check the timestamp if self.preserves_timestamp_exact: self.assertEqual(message_1.timestamp, message_2.timestamp) @@ -323,6 +332,8 @@ def assertMessageEqual(self, message_1, message_2): self.assertEqual(message_1.is_error_frame, message_2.is_error_frame) if self.preserves_channel: self.assertEqual(message_1.channel, message_2.channel) + else: + self.assertEqual(message_2.channel, self.adds_default_channel) self.assertEqual(message_1.dlc, message_2.dlc) self.assertEqual(message_1.data, message_2.data) self.assertEqual(message_1.is_fd, message_2.is_fd) @@ -353,7 +364,9 @@ def _setup_instance(self): super(TestAscFileFormat, self)._setup_instance_helper( can.ASCWriter, can.ASCReader, check_fd=False, - check_comments=True, round_timestamps=True + check_comments=True, + round_timestamps=True, + preserves_channel=False, adds_default_channel=0 ) @@ -367,7 +380,8 @@ def _setup_instance(self): can.BLFWriter, can.BLFReader, binary_file=True, check_fd=False, - check_comments=False + check_comments=False, + preserves_channel=False, adds_default_channel=0 ) def test_read_known_file(self): @@ -398,7 +412,8 @@ def _setup_instance(self): super(TestCanutilsFileFormat, self)._setup_instance_helper( can.CanutilsLogWriter, can.CanutilsLogReader, check_fd=False, - test_append=True, check_comments=False + test_append=True, check_comments=False, + preserves_channel=False, adds_default_channel='vcan0' ) @@ -411,7 +426,8 @@ def _setup_instance(self): super(TestCsvFileFormat, self)._setup_instance_helper( can.CSVWriter, can.CSVReader, check_fd=False, - test_append=True, check_comments=False + test_append=True, check_comments=False, + preserves_channel=False, adds_default_channel=None ) @@ -424,7 +440,8 @@ def _setup_instance(self): super(TestSqliteDatabaseFormat, self)._setup_instance_helper( can.SqliteWriter, can.SqliteReader, check_fd=False, - test_append=True, check_comments=False + test_append=True, check_comments=False, + preserves_channel=False, adds_default_channel=None ) @unittest.skip("not implemented") From a20f4f724fa0b3d4dc627ba00f7d59130a0d3822 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 10 Sep 2018 15:42:02 +0200 Subject: [PATCH 10/32] nicer __repr__ --- can/message.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/can/message.py b/can/message.py index e05690275..45574f5ff 100644 --- a/can/message.py +++ b/can/message.py @@ -127,9 +127,13 @@ def __nonzero__(self): def __repr__(self): args = ["timestamp={}".format(self.timestamp), "arbitration_id={:#x}".format(self.arbitration_id), - "extended_id={}".format(self.is_extended_id), - "is_remote_frame={}".format(self.is_remote_frame), - "is_error_frame={}".format(self.is_error_frame)] + "extended_id={}".format(self.is_extended_id)] + + if self.is_remote_frame: + args.append("is_remote_frame={}".format(self.is_remote_frame)) + + if self.is_error_frame: + args.append("is_error_frame={}".format(self.is_error_frame)) if self.channel is not None: args.append("channel={!r}".format(self.channel)) From 5217880d017eff0237923cff34ba8b5e846f0013 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 10 Sep 2018 16:11:34 +0200 Subject: [PATCH 11/32] add __slots__, correct __hash__. deprecate id_type --- can/message.py | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/can/message.py b/can/message.py index 45574f5ff..44734b494 100644 --- a/can/message.py +++ b/can/message.py @@ -25,7 +25,31 @@ class Message(object): are not used for comparing. """ - def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, + __slots__ = ( + "timestamp", + "arbitration_id", + "is_extended_id", + "is_remote_frame", + "is_error_frame", + "channel", + "dlc", + "data", + "is_fd", + "bitrate_switch", + "error_state_indicator", + ) + + @property + def id_type(self): + warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning) + return self.is_extended_id + + @id_type.setter + def id_type(self, value): + warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning) + self.is_extended_id = value + + def __init__(self, timestamp=0.0, arbitration_id=0, is_extended_id=True, is_remote_frame=False, is_error_frame=False, channel=None, dlc=None, data=None, is_fd=False, bitrate_switch=False, error_state_indicator=False, @@ -45,7 +69,6 @@ def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, self.timestamp = timestamp self.arbitration_id = arbitration_id - self.id_type = extended_id # deprecated self.is_extended_id = extended_id self.is_remote_frame = is_remote_frame @@ -175,15 +198,17 @@ def __ne__(self, other): def __hash__(self): return hash(( - self.arbitration_id, # self.timestamp # excluded, like in self.__eq__(self, other) + self.arbitration_id, self.is_extended_id, + self.is_remote_frame, + self.is_error_frame, + self.channel, self.dlc, self.data, self.is_fd, self.bitrate_switch, - self.is_remote_frame, - self.is_error_frame + self.error_state_indicator )) def __format__(self, format_spec): From edb2b6e7044c9e86a3b3eba4d06d13dd7fd28044 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 10 Sep 2018 16:14:37 +0200 Subject: [PATCH 12/32] make timestamp comparisoun a bit more forgiving --- test/logformats_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/logformats_test.py b/test/logformats_test.py index 03aad2c86..cfaf2df25 100644 --- a/test/logformats_test.py +++ b/test/logformats_test.py @@ -320,7 +320,7 @@ def assertMessageEqual(self, message_1, message_2): else: t1 = round(message_1.timestamp) if self.round_timestamps else message_1.timestamp t2 = round(message_2.timestamp) if self.round_timestamps else message_2.timestamp - self.assertAlmostEqual(message_2.timestamp, message_1.timestamp, places=6, + self.assertAlmostEqual(message_2.timestamp, message_1.timestamp, delta=0.0001, msg="message timestamps are not almost_equal ({!r} !~= {!r}{})" .format(message_1.timestamp, message_2.timestamp, "; rounded" if self.round_timestamps else "")) From ec9f9ed12806c959b89d2803ea4b44496a35bf50 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 10 Sep 2018 16:16:54 +0200 Subject: [PATCH 13/32] use extended_id in constructor --- can/message.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/message.py b/can/message.py index 44734b494..012368c52 100644 --- a/can/message.py +++ b/can/message.py @@ -49,7 +49,7 @@ def id_type(self, value): warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning) self.is_extended_id = value - def __init__(self, timestamp=0.0, arbitration_id=0, is_extended_id=True, + def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, is_remote_frame=False, is_error_frame=False, channel=None, dlc=None, data=None, is_fd=False, bitrate_switch=False, error_state_indicator=False, From 1a76644941d92cd6e4e986404287bafa1b943747 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 10 Sep 2018 16:29:07 +0200 Subject: [PATCH 14/32] fix TestBlfFileFormat test --- can/message.py | 2 +- test/logformats_test.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/can/message.py b/can/message.py index 012368c52..42df5cda6 100644 --- a/can/message.py +++ b/can/message.py @@ -9,7 +9,7 @@ starting with Python 3.7. """ -import logging +import warnings class Message(object): diff --git a/test/logformats_test.py b/test/logformats_test.py index cfaf2df25..fdec756ba 100644 --- a/test/logformats_test.py +++ b/test/logformats_test.py @@ -320,7 +320,7 @@ def assertMessageEqual(self, message_1, message_2): else: t1 = round(message_1.timestamp) if self.round_timestamps else message_1.timestamp t2 = round(message_2.timestamp) if self.round_timestamps else message_2.timestamp - self.assertAlmostEqual(message_2.timestamp, message_1.timestamp, delta=0.0001, + self.assertAlmostEqual(message_2.timestamp, message_1.timestamp, delta=1e-6, msg="message timestamps are not almost_equal ({!r} !~= {!r}{})" .format(message_1.timestamp, message_2.timestamp, "; rounded" if self.round_timestamps else "")) @@ -381,6 +381,7 @@ def _setup_instance(self): binary_file=True, check_fd=False, check_comments=False, + preserves_timestamp_exact=False, preserves_channel=False, adds_default_channel=0 ) From 26b2406a445ecf40ffe087e8109e19f89a919c90 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 10 Sep 2018 20:58:37 +0200 Subject: [PATCH 15/32] implemented Message._check --- can/message.py | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/can/message.py b/can/message.py index 42df5cda6..004b81d6b 100644 --- a/can/message.py +++ b/can/message.py @@ -56,7 +56,7 @@ def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, check=False): """ To create a message object, simply provide any of the below attributes - toghether with additional parameters as keyword arguments to the constructor. + together with additional parameters as keyword arguments to the constructor. :param bool check: By default, the constructor of this class does not strictly check the input. Thus, the caller must prevent the creation of invalid messages or @@ -215,18 +215,39 @@ def __format__(self, format_spec): if not format_spec: return self.__str__() else: - raise ValueError('non empty format_specs are not supported') + raise ValueError("non empty format_specs are not supported") def __bytes__(self): return bytes(self.data) def _check(self): - """Checks if the message parameters are valid. + """Checks if the message parameters are valid. Does assume that + the types are already correct. - :raises ValueError: iff one or more attributes are invalid + :raises AssertionError: iff one or more attributes are invalid """ - # TODO add checks - if is_fd and self.dlc > 64: - logger.warning("data link count was %d but it should be less than or equal to 64", self.dlc) - if not is_fd and self.dlc > 8: - logger.warning("data link count was %d but it should be less than or equal to 8", self.dlc) + + assert 0.0 <= self.timestamp, "timestamp may not negative" + + assert not (self.is_remote_frame and self.is_error_frame), \ + "a message cannot be a remote and an error frame at the sane time" + + assert 0 <= self.arbitration_id, "IDs may not ne negative" + + if self.is_extended_id: + assert self.arbitration_id < 0x20000000, "Extended arbitration IDs must be less than 2**29" + else: + assert self.arbitration_id < 0x800, "Normal arbitration IDs must be less than 2**11" + + assert 0 <= self.dlc, "DLC may not be negative" + if self.is_fd: + assert self.dlc > 64, "DLC was {} but it should be less than or equal to 64 for CAN FD frames".format(self.dlc) + else: + assert self.dlc > 8, "DLC was {} but it should be less than or equal to 8 for normal CAN frames".format(self.dlc) + + if not self.is_remote_frame: + assert self.dlc == len(self.data), "the length of the DLC and the length of the data must match up" + + if not self.is_fd: + assert not self.bitrate_switch, "bitrate switch is only allowed for CAN FD frames" + assert not self.error_state_indicator, "error stat indicator is only allowed for CAN FD frames" From 52ab605166e057d3b7ae436d98b7a11bb7b8fb20 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 10 Sep 2018 21:10:24 +0200 Subject: [PATCH 16/32] adjust logformat test --- test/logformats_test.py | 50 +++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/test/logformats_test.py b/test/logformats_test.py index fdec756ba..664570af8 100644 --- a/test/logformats_test.py +++ b/test/logformats_test.py @@ -152,7 +152,7 @@ def test_path_like_explicit_stop(self): self.assertEqual(len(read_messages), len(self.original_messages), "the number of written messages does not match the number of read messages") - self.assertMessagesEqual(read_messages) + self.assertMessagesEqual(self.original_messages, read_messages) self.assertIncludesComments(self.test_file_name) def test_path_like_context_manager(self): @@ -179,7 +179,7 @@ def test_path_like_context_manager(self): self.assertEqual(len(read_messages), len(self.original_messages), "the number of written messages does not match the number of read messages") - self.assertMessagesEqual(read_messages) + self.assertMessagesEqual(self.original_messages, read_messages) self.assertIncludesComments(self.test_file_name) def test_file_like_explicit_stop(self): @@ -209,7 +209,7 @@ def test_file_like_explicit_stop(self): self.assertEqual(len(read_messages), len(self.original_messages), "the number of written messages does not match the number of read messages") - self.assertMessagesEqual(read_messages) + self.assertMessagesEqual(self.original_messages, read_messages) self.assertIncludesComments(self.test_file_name) def test_file_like_context_manager(self): @@ -238,7 +238,7 @@ def test_file_like_context_manager(self): self.assertEqual(len(read_messages), len(self.original_messages), "the number of written messages does not match the number of read messages") - self.assertMessagesEqual(read_messages) + self.assertMessagesEqual(self.original_messages, read_messages) self.assertIncludesComments(self.test_file_name) def test_append_mode(self): @@ -275,7 +275,7 @@ def test_append_mode(self): with self.reader_constructor(self.test_file_name) as reader: read_messages = list(reader) - self.assertMessagesEqual(read_messages) + self.assertMessagesEqual(self.original_messages, read_messages) def _write_all(self, writer): """Writes messages and insert comments here and there.""" @@ -294,16 +294,18 @@ def _ensure_fsync(self, io_handler): io_handler.file.flush() os.fsync(io_handler.file.fileno()) - def assertMessagesEqual(self, read_messages): + def assertMessagesEqual(self, messages_1, messages_2): """ Checks the order and content of the individual messages. """ - for index, (original, read) in enumerate(zip(self.original_messages, read_messages)): + self.assertEqual(len(messages_1), len(messages_2)) + + for index, (message_1, message_2) in enumerate(zip(messages_1, messages_2)): try: - self.assertMessageEqual(original, read) + self.assertMessageEqual(message_1, message_2) except AssertionError as e: - print("Comparing: original message: {!r}".format(original)) - print(" read message: {!r}".format(read)) + print("Comparing: message 1: {!r}".format(message_1)) + print(" message 2: {!r}".format(message_2)) self.fail("messages are not equal at index #{}:\n{}".format(index, e)) def assertMessageEqual(self, message_1, message_2): @@ -389,19 +391,19 @@ def test_read_known_file(self): logfile = os.path.join(os.path.dirname(__file__), "data", "logfile.blf") with can.BLFReader(logfile) as reader: messages = list(reader) - self.assertEqual(len(messages), 2) - self.assertEqual(messages[0], - can.Message( - extended_id=False, - arbitration_id=0x64, - data=[0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8])) - self.assertEqual(messages[0].channel, 0) - self.assertEqual(messages[1], - can.Message( - is_error_frame=True, - extended_id=True, - arbitration_id=0x1FFFFFFF)) - self.assertEqual(messages[1].channel, 0) + + expected = [ + can.Message( + extended_id=False, + arbitration_id=0x64, + data=[0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8]), + can.Message( + extended_id=True, + arbitration_id=0x1FFFFFFF, + is_error_frame=True,) + ] + + self.assertMessagesEqual(messages, expected) class TestCanutilsFileFormat(ReaderWriterTest): @@ -471,7 +473,7 @@ def test_read_all(self): self.assertEqual(len(read_messages), len(self.original_messages), "the number of written messages does not match the number of read messages") - self.assertMessagesEqual(read_messages) + self.assertMessagesEqual(self.original_messages, read_messages) class TestPrinter(unittest.TestCase): From 5d8866268d9a95fcdfcab26ec28d64161a75a3e3 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 16 Sep 2018 01:39:26 +0200 Subject: [PATCH 17/32] try to add warning & compat __dict__ --- can/message.py | 12 ++++++++++-- test/logformats_test.py | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/can/message.py b/can/message.py index 42df5cda6..637a366e5 100644 --- a/can/message.py +++ b/can/message.py @@ -25,7 +25,7 @@ class Message(object): are not used for comparing. """ - __slots__ = ( + __slots__ = [ "timestamp", "arbitration_id", "is_extended_id", @@ -37,7 +37,15 @@ class Message(object): "is_fd", "bitrate_switch", "error_state_indicator", - ) + "__weakref__ ", + "__dict__" # TODO keep this for a version, to not break old code + ] + + def __getattr__(self, key, value): + # TODO keep this for a version, to not break old code + # called if the attribute was not found in __slots__ + warnings.warn("Custom attributes of messages are deprecated and will be removed in the next major version", DeprecationWarning) + return self.__dict__[key] @property def id_type(self): diff --git a/test/logformats_test.py b/test/logformats_test.py index fdec756ba..62b0ca63a 100644 --- a/test/logformats_test.py +++ b/test/logformats_test.py @@ -365,7 +365,7 @@ def _setup_instance(self): can.ASCWriter, can.ASCReader, check_fd=False, check_comments=True, - round_timestamps=True, + #round_timestamps=True, # TODO is this required? preserves_channel=False, adds_default_channel=0 ) From 5fb17752effc49544eec63a11025850d8c8f7ffd Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 16 Sep 2018 02:07:59 +0200 Subject: [PATCH 18/32] properly implement warning --- can/message.py | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/can/message.py b/can/message.py index 8d65e6650..6d8a34c8c 100644 --- a/can/message.py +++ b/can/message.py @@ -25,7 +25,7 @@ class Message(object): are not used for comparing. """ - __slots__ = [ + __slots__ = ( "timestamp", "arbitration_id", "is_extended_id", @@ -37,15 +37,28 @@ class Message(object): "is_fd", "bitrate_switch", "error_state_indicator", - "__weakref__ ", - "__dict__" # TODO keep this for a version, to not break old code - ] - - def __getattr__(self, key, value): - # TODO keep this for a version, to not break old code - # called if the attribute was not found in __slots__ - warnings.warn("Custom attributes of messages are deprecated and will be removed in the next major version", DeprecationWarning) - return self.__dict__[key] + "__weakref__", # support weak references to messages + "_dict" # see __getattr__ + ) + + 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 the next major version after 2.3 + # this method is only called if the attribute was not found elsewhere, like in __slots__ + try: + warnings.warn("Custom attributes of messages are deprecated and will be removed in the next major version", DeprecationWarning) + return self._dict[key] + except KeyError: + raise AttributeError("'message' object has no attribute '{}'".format(key)) + + def __setattr__(self, key, value): + # see __getattr__ + try: + super(Message, self).__setattr__(key, value) + except AttributeError: + warnings.warn("Custom attributes of messages are deprecated and will be removed in the next major version", DeprecationWarning) + self._dict[key] = value @property def id_type(self): @@ -74,6 +87,7 @@ def __init__(self, timestamp=0.0, arbitration_id=0, extended_id=True, :raises ValueError: iff `check` is set to `True` and one or more arguments were invalid """ + self._dict = dict() # see __getattr__ self.timestamp = timestamp self.arbitration_id = arbitration_id From eee819f8e8200f11325d26539928b72c7d0d0697 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 16 Sep 2018 02:19:05 +0200 Subject: [PATCH 19/32] update deprecation notes --- can/CAN.py | 4 ++-- can/broadcastmanager.py | 2 +- can/message.py | 6 ++++-- doc/bcm.rst | 2 +- doc/development.rst | 11 ++++++----- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/can/CAN.py b/can/CAN.py index 7f8469aee..4f5b566e3 100644 --- a/can/CAN.py +++ b/can/CAN.py @@ -23,10 +23,10 @@ log = logging.getLogger('can') -# See #267 +# See #267. # Version 2.0 - 2.1: Log a Debug message # Version 2.2: Log a Warning # Version 2.3: Log an Error # Version 2.4: Remove the module -log.warning('Loading python-can via the old "CAN" API is deprecated since v2.0 an will get removed in v2.4. ' +log.error('Loading python-can via the old "CAN" API is deprecated since v2.0 an will get removed in v2.4. ' 'Please use `import can` instead.') diff --git a/can/broadcastmanager.py b/can/broadcastmanager.py index 5e9ff7118..2d8b07242 100644 --- a/can/broadcastmanager.py +++ b/can/broadcastmanager.py @@ -152,5 +152,5 @@ def send_periodic(bus, message, period, *args, **kwargs): :return: A started task instance """ log.warning("The function `can.send_periodic` is deprecated and will " + - "be removed in version 2.3. Please use `can.Bus.send_periodic` instead.") + "be removed in version 3.0. Please use `can.Bus.send_periodic` instead.") return bus.send_periodic(message, period, *args, **kwargs) diff --git a/can/message.py b/can/message.py index 6d8a34c8c..816685307 100644 --- a/can/message.py +++ b/can/message.py @@ -43,8 +43,8 @@ class Message(object): 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 the next major version after 2.3 + # this entire method (as well as the _dict attribute in __slots__ and the __setattr__ method) + # can be removed in 3.0 # this method is only called if the attribute was not found elsewhere, like in __slots__ try: warnings.warn("Custom attributes of messages are deprecated and will be removed in the next major version", DeprecationWarning) @@ -62,11 +62,13 @@ def __setattr__(self, key, value): @property def id_type(self): + # TODO remove in 3.0 warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning) return self.is_extended_id @id_type.setter def id_type(self, value): + # TODO remove in 3.0 warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning) self.is_extended_id = value diff --git a/doc/bcm.rst b/doc/bcm.rst index 0676a77eb..dd4e6fede 100644 --- a/doc/bcm.rst +++ b/doc/bcm.rst @@ -45,7 +45,7 @@ Functional API .. warning:: The functional API in :func:`can.broadcastmanager.send_periodic` is now deprecated - and will be removed in version 2.3. + and will be removed in version 3.0. Use the object oriented API via :meth:`can.BusABC.send_periodic` instead. .. autofunction:: can.broadcastmanager.send_periodic diff --git a/doc/development.rst b/doc/development.rst index 51924be16..9e7b9d91c 100644 --- a/doc/development.rst +++ b/doc/development.rst @@ -118,14 +118,15 @@ Creating a new Release - Release from the ``master`` branch. - Update the library version in ``__init__.py`` using `semantic versioning `__. +- Check if any deprecations are pending. - Run all tests and examples against available hardware. - Update `CONTRIBUTORS.txt` with any new contributors. - For larger changes update ``doc/history.rst``. - Sanity check that documentation has stayed inline with code. -- Create a temporary virtual environment. Run ``python setup.py install`` and ``python setup.py test`` -- Create and upload the distribution: ``python setup.py sdist bdist_wheel`` -- Sign the packages with gpg ``gpg --detach-sign -a dist/python_can-X.Y.Z-py3-none-any.whl`` -- Upload with twine ``twine upload dist/python-can-X.Y.Z*`` -- In a new virtual env check that the package can be installed with pip: ``pip install python-can==X.Y.Z`` +- Create a temporary virtual environment. Run ``python setup.py install`` and ``python setup.py test``. +- Create and upload the distribution: ``python setup.py sdist bdist_wheel``. +- Sign the packages with gpg ``gpg --detach-sign -a dist/python_can-X.Y.Z-py3-none-any.whl``. +- Upload with twine ``twine upload dist/python-can-X.Y.Z*``. +- In a new virtual env check that the package can be installed with pip: ``pip install python-can==X.Y.Z``. - Create a new tag in the repository. - Check the release on PyPi, Read the Docs and GitHub. From 00f6b92e516cb5a93d7ba537f4bd4e73ae0d5346 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 16 Sep 2018 02:26:27 +0200 Subject: [PATCH 20/32] docs & remove undocumented setting of flags in kvaser backend --- can/interfaces/kvaser/canlib.py | 2 -- can/message.py | 6 ++++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/can/interfaces/kvaser/canlib.py b/can/interfaces/kvaser/canlib.py index 99850a2ec..00ef6cd6d 100644 --- a/can/interfaces/kvaser/canlib.py +++ b/can/interfaces/kvaser/canlib.py @@ -517,8 +517,6 @@ def _recv_internal(self, timeout=None): error_state_indicator=error_state_indicator, channel=self.channel, timestamp=msg_timestamp + self._timestamp_offset) - rx_msg.flags = flags - rx_msg.raw_timestamp = msg_timestamp #log.debug('Got message: %s' % rx_msg) return rx_msg, self._is_filtered else: diff --git a/can/message.py b/can/message.py index 816685307..2433b66f2 100644 --- a/can/message.py +++ b/can/message.py @@ -21,8 +21,10 @@ class Message(object): Messages can use extended identifiers, be remote or error frames, contain data and can be associated to a channel. - When testing for equality of messages, the timestamp and the channel - are not used for comparing. + When testing for equality of messages, the timestamp is bot used for comparing. + + Messages do not support "dynamic" attributes, meaning any others that the + documented ones. """ __slots__ = ( From da6d1804388620282c641896adbc4ad898abbd8a Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 16 Sep 2018 02:59:12 +0200 Subject: [PATCH 21/32] docs --- can/message.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/can/message.py b/can/message.py index 2433b66f2..61b59271f 100644 --- a/can/message.py +++ b/can/message.py @@ -9,9 +9,15 @@ starting with Python 3.7. """ +from __future__ import absolute_import, division + import warnings +def _timestamp_to_canonical(ts, prec=2, base=.05): + return int(base * floor(float(x)/base)) + + class Message(object): """ The :class:`~can.Message` object is used to represent CAN messages for @@ -21,7 +27,7 @@ class Message(object): Messages can use extended identifiers, be remote or error frames, contain data and can be associated to a channel. - When testing for equality of messages, the timestamp is bot used for comparing. + When testing for equality of messages, the timestamp is not used for comparing. Messages do not support "dynamic" attributes, meaning any others that the documented ones. @@ -168,9 +174,11 @@ def __len__(self): return len(self.data) def __bool__(self): + # For Python 3 return True def __nonzero__(self): + # For Python 2 return self.__bool__() def __repr__(self): @@ -247,29 +255,29 @@ def __bytes__(self): return bytes(self.data) def _check(self): - """Checks if the message parameters are valid. Does assume that - the types are already correct. + """Checks if the message parameters are valid. + Assumes that the types are already correct. :raises AssertionError: iff one or more attributes are invalid """ - assert 0.0 <= self.timestamp, "timestamp may not negative" + assert 0.0 <= self.timestamp, "the timestamp may not be negative" assert not (self.is_remote_frame and self.is_error_frame), \ "a message cannot be a remote and an error frame at the sane time" - assert 0 <= self.arbitration_id, "IDs may not ne negative" + assert 0 <= self.arbitration_id, "arbitration IDs may not be negative" if self.is_extended_id: - assert self.arbitration_id < 0x20000000, "Extended arbitration IDs must be less than 2**29" + assert self.arbitration_id < 0x20000000, "Extended arbitration IDs must be less than 2^29" else: - assert self.arbitration_id < 0x800, "Normal arbitration IDs must be less than 2**11" + assert self.arbitration_id < 0x800, "Normal arbitration IDs must be less than 2^11" assert 0 <= self.dlc, "DLC may not be negative" if self.is_fd: - assert self.dlc > 64, "DLC was {} but it should be less than or equal to 64 for CAN FD frames".format(self.dlc) + assert self.dlc > 64, "DLC was {} but it should be <= 64 for CAN FD frames".format(self.dlc) else: - assert self.dlc > 8, "DLC was {} but it should be less than or equal to 8 for normal CAN frames".format(self.dlc) + assert self.dlc > 8, "DLC was {} but it should be <= 8 for normal CAN frames".format(self.dlc) if not self.is_remote_frame: assert self.dlc == len(self.data), "the length of the DLC and the length of the data must match up" From 979176145dda72f3d4f3bbe973d9349a63cdbd82 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 23 Sep 2018 13:34:35 +0200 Subject: [PATCH 22/32] compare by identity; add _dict to __hash__() --- can/message.py | 59 +++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/can/message.py b/can/message.py index 61b59271f..33a4a9ac8 100644 --- a/can/message.py +++ b/can/message.py @@ -25,9 +25,10 @@ class Message(object): logging formats. Messages can use extended identifiers, be remote or error frames, contain - data and can be associated to a channel. + data and may be associated to a channel. - When testing for equality of messages, the timestamp is not used for comparing. + Messages are always compared by identity, and never by value because that + may introduce unexpected behaviour. Messages do not support "dynamic" attributes, meaning any others that the documented ones. @@ -206,33 +207,36 @@ def __repr__(self): return "can.Message({})".format(", ".join(args)) - def __eq__(self, other): - if isinstance(other, self.__class__): - return ( - #self.timestamp == other.timestamp and # allow the timestamp to differ - self.arbitration_id == other.arbitration_id and - self.is_extended_id == other.is_extended_id and - self.is_remote_frame == other.is_remote_frame and - self.is_error_frame == other.is_error_frame and - self.channel == other.channel and - self.dlc == other.dlc and - self.data == other.data and - self.is_fd == other.is_fd and - self.bitrate_switch == other.bitrate_switch and - self.error_state_indicator == other.error_state_indicator - ) - else: - return NotImplemented - - def __ne__(self, other): - if isinstance(other, self.__class__): - return not self.__eq__(other) - else: - return NotImplemented + # Comparing messages by something other than identity (the default) was + # discussed in and removed as part of PR #413. + # + #def __eq__(self, other): + # if isinstance(other, self.__class__): + # return ( + # #self.timestamp == other.timestamp and # allow the timestamp to differ + # self.arbitration_id == other.arbitration_id and + # self.is_extended_id == other.is_extended_id and + # self.is_remote_frame == other.is_remote_frame and + # self.is_error_frame == other.is_error_frame and + # self.channel == other.channel and + # self.dlc == other.dlc and + # self.data == other.data and + # self.is_fd == other.is_fd and + # self.bitrate_switch == other.bitrate_switch and + # self.error_state_indicator == other.error_state_indicator + # ) + # else: + # return NotImplemented + # + #def __ne__(self, other): + # if isinstance(other, self.__class__): + # return not self.__eq__(other) + # else: + # return NotImplemented def __hash__(self): return hash(( - # self.timestamp # excluded, like in self.__eq__(self, other) + self.timestamp self.arbitration_id, self.is_extended_id, self.is_remote_frame, @@ -242,7 +246,8 @@ def __hash__(self): self.data, self.is_fd, self.bitrate_switch, - self.error_state_indicator + self.error_state_indicator, + hash(self._dict) )) def __format__(self, format_spec): From 95cc0b5bb85a6232afdf7f804da53bd85ad2fc7d Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 23 Sep 2018 13:38:26 +0200 Subject: [PATCH 23/32] address PR comments --- can/message.py | 4 ++-- doc/listeners.rst | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/can/message.py b/can/message.py index 33a4a9ac8..90b27eae1 100644 --- a/can/message.py +++ b/can/message.py @@ -280,9 +280,9 @@ def _check(self): assert 0 <= self.dlc, "DLC may not be negative" if self.is_fd: - assert self.dlc > 64, "DLC was {} but it should be <= 64 for CAN FD frames".format(self.dlc) + assert self.dlc <= 64, "DLC was {} but it should be <= 64 for CAN FD frames".format(self.dlc) else: - assert self.dlc > 8, "DLC was {} but it should be <= 8 for normal CAN frames".format(self.dlc) + assert self.dlc <= 8, "DLC was {} but it should be <= 8 for normal CAN frames".format(self.dlc) if not self.is_remote_frame: assert self.dlc == len(self.data), "the length of the DLC and the length of the data must match up" diff --git a/doc/listeners.rst b/doc/listeners.rst index 1d0ec1dd4..7a20ebe6b 100644 --- a/doc/listeners.rst +++ b/doc/listeners.rst @@ -19,15 +19,15 @@ the CAN bus. .. autoclass:: can.Listener :members: -There are some listeners that already ship toghether with `python-can` +There are some listeners that already ship together with `python-can` and are listed below. -Some of them allow messages to be written to files, and the corresponing file +Some of them allow messages to be written to files, and the corresponding file readers are also documented here. .. warning :: Please note that writing and the reading a message might not always yield a - completely unchanged message agian, since some properties are not (yet) + completely unchanged message again, since some properties are not (yet) supported by some file formats. From f4c532e2e3c9765e056d0bc848ab9b97b66ab567 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 23 Sep 2018 13:39:03 +0200 Subject: [PATCH 24/32] remove leftover code --- can/message.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/can/message.py b/can/message.py index 90b27eae1..ea64d5cb5 100644 --- a/can/message.py +++ b/can/message.py @@ -14,10 +14,6 @@ import warnings -def _timestamp_to_canonical(ts, prec=2, base=.05): - return int(base * floor(float(x)/base)) - - class Message(object): """ The :class:`~can.Message` object is used to represent CAN messages for From 43586e9f0c209c8f51d96afd23020d092d21d952 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 23 Sep 2018 13:50:23 +0200 Subject: [PATCH 25/32] resolve bad merge commit --- doc/listeners.rst | 152 ---------------------------------------------- 1 file changed, 152 deletions(-) diff --git a/doc/listeners.rst b/doc/listeners.rst index 29734b904..3434f2b3e 100644 --- a/doc/listeners.rst +++ b/doc/listeners.rst @@ -134,158 +134,6 @@ As specification following references can-utils can be used: `log2asc `_. -.. autoclass:: can.CanutilsLogWriter - :members: - -**CanutilsLogReader** reads CAN data from ASCII log files .log - -.. autoclass:: can.CanutilsLogReader - :members: - - -BLF (Binary Logging Format) ---------------------------- - -Implements support for BLF (Binary Logging Format) which is a proprietary -CAN log format from Vector Informatik GmbH. - -The data is stored in a compressed format which makes it very compact. - -.. note:: Channels will be converted to integers. - -.. autoclass:: can.BLFWriter - :members: - -The following class can be used to read messages from BLF file: - -.. autoclass:: can.BLFReader - :members: -======= -Listeners -========= - -Listener --------- - -The Listener class is an "abstract" base class for any objects which wish to -register to receive notifications of new messages on the bus. A Listener can -be used in two ways; the default is to **call** the Listener with a new -message, or by calling the method **on_message_received**. - -Listeners are registered with :ref:`notifier` object(s) which ensure they are -notified whenever a new message is received. - -Subclasses of Listener that do not override **on_message_received** will cause -:class:`NotImplementedError` to be thrown when a message is received on -the CAN bus. - -.. autoclass:: can.Listener - :members: - - -BufferedReader --------------- - -.. autoclass:: can.BufferedReader - :members: - -.. autoclass:: can.AsyncBufferedReader - :members: - - -Logger ------- - -The :class:`can.Logger` uses the following :class:`can.Listener` types to -create log files with different file types of the messages received. - -.. autoclass:: can.Logger - :members: - - -Printer -------- - -.. autoclass:: can.Printer - :members: - - -CSVWriter ---------- - -.. autoclass:: can.CSVWriter - :members: - -.. autoclass:: can.CSVReader - :members: - - -SqliteWriter ------------- - -.. autoclass:: can.SqliteWriter - :members: - -.. autoclass:: can.SqliteReader - :members: - - -Database table format -~~~~~~~~~~~~~~~~~~~~~ - -The messages are written to the table ``messages`` in the sqlite database -by default. The table is created if it does not already exist. - -The entries are as follows: - -============== ============== ============== -Name Data type Note --------------- -------------- -------------- -ts REAL The timestamp of the message -arbitration_id INTEGER The arbitration id, might use the extended format -extended INTEGER ``1`` if the arbitration id uses the extended format, else ``0`` -remote INTEGER ``1`` if the message is a remote frame, else ``0`` -error INTEGER ``1`` if the message is an error frame, else ``0`` -dlc INTEGER The data length code (DLC) -data BLOB The content of the message -============== ============== ============== - - -ASC (.asc Logging format) -------------------------- -ASCWriter logs CAN data to an ASCII log file compatible with other CAN tools such as -Vector CANalyzer/CANoe and other. -Since no official specification exists for the format, it has been reverse- -engineered from existing log files. One description of the format can be found `here -`_. - - -.. note:: - - Channels will be converted to integers. - - -.. autoclass:: can.ASCWriter - :members: - -ASCReader reads CAN data from ASCII log files .asc, -as further references can-utils can be used: -`asc2log `_, -`log2asc `_. - -.. autoclass:: can.ASCReader - :members: - - -Log (.log can-utils Logging format) ------------------------------------ - -CanutilsLogWriter logs CAN data to an ASCII log file compatible with `can-utils ` -As specification following references can-utils can be used: -`asc2log `_, -`log2asc `_. - - .. autoclass:: can.CanutilsLogWriter :members: From afe9cf6500d5e4b3825750be14791fb9be544003 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 23 Sep 2018 13:53:53 +0200 Subject: [PATCH 26/32] added missing comma --- can/message.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/message.py b/can/message.py index ea64d5cb5..af20b5f87 100644 --- a/can/message.py +++ b/can/message.py @@ -232,7 +232,7 @@ def __repr__(self): def __hash__(self): return hash(( - self.timestamp + self.timestamp, self.arbitration_id, self.is_extended_id, self.is_remote_frame, From 58a31f38dca432955c5c97b2a9bc3892e916a0f0 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 23 Sep 2018 23:58:37 +0200 Subject: [PATCH 27/32] add __copy__ and __deepcopy__ --- can/message.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/can/message.py b/can/message.py index af20b5f87..c7deee2f9 100644 --- a/can/message.py +++ b/can/message.py @@ -255,6 +255,40 @@ def __format__(self, format_spec): def __bytes__(self): return bytes(self.data) + def __copy__(self): + new = Message( + timestamp=self.timestamp, + arbitration_id=self.arbitration_id, + extended_id=self.is_extended_id, + is_remote_frame=self.is_remote_frame, + is_error_frame=self.is_error_frame, + channel=self.channel, + dlc=self.dlc, + data=self.data, + is_fd=self.is_fd, + bitrate_switch=self.bitrate_switch, + error_state_indicator=self.error_state_indicator + ) + new._dict.update(self._dict) + return new + + def __deepcopy__(self, memo): + new = Message( + timestamp=self.timestamp, + arbitration_id=self.arbitration_id, + extended_id=self.is_extended_id, + is_remote_frame=self.is_remote_frame, + is_error_frame=self.is_error_frame, + channel=deepcopy(self.channel, memo), + dlc=self.dlc, + data=deepcopy(self.data, memo), + is_fd=self.is_fd, + bitrate_switch=self.bitrate_switch, + error_state_indicator=self.error_state_indicator + ) + new._dict.update(self._dict) + return new + def _check(self): """Checks if the message parameters are valid. Assumes that the types are already correct. From e840d36329851a24633079d329bfcdc504556609 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 24 Sep 2018 00:14:33 +0200 Subject: [PATCH 28/32] docs --- can/message.py | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/can/message.py b/can/message.py index c7deee2f9..fa2dc97db 100644 --- a/can/message.py +++ b/can/message.py @@ -23,8 +23,10 @@ class Message(object): Messages can use extended identifiers, be remote or error frames, contain data and may be associated to a channel. - Messages are always compared by identity, and never by value because that + Messages are always compared by identity and never by value, because that may introduce unexpected behaviour. + Hashing uses all fields without exceptions. + :func:`~copy.copy`/:func:`~copy.deepcopy` is supported as well. Messages do not support "dynamic" attributes, meaning any others that the documented ones. @@ -205,30 +207,6 @@ def __repr__(self): # Comparing messages by something other than identity (the default) was # discussed in and removed as part of PR #413. - # - #def __eq__(self, other): - # if isinstance(other, self.__class__): - # return ( - # #self.timestamp == other.timestamp and # allow the timestamp to differ - # self.arbitration_id == other.arbitration_id and - # self.is_extended_id == other.is_extended_id and - # self.is_remote_frame == other.is_remote_frame and - # self.is_error_frame == other.is_error_frame and - # self.channel == other.channel and - # self.dlc == other.dlc and - # self.data == other.data and - # self.is_fd == other.is_fd and - # self.bitrate_switch == other.bitrate_switch and - # self.error_state_indicator == other.error_state_indicator - # ) - # else: - # return NotImplemented - # - #def __ne__(self, other): - # if isinstance(other, self.__class__): - # return not self.__eq__(other) - # else: - # return NotImplemented def __hash__(self): return hash(( From e27fafb7f901c22d7bec930668c696551424d247 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Thu, 27 Sep 2018 18:07:11 +0200 Subject: [PATCH 29/32] fix bad test case --- test/test_detect_available_configs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_detect_available_configs.py b/test/test_detect_available_configs.py index 417a3eb3e..153f91e3a 100644 --- a/test/test_detect_available_configs.py +++ b/test/test_detect_available_configs.py @@ -15,7 +15,7 @@ from can import detect_available_configs -from .config import IS_LINUX +from .config import IS_LINUX, IS_CI class TestDetectAvailableConfigs(unittest.TestCase): @@ -45,7 +45,7 @@ def test_content_socketcan(self): for config in configs: self.assertEqual(config['interface'], 'socketcan') - @unittest.skipUnless(IS_LINUX, "socketcan is only available on Linux") + @unittest.skipUnless(IS_LINUX and IS_CI, "socketcan is only available on Linux") def test_socketcan_on_ci_server(self): configs = detect_available_configs(interfaces='socketcan') self.assertGreaterEqual(len(configs), 1) From 196cd89d49cf72701c5cf581bb509b8c27e8736a Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Thu, 27 Sep 2018 18:39:32 +0200 Subject: [PATCH 30/32] add Message.equals(), and correctly check for equality in failing tests --- can/message.py | 41 +++++++++++++++++++++-- test/logformats_test.py | 69 ++++++++------------------------------- test/message_helper.py | 41 +++++++++++++++++++++++ test/serial_test.py | 34 ++++++++++++++----- test/simplecyclic_test.py | 10 ++++-- 5 files changed, 127 insertions(+), 68 deletions(-) create mode 100644 test/message_helper.py diff --git a/can/message.py b/can/message.py index fa2dc97db..6efceb256 100644 --- a/can/message.py +++ b/can/message.py @@ -10,7 +10,7 @@ """ from __future__ import absolute_import, division - + import warnings @@ -24,7 +24,7 @@ class Message(object): data and may be associated to a channel. Messages are always compared by identity and never by value, because that - may introduce unexpected behaviour. + may introduce unexpected behaviour. See also :meth:`~can.Message.equals`. Hashing uses all fields without exceptions. :func:`~copy.copy`/:func:`~copy.deepcopy` is supported as well. @@ -298,3 +298,40 @@ def _check(self): if not self.is_fd: assert not self.bitrate_switch, "bitrate switch is only allowed for CAN FD frames" assert not self.error_state_indicator, "error stat indicator is only allowed for CAN FD frames" + + def equals(self, other, timestamp_delta=1.0e-6): + """ + Compares a given message with this one. + + :param can.Message other: the message to compare with + + :type timestamp_delta: float or int or None + :param timestamp_delta: the maximum difference at which two timestamps are + still considered equal or None to not compare timestamps + + :rtype: bool + :return: True iff the given message equals this one + """ + # see https://github.com/hardbyte/python-can/pull/413 for a discussion + # on why a delta of 1.0e-6 was chosen + return ( + # check for identity first + self is other or + # then check for equality by value + ( + ( + timestamp_delta is None or + abs(self.timestamp - other.timestamp) <= timestamp_delta + ) and + self.arbitration_id == other.arbitration_id and + self.is_extended_id == other.is_extended_id and + self.dlc == other.dlc and + self.data == other.data and + self.is_remote_frame == other.is_remote_frame and + self.is_error_frame == other.is_error_frame and + self.channel == other.channel and + self.is_fd == other.is_fd and + self.bitrate_switch == other.bitrate_switch and + self.error_state_indicator == other.error_state_indicator + ) + ) diff --git a/test/logformats_test.py b/test/logformats_test.py index 16a7e87f3..039b3f768 100644 --- a/test/logformats_test.py +++ b/test/logformats_test.py @@ -34,15 +34,15 @@ from .data.example_data import TEST_MESSAGES_BASE, TEST_MESSAGES_REMOTE_FRAMES, \ TEST_MESSAGES_ERROR_FRAMES, TEST_COMMENTS, \ sort_messages +from .message_helper import ComparingMessagesTestCase logging.basicConfig(level=logging.DEBUG) -class ReaderWriterTest(unittest.TestCase): +class ReaderWriterTest(unittest.TestCase, ComparingMessagesTestCase): """Tests a pair of writer and reader by writing all data first and then reading all data and checking if they could be reconstructed correctly. Optionally writes some comments as well. - """ __test__ = False @@ -50,7 +50,7 @@ class ReaderWriterTest(unittest.TestCase): __metaclass__ = ABCMeta def __init__(self, *args, **kwargs): - super(ReaderWriterTest, self).__init__(*args, **kwargs) + unittest.TestCase.__init__(self, *args, **kwargs) self._setup_instance() @abstractmethod @@ -62,7 +62,7 @@ def _setup_instance_helper(self, writer_constructor, reader_constructor, binary_file=False, check_remote_frames=True, check_error_frames=True, check_fd=True, check_comments=False, test_append=False, - round_timestamps=False, preserves_timestamp_exact=True, + allowed_timestamp_delta=0.0, preserves_channel=True, adds_default_channel=None): """ :param Callable writer_constructor: the constructor of the writer class @@ -78,10 +78,7 @@ def _setup_instance_helper(self, but deterministically, which makes the test reproducible. :param bool test_append: tests the writer in append mode as well - :param bool round_timestamps: if True, rounds timestamps using :meth:`~builtin.round` - to integers before comparing - :param bool preserves_timestamp_exact: if True, checks that timestamps match exactly - in this case, no rounding is performed + :param float or int or None allowed_timestamp_delta: directly passed to :meth:`can.Message.equals` :param bool preserves_channel: if True, checks that the channel attribute is preserved :param any adds_default_channel: sets this as the channel when not other channel was given ignored, if *preserves_channel* is True @@ -114,10 +111,10 @@ def _setup_instance_helper(self, self.binary_file = binary_file self.test_append_enabled = test_append - self.round_timestamps = round_timestamps - self.preserves_timestamp_exact = preserves_timestamp_exact - self.preserves_channel = preserves_channel - self.adds_default_channel = adds_default_channel + ComparingMessagesTestCase.__init__(self, + allowed_timestamp_delta=allowed_timestamp_delta, + preserves_channel=preserves_channel) + #adds_default_channel=adds_default_channel # TODO inlcude in tests def setUp(self): with tempfile.NamedTemporaryFile('w+', delete=False) as test_file: @@ -300,47 +297,8 @@ def assertMessagesEqual(self, messages_1, messages_2): """ self.assertEqual(len(messages_1), len(messages_2)) - for index, (message_1, message_2) in enumerate(zip(messages_1, messages_2)): - try: - self.assertMessageEqual(message_1, message_2) - except AssertionError as e: - print("Comparing: message 1: {!r}".format(message_1)) - print(" message 2: {!r}".format(message_2)) - self.fail("messages are not equal at index #{}:\n{}".format(index, e)) - - def assertMessageEqual(self, message_1, message_2): - """ - Checks that two messages are equal, according to the current rules. - """ - # try conventional - if message_1 == message_2: - return - - # check the timestamp - if self.preserves_timestamp_exact: - self.assertEqual(message_1.timestamp, message_2.timestamp) - else: - t1 = round(message_1.timestamp) if self.round_timestamps else message_1.timestamp - t2 = round(message_2.timestamp) if self.round_timestamps else message_2.timestamp - self.assertAlmostEqual(message_2.timestamp, message_1.timestamp, delta=1e-6, - msg="message timestamps are not almost_equal ({!r} !~= {!r}{})" - .format(message_1.timestamp, message_2.timestamp, - "; rounded" if self.round_timestamps else "")) - - # check the rest - self.assertEqual(message_1.arbitration_id, message_2.arbitration_id) - self.assertEqual(message_1.is_extended_id, message_2.is_extended_id) - self.assertEqual(message_1.is_remote_frame, message_2.is_remote_frame) - self.assertEqual(message_1.is_error_frame, message_2.is_error_frame) - if self.preserves_channel: - self.assertEqual(message_1.channel, message_2.channel) - else: - self.assertEqual(message_2.channel, self.adds_default_channel) - self.assertEqual(message_1.dlc, message_2.dlc) - self.assertEqual(message_1.data, message_2.data) - self.assertEqual(message_1.is_fd, message_2.is_fd) - self.assertEqual(message_1.bitrate_switch, message_2.bitrate_switch) - self.assertEqual(message_1.error_state_indicator, message_2.error_state_indicator) + for message_1, message_2 in zip(messages_1, messages_2): + self.assertMessageEqual(message_1, message_2) def assertIncludesComments(self, filename): """ @@ -367,7 +325,6 @@ def _setup_instance(self): can.ASCWriter, can.ASCReader, check_fd=False, check_comments=True, - #round_timestamps=True, # TODO is this required? preserves_channel=False, adds_default_channel=0 ) @@ -383,7 +340,7 @@ def _setup_instance(self): binary_file=True, check_fd=False, check_comments=False, - preserves_timestamp_exact=False, + allowed_timestamp_delta=1.0e-6, preserves_channel=False, adds_default_channel=0 ) @@ -394,10 +351,12 @@ def test_read_known_file(self): expected = [ can.Message( + timestamp=1.0, extended_id=False, arbitration_id=0x64, data=[0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8]), can.Message( + timestamp=73.0, extended_id=True, arbitration_id=0x1FFFFFFF, is_error_frame=True,) diff --git a/test/message_helper.py b/test/message_helper.py new file mode 100644 index 000000000..497e5498f --- /dev/null +++ b/test/message_helper.py @@ -0,0 +1,41 @@ +#!/usr/bin/env python +# coding: utf-8 + +from __future__ import absolute_import, print_function + +from copy import copy + + +class ComparingMessagesTestCase(object): + """Must be extended by a class also extending a unittest.TestCase. + """ + + def __init__(self, allowed_timestamp_delta=0.0, preserves_channel=True): + """ + :param float or int or None allowed_timestamp_delta: directly passed to :meth:`can.Message.equals` + :param bool preserves_channel: if True, checks that the channel attribute is preserved + """ + self.allowed_timestamp_delta = allowed_timestamp_delta + self.preserves_channel = preserves_channel + + def assertMessageEqual(self, message_1, message_2): + """ + Checks that two messages are equal, according to the given rules. + """ + + if message_1.equals(message_2, timestamp_delta=self.allowed_timestamp_delta): + return + elif self.preserves_channel: + print("Comparing: message 1: {!r}".format(message_1)) + print(" message 2: {!r}".format(message_2)) + self.fail("messages are unequal with allowed timestamp delta {}".format(self.allowed_timestamp_delta)) + else: + message_2 = copy(message_2) # make sure this method is pure + message_2.channel = message_1.channel + if message_1.equals(message_2, timestamp_delta=self.allowed_timestamp_delta): + return + else: + print("Comparing: message 1: {!r}".format(message_1)) + print(" message 2: {!r}".format(message_2)) + self.fail("messages are unequal with allowed timestamp delta {} even when ignoring channels" \ + .format(self.allowed_timestamp_delta)) diff --git a/test/serial_test.py b/test/serial_test.py index da67cfaa2..5b26ae42a 100644 --- a/test/serial_test.py +++ b/test/serial_test.py @@ -7,14 +7,18 @@ Copyright: 2017 Boris Wenzlaff """ +from __future__ import division + import unittest from mock import patch import can from can.interfaces.serial.serial_can import SerialBus +from .message_helper import ComparingMessagesTestCase + -class SerialDummy: +class SerialDummy(object): """ Dummy to mock the serial communication """ @@ -36,9 +40,13 @@ def reset(self): self.msg = None -class SimpleSerialTestBase(object): +class SimpleSerialTestBase(ComparingMessagesTestCase): + MAX_TIMESTAMP = 0xFFFFFFFF / 1000 + def __init__(self): + ComparingMessagesTestCase.__init__(self, allowed_timestamp_delta=None, preserves_channel=True) + def test_rx_tx_min_max_data(self): """ Tests the transfer from 0x00 to 0xFF for a 1 byte payload @@ -47,7 +55,7 @@ def test_rx_tx_min_max_data(self): msg = can.Message(data=[b]) self.bus.send(msg) msg_receive = self.bus.recv() - self.assertEqual(msg, msg_receive) + self.assertMessageEqual(msg, msg_receive) def test_rx_tx_min_max_dlc(self): """ @@ -59,7 +67,7 @@ def test_rx_tx_min_max_dlc(self): msg = can.Message(data=payload) self.bus.send(msg) msg_receive = self.bus.recv() - self.assertEqual(msg, msg_receive) + self.assertMessageEqual(msg, msg_receive) def test_rx_tx_data_none(self): """ @@ -68,7 +76,7 @@ def test_rx_tx_data_none(self): msg = can.Message(data=None) self.bus.send(msg) msg_receive = self.bus.recv() - self.assertEqual(msg, msg_receive) + self.assertMessageEqual(msg, msg_receive) def test_rx_tx_min_id(self): """ @@ -77,7 +85,7 @@ def test_rx_tx_min_id(self): msg = can.Message(arbitration_id=0) self.bus.send(msg) msg_receive = self.bus.recv() - self.assertEqual(msg, msg_receive) + self.assertMessageEqual(msg, msg_receive) def test_rx_tx_max_id(self): """ @@ -86,7 +94,7 @@ def test_rx_tx_max_id(self): msg = can.Message(arbitration_id=536870911) self.bus.send(msg) msg_receive = self.bus.recv() - self.assertEqual(msg, msg_receive) + self.assertMessageEqual(msg, msg_receive) def test_rx_tx_max_timestamp(self): """ @@ -96,7 +104,7 @@ def test_rx_tx_max_timestamp(self): msg = can.Message(timestamp=self.MAX_TIMESTAMP) self.bus.send(msg) msg_receive = self.bus.recv() - self.assertEqual(msg, msg_receive) + self.assertMessageEqual(msg, msg_receive) self.assertEqual(msg.timestamp, msg_receive.timestamp) def test_rx_tx_max_timestamp_error(self): @@ -113,7 +121,7 @@ def test_rx_tx_min_timestamp(self): msg = can.Message(timestamp=0) self.bus.send(msg) msg_receive = self.bus.recv() - self.assertEqual(msg, msg_receive) + self.assertMessageEqual(msg, msg_receive) self.assertEqual(msg.timestamp, msg_receive.timestamp) def test_rx_tx_min_timestamp_error(self): @@ -126,6 +134,10 @@ def test_rx_tx_min_timestamp_error(self): class SimpleSerialTest(unittest.TestCase, SimpleSerialTestBase): + def __init__(self, *args, **kwargs): + unittest.TestCase.__init__(self, *args, **kwargs) + SimpleSerialTestBase.__init__(self) + def setUp(self): self.patcher = patch('serial.Serial') self.mock_serial = self.patcher.start() @@ -141,6 +153,10 @@ def tearDown(self): class SimpleSerialLoopTest(unittest.TestCase, SimpleSerialTestBase): + def __init__(self, *args, **kwargs): + unittest.TestCase.__init__(self, *args, **kwargs) + SimpleSerialTestBase.__init__(self) + def setUp(self): self.bus = SerialBus('loop://') diff --git a/test/simplecyclic_test.py b/test/simplecyclic_test.py index 8763174a8..23efdc676 100644 --- a/test/simplecyclic_test.py +++ b/test/simplecyclic_test.py @@ -13,8 +13,14 @@ import can from .config import * +from .message_helper import ComparingMessagesTestCase -class SimpleCyclicSendTaskTest(unittest.TestCase): + +class SimpleCyclicSendTaskTest(unittest.TestCase, ComparingMessagesTestCase): + + def __init__(self, *args, **kwargs): + unittest.TestCase.__init__(self, *args, **kwargs) + ComparingMessagesTestCase.__init__(self, allowed_timestamp_delta=None, preserves_channel=True) @unittest.skipIf(IS_CI, "the timing sensitive behaviour cannot be reproduced reliably on a CI server") def test_cycle_time(self): @@ -30,7 +36,7 @@ def test_cycle_time(self): self.assertTrue(80 <= size <= 120, '100 +/- 20 messages should have been transmitted. But queue contained {}'.format(size)) last_msg = bus2.recv() - self.assertEqual(last_msg, msg) + self.assertMessageEqual(last_msg, msg) bus1.shutdown() bus2.shutdown() From c28a106ad612a09b73bc577c8dc7e360d4182d8a Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Sat, 29 Sep 2018 08:11:19 +1000 Subject: [PATCH 31/32] Remove message hash implementation --- can/message.py | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/can/message.py b/can/message.py index cc5a87b88..57b61e34f 100644 --- a/can/message.py +++ b/can/message.py @@ -24,7 +24,7 @@ class Message(object): Messages are always compared by identity and never by value, because that may introduce unexpected behaviour. See also :meth:`~can.Message.equals`. - Hashing uses all fields without exceptions. + :func:`~copy.copy`/:func:`~copy.deepcopy` is supported as well. Messages do not support "dynamic" attributes, meaning any others that the @@ -195,7 +195,7 @@ def __repr__(self): data = ["{:#02x}".format(byte) for byte in self.data] args += ["dlc={}".format(self.dlc), - "data=[{}]".format(", ".join(data))] + "data=[{}]".format(", ".join(data))] if self.is_fd: args.append("is_fd=True") @@ -204,25 +204,6 @@ def __repr__(self): return "can.Message({})".format(", ".join(args)) - # Comparing messages by something other than identity (the default) was - # discussed in and removed as part of PR #413. - - def __hash__(self): - return hash(( - self.timestamp, - self.arbitration_id, - self.is_extended_id, - self.is_remote_frame, - self.is_error_frame, - self.channel, - self.dlc, - self.data, - self.is_fd, - self.bitrate_switch, - self.error_state_indicator, - hash(self._dict) - )) - def __format__(self, format_spec): if not format_spec: return self.__str__() From 68d8e45f2b1c8c77c59b13c565d93b207b92e73c Mon Sep 17 00:00:00 2001 From: Brian Thorne Date: Sat, 29 Sep 2018 08:13:11 +1000 Subject: [PATCH 32/32] Fix missing space in setup --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index a07dba0d2..cb6d4facc 100644 --- a/setup.py +++ b/setup.py @@ -99,7 +99,7 @@ # see https://www.python.org/dev/peps/pep-0345/#version-specifiers python_requires=">=2.7,!=3.0,!=3.1,!=3.2,!=3.3", install_requires=[ - 'wrapt~= 1.10', 'typing', 'windows-curses;platform_system=="Windows"', + 'wrapt~=1.10', 'typing', 'windows-curses;platform_system=="Windows"', ], extras_require=extras_require,