Fix broken isotp socket connection (#201)

pylessard · web-flow · commit d90acfd9ae92 · 2024-01-22T21:33:37.000-05:00
diff --git a/doc/source/udsoncan/questions_answers.rst b/doc/source/udsoncan/questions_answers.rst
@@ -77,4 +77,24 @@ What makes a DTC permanent?
 How can I contribute?
 ---------------------
 
-Create a Github issue, fork the project, propose a pull request and I will review it; that's the best way.
+.. epigraph::
+
+    Create a Github issue, fork the project, propose a pull request and I will review it; that's the best way.
+
+-----
+
+My IsoTPSocketConnection raises an error after updating udsoncan
+----------------------------------------------------------------
+
+.. epigraph::
+
+    With a breaking change of the isotp v2 socket module, it was necessary to change the signature of the :class:`IsoTPSocketConnection<udsoncan.connections.IsoTPSocketConnection>`. 
+    The change has been carried in v1.21.2. It is not possible to pass ``rxid`` and ``txid`` parameter. A full ``isotp.Address`` must be provided.
+
+.. code-block:: python
+    
+    # Before 1.21
+    IsoTPSocketConnection('vcan0', rxid=123, txid=456)  
+
+    # After 1.21
+    IsoTPSocketConnection('vcan0', isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=123, txid=456)) 
diff --git a/test/stub.py b/test/stub.py
@@ -3,49 +3,72 @@
 import queue
 import logging
 import socket
+from dataclasses import dataclass
+
 
 class StubbedIsoTPSocket(object):
     conns = {}
 
     def __init__(self, name=None, timeout=1):
         self.bound = False
-        self.interface=None
-        self.rxid=None
-        self.txid=None
+        self.interface = None
+        self.address = None
         self.timeout = timeout
 
-        self.queue_in = queue.Queue()	# Client reads from this queue. Other end is simulated
+        self.queue_in = queue.Queue()  # Client reads from this queue. Other end is simulated
 
-    def bind(self, interface, rxid, txid):
+    def bind(self, interface, address):
         self.interface = interface
-        self.rxid = rxid
-        self.txid = txid
+        self.address = address
         self.bound = True
-        sockkey = (self.interface, self.rxid, self.txid)
+        sockkey = (self.interface, self.address)
         if sockkey not in StubbedIsoTPSocket.conns:
             StubbedIsoTPSocket.conns[sockkey] = dict()
         StubbedIsoTPSocket.conns[sockkey][id(self)] = self
 
     def close(self):
-        self.bound=False
-        sockkey = (self.interface, self.rxid, self.txid)
+        self.bound = False
+        sockkey = (self.interface, self.address)
         if sockkey in StubbedIsoTPSocket.conns:
             if id(self) in StubbedIsoTPSocket.conns[sockkey]:
                 del StubbedIsoTPSocket.conns[sockkey][id(self)]
         while not self.queue_in.empty():
             self.queue_in.get()
 
+    def must_receive(self, dst_interface: str, srcaddr: "isotp.Address", dstaddr: "isotp.Address"):
+        if dst_interface != self.interface:
+            return False
+
+        @dataclass
+        class StubbedCanMsg:
+            is_extended_id: bool
+            arbitration_id: int
+            data: bytes
+
+        # Simulate an isotp.CanMessage to work with addresses
+        msg = StubbedCanMsg(
+            is_extended_id=srcaddr.is_tx_29bits(),
+            arbitration_id=srcaddr.get_tx_arbitration_id(),
+            data=bytes([srcaddr.get_tx_extension_byte()]) if srcaddr.requires_tx_extension_byte() else bytes()
+        )
+
+        if dstaddr.is_for_me(msg):
+            return True
+
+        return False
+
     def send(self, payload):
