Skip to content

Commit 4ea358f

Browse files
felixdivohardbyte
felixdivo
authored and
hardbyte
committed
Message class changes (hardbyte#497)
* add hypothesis test for Message class * add runtime warning to deprecated Message's __init__() parameter 'extended_id' * move remaining parameter testing into _check() * remove data from remote test frames * add check for remote frames back to message constructor * cleanups & some docs for TestMessageClass * fix unicode error and change message's __len__ to use the DLC * correct behavior of check=True * fix remote_frame special case * use ValueError instead of assert statements in message's _check()
1 parent de94772 commit 4ea358f

File tree

6 files changed

+153
-43
lines changed

6 files changed

+153
-43
lines changed

‎can/message.py‎

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
"""
1010

1111
from __future__ import absolute_import, division
12-
12+
1313
import warnings
1414
from copy import deepcopy
15+
from math import isinf, isnan
16+
1517

1618
class Message(object):
1719
"""
@@ -53,7 +55,7 @@ def __getattr__(self, key):
5355
# can be removed in 4.0
5456
# this method is only called if the attribute was not found elsewhere, like in __slots__
5557
try:
56-
warnings.warn("Custom attributes of messages are deprecated and will be removed in the next major version", DeprecationWarning)
58+
warnings.warn("Custom attributes of messages are deprecated and will be removed in 4.0", DeprecationWarning)
5759
return self._dict[key]
5860
except KeyError:
5961
raise AttributeError("'message' object has no attribute '{}'".format(key))
@@ -63,26 +65,26 @@ def __setattr__(self, key, value):
6365
try:
6466
super(Message, self).__setattr__(key, value)
6567
except AttributeError:
66-
warnings.warn("Custom attributes of messages are deprecated and will be removed in the next major version", DeprecationWarning)
68+
warnings.warn("Custom attributes of messages are deprecated and will be removed in 4.0", DeprecationWarning)
6769
self._dict[key] = value
6870

6971
@property
7072
def id_type(self):
7173
# TODO remove in 4.0
72-
warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning)
74+
warnings.warn("Message.id_type is deprecated and will be removed in 4.0, use is_extended_id instead", DeprecationWarning)
7375
return self.is_extended_id
7476

7577
@id_type.setter
7678
def id_type(self, value):
7779
# TODO remove in 4.0
78-
warnings.warn("Message.id_type is deprecated, use is_extended_id", DeprecationWarning)
80+
warnings.warn("Message.id_type is deprecated and will be removed in 4.0, use is_extended_id instead", DeprecationWarning)
7981
self.is_extended_id = value
8082

