P2 timeout is not checked correctly when response has consecutive frame #42
Comments
|
Hi, |
|
Hi pylessard, |
|
Thank you for your input. Regards |
|
Hi pylessard, |
|
according to ISO-15765-3, the P2_can_client is defined as: |
|
ISO14229-2:2013(E) defines P2Client as
Figure 9 shows the P2Client timer being stopped at
|
|
Hi all, Is there a standardized timeout/interval specified when performing CAN ISO-TP requests? Specifically, I am thinking of the following intervals to be satisfied by the ECU and tester: It is unclear to me if Further, if they are "dynamic" and depend on the specific ECU implementation, how is the |
|
Hi @MatinF
|
|
Thanks for the clarification. So just to verify that I've understood this correctly:
In regards to the Timeout, it looks like this is up to 1000 ms based on the above I guess what I am unsure about is whether one can rely on the 1000 ms as a standard across all ECUs - or if this is implementation specific. If the latter is the case, do you know if there are some general conventions/good practices to follow in terms of what timings to expect? Thanks! |
|
Hello @MatinF,
|
|
Thanks for the clarification. So just to verify that I've understood this correctly:
Single frame carries an isotp frame in a single CAN message. First Frame is the begining of an isotp frame that requires multiple CAN message. So they are both the beginning of a transmission. They are unrelated.
On a additional note. If you want to be safe in your implementation, you can leave a 1 or 2 millisecond delay where there is no specification. That will ensure maximum compatibility. Regards |
|
Hello @pylessard, Concerning the hard-coded delay, I'm not sure to understand. The transport layer also specifies the STMin parameter (minimum separation time between two consecutive frames), which is communicated to the client/tester by the ECU trough the Flow Control frame (byte 3). Wouldn't it be the responsibility of the ECU to tell the client/tester to lower the communication speed? |
|
Yes, between consecutive frames. (I think) |
|
Just to clarify: When I mention the time between the Single Frame and First Frame, by Single Frame I refer to the "request" frame to be sent by the testing tool (which will have the same format as a Single Frame in the ISO TP context). Hence, my question was related to what the defined max timeout would be before the ECU must send the First Frame response to the request. My understanding here is that this delay may be 1000 ms in theory, but in practice it will most likely be far less (e.g. 1-10 ms). Similarly, the timeout before the Flow Control must be sent by the tester device should be 1000 ms in practice (i.e. Let me know if I missed something here. |
|
Hi @pylessard, I have just faced this issue during testing read dtc request. The expected data flow is as below: Then I got the P2 timeout after 1 second. From my understanding, the lib is waiting for isotp to send receive done notification during waiting for P2 time. However, according to ISO14229-2, the P2 timeout should be the timing between tester request and first response from ECU (which is the first frame). I believe there should be another notification from isotp like start receiving notification to stop the P2 timeout. Reference from ISO14229-2: |
|
Just to let you know that isotp v2.x will be released soon and the possibility of doing a blocking send has been added. This will allow starting the timer at the right moment. I will see how I can bring that api up to the UDS level so it can be used across connections (that supports it) |
|
isotp v2.x has been released. and udsoncan 1.21 supports it. using the new parameter from isotp v2.x |
|
Hi, I believe the problem a still here. I am trying to read DID which is 20b long and get P2Server timeout error. Python 3.11.8 (tags/v3.11.8:db85d51, Feb 6 2024, 22:03:32) [MSC v.1937 64 bit (AMD64)] on win32 pip3 list annotated-types 0.6.0 |
|
you're not having the same issue here. Check this parameter : https://udsoncan.readthedocs.io/en/latest/udsoncan/client.html#use_server_timing |
|
Hello, I have encountered this issue recently and was surprised to see it is a couple of years old and has not been tackled yet. So I took the time to build a sample script to have a concrete example of how the issue occurs and then I looked at the root cause and possible fixes. Test script (using a virtual CAN channel named vcan0)import logging
from threading import Thread, Event
from time import sleep
from can import Notifier, Message
from can.interfaces.socketcan import SocketcanBus
from isotp import Address, AddressingMode
from udsoncan import Request
from udsoncan.client import Client
from isotp.protocol import NotifierBasedCanStack
from udsoncan.connections import PythonIsoTpConnection
reply_is_ready = Event()
def create_connection() -> PythonIsoTpConnection:
bus = SocketcanBus(channel="vcan0", fd=False)
isotp_params = {
'stmin': 10,
'blocksize': 48,
'wftmax': 0,
'tx_padding': 0,
'rx_flowcontrol_timeout': 1000,
'rx_consecutive_frame_timeout': 1000,
'blocking_send': True
}
address = Address(
AddressingMode.Normal_11bits, txid=0x123, rxid=0x321
)
notifier = Notifier(bus, [], timeout=0.1)
stack = NotifierBasedCanStack(bus, notifier, address=address, params=isotp_params)
return PythonIsoTpConnection(stack)
def simulate_reply():
payloads = map(bytes.fromhex, (
"1023620123aaaaaa",
"21bbbbbbbbbbbbbb",
"22cccccccccccccc",
"23dddddddddddddd",
"24eeeeeeeeeeeeee",
"25ffffffffffffff",
))
messages = [
Message(arbitration_id=0x321, dlc=8, is_extended_id=False, is_fd=False, is_rx=False, data=payload)
for payload in payloads
]
with SocketcanBus(channel="vcan0", fd=False) as bus:
reply_is_ready.set()
bus.recv()
for message in messages:
sleep(0.02)
bus.send(message)
def main():
logging.basicConfig(level=logging.INFO)
connection = create_connection()
connection.open()
client = Client(connection, config={"p2_timeout": 0.05})
request = Request.from_payload(b'\x22\x01\x23')
reply_thread = Thread(target=simulate_reply)
reply_thread.start()
reply_is_ready.wait()
client.send_request(request)
reply_thread.join()
client.close()
if __name__ == '__main__':
main()What the script does:
Root cause of the issueThe line that causes the failure is the following one, defined in This wait_frame() call will cause a TimeoutException when timeout_value is P2_timeout (default) and the Response from the UDS Server is a multi-frame one. That is because that method simply calls Looking inside the TransportLayerLogic class, the Queue that is being used for the previously mentioned The problem lies in the fact that no data is added to rx_queue until all CAN Frames of a multi-frame message have been received and the upper layer (UDS Client) has no knowledge that the Server has sent the First Frame within the expected P2 timeout and is in the process of sending the Consecutive Frames. The one field that gets populated after a First Frame is received (and validated) is actual_rxdl. A possible solutionThe quick solution I came up with is to add a secondary wait_frame call in case the first one times out and actual_rxdl is not None. Changing from this: try:
recv_payload = self.conn.wait_frame(timeout=timeout_value, exception=True)
except TimeoutException:
timed_out = True
except Exception as e:
raise eTo this: try:
recv_payload = self.conn.wait_frame(timeout=timeout_value, exception=True)
except TimeoutException:
timed_out = True
except Exception as e:
raise e
if isinstance(self.conn, PythonIsoTpConnection):
actual_rxdl = self.conn.subconn.isotp_layer.actual_rxdl
else:
actual_rxdl = None
if timed_out and actual_rxdl:
timed_out = False
try:
recv_payload = self.conn.wait_frame(timeout=overall_timeout, exception=True)
except TimeoutException:
timed_out = True
except Exception as e:
raise eHowever, this solution is not usable for anything other than a PythonIsoTpConnection (as far as I can tell), since the other connection types don't have an isotp_layer that has the actual_rxdl field to check for. Also, this definitely feels like a hack and my proposal for a better solution would be to have a What do you think? |
|
Hi, The problem is whether you start the timer at the beginning of the transmission or at the end. The actual situation is that existing transports layer aren't able to provide a reliable feedback when the transmission is complete. I implemented a blocking send feature inside my isotp TransportLayer (see here). If you enable that, it will improve because send() will block until the transmission is finished before calling This being said, we would also need to change the interface of the connection to add something that tells if the message is fully transmitted and wait for that if supported. I never dived into implementing such thing since the low-level layers are not reliable in this regard... and the problem can be worked around quite easily by extending the timeout. It all boils down to the modularity of the design and trying to have all the layers independent, yet compatible. You may not see that kind of issue if you use a library that implements all layers in a single stack. Something maybe like what Vector and Peak can offer... but that mean you're stuck with their ecosystem |
Partially correct:
It is enabled, please look carefully at the sample script I provided. :) The send has nothing to do with the root issue. If you don't want to have a retry in the case I explained in detail above, then what needs to change is for the P2, P2* and Request timeout values to be handled by the Connection, not the Client, since it is that low level layer dealing with the Single Frame vs First Frame + Consecutive Frame situation causing the issue today. |
|
I get what you mean. Check ISO-14229 I'd rather improve the whole stack to add the missing
|
|
"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." (Source: https://udsoncan.readthedocs.io/en/latest/udsoncan/client.html#p2_timeout) Yes, you are correct about starting the P2 timer/timeout period only after doing the send, which is what that blocking_send parameter you added handles. However, I see you are still not getting the point, that the P2 timer shall be cancelled when a first response is received, which does happen in the sample code I provided (payload: 1023620123aaaaaa), yet without the proposed changes, the Request will always fail, because the P2 timeout is not disregarded at the point when the First Frame is received. No matter how you look at it, in the example I provided, by sending each frame at 20 ms, it is well bellow the 50 ms target, and so the P2 timeout exception should never occur. The problem with your understanding is that you expect the entire response message (FF + all CFs) to be received in the P2 timeout period, which is not what the spec, the documentation of your project and the various snippets attached to the issue mention. |
|
I also think you are not getting the point :) UDS has a mechanism to respond with a message that say, "acknowledge, will respond later". That is the first response. P2 timeout applies to that message, then P2* for subsequent messages. Nothing to do with how the underlying protocol segment a big message in smaller frames The diagram cannot be more clear about when to starts timers |
|
Hello, After taking a step back and looking at all of your explanations throughout this thread, I think I see your point. UDS is a higher level protocol that can be implemented over CAN (DoCAN) or Ethernet (DoIP), and as such timings related to it should not be directly tied to timings of the lower layer protocol. So, when a UDS Server sends back timings (P2 and P2*) during a Response to a Change Diagnostic Session Request, the Server is supposed to honor that P2 timing going forward for all future requests. In order to do that, it needs to ensure that, no matter what shape the UDS payload is packaged in (Single Frame or First Frame + Consecutive Frames), those all need to arrive to the Client within P2 timeout period, otherwise it is supposed to use NRC 0x78 to signal to the Client that it should switch to P2* timeout as will take longer to send the entire UDS payload to it. Is that correct? |
|
Yes, that is the how the protocol is designed. |
In client.py, function send_request wait for entire response with timeout is P2 timeout. But as I know, P2 timeout is timeout to the first response(can be first frame) not entire response. So with long response, timeout error will occur
The text was updated successfully, but these errors were encountered: