Message class changes

can/message.py

@@ -9,9 +9,11 @@
 """
 
 from __future__ import absolute_import, division
-        
+
 import warnings
 from copy import deepcopy
+from math import isinf, isnan
+
 
 class Message(object):
     """
@@ -53,7 +55,7 @@ def __getattr__(self, key):
         # can be removed in 4.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)
+            warnings.warn("Custom attributes of messages are deprecated and will be removed in 4.0", DeprecationWarning)
             return self._dict[key]
         except KeyError:
             raise AttributeError("'message' object has no attribute '{}'".format(key))
@@ -63,26 +65,26 @@ def __setattr__(self, key, value):
         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)
+            warnings.warn("Custom attributes of messages are deprecated and will be removed in 4.0", DeprecationWarning)
             self._dict[key] = value
 
     @property
     def id_type(self):
         # TODO remove in 4.0
-        warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning)
+        warnings.warn("Message.id_type is deprecated and will be removed in 4.0, use is_extended_id instead", DeprecationWarning)
         return self.is_extended_id
 
     @id_type.setter
     def id_type(self, value):
         # TODO remove in 4.0
-        warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning)
+        warnings.warn("Message.id_type is deprecated and will be removed in 4.0, use is_extended_id instead", DeprecationWarning)
         self.is_extended_id = value
 
     def __init__(self, timestamp=0.0, arbitration_id=0, is_extended_id=None,
                  is_remote_frame=False, is_error_frame=False, channel=None,
                  dlc=None, data=None,
                  is_fd=False, bitrate_switch=False, error_state_indicator=False,
-                 extended_id=True, # deprecated in 3.x, removed in 4.x
+                 extended_id=None, # deprecated in 3.x, TODO remove in 4.x
                  check=False):
         """
         To create a message object, simply provide any of the below attributes
@@ -100,10 +102,15 @@ def __init__(self, timestamp=0.0, arbitration_id=0, is_extended_id=None,
 
         self.timestamp = timestamp
         self.arbitration_id = arbitration_id
+
+        if extended_id is not None:
+            # TODO remove in 4.0
+            warnings.warn("The extended_id parameter is deprecated and will be removed in 4.0, use is_extended_id instead", DeprecationWarning)
+
         if is_extended_id is not None:
             self.is_extended_id = is_extended_id
         else:
-            self.is_extended_id = extended_id
+            self.is_extended_id = True if extended_id is None else extended_id
 
         self.is_remote_frame = is_remote_frame
         self.is_error_frame = is_error_frame
@@ -162,18 +169,19 @@ def __str__(self):
             field_strings.append(" " * 24)
 
         if (self.data is not None) and (self.data.isalnum()):
-            try:
-                field_strings.append("'{}'".format(self.data.decode('utf-8')))
-            except UnicodeError:
-                pass
+            field_strings.append("'{}'".format(self.data.decode('utf-8', 'replace')))
 
         if self.channel is not None:
-            field_strings.append("Channel: {}".format(self.channel))
+            try:
+                field_strings.append("Channel: {}".format(self.channel))
+            except UnicodeEncodeError:
+                pass
 
         return "    ".join(field_strings).strip()
 
     def __len__(self):
-        return len(self.data)
+        # return the dlc such that it also works on remote frames
+        return self.dlc
 
     def __bool__(self):
         # For Python 3
@@ -255,33 +263,47 @@ def _check(self):
         """Checks if the message parameters are valid.
         Assumes that the types are already correct.
 
