Fix wwhobd dtc subfunctions (#278)

Author: pylessard

test/ClientServerTest.py

@@ -1,10 +1,17 @@
 from udsoncan.connections import QueueConnection
 from test.ThreadableTest import ThreadableTest
 from udsoncan.client import Client
+from typing import Optional
 
 class ClientServerTest(ThreadableTest):
+    _standard_version:Optional[int]
+
     def __init__(self, *args, **kwargs):
+        self._standard_version = None
         ThreadableTest.__init__(self, *args, **kwargs)
+    
+    def set_standard(self, version:int):
+        self._standard_version = version
 
     def setUp(self):
         self.conn = QueueConnection(name='unittest', mtu=4095)
@@ -15,6 +22,8 @@ def clientSetUp(self):
         self.udsclient.set_config('exception_on_invalid_response', True)
         self.udsclient.set_config('exception_on_unexpected_response', True)
         self.udsclient.set_config('exception_on_negative_response', True)
+        if self._standard_version is not None:
+            self.udsclient.set_config('standard_version', self._standard_version)
 
         self.udsclient.open()
         if hasattr(self, "postClientSetUp"):

test/client/test_read_dtc_information.py

@@ -2347,26 +2347,31 @@ def __init__(self, *args, **kwargs):
 class TestReportMirrorMemoryDTCByStatusMask(ClientServerTest, GenericTestStatusMaskRequest_DtcAndStatusMaskResponse):	# Subfn = 0xF
     def __init__(self, *args, **kwargs):
         ClientServerTest.__init__(self, *args, **kwargs)
+        self.set_standard(2013) # Removed starting form 2020
         GenericTestStatusMaskRequest_DtcAndStatusMaskResponse.__init__(self, subfunction=0xf, client_function = 'get_mirrormemory_dtc_by_status_mask')
 
 class TestReportMirrorMemoryDTCExtendedDataRecordByDTCNumber(ClientServerTest, GenericReportExtendedDataByRecordNumber):	# Subfn = 0x10
     def __init__(self, *args, **kwargs):
         ClientServerTest.__init__(self, *args, **kwargs)
+        self.set_standard(2013) # Removed starting form 2020
         GenericReportExtendedDataByRecordNumber.__init__(self, subfunction=0x10, client_function = 'get_mirrormemory_dtc_extended_data_by_dtc_number')
 
 class TestReportNumberOfMirrorMemoryDTCByStatusMask(ClientServerTest, GenericTest_RequestStatusMask_ResponseNumberOfDTC):	# Subfn = 0x11
     def __init__(self, *args, **kwargs):
         ClientServerTest.__init__(self, *args, **kwargs)
+        self.set_standard(2013) # Removed starting form 2020
         GenericTestStatusMaskRequest_DtcAndStatusMaskResponse.__init__(self, subfunction=0x11, client_function = 'get_mirrormemory_number_of_dtc_by_status_mask')
 
 class TestReportNumberOfEmissionsRelatedOBDDTCByStatusMask(ClientServerTest, GenericTest_RequestStatusMask_ResponseNumberOfDTC):	# Subfn = 0x12
     def __init__(self, *args, **kwargs):
         ClientServerTest.__init__(self, *args, **kwargs)
+        self.set_standard(2013) # Removed starting form 2020
         GenericTestStatusMaskRequest_DtcAndStatusMaskResponse.__init__(self, subfunction=0x12, client_function = 'get_number_of_emission_dtc_by_status_mask')
 
 class TestReportEmissionsRelatedOBDDTCByStatusMask(ClientServerTest, GenericTestStatusMaskRequest_DtcAndStatusMaskResponse):	# Subfn = 0x13
     def __init__(self, *args, **kwargs):
         ClientServerTest.__init__(self, *args, **kwargs)
+        self.set_standard(2013) # Removed starting form 2020
         GenericTestStatusMaskRequest_DtcAndStatusMaskResponse.__init__(self, subfunction=0x13, client_function = 'get_emission_dtc_by_status_mask')
 
 class TestReportDTCFaultDetectionCounter(ClientServerTest):	# Subfn = 0x14
@@ -2605,9 +2610,6 @@ def _test_functional_group_verification(self):
         with self.assertRaises(ValueError):
             self.udsclient.get_wwh_obd_dtc_by_status_mask(None, status_mask=2, severity_mask=0xA0, dtc_class=4)
 
-        with self.assertRaises(ValueError):
-            self.udsclient.get_wwh_obd_dtc_by_status_mask(0xff, status_mask=2, severity_mask=0xA0, dtc_class=4)
-
     def assert_no_data_response_with_severity_class(self, response):
         self.assertEqual(len(response.service_data.dtcs), 0)
         self.assertEqual(response.service_data.dtc_count, 0)
@@ -2732,8 +2734,6 @@ def _test_functional_group_verification(self):
         with self.assertRaises(ValueError):
             self.udsclient.get_wwh_obd_dtc_with_permanent_status(None)
 
-        with self.assertRaises(ValueError):
-            self.udsclient.get_wwh_obd_dtc_with_permanent_status(0xff)
 
     def assert_no_data_response_with_severity_class(self, response):
         self.assertEqual(len(response.service_data.dtcs), 0)
@@ -3092,10 +3092,10 @@ def test_record_number_out_of_range_response_exception(self):
         pass
 
     def _test_record_number_out_of_range_response_exception(self):
-        response = Response(service = ReadDTCInformation, code = Response.Code.PositiveResponse, data = self.sb + b'\xF0\x12\x34\x56\x20\x01\x02\x03\x04\x05')
+        response = Response(service = ReadDTCInformation, code = Response.Code.PositiveResponse, data = self.sb + b'\xF0\x12\x34\x56\x20\x01\x02\x03\x04')
 
         with self.assertRaises(InvalidResponseException):  
-            # Do not go thourgh the client because out of range would raised at request time.
+            # Do not go through the client because out of range would raised at request time.
             ReadDTCInformation.interpret_response(response, ReadDTCInformation.Subfunction.reportDTCExtDataRecordByRecordNumber, extended_data_size={0x123456 : 5})
 
     def test_duplicate_dtc(self):
@@ -3126,7 +3126,7 @@ def _test_oob_values(self):
             self.udsclient.get_dtc_extended_data_by_record_number(record_number = 'asd', data_size=5)
 
         with self.assertRaises(NotImplementedError):
-            self.udsclient.set_config('standard_version', 2013)
+            self.udsclient.set_config('standard_version', 2006)
             self.udsclient.get_dtc_extended_data_by_record_number(record_number = 0x33, data_size=5)
         self.udsclient.set_config('standard_version', latest_standard)    
 
@@ -3417,7 +3417,7 @@ def _test_oob_value(self):
             self.udsclient.get_user_defined_memory_dtc_by_status_mask(0x12, 'aaa')
 
         with self.assertRaises(NotImplementedError):
-            self.udsclient.set_config('standard_version', 2013)
+            self.udsclient.set_config('standard_version', 2006)
             self.udsclient.get_user_defined_memory_dtc_by_status_mask(0x10, 0x20)
         self.udsclient.set_config('standard_version', latest_standard)
 
@@ -3857,7 +3857,7 @@ def _test_oob_values(self):
             self.udsclient.get_user_defined_dtc_snapshot_by_dtc_number(dtc=0x123456, record_number=0x02, memory_selection = 0x100)
 
         with self.assertRaises(NotImplementedError):
-            self.udsclient.set_config('standard_version', 2013)
+            self.udsclient.set_config('standard_version', 2006)
             self.udsclient.get_user_defined_dtc_snapshot_by_dtc_number(dtc=0x123456, record_number=0x02, memory_selection = 0x99)
         self.udsclient.set_config('standard_version', latest_standard)
 
@@ -4182,7 +4182,7 @@ def _test_oob_values(self):
 
 
         with self.assertRaises(NotImplementedError):
-            self.udsclient.set_config('standard_version', 2013)
+            self.udsclient.set_config('standard_version', 2006)
             self.udsclient.get_user_defined_dtc_extended_data_by_dtc_number(dtc=0x123456, data_size=3, record_number=0x99, memory_selection=0x88)  
         self.udsclient.set_config('standard_version', latest_standard)          
 

udsoncan/__init__.py

@@ -26,10 +26,12 @@
 __author__ = 'Pier-Yves Lessard'
 
 latest_standard = 2020
+valid_standards = [2006,2013,2020]
+
 __default_log_config_file = path.join(path.dirname(path.abspath(__file__)), 'logging.conf')
 
 
-def setup_logging(config_file=__default_log_config_file):
+def setup_logging(config_file:str=__default_log_config_file):
     """
     This function setup the logger accordingly to the module provided cfg file
     """

udsoncan/client.py

@@ -14,7 +14,8 @@
 
 from udsoncan.exceptions import *
 from udsoncan.configs import default_client_config
-from udsoncan.typing import ClientConfig, TypedDict
+from udsoncan.typing import ClientConfig
+from udsoncan import valid_standards
 import logging
 import binascii
 import functools
@@ -160,7 +161,7 @@ def refresh_config(self) -> None:
         self.validate_config()
 
     def validate_config(self) -> None:
-        if self.config['standard_version'] not in [2006, 2013, 2020]:
+        if self.config['standard_version'] not in valid_standards:
             raise ConfigError('Valid standard versions are 2006, 2013, 2020. %s is not supported' % self.config['standard_version'])
 
     # Decorator to apply on functions that the user will call.

udsoncan/services/ReadDTCInformation.py

@@ -1,5 +1,5 @@
 import struct
-from udsoncan import Dtc, check_did_config, make_did_codec_from_definition, fetch_codec_definition_from_config, latest_standard, DIDConfig
+from udsoncan import Dtc, check_did_config, make_did_codec_from_definition, fetch_codec_definition_from_config, latest_standard, valid_standards, DIDConfig
 from udsoncan.Request import Request
 from udsoncan.Response import Response
 from udsoncan.exceptions import *
@@ -168,10 +168,12 @@ def assert_extended_data_size_int_or_dict(cls,
                 tools.validate_int(extended_data_size[dtcid], min=0, max=0xFFF, name='Extended data size for DTC=0x%06x' % dtcid)
 
     @classmethod
-    def assert_functional_group_id(cls, functional_group_id: Optional[int], subfunction) -> None:
+    def assert_functional_group_id(cls, functional_group_id: Optional[int], subfunction:int) -> None:
         if functional_group_id is None:
             raise ValueError('functional_group_id must be provided for subfunction 0x%02x' % subfunction)
-        tools.validate_int(functional_group_id, min=0, max=0xFE, name='Functional Group ID')
+        # ISO-14229:2020 Disallow a value of 0xFF. But ISO-27145-3 Defines it as "All functional system groups".
+        # Allowing 0xFF because of the ambiguity
+        tools.validate_int(functional_group_id, min=0, max=0xFF, name='Functional Group ID')    
 
     @classmethod
     def pack_dtc(cls, dtcid: int) -> bytes:
@@ -180,6 +182,9 @@ def pack_dtc(cls, dtcid: int) -> bytes:
     @classmethod
     def check_subfunction_valid(cls, subfunction: int, standard_version: int = latest_standard) -> None:
         tools.validate_int(subfunction, min=1, max=0xFF, name='Subfunction')
+        if standard_version not in valid_standards:
+            raise ValueError(f"Standard version {standard_version} is not valid")
+
         vlist = vars(cls)
         ok = True
         for v in vlist:
@@ -190,20 +195,33 @@ def check_subfunction_valid(cls, subfunction: int, standard_version: int = lates
             raise ValueError('Unknown subfunction : 0x%02x', subfunction)
 
         # These subfunction have been added in the 2020 version of the standard
-        subfunction2020 = [
-            cls.Subfunction.reportUserDefMemoryDTCByStatusMask,
-            cls.Subfunction.reportDTCExtDataRecordByRecordNumber,
-            cls.Subfunction.reportUserDefMemoryDTCSnapshotRecordByDTCNumber,
-            cls.Subfunction.reportUserDefMemoryDTCExtDataRecordByDTCNumber,
-            cls.Subfunction.reportSupportedDTCExtDataRecord,
-            cls.Subfunction.reportWWHOBDDTCByMaskRecord,
-            cls.Subfunction.reportWWHOBDDTCWithPermanentStatus,
-            cls.Subfunction.reportDTCInformationByDTCReadinessGroupIdentifier,
-        ]
-
-        if subfunction in subfunction2020 and standard_version < 2020:
-            raise NotImplementedError('The subfunction 0x%02x has been introduced in standard version 2020 but decoding is requested to be done as per %d version' % (
-                subfunction, standard_version))
+        subfunction_disallow_map:Dict[int, List[int]] = {
+            2006: [
+                cls.Subfunction.reportDTCExtDataRecordByRecordNumber,
+                cls.Subfunction.reportUserDefMemoryDTCByStatusMask,
+                cls.Subfunction.reportUserDefMemoryDTCSnapshotRecordByDTCNumber,
+                cls.Subfunction.reportUserDefMemoryDTCExtDataRecordByDTCNumber,
+                #cls.Subfunction.reportDTCExtendedDataRecordIdentification, # Not implemented
+                cls.Subfunction.reportWWHOBDDTCByMaskRecord,
+                cls.Subfunction.reportWWHOBDDTCWithPermanentStatus,
+                cls.Subfunction.reportDTCInformationByDTCReadinessGroupIdentifier,
+            ],
+            2013: [
+                #cls.Subfunction.reportDTCExtendedDataRecordIdentification, # Not implemented
+                cls.Subfunction.reportDTCInformationByDTCReadinessGroupIdentifier,
+            ],
+            2020: [
+                cls.Subfunction.reportMirrorMemoryDTCByStatusMask,
+                #cls.Subfunction.reportMirrorMemoryDTCExtDataRecordByDTCNumber, # Not implemented
+                cls.Subfunction.reportNumberOfMirrorMemoryDTCByStatusMask,
+                #cls.Subfunction.reportNumberOfEmissionsOBDDTCByStatusMask, # Not implemented
+                #cls.Subfunction.reportEmissionsOBDDTCByStatusMask, # Not implemented
+            ]
+        }
+
+        if standard_version in subfunction_disallow_map:
+            if subfunction in subfunction_disallow_map[standard_version]:
+                raise NotImplementedError(f"Subfunction 0x{subfunction:02x} is not allowed by ISO-14229:{standard_version}. Check for a different version of the standard")
 
     @classmethod
     def make_request(cls,
@@ -248,6 +266,8 @@ def make_request(cls,
 
 
         :raises ValueError: If parameters are out of range, missing or wrong type
+        :raises NotImplementedError: If the requested subfunction is not supported by the active ISO-14229 standard version
+
         """
 
         # Request grouping for subfunctions that have the same request format
@@ -391,11 +411,11 @@ def make_request(cls,
         elif subfunction == ReadDTCInformation.Subfunction.reportWWHOBDDTCByMaskRecord:
             cls.assert_status_mask(status_mask, subfunction)
             cls.assert_severity_mask(severity_mask, subfunction)
-            cls.assert_functional_group_id(functional_group_id, subfunction)  # Maximum specified by ISO-14229:2020 
+            cls.assert_functional_group_id(functional_group_id, subfunction)
             req.data = struct.pack('BBB', functional_group_id, status_mask, severity_mask)
 
         elif subfunction == ReadDTCInformation.Subfunction.reportWWHOBDDTCWithPermanentStatus:
-            cls.assert_functional_group_id(functional_group_id, subfunction)  # Maximum specified by ISO-14229:2020 
+            cls.assert_functional_group_id(functional_group_id, subfunction)
             req.data = struct.pack('B', functional_group_id)
 
         return req
@@ -852,9 +872,6 @@ def interpret_response(cls,
                 raise InvalidResponseException(response, 'Incomplete response from server. Missing DTCExtDataRecordNumber')
 
             record_number = int(response.data[1])
-            if record_number > 0xEF:
-                raise InvalidResponseException(
-                    response, 'Server returned a RecordNumber of %d which is out of range (00-EF) according to ISO-14229:2020', record_number)
 
             actual_byte = 2
             received_dtcs = set()
@@ -931,9 +948,8 @@ def interpret_response(cls,
             else:
                 raise NotImplementedError("Unreachable code")
 
-            if response.service_data.functional_group_id > 0xFE:
-                raise InvalidResponseException(response, "FunctionalGroupIdentifier returned by the server is not smaller or equal than 0xFE")
-            
+            # Don't check functional_group_id on purpose. range 0 to FF is accepted
+
             if response.service_data.dtc_format not in [Dtc.Format.SAE_J2012_DA_DTCFormat_04, Dtc.Format.SAE_J1939_73]:
                 raise InvalidResponseException(response, "DTCFormatIdentifier returned by the server is not one of the following: SAE_J2012-DA_DTCFormat_04 (4), SAE_J1939-73_DTCFormat(2). Got 0x%02x" % response.service_data.dtc_format)