struct.error on change_session interpret_response

jeanot1314 commented 7 months ago

Test with PCAN hardware.

I am starting to play with udsoncan.

I had no problem to send ecu_reset to my target but when I tried the change_session method from DiagnosticSessionControl.py with number 1/2/3 I catched an error "struct.error: unpack requires a buffer of 4 bytes"

(a, b) = struct.unpack('>HH', response.data[1:])

My communication on CAN is : 02 10 02 00 00 00 00 00 as Tx 07 50 02 00 32 01 F4 0A as Rx. it seem that we enter unpack with the 5 last bytes and create the problem

If I change with (a, b) = struct.unpack('>HH', response.data[2:]) everything work fine.

Do you have any ideas why there is 5 bytes controlled when the server send 8 bytes?

Best regards

pylessard commented 7 months ago

EDIT. The below answer is wrong. I was confused

Hi
You title talks about "interpret_response" but in your code example, you decode the response yourself. Not sure what,s going on here.

For the explanation, your can response can be dissected like this:
 - 07  - isotp header. length of 7 bytes
 - 50 - service ID with bit 7 set to 1  (0x40 | 0x10)
 - 0200 - DID id in a 16bits field
 - 3201f404 : you did payload, which is indeed 4 bytes long.

Cheers

jeanot1314 commented 7 months ago

Thanks you for the quick answer.

I am not sure I understand what you mean by decoding yourself.

When the UDS server device answer, I have the error "struct.error: unpack requires a buffer of 4 bytes"

it's coming from (a, b) = struct.unpack('>HH', response.data[1:])

If I print (response.data[1:]) I have 5 bytes instead of 4.

Could it be possible that my DID configuration client is 1byte instead of 2? So I have a 5bytes long payload?

pylessard commented 7 months ago

Ohhh my bad, sorry. I somehow was conviced you were parsing DIDs, but your not. I see, I see.

Well.. your server is wrong here. It seems to encode the subfunction on a 16 bits ID. It should be 8bits. See the standard

pylessard commented 7 months ago

I have push a correction so that the reported error is better. Here : https://github.com/pylessard/python-udsoncan/commit/a4b40472877b5a8f7229b48fc0634b50dc02bac3

jeanot1314 commented 7 months ago

I am sorry, I was sure the server part was reliable... that's the reason I was looking at the library.

Merci beaucoup pour le support ! ;)

pylessard commented 7 months ago

That's the purpose of the library, finding those defects :) Glad it helped

Cheers

jeanot1314 commented 5 months ago

Hi Pylessard,

I have another strange behavior but I can't find if it's coming from client or server. I already used ReadDataByIdentifier on others DID, so it's surprising.

Here are the Log :

2024-02-09 09:26:53 [INFO] Connection: Connection opened 2024-02-09 09:26:53 [INFO] UdsClient: ReadDataByIdentifier<0x22> - Reading data identifier : 0xf080 (VehicleManufacturerSpecific) 2024-02-09 09:26:53 [DEBUG] Connection: Sending 3 bytes : [22f080] 2024-02-09 09:26:53 [DEBUG] Connection: Received 25 bytes : [62f080985513198000b09855131980ffffffffff01ffffffff] 2024-02-09 09:26:53 [INFO] UdsClient: Received positive response for service ReadDataByIdentifier (0x22) from server. 2024-02-09 09:26:53 [ERROR] UdsClient: [InvalidResponseException] : ReadDataByIdentifier service execution returned an invalid response. Response given by server is incomplete. 2024-02-09 09:26:53 [INFO] Connection: Connection closed

The exchange seems allright because it expect 25 bytes (0x19) and receive 25 bytes. To better understand, I recorded the traffic, but everything seems good

ID=1711,Type=D,Length=8,Data=0322F080 ID=1679,Type=D,Length=8,Data=101962F080985513

ID=1711,Type=D,Length=8,Data=300820 ID=1679,Type=D,Length=8,Data=21198000B0985513 ID=1679,Type=D,Length=8,Data=221980FFFFFFFFFF ID=1679,Type=D,Length=6,Data=2301FFFFFFFF

Do you want me to open another topic?

Thanks a lot

jeanot1314 commented 5 months ago

The problem seems to come from here :

https://github.com/pylessard/python-udsoncan/blob/8f97ee445882421244b5f7270ca075d5d7f5414c/udsoncan/services/ReadDataByIdentifier.py#L142

At this moment, "len(response.data)" is 24 bytes ( 25 bytes minus 0x62 answer ) The offset+1 is 24 as well

jeanot1314 commented 5 months ago

Ok, nevermind. It come from the length I send Really sorry to have disturb you. :(

pylessard / python-udsoncan

struct.error on change_session interpret_response #192