8183
def __init__(self, timestamp=0.0, arbitration_id=0, is_extended_id=None,
8284
is_remote_frame=False, is_error_frame=False, channel=None,
8385
dlc=None, data=None,
8486
is_fd=False, bitrate_switch=False, error_state_indicator=False,
85-
extended_id=True, # deprecated in 3.x, removed in 4.x
87+
extended_id=None, # deprecated in 3.x, TODO remove in 4.x
8688
check=False):
8789
"""
8890
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,
100102

101103
self.timestamp = timestamp
102104
self.arbitration_id = arbitration_id
105+
106+
if extended_id is not None:
107+
# TODO remove in 4.0
108+
warnings.warn("The extended_id parameter is deprecated and will be removed in 4.0, use is_extended_id instead", DeprecationWarning)
109+
103110
if is_extended_id is not None:
104111
self.is_extended_id = is_extended_id
105112
else:
106-
self.is_extended_id = extended_id
113+
self.is_extended_id = True if extended_id is None else extended_id
107114

108115
self.is_remote_frame = is_remote_frame
109116
self.is_error_frame = is_error_frame
@@ -162,18 +169,19 @@ def __str__(self):
162169
field_strings.append(" " * 24)
163170

164171
if (self.data is not None) and (self.data.isalnum()):
165-
try:
166-
field_strings.append("'{}'".format(self.data.decode('utf-8')))
167-
except UnicodeError:
168-
pass
172+
field_strings.append("'{}'".format(self.data.decode('utf-8', 'replace')))
169173

170174
if self.channel is not None:
171-
field_strings.append("Channel: {}".format(self.channel))
175+
try:
176+
field_strings.append("Channel: {}".format(self.channel))
177+
except UnicodeEncodeError:
178+
pass
172179

173180
return " ".join(field_strings).strip()
174181

175182
def __len__(self):
176-
return len(self.data)
183+
# return the dlc such that it also works on remote frames
184+
return self.dlc
177185

178186
def __bool__(self):
179187
# For Python 3
@@ -255,33 +263,47 @@ def _check(self):
255263
"""Checks if the message parameters are valid.
256264
Assumes that the types are already correct.
257265
258-
:raises AssertionError: iff one or more attributes are invalid
266+
:raises ValueError: iff one or more attributes are invalid
259267
"""
260268

261-
assert 0.0 <= self.timestamp, "the timestamp may not be negative"
269+
if self.timestamp < 0.0:
270+
raise ValueError("the timestamp may not be negative")
271+
if isinf(self.timestamp):
272+
raise ValueError("the timestamp may not be infinite")
273+
if isnan(self.timestamp):
274+
raise ValueError("the timestamp may not be NaN")
262275

263-
assert not (self.is_remote_frame and self.is_error_frame), \
264-
"a message cannot be a remote and an error frame at the sane time"
276+
if self.is_remote_frame and self.is_error_frame:
277+
raise ValueError("a message cannot be a remote and an error frame at the sane time")
265278

266-
assert 0 <= self.arbitration_id, "arbitration IDs may not be negative"
279+
if self.arbitration_id < 0:
280+
raise ValueError("arbitration IDs may not be negative")
267281

268282
if self.is_extended_id:
269-
assert self.arbitration_id < 0x20000000, "Extended arbitration IDs must be less than 2^29"
270-
else:
271-
assert self.arbitration_id < 0x800, "Normal arbitration IDs must be less than 2^11"
283+
if 0x20000000 <= self.arbitration_id:
284+
raise ValueError("Extended arbitration IDs must be less than 2^29")
285+
elif 0x800 <= self.arbitration_id:
286+
raise ValueError("Normal arbitration IDs must be less than 2^11")
272287

273-
assert 0 <= self.dlc, "DLC may not be negative"
288+
if self.dlc < 0:
289+
raise ValueError("DLC may not be negative")
274290
if self.is_fd:
275-
assert self.dlc <= 64, "DLC was {} but it should be <= 64 for CAN FD frames".format(self.dlc)
276-
else:
277-
assert self.dlc <= 8, "DLC was {} but it should be <= 8 for normal CAN frames".format(self.dlc)
291+
if 64 < self.dlc:
292+
raise ValueError("DLC was {} but it should be <= 64 for CAN FD frames".format(self.dlc))
293+
elif 8 < self.dlc:
294+
raise ValueError("DLC was {} but it should be <= 8 for normal CAN frames".format(self.dlc))
278295

279-
if not self.is_remote_frame:
280-
assert self.dlc == len(self.data), "the length of the DLC and the length of the data must match up"
296+
if self.is_remote_frame:
297+
if self.data is not None and len(self.data) != 0:
298+
raise ValueError("remote frames may not carry any data")
299+
elif self.dlc != len(self.data):
300+
raise ValueError("the DLC and the length of the data must match up for non remote frames")
281301

282302
if not self.is_fd:
283-
assert not self.bitrate_switch, "bitrate switch is only allowed for CAN FD frames"
284-
assert not self.error_state_indicator, "error stat indicator is only allowed for CAN FD frames"
303+
if self.bitrate_switch:
304+
raise ValueError("bitrate switch is only allowed for CAN FD frames")
305+
if self.error_state_indicator:
306+
raise ValueError("error stat indicator is only allowed for CAN FD frames")
285307

286308
def equals(self, other, timestamp_delta=1.0e-6):
287309
"""
@@ -299,7 +321,7 @@ def equals(self, other, timestamp_delta=1.0e-6):
299321
# see https://github.com/hardbyte/python-can/pull/413 for a discussion
300322
# on why a delta of 1.0e-6 was chosen
301323
return (
302-
# check for identity first
324+
# check for identity first and finish fast
303325
self is other or
304326
# then check for equality by value
305327
(

‎setup.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
'pytest-cov~=2.5',
3737
'codecov~=2.0',
3838
'future',
39-
'six'
39+
'six',
40+
'hypothesis'
4041
] + extras_require['serial']
4142

4243
extras_require['test'] = tests_require

‎test/data/example_data.py‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def sort_messages(messages):
3030

3131

3232
# List of messages of different types that can be used in tests
33-
TEST_MESSAGES_BASE = [
33+
TEST_MESSAGES_BASE = sort_messages([
3434
Message(
3535
# empty
3636
),
@@ -98,20 +98,17 @@ def sort_messages(messages):
9898
arbitration_id=0x768, is_extended_id=False,
9999
timestamp=TEST_TIME + 3.165
100100
),
101-
]
102-
TEST_MESSAGES_BASE = sort_messages(TEST_MESSAGES_BASE)
101+
])
103102

