move j1939 (pgn, prio, source) handling direct to arbitration-id class

(#238)

Author: ebroecker

src/canmatrix/canmatrix.py

@@ -613,6 +613,20 @@ def pgn(self):
             _pgn += ps
         return _pgn
 
+    @pgn.setter
+    def pgn(self, value):  # type: (int) -> None
+        self.extended = True
+        ps = value & 0xff
+        pf = (value >> 8) & 0xFF
+        _pgn = pf << 8
+        if pf >= 240:
+            _pgn += ps
+
+        self.id &= 0xff0000ff
+        self.id |= (_pgn & 0xffff) << 8  # default pgn is None -> mypy reports error
+
+
+
     @property
     def j1939_tuple(self):  # type: () -> typing.Tuple[int, int, int]
         """Get tuple (destination, PGN, source)
@@ -637,6 +651,11 @@ def j1939_source(self):
             raise J1939needsExtendedIdetifier
         return self.id & 0xFF
 
+    @j1939_source.setter
+    def j1939_source(self, value):  # type: (int) -> None
+        self.extended = True
+        self.id = (self.id & 0xffffff00) | (value & 0xff)
+
     @property
     def j1939_ps(self):
         if not self.extended:
@@ -659,7 +678,12 @@ def j1939_edp(self):
     def j1939_priority(self):
         if not self.extended:
             raise J1939needsExtendedIdetifier
-        return (self.id >> 25) & 0x7
+        return (self.id >> 26) & 0x7
+
+    @j1939_priority.setter
+    def j1939_priority(self, value):  # type: (int) -> None
+        self.extended = True
+        self.id = (self.id & 0x2ffffff) | ((value & 0x7) << 26)
 
     @property
     def j1939_str(self):  # type: () -> str
@@ -733,9 +757,6 @@ class Frame(object):
     receivers = attr.ib(factory=list)  # type: typing.MutableSequence[str]
     signalGroups = attr.ib(factory=list)  # type: typing.MutableSequence[SignalGroup]
 
-    j1939_pgn = attr.ib(default=0)  # type: typing.Optional[int]
-    j1939_source = attr.ib(default=0)  # type: int
-    j1939_prio = attr.ib(default=0)  # type: int
     is_j1939 = attr.ib(default=False)  # type: bool
     # ('cycleTime', '_cycleTime', int, None),
     # ('sendType', '_sendType', str, None),
@@ -787,39 +808,28 @@ def pgn(self):  # type: () -> int
 
     @pgn.setter
     def pgn(self, value):  # type: (int) -> None
-        self.j1939_pgn = value
-        self.recalc_J1939_id()
-        self.j1939_pgn = self.arbitration_id.pgn
+        self.arbitration_id.pgn = value
 
     @property
     def priority(self):  # type: () -> int
         """Get J1939 priority."""
-        return self.j1939_prio
+        return self.arbitration_id.j1939_prio
 
     @priority.setter
     def priority(self, value):  # type: (int) -> None
         """Set J1939 priority."""
-        self.j1939_prio = value
-        self.recalc_J1939_id()
+        self.arbitration_id.j1939_priority = value
 
     @property
     def source(self):  # type: () -> int
         """Get J1939 source."""
-        return self.j1939_source
+        return  self.arbitration_id.j1939_source
 
     @source.setter
     def source(self, value):  # type: (int) -> None
         """Set J1939 source."""
-        self.j1939_source = value
-        self.recalc_J1939_id()
-
-    def recalc_J1939_id(self):  # type: () -> None
-        """Recompute J1939 ID"""
-        self.arbitration_id.id = self.j1939_source & 0xff
-        self.arbitration_id.id += (self.j1939_pgn & 0xffff) << 8  # default pgn is None -> mypy reports error
-        self.arbitration_id.id += (self.j1939_prio & 0x7) << 26
-        self.arbitration_id.extended = True
-        self.is_j1939 = True
+        self.arbitration_id.j1939_source = value
+
 
     # @property
     # def cycleTime(self):

src/canmatrix/tests/test_canmatrix.py

@@ -557,32 +557,15 @@ def test_frame_not_multiplexed():
     frame.add_signal(canmatrix.canmatrix.Signal(name="some"))
     assert not frame.is_multiplexed
 
-def test_j1939_frame_construction():
-    frame = canmatrix.canmatrix.Frame()
-    frame.source = 0x22
-    frame.pgn = 0xAAAA
-    frame.priority = 3
-    assert frame.pgn == frame.j1939_pgn
-    assert frame.source == frame.j1939_source
-    assert frame.priority == frame.j1939_prio
-
 def test_frame_calc_j1939_id():
     # we have to set all j1939 properties in the __init__ otherwise the setters crash
-    frame = canmatrix.canmatrix.Frame(j1939_source=0x11, j1939_pgn=0xFFFF, j1939_prio=0)
+    frame = canmatrix.canmatrix.Frame()
     frame.source = 0x22
     frame.pgn = 0xAAAA
     frame.priority = 3
     assert hex(frame.arbitration_id.id) == hex(0xcaa0022)
 
 
-def test_frame_get_j1939_properties():
-    frame = canmatrix.canmatrix.Frame(j1939_source=0x11, j1939_pgn=0xFFFF, j1939_prio=1)
-    frame.recalc_J1939_id()  # pgn property is computed from id!
-    assert frame.pgn == frame.j1939_pgn
-    assert frame.source == frame.j1939_source
-    assert frame.priority == frame.j1939_prio
-
-
 def test_frame_add_transmitter(empty_frame):
     empty_frame.add_transmitter("BCM")
     assert empty_frame.transmitters == ["BCM"]
@@ -822,7 +805,14 @@ def test_arbitration_id_is_instance():
     assert frame1.arbitration_id.id == 42
     assert frame2.arbitration_id.id == 0
 
-
+def test_arbitration_id_j1939_direct_setters():
+    arb_id = canmatrix.ArbitrationId(0)
+    arb_id.pgn = 0xF1AA
+    arb_id.j1939_source = 0x22
+    arb_id.j1939_priority = 3
+    assert arb_id.pgn == 0xF1AA
+    assert arb_id.j1939_source == 0x22
+    assert arb_id.j1939_priority == 3
 
 @pytest.fixture
 def empty_matrix():