-        :raises AssertionError: iff one or more attributes are invalid
+        :raises ValueError: iff one or more attributes are invalid
         """
 
-        assert 0.0 <= self.timestamp, "the timestamp may not be negative"
+        if self.timestamp < 0.0:
+            raise ValueError("the timestamp may not be negative")
+        if isinf(self.timestamp):
+            raise ValueError("the timestamp may not be infinite")
+        if isnan(self.timestamp):
+            raise ValueError("the timestamp may not be NaN")
 
-        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"
+        if self.is_remote_frame and self.is_error_frame:
+            raise ValueError("a message cannot be a remote and an error frame at the sane time")
 
-        assert 0 <= self.arbitration_id, "arbitration IDs may not be negative"
+        if self.arbitration_id < 0:
+            raise ValueError("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"
-        else:
-            assert self.arbitration_id < 0x800, "Normal arbitration IDs must be less than 2^11"
+            if 0x20000000 <= self.arbitration_id:
+                raise ValueError("Extended arbitration IDs must be less than 2^29")
+        elif 0x800 <= self.arbitration_id:
+            raise ValueError("Normal arbitration IDs must be less than 2^11")
 
-        assert 0 <= self.dlc, "DLC may not be negative"
+        if self.dlc < 0:
+            raise ValueError("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)
-        else:
-            assert self.dlc <= 8, "DLC was {} but it should be <= 8 for normal CAN frames".format(self.dlc)
+            if 64 < self.dlc:
+                raise ValueError("DLC was {} but it should be <= 64 for CAN FD frames".format(self.dlc))
+        elif 8 < self.dlc:
+            raise ValueError("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"
+        if self.is_remote_frame:
+            if self.data is not None and len(self.data) != 0:
+                raise ValueError("remote frames may not carry any data")
+        elif self.dlc != len(self.data):
+            raise ValueError("the DLC and the length of the data must match up for non remote frames")
 
         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"
+            if self.bitrate_switch:
+                raise ValueError("bitrate switch is only allowed for CAN FD frames")
+            if self.error_state_indicator:
+                raise ValueError("error stat indicator is only allowed for CAN FD frames")
 
     def equals(self, other, timestamp_delta=1.0e-6):
         """
@@ -299,7 +321,7 @@ def equals(self, other, timestamp_delta=1.0e-6):
         # 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
+            # check for identity first and finish fast
             self is other or
             # then check for equality by value
             (

setup.py

@@ -36,7 +36,8 @@
     'pytest-cov~=2.5',
     'codecov~=2.0',
     'future',
-    'six'
+    'six',
+    'hypothesis'
 ] + extras_require['serial']
 
 extras_require['test'] = tests_require

test/data/example_data.py

@@ -30,7 +30,7 @@ def sort_messages(messages):
 
 
 # List of messages of different types that can be used in tests