104103

105-
TEST_MESSAGES_REMOTE_FRAMES = [
104+
TEST_MESSAGES_REMOTE_FRAMES = sort_messages([
106105
Message(
107106
arbitration_id=0xDADADA, is_extended_id=True, is_remote_frame=True,
108107
timestamp=TEST_TIME + .165,
109-
data=[1, 2, 3, 4, 5, 6, 7, 8]
110108
),
111109
Message(
112110
arbitration_id=0x123, is_extended_id=False, is_remote_frame=True,
113111
timestamp=TEST_TIME + .365,
114-
data=[254, 255]
115112
),
116113
Message(
117114
arbitration_id=0x768, is_extended_id=False, is_remote_frame=True,
@@ -121,11 +118,10 @@ def sort_messages(messages):
121118
arbitration_id=0xABCDEF, is_extended_id=True, is_remote_frame=True,
122119
timestamp=TEST_TIME + 7858.67
123120
),
124-
]
125-
TEST_MESSAGES_REMOTE_FRAMES = sort_messages(TEST_MESSAGES_REMOTE_FRAMES)
121+
])
126122

127123

128-
TEST_MESSAGES_ERROR_FRAMES = [
124+
TEST_MESSAGES_ERROR_FRAMES = sort_messages([
129125
Message(
130126
is_error_frame=True
131127
),
@@ -137,8 +133,7 @@ def sort_messages(messages):
137133
is_error_frame=True,
138134
timestamp=TEST_TIME + 17.157
139135
)
140-
]
141-
TEST_MESSAGES_ERROR_FRAMES = sort_messages(TEST_MESSAGES_ERROR_FRAMES)
136+
])
142137

143138

