device address and pgn request corrections (juergenH87#5)

* Correct device address
Add state check to the function send request
Correct a missing part at the remove timer function

* Addressclaim is allowed without an address.
PGN Addressclaim are allowed for nodes without a device address

* Adapt examples and Tests
Adapt the examples to wait for the device address
Adapt the tests to work with the current version of the library

* Start the controller application before subscribe.

* Subscribe accept now a function

* Revert some unneeded changes

Author: linzbdav

j1939/controller_application.py

@@ -68,7 +68,7 @@ def subscribe(self, callback):
         :param callback:
             Function to call when message is received.
         """
-        self._ecu.subscribe(callback, self._device_address_preferred)
+        self._ecu.subscribe(callback, self.message_acceptable)
 
     def unsubscribe(self, callback):
         """Stop listening for message.
@@ -121,7 +121,7 @@ def remove_timer(self, callback):
         :param callback:
             The callback to be removed from the timer event list
         """
-        remove_timer(callback)
+        self._ecu.remove_timer(callback)
 
     def start(self):
         """Starts the CA
@@ -263,16 +263,23 @@ def send_pgn(self, data_page, pdu_format, pdu_specific, priority, data):
         if self.state != ControllerApplication.State.NORMAL:
             raise RuntimeError("Could not send message unless address claiming has finished")
 
-        self._ecu.send_pgn(data_page, pdu_format, pdu_specific, priority, self._device_address_preferred, data)
+        self._ecu.send_pgn(data_page, pdu_format, pdu_specific, priority, self._device_address, data)
 
     def send_request(self, data_page, pgn, destination):
         """send a request message
         :param int data_page: data page
         :param int pgn: pgn to be requested
         :param list data: destination address
         """
+        if self.state != ControllerApplication.State.NORMAL:
+            if pgn != j1939.ParameterGroupNumber.PGN.ADDRESSCLAIM:
+                raise RuntimeError("Could not send request message unless address claiming has finished")
+            source_address = j1939.ParameterGroupNumber.Address.NULL
+        else:
+            source_address = self._device_address
+
         data = [(pgn & 0xFF), ((pgn >> 8) & 0xFF), ((pgn >> 16) & 0xFF)]
-        self._ecu.send_pgn(data_page, (j1939.ParameterGroupNumber.PGN.REQUEST >> 8) & 0xFF, destination & 0xFF, 6, self._device_address_preferred, data)
+        self._ecu.send_pgn(data_page, (j1939.ParameterGroupNumber.PGN.REQUEST >> 8) & 0xFF, destination & 0xFF, 6, source_address, data)
 
     def _send_address_claimed(self, address):
         # TODO: Normally the (initial) address claimed message must not be an auto repeat message.

j1939/electronic_control_unit.py

@@ -561,7 +561,7 @@ def notify_subscribers(self, priority, pgn, sa, dest, timestamp, data):
         # notify only the CA for which the message is intended
         # each CA receives all broadcast messages
         for dic in self._subscribers:
-            if (dic['dev_adr'] == None) or (dest == j1939.ParameterGroupNumber.Address.GLOBAL) or (dest == dic['dev_adr']):
+            if (dic['dev_adr'] == None) or (dest == j1939.ParameterGroupNumber.Address.GLOBAL) or (callable(dic['dev_adr']) and dic['dev_adr'](dest)) or (dest == dic['dev_adr']):
                 dic['cb'](priority, pgn, sa, timestamp, data)
 
     def add_ca(self, **kwargs):

test/test_ca.py

@@ -50,7 +50,7 @@ def _send_message(self, can_id, data):
         expected_data = self.can_messages.pop(0)
         self.assertEqual(expected_data[0], TestCA.MsgType.CANTX, "No transmission was expected")
         self.assertEqual(can_id, expected_data[1])
-        self.assertSequenceEqual(data, expected_data[2])
+        self.assertSequenceEqual(data[::-1], expected_data[2])
         self._inject_messages_into_ecu()
 
     def _on_message(self, pgn, data):
@@ -64,7 +64,7 @@ def _on_message(self, pgn, data):
         expected_data = self.pdus.pop(0)
         self.assertEqual(expected_data[0], TestCA.MsgType.PDU)
         self.assertEqual(pgn, expected_data[1])
-        self.assertSequenceEqual(data, expected_data[2])
+        self.assertSequenceEqual(data[::-1], expected_data[2])
 
     def setUp(self):
         """Called before each test methode.

test/test_ecu.py

@@ -85,7 +85,10 @@ def _on_message(self, priority, pgn, sa, timestamp, data):
         expected_data = self.pdus.pop(0)
         self.assertEqual(expected_data[0], TestECU.MsgType.PDU)
         self.assertEqual(pgn, expected_data[1])
-        self.assertSequenceEqual(data, expected_data[2])
+        if isinstance(data, list):
+            self.assertSequenceEqual(data, expected_data[2])
+        else:
+            self.assertIsNone(data)
 
     def setUp(self):
         """Called before each test methode.
@@ -128,7 +131,7 @@ def tearDown(self):
     #def test_connect(self):
     #    self.ecu.connect(bustype="virtual", channel=1)
     #    self.ecu.disconnect()
-
+    
     def test_broadcast_receive_short(self):
         """Test the receivement of a normal broadcast message
 
@@ -151,7 +154,7 @@ def test_broadcast_receive_short(self):
         # wait for final processing    
         time.sleep(0.100)
         self.ecu.unsubscribe(self._on_message)
-
+    
     def test_broadcast_receive_long(self):
         """Test the receivement of a long broadcast message
 
@@ -240,15 +243,15 @@ def test_peer_to_peer_send_short(self):
         Its length is 8 Bytes. The contained values are bogous of cause.
         """
         self.can_messages = [
-            (TestECU.MsgType.CANTX, 0x18009B90, [1, 2, 3, 4, 5, 6, 7, 8], 0.0),      # PGN 61440
+            (TestECU.MsgType.CANTX, 0x18F09B90, [1, 2, 3, 4, 5, 6, 7, 8], 0.0),      # PGN 61440
         ]
 
         pdu = (TestECU.MsgType.PDU, 61440, [1, 2, 3, 4, 5, 6, 7, 8])
 
         self.ecu.subscribe(self._on_message)
 
         # sending from 144 to 155 with prio 6
-        self.ecu.send_pgn(0, pdu[1], 155, 6, 144, pdu[2])
+        self.ecu.send_pgn(0, pdu[1]>>8, 155, 6, 144, pdu[2])
 
         # wait until all messages are processed asynchronously
         while len(self.can_messages)>0:
@@ -265,22 +268,26 @@ def test_peer_to_peer_send_long(self):
         Its length is 20 Bytes.
         """
         self.can_messages = [
-            (TestECU.MsgType.CANTX, 0x18EC9B90, [16, 20, 0, 3, 255, 155, 0, 0], 0.0),        # TP.CM RTS 1
-            (TestECU.MsgType.CANRX, 0x1CEC909B, [17, 1, 1, 255, 255, 155, 0, 0], 0.0),       # TP.CM CTS 1
+            (TestECU.MsgType.CANTX, 0x18EC9B90, [16, 20, 0, 3, 255, 0, 223, 0], 0.0),        # TP.CM RTS 1
+            (TestECU.MsgType.CANRX, 0x1CEC909B, [17, 1, 1, 255, 255, 0, 223, 0], 0.0),       # TP.CM CTS 1
             (TestECU.MsgType.CANTX, 0x1CEB9B90, [1, 1, 2, 3, 4, 5, 6, 7], 0.0),              # TP.DT 1
-            (TestECU.MsgType.CANRX, 0x1CEC909B, [17, 1, 2, 255, 255, 155, 0, 0], 0.0),       # TP.CM CTS 2
+            (TestECU.MsgType.CANRX, 0x1CEC909B, [17, 1, 2, 255, 255, 0, 223, 0], 0.0),       # TP.CM CTS 2
             (TestECU.MsgType.CANTX, 0x1CEB9B90, [2, 1, 2, 3, 4, 5, 6, 7], 0.0),              # TP.DT 2
-            (TestECU.MsgType.CANRX, 0x1CEC909B, [17, 1, 3, 255, 255, 155, 0, 0], 0.0),       # TP.CM CTS 3
+            (TestECU.MsgType.CANRX, 0x1CEC909B, [17, 1, 3, 255, 255, 0, 223, 0], 0.0),       # TP.CM CTS 3
             (TestECU.MsgType.CANTX, 0x1CEB9B90, [3, 1, 2, 3, 4, 5, 6, 255], 0.0),            # TP.DT 3
-            (TestECU.MsgType.CANRX, 0x1CEC909B, [19, 20, 0, 3, 255, 155, 0, 0], 0.0),        # TP.CM EOMACK
+            (TestECU.MsgType.CANRX, 0x1CEC909B, [19, 20, 0, 3, 255, 0, 223, 0], 0.0),        # TP.CM EOMACK
+        ]
+
+        self.pdus = [
+            (TestECU.MsgType.PDU, 57088, None),
         ]
 
         pdu = (TestECU.MsgType.PDU, 57088, [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6])
 
         self.ecu.subscribe(self._on_message)
-
+        
         # sending from 144 to 155 with prio 6
-        self.ecu.send_pgn(0, pdu[1], 155, 6, 144, pdu[2])
+        self.ecu.send_pgn(0, pdu[1]>>8, 155, 6, 144, pdu[2])
 
         # wait until all messages are processed asynchronously
         while len(self.can_messages)>0:
@@ -296,7 +303,7 @@ def test_broadcast_send_long(self):
         Its length is 20 Bytes. The contained values are bogous of cause.
         """
         self.can_messages = [
-            (TestECU.MsgType.CANTX, 0x18ECFF90, [32, 20, 0, 3, 255, 255, 176, 0], 0.0),      # TP.BAM
+            (TestECU.MsgType.CANTX, 0x18ECFF90, [32, 20, 0, 3, 255, 176, 254, 0], 0.0),      # TP.BAM
             (TestECU.MsgType.CANTX, 0x1CEBFF90, [1, 1, 2, 3, 4, 5, 6, 7], 0.0),              # TP.DT 1
             (TestECU.MsgType.CANTX, 0x1CEBFF90, [2, 1, 2, 3, 4, 5, 6, 7], 0.0),              # TP.DT 2
             (TestECU.MsgType.CANTX, 0x1CEBFF90, [3, 1, 2, 3, 4, 5, 6, 255], 0.0),            # TP.DT 3
@@ -307,7 +314,7 @@ def test_broadcast_send_long(self):
         self.ecu.subscribe(self._on_message)
 
         # sending from 144 to GLOABL with prio 6
-        self.ecu.send_pgn(0, pdu[1], j1939.ParameterGroupNumber.Address.GLOBAL, 6, 144, pdu[2])
+        self.ecu.send_pgn(0, pdu[1]>>8, pdu[1], 6, 144, pdu[2])
 
         # wait until all messages are processed asynchronously
         while len(self.can_messages)>0: