Improved Connector Close Behavior and Auto TCP Reset (#5)

* Update udsoncan adapter to not close the transport by default (that functionality can be re-enabled with a kwarg in the constructor)
* Add kwarg to DoIPClient allowing reconnect/reactivate if the TCP socket breaks
* Cleaned up a few typos in documentation

README.rst

@@ -57,7 +57,7 @@ Updated version of udsoncan's example using `python_doip` instead of IsoTPSocket
    ecu_logical_address = 0x00E0
    doip_client = DoIPClient(ecu_ip, ecu_logical_address)
    conn = DoIPClientUDSConnector(doip_client)
-   with Client(conn,  request_timeout=2, config=MyCar.config) as client:
+   with Client(conn, request_timeout=2, config=MyCar.config) as client:
       try:
          client.change_session(DiagnosticSessionControl.Session.extendedDiagnosticSession)  # integer with value of 3
          client.unlock_security_access(MyCar.debug_level)                                   # Fictive security level. Integer coming from fictive lib, let's say its value is 5
@@ -66,14 +66,24 @@ Updated version of udsoncan's example using `python_doip` instead of IsoTPSocket
          client.ecu_reset(ECUReset.ResetType.hardReset)                                     # HardReset = 0x01
       except NegativeResponseException as e:
          print('Server refused our request for service %s with code "%s" (0x%02x)' % (e.response.service.get_name(), e.response.code_name, e.response.code))
-      except InvalidResponseException, UnexpectedResponseException as e:
+      except (InvalidResponseException, UnexpectedResponseException) as e:
          print('Server sent an invalid payload : %s' % e.response.original_payload)
 
+      # Because we reset our UDS server, we must also reconnect/reactivate the DoIP socket
+      # Alternatively, we could have used the auto_reconnect_tcp flag on the DoIPClient
+      # Note: ECU's do not restart instantly, so you may need a sleep() before moving on
+      doip_client.reconnect()
+      client.tester_present()
+
+   # Cleanup the DoIP Socket when we're done. Alternatively, we could have used the
+   # close_connection flag on conn so that the udsoncan client would clean it up
+   doip_client.close()
+
 python-uds Support
 ------------------
 The `python-uds <https://github.com/richClubb/python-uds>`_ can also be used
 but requires a fork until the owner merges this PR
-`Doip #63 <https://github.com/richClubb/python-uds/pull/63>`. For now, to use
+`Doip #63 <https://github.com/richClubb/python-uds/pull/63>`_. For now, to use
 the port:
 
 using pip::
@@ -92,6 +102,6 @@ Example:
    ecu = Uds(transportProtocol="DoIP", ecu_ip="192.168.1.1", ecu_logical_address=1)
    try:
        response = ecu.send([0x3E, 0x00])
-       print(TesterPresent)  # This should be [0x7E, 0x00]
+       print(response)  # This should be [0x7E, 0x00]
    except:
        print("Send did not complete")

doipclient/client.py

@@ -34,6 +34,9 @@ def __init__(self):
         self.payload = bytearray()
         self._state = Parser.ParserState.READ_PROTOCOL_VERSION     
 
+    def push_bytes(self, data_bytes):
+        self.rx_buffer += data_bytes
+
     def read_message(self, data_bytes):
         self.rx_buffer += data_bytes
         while self.rx_buffer:
@@ -131,6 +134,8 @@ class DoIPClient:
     :type use_secure: bool
     :param log_level: Logging level
     :type log_level: int
+    :param auto_reconnect_tcp: Attempt to automatically reconnect TCP sockets that were closed by peer
+    :type auto_reconnect_tcp: bool
 
     :raises ConnectionRefusedError: If the activation request fails
     """
@@ -146,6 +151,7 @@ def __init__(
         client_logical_address=0x0E00,
         client_ip_address=None,
         use_secure=False,
+        auto_reconnect_tcp=False,
     ):
         self._ecu_logical_address = ecu_logical_address
         self._client_logical_address = client_logical_address
@@ -159,6 +165,7 @@ def __init__(
         self._tcp_parser = Parser()
         self._protocol_version = protocol_version
         self._connect()
+        self._auto_reconnect_tcp = auto_reconnect_tcp
         if self._activation_type is not None:
             result = self.request_activation(self._activation_type)
             if result.response_code != RoutingActivationResponse.ResponseCode.Success:
@@ -170,6 +177,12 @@ class TransportType(IntEnum):
         TRANSPORT_UDP = 1
         TRANSPORT_TCP = 2
 
+    def __enter__(self):
+        return self
+
+    def __exit__(self, type, value, traceback):
+        self.close()
+
     @classmethod
     def await_vehicle_announcement(cls, udp_port=UDP_DISCOVERY, timeout=None):
         """Receive Vehicle Announcement Message
@@ -288,6 +301,14 @@ def send_doip(self, payload_type, payload_data, transport=TransportType.TRANSPOR
         )
         if transport == DoIPClient.TransportType.TRANSPORT_TCP:
             self._tcp_sock.send(data_bytes)
+
+            if self._auto_reconnect_tcp:
+                try:
+                    self._tcp_parser.push_bytes(self._tcp_sock.recv(1024))
+                except (ConnectionResetError, BrokenPipeError):
+                    logger.debug("TCP Connection broken, attempting to reset")
+                    self.reconnect()
+                    self._tcp_sock.send(data_bytes)
         else:
             self._udp_sock.sendto(data_bytes, (self._ecu_ip_address, self._udp_port))
 
@@ -496,6 +517,7 @@ def close(self):
 
     def reconnect(self, close_delay=A_PROCESSING_TIME):
         """Attempts to re-establish the connection. Useful after an ECU reset
+
         :param close_delay: Time to wait between closing and re-opening socket
         :type close_delay: float, optional
         """

doipclient/connectors.py

@@ -11,15 +11,19 @@ class DoIPClientUDSConnector(BaseConnection):
     :param name: This name is included in the logger name so that its output can be redirected. The logger name will be ``Connection[<name>]``
     :type name: string
 
+    :param close_connection: True if the wrapper's close() function should close the associated DoIP client. This is not the default
+    :type name: bool
+
     """
 
-    def __init__(self, doip_layer, name=None):
+    def __init__(self, doip_layer, name=None, close_connection=False):
         BaseConnection.__init__(self, name)
         self._connection = doip_layer
-        self.opened = True
+        self._close_connection = close_connection
+        self.opened = False
 
     def open(self):
-        pass
+        self.opened = True
 
     def __enter__(self):
         return self
@@ -28,7 +32,8 @@ def __exit__(self, type, value, traceback):
         self.close()
 
     def close(self):
-        self._connection.close()
+        if self._close_connection:
+            self._connection.close()
         self.opened = False
 
     def is_open(self):

setup.py

@@ -6,7 +6,7 @@
     long_description = fh.read()
 
 setuptools.setup(name='doipclient',
-      version='1.0.2',
+      version='1.0.3',
       description='A Diagnostic over IP (DoIP) client implementing ISO-13400-2.',
       long_description=long_description,
       author='Jacob Schaer',

tests/test_client.py

@@ -62,17 +62,23 @@ def bind(self, address):
 
     def recv(self, bufflen):
         try:
-            return self.rx_queue.pop(0)
+            result = self.rx_queue.pop(0)
+            if type(result) == bytearray:
+                return result
+            else:
+                raise(result)
         except IndexError:
             raise socket.timeout()
-        self.tx_queue
 
     def send(self, buffer):
         self.tx_queue.append(buffer)
 
     def sendto(self, data_bytes, destination):
         self.tx_queue.append(data_bytes)
 
+    def close(self):
+        pass
+
 @pytest.fixture
 def mock_socket(monkeypatch):
     a = MockSocket()
@@ -203,11 +209,42 @@ def test_does_not_activate_with_none(mock_socket, mocker):
     sut = DoIPClient(test_ip, test_logical_address, activation_type=None)
     assert spy.call_count == 0
 
+def test_resend_reactivate_broken_socket(mock_socket, mocker):
+    request_activation_spy = mocker.spy(DoIPClient, "request_activation")
+    reconnect_spy = mocker.spy(DoIPClient, "reconnect")
+    sut = DoIPClient(test_ip, test_logical_address, auto_reconnect_tcp=True)
+    mock_socket.rx_queue.append(ConnectionResetError(""))
+    mock_socket.rx_queue.append(successful_activation_response)
+    mock_socket.rx_queue.append(diagnostic_positive_response)
+    assert None == sut.send_diagnostic(bytearray([0, 1, 2]))
+    assert request_activation_spy.call_count == 2
+    assert reconnect_spy.call_count == 1
+
+def test_no_resend_reactivate_broken_socket(mock_socket, mocker):
+    request_activation_spy = mocker.spy(DoIPClient, "request_activation")
+    reconnect_spy = mocker.spy(DoIPClient, "reconnect")
+    sut = DoIPClient(test_ip, test_logical_address)
+    mock_socket.rx_queue.append(ConnectionResetError(""))
+    mock_socket.rx_queue.append(successful_activation_response)
+    mock_socket.rx_queue.append(diagnostic_positive_response)
+    with pytest.raises(ConnectionResetError):
+        sut.send_diagnostic(bytearray([0, 1, 2]))
+    assert request_activation_spy.call_count == 1
+    assert reconnect_spy.call_count == 0
+
 def test_connect_with_bind(mock_socket):
     sut = DoIPClient(test_ip, test_logical_address, client_ip_address='192.168.1.1')
     assert mock_socket._bound_ip == '192.168.1.1'
     assert mock_socket._bound_port == 0
 
+def test_context_manager(mock_socket, mocker):
+    close_spy = mocker.spy(DoIPClient, "close")
+    mock_socket.rx_queue.append(diagnostic_positive_response)
+
+    with DoIPClient(test_ip, test_logical_address) as sut:
+        assert None == sut.send_diagnostic(bytearray([0, 1, 2]))
+    assert close_spy.call_count == 1
+
 def test_send_good_activation_request(mock_socket):
     sut = DoIPClient(test_ip, test_logical_address)
     mock_socket.rx_queue.append(successful_activation_response)