-TEST_MESSAGES_BASE = [
+TEST_MESSAGES_BASE = sort_messages([
     Message(
         # empty
     ),
@@ -98,20 +98,17 @@ def sort_messages(messages):
         arbitration_id=0x768, is_extended_id=False,
         timestamp=TEST_TIME + 3.165
     ),
-]
-TEST_MESSAGES_BASE = sort_messages(TEST_MESSAGES_BASE)
+])
 
 
-TEST_MESSAGES_REMOTE_FRAMES = [
+TEST_MESSAGES_REMOTE_FRAMES = sort_messages([
     Message(
         arbitration_id=0xDADADA, is_extended_id=True, is_remote_frame=True,
         timestamp=TEST_TIME + .165,
-        data=[1, 2, 3, 4, 5, 6, 7, 8]
     ),
     Message(
         arbitration_id=0x123, is_extended_id=False, is_remote_frame=True,
         timestamp=TEST_TIME + .365,
-        data=[254, 255]
     ),
     Message(
         arbitration_id=0x768, is_extended_id=False, is_remote_frame=True,
@@ -121,11 +118,10 @@ def sort_messages(messages):
         arbitration_id=0xABCDEF, is_extended_id=True, is_remote_frame=True,
         timestamp=TEST_TIME + 7858.67
     ),
-]
-TEST_MESSAGES_REMOTE_FRAMES = sort_messages(TEST_MESSAGES_REMOTE_FRAMES)
+])
 
 
-TEST_MESSAGES_ERROR_FRAMES = [
+TEST_MESSAGES_ERROR_FRAMES = sort_messages([
     Message(
         is_error_frame=True
     ),
@@ -137,8 +133,7 @@ def sort_messages(messages):
         is_error_frame=True,
         timestamp=TEST_TIME + 17.157
     )
-]
-TEST_MESSAGES_ERROR_FRAMES = sort_messages(TEST_MESSAGES_ERROR_FRAMES)
+])
 
 
 TEST_ALL_MESSAGES = sort_messages(TEST_MESSAGES_BASE + TEST_MESSAGES_REMOTE_FRAMES + \

test/message_helper.py

@@ -1,13 +1,21 @@
 #!/usr/bin/env python
 # coding: utf-8
 
+"""
+This module contains a helper for writing test cases that need to compare messages.
+"""
+
 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.
+    """
+    Must be extended by a class also extending a unittest.TestCase.
+
+    .. note:: This class does not extend unittest.TestCase so it does not get
+              run as a test itself.
     """
 
     def __init__(self, allowed_timestamp_delta=0.0, preserves_channel=True):

test/notifier_test.py

@@ -1,5 +1,6 @@
 #!/usr/bin/env python
 # coding: utf-8
+
 import unittest
 import time
 try:

test/test_message_class.py

@@ -0,0 +1,83 @@
+#!/usr/bin/env python
+# coding: utf-8
+
+import unittest
+import sys
+from math import isinf, isnan
+from copy import copy, deepcopy
+
+from hypothesis import given, settings, reproduce_failure
+import hypothesis.strategies as st
+
+from can import Message
+
+
+class TestMessageClass(unittest.TestCase):
+    """
+    This test tries many inputs to the message class constructor and then sanity checks
+    all methods and ensures that nothing crashes. It also checks whether Message._check()
+    allows all valid can frames.
+    """
+
+    @given(
+        timestamp=st.floats(min_value=0.0),
+        arbitration_id=st.integers(),
+        is_extended_id=st.booleans(),
+        is_remote_frame=st.booleans(),
+        is_error_frame=st.booleans(),
+        channel=st.one_of(st.text(), st.integers()),
+        dlc=st.integers(min_value=0, max_value=8),
+        data=st.one_of(st.binary(min_size=0, max_size=8), st.none()),
+        is_fd=st.booleans(),
+        bitrate_switch=st.booleans(),
+        error_state_indicator=st.booleans()
+    )
+    @settings(max_examples=2000)
+    def test_methods(self, **kwargs):
+        is_valid = not (
+            (not kwargs['is_remote_frame'] and (len(kwargs['data'] or []) != kwargs['dlc'])) or
+            (kwargs['arbitration_id'] >= 0x800 and not kwargs['is_extended_id']) or
+            kwargs['arbitration_id'] >= 0x20000000 or
+            kwargs['arbitration_id'] < 0 or
+            (kwargs['is_remote_frame'] and kwargs['is_error_frame']) or
+            (kwargs['is_remote_frame'] and len(kwargs['data'] or []) > 0) or
+            ((kwargs['bitrate_switch'] or kwargs['error_state_indicator']) and not kwargs['is_fd']) or
+            isnan(kwargs['timestamp']) or
+            isinf(kwargs['timestamp'])
+        )
+
+        # this should return normally and not throw an exception
+        message = Message(check=is_valid, **kwargs)
+
+        if kwargs['data'] is None or kwargs['is_remote_frame']:
+            kwargs['data'] = bytearray()
+
+        if not is_valid and not kwargs['is_remote_frame']:
+            with self.assertRaises(ValueError):
+                Message(check=True, **kwargs)
+
+        self.assertGreater(len(str(message)), 0)
+        self.assertGreater(len(message.__repr__()), 0)
+        if is_valid:
+            self.assertEqual(len(message), kwargs['dlc'])
+        self.assertTrue(bool(message))
+        self.assertGreater(len("{}".format(message)), 0)
+        _ = "{}".format(message)
+        with self.assertRaises(Exception):
+            _ = "{somespec}".format(message)
+        if sys.version_info.major > 2:
+            self.assertEqual(bytearray(bytes(message)), kwargs['data'])
+
+        # check copies and equalities
+        if is_valid:
+            self.assertEqual(message, message)        
+            normal_copy = copy(message)
+            deep_copy = deepcopy(message)
+            for other in (normal_copy, deep_copy, message):
+                self.assertTrue(message.equals(other, timestamp_delta=None))
+                self.assertTrue(message.equals(other))
+                self.assertTrue(message.equals(other, timestamp_delta=0))
+
+
+if __name__ == '__main__':
+    unittest.main()