commaai / panda

code powering the comma.ai panda
MIT License
1.5k stars 744 forks source link

[ISO-TP] Number of consecutive frames is wrong when FC contains Block Size #1961

Open pd0wm opened 1 month ago

pd0wm commented 1 month ago

When an ECU responds with a non zero Block Size in the flow control, the number of consecutive frames might be wrong. E.g. if the ECU responds with a block size of 8, but there are less consecutive frames to send it will send zeroes instead.

Example:

uds_client._uds_request(0x23, None, b"\x00" * 7)

Debug output:

ISO-TP: REQUEST - 0x7b7 0x2300000000000000
ISO-TP: TX - first frame - 0x7b7
CAN-TX: 0x7b7 - 0x1008230000000000
CAN-RX: 0x7bf - 0x300814aaaaaaaaaa
ISO-TP: RX - flow control continue - 0x7b7
CAN-TX: 0x7b7 - 0x2100000000000000
CAN-TX: delay - 0.02
CAN-TX: 0x7b7 - 0x2200000000000000
CAN-TX: delay - 0.02
CAN-TX: 0x7b7 - 0x2300000000000000
CAN-TX: delay - 0.02
CAN-TX: 0x7b7 - 0x2400000000000000
CAN-TX: delay - 0.02
CAN-TX: 0x7b7 - 0x2500000000000000
CAN-TX: delay - 0.02
CAN-TX: 0x7b7 - 0x2600000000000000
CAN-TX: delay - 0.02
CAN-TX: 0x7b7 - 0x2700000000000000
CAN-TX: delay - 0.02
CAN-TX: 0x7b7 - 0x2800000000000000
ISO-TP: TX - consecutive frame - 0x7b7 idx=8 done=True
CAN-RX: 0x7bf - 0x037f2331aaaaaaaa
ISO-TP: RX - single frame - 0x7bf idx=0 done=True
ISO-TP: RESPONSE - 0x7bf 0x7f2331

I think the fix should be to change https://github.com/commaai/panda/blob/master/python/uds.py#L535 to

end = min(start + count * num_bytes, self.tx_len)  if count > 0 else self.tx_len

But don't have time right now to verify.