-        target_sockkey = (self.interface, self.txid, self.rxid)
-        if target_sockkey in StubbedIsoTPSocket.conns:
-            for sockid in StubbedIsoTPSocket.conns[target_sockkey]:
-                StubbedIsoTPSocket.conns[target_sockkey][sockid].queue_in.put(payload)
+        # if target_sockkey in StubbedIsoTPSocket.conns:
+        for target_sockkey in StubbedIsoTPSocket.conns:
+            dst_interface = target_sockkey[0]
+            dstaddr = target_sockkey[1]
+            if self.must_receive(dst_interface, self.address, dstaddr):
+                for sockid in StubbedIsoTPSocket.conns[target_sockkey]:
+                    StubbedIsoTPSocket.conns[target_sockkey][sockid].queue_in.put(payload)
 
     def recv(self):
         try:
             payload = self.queue_in.get(block=True, timeout=self.timeout)
         except queue.Empty:
-            raise socket.timeout 
+            raise socket.timeout
         return payload
-
-
diff --git a/test/test_connection.py b/test/test_connection.py
@@ -12,7 +12,7 @@
     import isotp
     import can
     s = isotp.socket()
-    s.bind(_interface_name,rxid=1,txid=2)
+    s.bind(_interface_name, isotp.Address(rxid=1, txid=2))
     s.close()
     _STACK_POSSIBLE = True
 except Exception as e:
@@ -25,34 +25,39 @@
 except Exception as e:
     _AISOTP_POSSIBLE = False
 
+
 class TestIsoTPSocketConnection(UdsTest):
 
     def setUp(self):
         self.tpsock1 = StubbedIsoTPSocket(timeout=0.1)
         self.tpsock2 = StubbedIsoTPSocket(timeout=0.1)
 
     def test_open(self):
-        conn = IsoTPSocketConnection(interface='vcan0', rxid=0x001, txid=0x002, tpsock=self.tpsock1, name='unittest')
+        addr = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x001, txid=0x002)
+        conn = IsoTPSocketConnection(interface='vcan0', address=addr, tpsock=self.tpsock1, name='unittest')
         self.assertFalse(conn.is_open())
         conn.open()
         self.assertTrue(conn.is_open())
         conn.close()
         self.assertFalse(conn.is_open())
 
     def test_transmit(self):
-        conn1 = IsoTPSocketConnection(interface='vcan0', rxid=0x100, txid=0x101, tpsock=self.tpsock1, name='unittest')
-        conn2 = IsoTPSocketConnection(interface='vcan0', rxid=0x101, txid=0x100, tpsock=self.tpsock2, name='unittest')
+        addr1 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x100, txid=0x101)
+        addr2 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x101, txid=0x100)
+        conn1 = IsoTPSocketConnection(interface='vcan0', address=addr1, tpsock=self.tpsock1, name='unittest')
+        conn2 = IsoTPSocketConnection(interface='vcan0', address=addr2, tpsock=self.tpsock2, name='unittest')
 
         with conn1.open():
             with conn2.open():
                 payload1 = b"\x00\x01\x02\x03\x04"
                 conn1.send(payload1)
-                payload2 = conn2.wait_frame(timeout=0.3)
+                payload2 = conn2.wait_frame(timeout=0.3, exception=True)
                 self.assertEqual(payload1, payload2)
 
+
 class TestSocketConnection(UdsTest):
     def server_sock_thread_task(self):
-        self.thread_started=True
+        self.thread_started = True
         self.sock1, addr = self.server_sock.accept()
 
     def setUp(self):
@@ -112,6 +117,7 @@ def test_transmit(self):
                 payload2 = conn2.wait_frame(timeout=1, exception=True)
                 self.assertEqual(payload1, payload2)
 
+
 class TestQueueConnection(UdsTest):
     def setUp(self):
         self.conn = QueueConnection(name='unittest')
@@ -136,7 +142,7 @@ def test_send(self):
         self.assertEqual(frame, payload)
 
     def test_truncate(self):
-        payload = b"\x00\x01\x02\x03"*5000
+        payload = b"\x00\x01\x02\x03" * 5000
         self.conn.send(payload)
         frame = self.conn.touserqueue.get()
         self.assertEqual(len(frame), 4095)
@@ -161,48 +167,48 @@ def test_reopen(self):
 
         self.assertTrue(self.conn.touserqueue.empty())
 
+
 @unittest.skipIf(_STACK_POSSIBLE == False, 'Cannot test TestPythonIsoTpConnection. %s' % _STACK_UNVAILABLE_REASON)
 class TestPythonIsoTpConnection(UdsTest):
     def __init__(self, *args, **kwargs):
         UdsTest.__init__(self, *args, **kwargs)
         if not hasattr(self.__class__, '_next_id'):
-            self.__class__._next_id=1
+            self.__class__._next_id = 1
 
         self.stack_txid = self.__class__._next_id
-        self.stack_rxid = self.__class__._next_id +1
+        self.stack_rxid = self.__class__._next_id + 1
         self.__class__._next_id += 2
 
     def make_bus(self):
         return can.interface.Bus(bustype='socketcan', channel='vcan0', bitrate=500000, receive_own_messages=True)
 
     def setUp(self):
         self.vcan0_bus = self.make_bus()
+        self.reader = can.BufferedReader()
+        self.notifier = can.Notifier(self.vcan0_bus, [self.reader])
         addr = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=self.stack_rxid, txid=self.stack_txid)
-        self.conn = PythonIsoTpConnection(isotp.CanStack(bus=self.vcan0_bus, address=addr), name='unittest')
+        self.conn = PythonIsoTpConnection(isotp.NotifierBasedCanStack(bus=self.vcan0_bus, notifier=self.notifier, address=addr), name='unittest')
         self.conn.open()
 
     def test_open(self):
         self.assertTrue(self.conn.is_open())
 
     def test_receive(self):
-        self.vcan0_bus.send(can.Message(arbitration_id = self.stack_rxid, data =  b"\x03\x01\x02\x03", is_extended_id = False))
+        self.vcan0_bus.send(can.Message(arbitration_id=self.stack_rxid, data=b"\x03\x01\x02\x03", is_extended_id=False))
         frame = self.conn.wait_frame(timeout=1)
         self.assertEqual(frame, b"\x01\x02\x03")
 
     def test_send(self):
         self.conn.send(b"\xAA\xBB\xCC\xDD\xEE\xFF")
-        t1 = time.time()
-        msg = self.vcan0_bus.recv(1)
+        msg = self.reader.get_message(1)
         self.assertIsNotNone(msg)
         self.assertEqual(msg.data, b'\x06\xAA\xBB\xCC\xDD\xEE\xFF')
 
     def test_reopen(self):
         self.conn.send(b"\x0A\x0B\x0C\x0D")
-        self.vcan0_bus.send(can.Message(arbitration_id = self.stack_rxid, data =  b"\x03\x01\x02\x03", is_extended_id = False))
+        self.vcan0_bus.send(can.Message(arbitration_id=self.stack_rxid, data=b"\x03\x01\x02\x03", is_extended_id=False))
         self.conn.close()
-        self.vcan0_bus.shutdown()
-        self.vcan0_bus = self.make_bus()
-        self.conn.open(bus=self.vcan0_bus)
+        self.conn.open()
 
         with self.assertRaises(TimeoutException):
             self.conn.wait_frame(timeout=0.05, exception=True)
@@ -211,8 +217,10 @@ def test_reopen(self):
 
     def tearDown(self):
         self.conn.close()
+        self.notifier.stop()
         self.vcan0_bus.shutdown()
 
+
 @unittest.skipIf(_AISOTP_POSSIBLE == False, "aisotp module is not present.")
 class TestSyncAioIsotpConnection(UdsTest):
 
@@ -232,7 +240,7 @@ def test_open(self):
     def test_transmit(self):
         conn0 = SyncAioIsotpConnection(interface="virtual", channel=0, bitrate=500000, rx_id=0x123, tx_id=0x456, name="unittest")
         conn1 = SyncAioIsotpConnection(interface="virtual", channel=0, bitrate=500000, rx_id=0x456, tx_id=0x123, name="unittest")
-    
+
         with conn0.open():
             with conn1.open():
                 tx_data = bytes([i for i in range(256)])
diff --git a/test/test_stubbed_isotpsock.py b/test/test_stubbed_isotpsock.py
@@ -1,22 +1,34 @@
+import unittest
 from test.UdsTest import UdsTest
 from test.stub import StubbedIsoTPSocket
 from udsoncan.exceptions import *
 import socket
 
+try:
+    import isotp
+    _isotp_available = True
+except ImportError:
+    _isotp_available = False
+
+
+@unittest.skipIf(_isotp_available == False, "isotp module not available")
 class TestStubbedIsoTPSocket(UdsTest):
     def test_open(self):
         tpsock = StubbedIsoTPSocket()
         self.assertFalse(tpsock.bound)
-        tpsock.bind(interface='vcan0', rxid=0x100, txid=0x101)
+        addr = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x100, txid=0x101)
+        tpsock.bind(interface='vcan0', address=addr)
         self.assertTrue(tpsock.bound)
         tpsock.close()
         self.assertFalse(tpsock.bound)
 
     def test_transmit(self):
         tpsock1 = StubbedIsoTPSocket()
         tpsock2 = StubbedIsoTPSocket(timeout=0.5)
-        tpsock1.bind(interface='vcan0', rxid=0x200, txid=0x201)
-        tpsock2.bind(interface='vcan0', rxid=0x201, txid=0x200)
+        addr1 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x200, txid=0x201)
+        addr2 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x201, txid=0x200)
+        tpsock1.bind(interface='vcan0', address=addr1)
+        tpsock2.bind(interface='vcan0', address=addr2)
 
         payload1 = b"\x01\x02\x03\x04"
         tpsock1.send(payload1)
@@ -27,9 +39,13 @@ def test_multicast(self):
         tpsock1 = StubbedIsoTPSocket()
         tpsock2 = StubbedIsoTPSocket(timeout=0.5)
         tpsock3 = StubbedIsoTPSocket(timeout=0.5)
-        tpsock1.bind(interface='vcan0', rxid=0x300, txid=0x301)
-        tpsock2.bind(interface='vcan0', rxid=0x301, txid=0x300)
-        tpsock3.bind(interface='vcan0', rxid=0x301, txid=0x300)
+        addr1 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x300, txid=0x301)
+        addr2 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x301, txid=0x300)
+        addr3 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x301, txid=0x300)
+
+        tpsock1.bind(interface='vcan0', address=addr1)
+        tpsock2.bind(interface='vcan0', address=addr2)
+        tpsock3.bind(interface='vcan0', address=addr3)
 
         payload1 = b"\x01\x02\x03\x04"
         tpsock1.send(payload1)
@@ -41,8 +57,10 @@ def test_multicast(self):
     def test_empty_on_close(self):
         tpsock1 = StubbedIsoTPSocket()
         tpsock2 = StubbedIsoTPSocket(timeout=0.2)
-        tpsock1.bind(interface='vcan0', rxid=0x400, txid=0x401)
-        tpsock2.bind(interface='vcan0', rxid=0x401, txid=0x400)
+        addr1 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x400, txid=0x401)
+        addr2 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x401, txid=0x400)
+        tpsock1.bind(interface='vcan0', address=addr1)
+        tpsock2.bind(interface='vcan0', address=addr2)
 
         payload = b"\x01\x02\x03\x04"
         tpsock1.send(payload)
@@ -54,12 +72,13 @@ def test_empty_on_close(self):
     def test_no_listener(self):
         tpsock1 = StubbedIsoTPSocket()
         tpsock2 = StubbedIsoTPSocket(timeout=0.2)
-        tpsock1.bind(interface='vcan0', rxid=0x400, txid=0x401)
+        addr1 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x400, txid=0x401)
+        addr2 = isotp.Address(isotp.AddressingMode.Normal_11bits, rxid=0x401, txid=0x400)
+        tpsock1.bind(interface='vcan0', address=addr1)
 
         payload = b"\x01\x02\x03\x04"
         tpsock1.send(payload)
-        tpsock2.bind(interface='vcan0', rxid=0x401, txid=0x400)
+        tpsock2.bind(interface='vcan0', address=addr2)
 
         with self.assertRaises(socket.timeout):
             tpsock2.recv()
-
diff --git a/udsoncan/connections.py b/udsoncan/connections.py
