From f26a6b3f9b81a8a7459ee06135075f10e59f1c6a Mon Sep 17 00:00:00 2001 From: Jacob Schaer Date: Sun, 26 Sep 2021 17:39:46 -0700 Subject: [PATCH] Message representation (#7) * Added forgotten test case for reconnection on graceful shutdown. * Added parent class for all message types implementing repr/str/eq * Fixed possible issue with VIN being either str or bytes --- doipclient/messages.py | 143 ++++++++++++++++---- tests/test_client.py | 300 ++++++++++++++++++++++------------------- 2 files changed, 279 insertions(+), 164 deletions(-) diff --git a/doipclient/messages.py b/doipclient/messages.py index c13f35f..c897770 100644 --- a/doipclient/messages.py +++ b/doipclient/messages.py @@ -4,7 +4,42 @@ # Quoted descriptions were copied or paraphrased from ISO-13400-2-2019 (E). -class ReservedMessage: +class DoIPMessage: + """Base class for DoIP messages implementing common features like comparison, + and representation""" + + def __repr__(self): + formatted_field_values = [] + for field in self._fields: + value = getattr(self, "_" + field) + if type(value) == str: + formatted_field_values.append(f'"{value}"') + else: + formatted_field_values.append(str(value)) + args = ", ".join(formatted_field_values) + classname = type(self).__name__ + return f"{classname}({args})" + + def __str__(self): + formatted_field_values = [] + for field in self._fields: + value = getattr(self, field) + if type(value) == str: + formatted_field_values.append(f'{field}: "{value}"') + else: + formatted_field_values.append(f"{field} : {str(value)}") + args = ", ".join(formatted_field_values) + classname = type(self).__name__ + if args: + return f"{classname} (0x{self.payload_type:X}): {{ {args} }}" + else: + return f"{classname} (0x{self.payload_type:X})" + + def __eq__(self, other): + return (type(self) == type(other)) and (self.pack() == other.pack()) + + +class ReservedMessage(DoIPMessage): """DoIP message whose payload ID is reserved either for manufacturer use or future expansion of DoIP protocol""" @@ -15,6 +50,8 @@ def unpack(cls, payload_type, payload_bytes, payload_length): def pack(self): self._payload + _fields = ["payload_type", "payload"] + def __init__(self, payload_type, payload): self._payload_type = payload_type self._payload = payload @@ -30,7 +67,7 @@ def payload_type(self): return self._payload_type -class GenericDoIPNegativeAcknowledge: +class GenericDoIPNegativeAcknowledge(DoIPMessage): """Generic header negative acknowledge structure. See Table 18""" payload_type = 0x0000 @@ -51,6 +88,8 @@ def unpack(cls, payload_bytes, payload_length): def pack(self): return struct.pack("!B", self._nack_code) + _fields = ["nack_code"] + def __init__(self, nack_code): self._nack_code = nack_code @@ -65,11 +104,13 @@ def nack_code(self): return self._nack_code -class AliveCheckRequest: +class AliveCheckRequest(DoIPMessage): """Alive check request - Table 27""" payload_type = 0x0007 + _fields = [] + @classmethod def unpack(cls, payload_bytes, payload_length): return AliveCheckRequest() @@ -78,7 +119,7 @@ def pack(self): return bytearray() -class AliveCheckResponse: +class AliveCheckResponse(DoIPMessage): """Alive check resopnse - Table 28""" payload_type = 0x0008 @@ -90,6 +131,8 @@ def unpack(cls, payload_bytes, payload_length): def pack(self): return struct.pack("!H", self._source_address) + _fields = ["source_address"] + def __init__(self, source_address): self._source_address = source_address @@ -113,11 +156,13 @@ def source_address(self): return self._source_address -class DoipEntityStatusRequest: +class DoipEntityStatusRequest(DoIPMessage): """DoIP entity status request - Table 10""" payload_type = 0x4001 + _fields = [] + @classmethod def unpack(cls, payload_bytes, payload_length): return DoipEntityStatusRequest() @@ -126,11 +171,13 @@ def pack(self): return bytearray() -class DiagnosticPowerModeRequest: +class DiagnosticPowerModeRequest(DoIPMessage): """Diagnostic power mode information request - Table 8""" payload_type = 0x4003 + _fields = [] + @classmethod def unpack(cls, payload_bytes, payload_length): return DiagnosticPowerModeRequest() @@ -139,11 +186,13 @@ def pack(self): return bytearray() -class DiagnosticPowerModeResponse: +class DiagnosticPowerModeResponse(DoIPMessage): """Diagnostic power mode information response - Table 9""" payload_type = 0x4004 + _fields = ["diagnostic_power_mode"] + class DiagnosticPowerMode(IntEnum): """Diagnostic power mode - See Table 9""" @@ -174,11 +223,13 @@ def diagnostic_power_mode(self): ) -class RoutingActivationRequest: +class RoutingActivationRequest(DoIPMessage): """Routing activation request. Table 46""" payload_type = 0x0005 + _fields = ["source_address", "activation_type", "reserved", "vm_specific"] + class ActivationType(IntEnum): """See Table 47 - Routing activation request activation types""" @@ -253,11 +304,13 @@ def vm_specific(self): return self._vm_specific -class VehicleIdentificationRequest: +class VehicleIdentificationRequest(DoIPMessage): """Vehicle identification request message. See Table 2""" payload_type = 0x0001 + _fields = [] + @classmethod def unpack(cls, payload_bytes, payload_length): return VehicleIdentificationRequest() @@ -266,11 +319,13 @@ def pack(self): return bytearray() -class VehicleIdentificationRequestWithEID: +class VehicleIdentificationRequestWithEID(DoIPMessage): """Vehicle identification request message with EID. See Table 3""" payload_type = 0x0002 + _fields = ["eid"] + @classmethod def unpack(cls, payload_bytes, payload_length): return VehicleIdentificationRequestWithEID( @@ -294,11 +349,13 @@ def eid(self): return self._eid -class VehicleIdentificationRequestWithVIN: +class VehicleIdentificationRequestWithVIN(DoIPMessage): """Vehicle identification request message with VIN. See Table 4""" payload_type = 0x0003 + _fields = ["vin"] + @classmethod def unpack(cls, payload_bytes, payload_length): return VehicleIdentificationRequestWithVIN( @@ -322,14 +379,25 @@ def vin(self): Values: ASCII """ - return self._vin.decode("ascii") + if type(self._vin) is bytes: + return self._vin.decode("ascii") + else: + return self._vin -class RoutingActivationResponse: +class RoutingActivationResponse(DoIPMessage): """Payload type routing activation response.""" payload_type = 0x0006 + _fields = [ + "client_logical_address", + "logical_address", + "response_code", + "reserved", + "vm_specific", + ] + class ResponseCode(IntEnum): """See Table 48""" @@ -440,7 +508,7 @@ def vm_specific(self): return self._vm_specific -class DiagnosticMessage: +class DiagnosticMessage(DoIPMessage): """Diagnostic Message - see Table 21 "Payload type diagnostic message structure" Description: Wrapper for diagnostic (UDS) payloads. The same message is used for @@ -450,6 +518,8 @@ class DiagnosticMessage: payload_type = 0x8001 + _fields = ["source_address", "target_address", "user_data"] + @classmethod def unpack(cls, payload_bytes, payload_length): return DiagnosticMessage( @@ -517,7 +587,7 @@ def user_data(self): return self._user_data -class DiagnosticMessageNegativeAcknowledgement: +class DiagnosticMessageNegativeAcknowledgement(DoIPMessage): """A negative acknowledgement of the previously received diagnostic (UDS) message. Indicates that the previously received diagnostic message was rejected. Reasons could @@ -528,6 +598,8 @@ class DiagnosticMessageNegativeAcknowledgement: payload_type = 0x8003 + _fields = ["source_address", "target_address", "nack_code", "previous_message_data"] + class NackCodes(IntEnum): """Diagnostic message negative acknowledge codes (See Table 26)""" @@ -615,7 +687,7 @@ def previous_message_data(self): return None -class DiagnosticMessagePositiveAcknowledgement: +class DiagnosticMessagePositiveAcknowledgement(DoIPMessage): """A positive acknowledgement of the previously received diagnostic (UDS) message. "...indicates a correctly received diagnostic message, which is processed and put into the transmission @@ -626,6 +698,8 @@ class DiagnosticMessagePositiveAcknowledgement: payload_type = 0x8002 + _fields = ["source_address", "target_address", "ack_code", "previous_message_data"] + @classmethod def unpack(cls, payload_bytes, payload_length): return DiagnosticMessagePositiveAcknowledgement( @@ -702,11 +776,18 @@ def previous_message_data(self): return None -class EntityStatusResponse: +class EntityStatusResponse(DoIPMessage): """DoIP entity status response. Table 11""" payload_type = 0x4002 + _fields = [ + "node_type", + "max_concurrent_sockets", + "currently_open_sockets", + "max_data_size", + ] + @classmethod def unpack(cls, payload_bytes, payload_length): if payload_length == 3: @@ -794,11 +875,20 @@ def max_data_size(self): return self._max_data_size -class VehicleIdentificationResponse: +class VehicleIdentificationResponse(DoIPMessage): """Payload type vehicle announcement/identification response message Table 5""" payload_type = 0x0004 + _fields = [ + "vin", + "logical_address", + "eid", + "gid", + "further_action_required", + "vin_sync_status", + ] + class SynchronizationStatusCodes(IntEnum): """VIN/GID synchronization status code values (Table 7) @@ -834,7 +924,7 @@ def unpack(cls, payload_bytes, payload_length): ) def pack(self): - if self._vin_gid_sync_status is not None: + if self._vin_sync_status is not None: return struct.pack( "!17sH6s6sBB", self._vin.encode("ascii"), @@ -842,7 +932,7 @@ def pack(self): self._eid, self._gid, self._further_action_required, - self._vin_gid_sync_status, + self._vin_sync_status, ) else: return struct.pack( @@ -861,14 +951,14 @@ def __init__( eid, gid, further_action_required, - vin_gid_sync_staus=None, + vin_gid_sync_status=None, ): self._vin = vin self._logical_address = logical_address self._eid = eid self._gid = gid self._further_action_required = further_action_required - self._vin_gid_sync_status = vin_gid_sync_staus + self._vin_sync_status = vin_gid_sync_status @property def vin(self): @@ -880,7 +970,10 @@ def vin(self): Values: ASCII """ - return self._vin.decode("ascii") + if type(self._vin) is bytes: + return self._vin.decode("ascii") + else: + return self._vin @property def logical_address(self): @@ -945,9 +1038,9 @@ def vin_sync_status(self): Description: "This is the additional information to notify the client DoIP entity that all DoIP entities have synchronized their information about the VIN or GID of the vehicle" """ - if self._vin_gid_sync_status is not None: + if self._vin_sync_status is not None: return VehicleIdentificationResponse.SynchronizationStatusCodes( - self._vin_gid_sync_status + self._vin_sync_status ) else: return None diff --git a/tests/test_client.py b/tests/test_client.py index f74ae4c..02c8f63 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -170,145 +170,145 @@ def mock_construct(*args, **kwargs): yield a -@pytest.mark.parametrize( - "message, fields", - [ - ( - VehicleIdentificationResponse, - [ - ("vin", "1" * 17), - ("logical_address", 1234), - ("eid", b"1" * 6), - ("gid", b"1" * 6), - ("further_action_required", 0x10), - ], - ), - ( - VehicleIdentificationResponse, - [ - ("vin", "1" * 17), - ("logical_address", 1234), - ("eid", b"1" * 6), - ("gid", b"1" * 6), - ("further_action_required", 0x10), - ("vin_sync_status", None), - ], - ), - ( - VehicleIdentificationResponse, - [ - ("vin", "1" * 17), - ("logical_address", 1234), - ("eid", b"2" * 6), - ("gid", b"2" * 6), - ("further_action_required", 0x00), - ("vin_sync_status", 0x10), - ], - ), - ( - EntityStatusResponse, - [ - ("node_type", 0x01), - ("max_concurrent_sockets", 13), - ("currently_open_sockets", 5), - ], - ), - ( - EntityStatusResponse, - [ - ("node_type", 0x00), - ("max_concurrent_sockets", 1), - ("currently_open_sockets", 28), - ("max_data_size", 0xFFF), - ], - ), - (GenericDoIPNegativeAcknowledge, [("nack_code", 1)]), - (VehicleIdentificationRequest, []), - (VehicleIdentificationRequestWithEID, [("eid", b"2" * 6)]), - (VehicleIdentificationRequestWithVIN, [("vin", "1" * 17)]), - ( - RoutingActivationRequest, - [ - ("source_address", 0x00E0), - ("activation_type", 1), - ], - ), - ( - RoutingActivationRequest, - [ - ("source_address", 0x00E0), - ("activation_type", 1), - ("reserved", 0), - ("vm_specific", 0x1234), - ], - ), - ( - RoutingActivationResponse, - [ - ("client_logical_address", 0x00E0), - ("logical_address", 1), - ("response_code", 0), - ], - ), - ( - RoutingActivationResponse, - [ - ("client_logical_address", 0x00E0), - ("logical_address", 1), - ("response_code", 0), - ("reserved", 0), - ("vm_specific", 0x1234), - ], - ), - (AliveCheckRequest, []), - (AliveCheckResponse, [("source_address", 0x00E0)]), - (DoipEntityStatusRequest, []), - (DiagnosticPowerModeRequest, []), - (DiagnosticPowerModeResponse, [("diagnostic_power_mode", 0x01)]), - ( - DiagnosticMessage, - [ - ("source_address", 0x00E0), - ("target_address", 0x00E0), - ("user_data", bytearray([0, 1, 2, 3])), - ], - ), - ( - DiagnosticMessagePositiveAcknowledgement, - [ - ("source_address", 0x00E0), - ("target_address", 0x00E0), - ("ack_code", 0), - ], - ), - ( - DiagnosticMessagePositiveAcknowledgement, - [ - ("source_address", 0x00E0), - ("target_address", 0x00E0), - ("ack_code", 0), - ("previous_message_data", bytearray([1, 2, 3])), - ], - ), - ( - DiagnosticMessageNegativeAcknowledgement, - [ - ("source_address", 0x00E0), - ("target_address", 0x00E0), - ("nack_code", 2), - ], - ), - ( - DiagnosticMessageNegativeAcknowledgement, - [ - ("source_address", 0x00E0), - ("target_address", 0x00E0), - ("nack_code", 2), - ("previous_message_data", bytearray([1, 2, 3])), - ], - ), - ], -) +parameterized_class_fields = [ + ( + VehicleIdentificationResponse, + [ + ("vin", "1" * 17), + ("logical_address", 1234), + ("eid", b"1" * 6), + ("gid", b"1" * 6), + ("further_action_required", 0x10), + ], + ), + ( + VehicleIdentificationResponse, + [ + ("vin", "1" * 17), + ("logical_address", 1234), + ("eid", b"1" * 6), + ("gid", b"1" * 6), + ("further_action_required", 0x10), + ("vin_sync_status", None), + ], + ), + ( + VehicleIdentificationResponse, + [ + ("vin", "1" * 17), + ("logical_address", 1234), + ("eid", b"2" * 6), + ("gid", b"2" * 6), + ("further_action_required", 0x00), + ("vin_sync_status", 0x10), + ], + ), + ( + EntityStatusResponse, + [ + ("node_type", 0x01), + ("max_concurrent_sockets", 13), + ("currently_open_sockets", 5), + ], + ), + ( + EntityStatusResponse, + [ + ("node_type", 0x00), + ("max_concurrent_sockets", 1), + ("currently_open_sockets", 28), + ("max_data_size", 0xFFF), + ], + ), + (GenericDoIPNegativeAcknowledge, [("nack_code", 1)]), + (VehicleIdentificationRequest, []), + (VehicleIdentificationRequestWithEID, [("eid", b"2" * 6)]), + (VehicleIdentificationRequestWithVIN, [("vin", "1" * 17)]), + ( + RoutingActivationRequest, + [ + ("source_address", 0x00E0), + ("activation_type", 1), + ], + ), + ( + RoutingActivationRequest, + [ + ("source_address", 0x00E0), + ("activation_type", 1), + ("reserved", 0), + ("vm_specific", 0x1234), + ], + ), + ( + RoutingActivationResponse, + [ + ("client_logical_address", 0x00E0), + ("logical_address", 1), + ("response_code", 0), + ], + ), + ( + RoutingActivationResponse, + [ + ("client_logical_address", 0x00E0), + ("logical_address", 1), + ("response_code", 0), + ("reserved", 0), + ("vm_specific", 0x1234), + ], + ), + (AliveCheckRequest, []), + (AliveCheckResponse, [("source_address", 0x00E0)]), + (DoipEntityStatusRequest, []), + (DiagnosticPowerModeRequest, []), + (DiagnosticPowerModeResponse, [("diagnostic_power_mode", 0x01)]), + ( + DiagnosticMessage, + [ + ("source_address", 0x00E0), + ("target_address", 0x00E0), + ("user_data", bytearray([0, 1, 2, 3])), + ], + ), + ( + DiagnosticMessagePositiveAcknowledgement, + [ + ("source_address", 0x00E0), + ("target_address", 0x00E0), + ("ack_code", 0), + ], + ), + ( + DiagnosticMessagePositiveAcknowledgement, + [ + ("source_address", 0x00E0), + ("target_address", 0x00E0), + ("ack_code", 0), + ("previous_message_data", bytearray([1, 2, 3])), + ], + ), + ( + DiagnosticMessageNegativeAcknowledgement, + [ + ("source_address", 0x00E0), + ("target_address", 0x00E0), + ("nack_code", 2), + ], + ), + ( + DiagnosticMessageNegativeAcknowledgement, + [ + ("source_address", 0x00E0), + ("target_address", 0x00E0), + ("nack_code", 2), + ("previous_message_data", bytearray([1, 2, 3])), + ], + ), +] + + +@pytest.mark.parametrize("message, fields", parameterized_class_fields) def test_packer_unpackers(mock_socket, message, fields): values = [x for _, x in fields] a = message(*values) @@ -318,6 +318,15 @@ def test_packer_unpackers(mock_socket, message, fields): assert getattr(b, field_name) == field_value +@pytest.mark.parametrize("message, fields", parameterized_class_fields) +def test_repr(mock_socket, message, fields): + values = [x for _, x in fields] + a = message(*values) + print(repr(a)) + print(str(a)) + assert eval(repr(a)) == a + + def test_does_not_activate_with_none(mock_socket, mocker): spy = mocker.spy(DoIPClient, "request_activation") mock_socket.rx_queue = [] @@ -325,6 +334,19 @@ def test_does_not_activate_with_none(mock_socket, mocker): assert spy.call_count == 0 +def test_resend_reactivate_closed_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(bytearray()) + 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 + assert mock_socket.timeout == 2 + + def test_resend_reactivate_broken_socket(mock_socket, mocker): request_activation_spy = mocker.spy(DoIPClient, "request_activation") reconnect_spy = mocker.spy(DoIPClient, "reconnect")