144139
TEST_ALL_MESSAGES = sort_messages(TEST_MESSAGES_BASE + TEST_MESSAGES_REMOTE_FRAMES + \

‎test/message_helper.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,21 @@
11
#!/usr/bin/env python
22
# coding: utf-8
33

4+
"""
5+
This module contains a helper for writing test cases that need to compare messages.
6+
"""
7+
48
from __future__ import absolute_import, print_function
59

610
from copy import copy
711

812

913
class ComparingMessagesTestCase(object):
10-
"""Must be extended by a class also extending a unittest.TestCase.
14+
"""
15+
Must be extended by a class also extending a unittest.TestCase.
16+
17+
.. note:: This class does not extend unittest.TestCase so it does not get
18+
run as a test itself.
1119
"""
1220

1321
def __init__(self, allowed_timestamp_delta=0.0, preserves_channel=True):

‎test/notifier_test.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#!/usr/bin/env python
22
# coding: utf-8
3+
34
import unittest
45
import time
56
try:

‎test/test_message_class.py‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
#!/usr/bin/env python
2+
# coding: utf-8
3+
4+
import unittest
5+
import sys
6+
from math import isinf, isnan
7+
from copy import copy, deepcopy
8+
9+
from hypothesis import given, settings, reproduce_failure
10+
import hypothesis.strategies as st
11+
12+
from can import Message
13+
14+
15+
class TestMessageClass(unittest.TestCase):
16+
"""
17+
This test tries many inputs to the message class constructor and then sanity checks
18+
all methods and ensures that nothing crashes. It also checks whether Message._check()
19+
allows all valid can frames.
20+
"""
21+
22+
@given(
23+
timestamp=st.floats(min_value=0.0),
24+
arbitration_id=st.integers(),
25+
is_extended_id=st.booleans(),
26+
is_remote_frame=st.booleans(),
27+
is_error_frame=st.booleans(),
28+
channel=st.one_of(st.text(), st.integers()),
29+
dlc=st.integers(min_value=0, max_value=8),
30+
data=st.one_of(st.binary(min_size=0, max_size=8), st.none()),
31+
is_fd=st.booleans(),
32+
bitrate_switch=st.booleans(),
33+
error_state_indicator=st.booleans()
34+
)
35+
@settings(max_examples=2000)
36+
def test_methods(self, **kwargs):
37+
is_valid = not (
38+
(not kwargs['is_remote_frame'] and (len(kwargs['data'] or []) != kwargs['dlc'])) or
39+
(kwargs['arbitration_id'] >= 0x800 and not kwargs['is_extended_id']) or
40+
kwargs['arbitration_id'] >= 0x20000000 or
41+
kwargs['arbitration_id'] < 0 or
42+
(kwargs['is_remote_frame'] and kwargs['is_error_frame']) or
43+
(kwargs['is_remote_frame'] and len(kwargs['data'] or []) > 0) or
44+
((kwargs['bitrate_switch'] or kwargs['error_state_indicator']) and not kwargs['is_fd']) or
45+
isnan(kwargs['timestamp']) or
46+
isinf(kwargs['timestamp'])
47+
)
48+
49+
# this should return normally and not throw an exception
50+
message = Message(check=is_valid, **kwargs)
51+
52+
if kwargs['data'] is None or kwargs['is_remote_frame']:
53+
kwargs['data'] = bytearray()
54+
55+
if not is_valid and not kwargs['is_remote_frame']:
56+
with self.assertRaises(ValueError):
57+
Message(check=True, **kwargs)
58+
59+
self.assertGreater(len(str(message)), 0)
60+
self.assertGreater(len(message.__repr__()), 0)
61+
if is_valid:
62+
self.assertEqual(len(message), kwargs['dlc'])
63+
self.assertTrue(bool(message))
64+
self.assertGreater(len("{}".format(message)), 0)
65+
_ = "{}".format(message)
66+
with self.assertRaises(Exception):
67+
_ = "{somespec}".format(message)
68+
if sys.version_info.major > 2:
69+
self.assertEqual(bytearray(bytes(message)), kwargs['data'])
70+
71+
# check copies and equalities
72+
if is_valid:
73+
self.assertEqual(message, message)
74+
normal_copy = copy(message)
75+
deep_copy = deepcopy(message)
76+
for other in (normal_copy, deep_copy, message):
77+
self.assertTrue(message.equals(other, timestamp_delta=None))
78+
self.assertTrue(message.equals(other))
79+
self.assertTrue(message.equals(other, timestamp_delta=0))
80+
81+
82+
if __name__ == '__main__':
83+
unittest.main()

0 commit comments

Comments
 (0)