Handle client timeouts in configuration. Ensure to never exceed global timeout + minor tweaks in timeout logic.

Author: pylessard

doc/source/udsoncan/client.rst

@@ -182,6 +182,36 @@ The client configuration must be a dictionary with the following keys defined:
 
    The number of bytes used to encode a data identifier specifically for :ref:`ReadDTCInformation<ReadDTCInformation>` subfunction ``reportDTCSnapshotRecordByDTCNumber`` and ``reportDTCSnapshotRecordByRecordNumber``. The UDS standard does not specify a DID size although all other services expect a DID encoded over 2 bytes (16 bits). Default value of 2
 
+.. _config_timeouts:
+.. _config_request_timeout:
+
+.. attribute:: request_timeout
+   :annotation: (int)
+
+   Maximum amount of time in seconds to wait for a response (positive or negative except NRC 0x78) after sending a request.
+   After this time is elapsed, a TimeoutException will be raised regardless of other timeouts value or previous client response.
+   Ensure an exit path if the ECU keeps requesting to wait.
+   Default value of 5
+
+.. _config_p2_timeout:
+
+.. attribute:: p2_timeout
+   :annotation: (int)
+
+   Maximum amount of time in seconds to wait for a first response (positive, negative, or NRC 0x78). After this time is elapsed, a TimeoutException will be raised if no response has been received.
+   See ISO 14229-2:2013 (UDS Session Layer Services) for more details. 
+   Default value of 1
+
+.. _config_p2_star_timeout:
+
+.. attribute:: p2_star_timeout
+   :annotation: (int)
+
+   Maximum amount of time in seconds to wait for a response (positive, negative, or NRC0x78) after the reception of a negative response with code 0x78
+   (requestCorrectlyReceived-ResponsePending). After this time is elapsed, a TimeoutException will be raised if no response has been received. 
+   See ISO 14229-2:2013 (UDS Session Layer Services) for more details.
+   Default value of 5
+
 -------------
 
 Suppress positive response

test/client/test_client.py

@@ -9,10 +9,10 @@ class TestClient(ClientServerTest):
 	def __init__(self, *args, **kwargs):
 		ClientServerTest.__init__(self, *args, **kwargs)
 
-	def test_timeout(self):
+	def test_timeout_override(self):
 		pass
 
-	def _test_timeout(self):
+	def _test_timeout_override(self):
 		req = Request(service = services.TesterPresent, subfunction=0) 
 		timeout = 0.5
 		try:
@@ -24,13 +24,14 @@ def _test_timeout(self):
 			self.assertGreater(diff, timeout, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
 			self.assertLess(diff, timeout+0.5, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
 
-	def test_param_timeout(self):
+	# Server does not respond. Overall timeout is set smaller than P2 timeout. Overall timeout should trig
+	def test_no_response_overall_timeout(self):
 		pass
 
-	def _test_param_timeout(self):
+	def _test_no_response_overall_timeout(self):
 		req = Request(service = services.TesterPresent, subfunction=0) 
 		timeout = 0.5
-		self.udsclient.request_timeout = timeout
+		self.udsclient.set_configs({'request_timeout': timeout, 'p2_timeout' : 2, 'p2_star_timeout':2})
 		try:
 			t1 = time.time()
 			response = self.udsclient.send_request(req)
@@ -40,8 +41,25 @@ def _test_param_timeout(self):
 			self.assertGreater(diff, timeout, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
 			self.assertLess(diff, timeout+0.5, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
 
-	def test_timeout_pending_response(self):
+	# Server does not respond. P2 timeout is smaller than overall timeout. P2 timeout should trig
+	def test_no_response_p2_timeout(self):
+		pass
 
+	def _test_no_response_p2_timeout(self):
+		req = Request(service = services.TesterPresent, subfunction=0) 
+		timeout = 0.5
+		self.udsclient.set_configs({'request_timeout': 2, 'p2_timeout' : timeout, 'p2_star_timeout':2})
+		try:
+			t1 = time.time()
+			response = self.udsclient.send_request(req)
+			raise Exception('Request did not raise a TimeoutException')
+		except TimeoutException as e:
+			diff = time.time() - t1
+			self.assertGreater(diff, timeout, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
+			self.assertLess(diff, timeout+0.5, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
+
+	#  overall timeout is set to 0.5. Server respond "pendingResponse" for 1 sec. Client should timeout first.
+	def test_overall_timeout_pending_response(self):
 		if not hasattr(self, 'completed'):
 			self.completed = False
 		self.conn.touserqueue.get(timeout=0.2)
@@ -51,17 +69,41 @@ def test_timeout_pending_response(self):
 			self.conn.fromuserqueue.put(response.get_payload())
 			time.sleep(0.1)
 
-	def _test_timeout_pending_response(self):
+	def _test_overall_timeout_pending_response(self):
 		req = Request(service = services.TesterPresent, subfunction=0) 
 		timeout = 0.5
+		self.udsclient.set_configs({'request_timeout': timeout, 'p2_timeout' : 2, 'p2_star_timeout':2})
 		try:
 			t1 = time.time()
-			response = self.udsclient.send_request(req, timeout=timeout)
+			response = self.udsclient.send_request(req)
 			raise Exception('Request did not raise a TimeoutException')
 		except TimeoutException as e:
 			self.assertIsNotNone(self.udsclient.last_response, 'Client never received the PendingResponse message')
+			self.completed = True
+			diff = time.time() - t1
+			self.assertGreater(diff, timeout, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
+			self.assertLess(diff, timeout+0.5, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
 
+	#  Sends 2 "pending response" response to switch to P2* timeout.
+	def test_p2_star_timeout(self):
+		self.conn.touserqueue.get(timeout=0.2)
+		response = Response(service=services.TesterPresent, code=Response.Code.RequestCorrectlyReceived_ResponsePending)
+		self.conn.fromuserqueue.put(response.get_payload())
+		time.sleep(0.1)
+		self.conn.fromuserqueue.put(response.get_payload())
+
+	def _test_p2_star_timeout(self):
+		req = Request(service = services.TesterPresent, subfunction=0) 
+		timeout = 2
+		self.udsclient.set_configs({'request_timeout': 5, 'p2_timeout' : 0.5, 'p2_star_timeout':timeout})
+		try:
+			t1 = time.time()
+			response = self.udsclient.send_request(req)
+			raise Exception('Request did not raise a TimeoutException')
+		except TimeoutException as e:
+			self.assertIsNotNone(self.udsclient.last_response, 'Client never received the PendingResponse message')
 			self.completed = True
 			diff = time.time() - t1
 			self.assertGreater(diff, timeout, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
 			self.assertLess(diff, timeout+0.5, 'Timeout raised after %.3f seconds when it should be %.3f sec' % (diff, timeout))
+

test/client/test_request_download.py

@@ -35,7 +35,7 @@ def test_request_download_config_format(self):
 		self.conn.fromuserqueue.put(b"\x74\x20\xab\xcd")	# Positive response
 
 	def _test_request_download_config_format(self):
-		self.udsclient.config = {'server_address_format':32, 'server_memorysize_format':16}
+		self.udsclient.set_configs({'server_address_format':32, 'server_memorysize_format':16})
 		memloc = MemoryLocation(address=0x1234, memorysize=0xFF)
 		response = self.udsclient.request_download(memory_location=memloc)
 		self.assertEqual(response.service_data.max_length,0xabcd)

test/client/test_request_upload.py

@@ -35,7 +35,7 @@ def test_request_upload_config_format(self):
 		self.conn.fromuserqueue.put(b"\x75\x20\xab\xcd")	# Positive response
 
 	def _test_request_upload_config_format(self):
-		self.udsclient.config = {'server_address_format':32, 'server_memorysize_format':16}
+		self.udsclient.set_configs({'server_address_format':32, 'server_memorysize_format':16})
 		memloc = MemoryLocation(address=0x1234, memorysize=0xFF)
 		response = self.udsclient.request_upload(memory_location=memloc)
 		self.assertEqual(response.service_data.max_length,0xabcd)

udsoncan/client.py

@@ -11,7 +11,7 @@
 
 class Client:
 	"""
-	__init__(self, conn, config=default_client_config, request_timeout = 1)
+	__init__(self, conn, config=default_client_config, request_timeout = None)
 
 	Object that interacts with a UDS server. 
 	It builds a service request, sends it to the server, receives and parses its response, detects communication anomalies and logs what it is doing for further debugging.
@@ -22,17 +22,8 @@ class Client:
 	:param config: The :ref:`client configuration<client_config>`
 	:type config: dict
 
-	:param request_timeout: Maximum amount of time in seconds to wait for the final response (positive or negative except NRC 0x78) after sending a request.
-	After this time is elapsed, a TimeoutException will be raised. If less than or equal to p2_star, NRC 0x78 (requestCorrectlyReceived-ResponsePending) responses are not supported.
+	:param request_timeout: Maximum amount of time to wait for a response. This parameter exists for backward compatibility only. For detailed timeout handling, see :ref:`Client configuration<config_timeouts>`
 	:type request_timeout: int
-
-	:param p2_timeout: Maximum amount of time in seconds to wait for a response (positive, negative, or NRC 0x78). After this time is elapsed, a TimeoutException will be raised.
-	See ISO 14229-2 (UDS Session Layer Services).
-	:type p2_timeout: int
-
-	:param p2_star_timeout: Maximum amount of time in seconds to wait for a response (positive, negative, or NRC0x78) after the reception of a negative response with code 0x78
-	(requestCorrectlyReceived-ResponsePending). After this time is elapsed, a TimeoutException will be raised. See ISO 14229-2 (UDS Session Layer Services).
-	:type p2_star_timeout: int
 	"""
 
 	class SuppressPositiveResponse:
@@ -46,12 +37,13 @@ def __enter__(self):
 		def __exit__(self, type, value, traceback):
 			self.enabled = False
 
-	def __init__(self, conn, config=default_client_config, request_timeout=5, p2_timeout=1, p2_star_timeout=5):
+	def __init__(self, conn, config=default_client_config, request_timeout=None):
 		self.conn = conn
-		self.request_timeout = request_timeout
-		self.p2_timeout = p2_timeout
-		self.p2_star_timeout = p2_star_timeout
 		self.config = dict(config) # Makes a copy of given configuration
+		
+		#For backward compatibility
+		if request_timeout is not None:
+			self.config['request_timeout'] = request_timeout
 		self.suppress_positive_response = Client.SuppressPositiveResponse()
 		self.last_response = None
 
@@ -82,6 +74,10 @@ def set_config(self, key, value):
 		self.config[key] = value
 		self.refresh_config()
 
+	def set_configs(self, dic):
+		for k in dic:
+			self.set_config(k, dic[k])
+
 	def refresh_config(self):
 		self.configure_logger()
 		for k in default_client_config:
@@ -1422,18 +1418,14 @@ def read_dtc_information(self, subfunction, status_mask=None, severity_mask=None
 	def send_request(self, request, timeout=-1):
 		if timeout < 0:
 			# Timeout not provided by user: defaults to Client request_timeout value
-			overall_timeout = self.request_timeout
+			overall_timeout = self.config['request_timeout']
+			single_request_timeout = min(overall_timeout, self.config['p2_timeout'])
 		else:
 			overall_timeout = timeout
-
-		if overall_timeout > self.p2_star_timeout:
-			current_timeout = self.p2_timeout
-			response_pending_supported = True
-		else:
-			# NRC 0x78 (RequestCorrectlyReceived_ResponsePending) not supported
-			current_timeout = overall_timeout
-			response_pending_supported = False
-
+			single_request_timeout = timeout
+		overall_timeout_time = time.time() + overall_timeout
+		using_p2_star = False	# Will switch to true when Nrc 0x78 will be received the first time.
+		
 		self.conn.empty_rxqueue()
 		self.logger.debug("Sending request to server")
 		override_suppress_positive_response = False
@@ -1453,22 +1445,29 @@ def send_request(self, request, timeout=-1):
 
 		done_receiving = False
 
-		if response_pending_supported:
-			overall_timeout_time = time.time() + overall_timeout
-		else:
-			overall_timeout_time = None
-
 		while not done_receiving:
 			done_receiving = True
 			self.logger.debug("Waiting for server response")
+
 			try:
-				payload = self.conn.wait_frame(timeout=current_timeout, exception=True)
+				if time.time() + single_request_timeout < overall_timeout_time:	
+					timeout_type_used 	= 'single_request'
+					timeout_value 		= single_request_timeout
+				else:	
+					timeout_type_used 	= 'overall'
+					timeout_value 		= max(overall_timeout_time - time.time(), 0)
+
+				payload = self.conn.wait_frame(timeout=timeout_value, exception=True)	
 			except TimeoutException:
-				raise TimeoutException('Did not receive response in time (timeout=%.3f sec)' % current_timeout)
+				if timeout_type_used == 'single_request':
+					timeout_name_to_report = 'P2* timeout' if using_p2_star else 'P2 timeout'
+				elif timeout_type_used == 'overall':
+					timeout_name_to_report = 'Global request timeout'
+				else:	# Shouldn't go here.
+					timeout_name_to_report = 'Timeout'
+				raise TimeoutException('Did not receive response in time. %s time has expired (timeout=%.3f sec)' % (timeout_name_to_report, timeout_value))
 			except Exception as e:
 				raise e
-			if overall_timeout_time is not None and overall_timeout_time - time.time() < 0:
-				raise TimeoutException('Did not receive final response in time (request timeout=%.3f sec)' % overall_timeout)
 
 			response = Response.from_payload(payload)
 			self.last_response = response
@@ -1486,13 +1485,12 @@ def send_request(self, request, timeout=-1):
 					self.logger.warning('Given response code "%s" (0x%02x) is not a supported negative response code according to UDS standard.' % (response.code_name, response.code))
 
 				if response.code == Response.Code.RequestCorrectlyReceived_ResponsePending:
-					if response_pending_supported:
-						# Received a 0x78 NRC: timeout is now set to P2*
-						current_timeout = self.p2_star_timeout
 						done_receiving = False
-						self.logger.debug("Server requested to wait with response code %s (0x%02x), timeout is now set to %.3f seconds" % (response.code_name, response.code, current_timeout))
-					else:
-						raise NegativeResponseException(response, "RequestCorrectlyReceived_ResponsePending not supported")
+						if not using_p2_star:
+							# Received a 0x78 NRC: timeout is now set to P2*
+							single_request_timeout = self.config['p2_star_timeout']
+							using_p2_star = True
+							self.logger.debug("Server requested to wait with response code %s (0x%02x), single request timeout is now set to P2* (%.3f seconds)" % (response.code_name, response.code, single_request_timeout))
 				else:
 					raise NegativeResponseException(response)
 

udsoncan/configs.py

@@ -10,5 +10,8 @@
 	'server_address_format'		: None,		# 8,16,24,32,40
 	'server_memorysize_format'	: None,		# 8,16,24,32,40
 	'data_identifiers' 			: {},
-	'input_output' 				: {}
+	'input_output' 				: {},
+	'request_timeout'			: 5,
+	'p2_timeout'				: 1, 
+	'p2_star_timeout'			: 5,
 }