DanielMartensson / Open-SAE-J1939

SAE J1939 protocol free to use for embedded systems or PC with CAN-bus
MIT License
455 stars 164 forks source link

Is TP.CM_RTS implemented correctly? #49

Closed alexch-m closed 1 week ago

alexch-m commented 1 month ago

Hello, Daniel!

I played with DM1 example to validate my own partial implementation of J1939 and found that possibly TP.CM works not according to the standard (sorry if I'm wrong - I'm a beginner in J1939). The test app is a modified version of your DM1 example which uses Socket CAN (vcan) and supports two modes (request and reply) for easy debugging. This is a log of CAN frames sent in and out when one instance of the test app requests DM1 from another one:

-> ID = 0x18EA5A6B, DLC = 3 [CA FE 00]
<- ID = 0x1CEC6B5A, DLC = 8, data = [10 12 00 03 FF CA FE 00]
-> ID = 0x1CEC5A6B, DLC = 8 [11 00 00 FF FF CA FE 00]
<- ID = 0x1CEB6B5A, DLC = 8, data = [01 44 11 6C 02 06 01 37]
-> ID = 0x1CEC5A6B, DLC = 8 [11 01 01 FF FF CA FE 00]
<- ID = 0x1CEB6B5A, DLC = 8, data = [02 04 04 02 6F 00 10 83]
-> ID = 0x1CEC5A6B, DLC = 8 [11 02 02 FF FF CA FE 00]
<- ID = 0x1CEB6B5A, DLC = 8, data = [03 46 00 0B 82 FF FF FF]
-> ID = 0x1CEC5A6B, DLC = 8 [13 12 00 03 FF CA FE 00]

Looking at TP.CM_CTS frames (starting from 0x11), we see that bytes n.2 and n.3 (numeration starts from 1) are "00 00", "01 01", "02 02". According to SAE J1939-21, byte n.2 is a "Number of packets that can be sent. This value shall be no larger than the value in byte 5 of the RTS message." I'd expect to see e.g. value 1 here (if the party which sends CTS wants to receive frames one-by-one), but we see numbers identical to byte n.3 ("Next packet number to be sent").

The code in Transport_Protocol_Connection_Management.c:

    case CONTROL_BYTE_TP_CM_CTS:
        data[1] = j1939->this_ecu_tp_cm.total_number_of_packages_transmitted;
        data[2] = j1939->this_ecu_tp_cm.next_packet_number_transmitted;

-- is it correct?

DanielMartensson commented 1 month ago

Hi!

@alexch-m

Se here: https://github.com/DanielMartensson/Open-SAE-J1939/blob/main/Src/Documentation/Pictures/SAE%20J1939%20Resources/TCI%20%20SENSE%2042%20Version%203.0-25-108.pdf

Notice! That's for BAM messages, but the CTS has a different data frame.

Value 1 is the Control Byte and it should not hold the package size.

alexch-m commented 1 month ago

Hi!

Preword: my apologies for misleading issue title: it shall be "Is TP.CM_CTS implemented correctly?"!

So, regarding CTS, please look at the picture below (TP.CM_CTS description), it's from SAE-J1939-21-2006.pdf. Other libraries I tried (e.g. python-can-j1939) work according to this definition. I can provide their CAN dumps later if needed. Unfortunately I don't have examples of TP.CM communications from real hardware yet. SAE-J1939-21-2006 Fig 15

DanielMartensson commented 1 month ago

@alexch-m

The reason why you might be confused, is because I'm re-using the same data frame and only change some new numbers. This is due to saving memory because Open SAE J1939 runs on Arduino as well. As you can see, byte 6 to byte 8 (indexing from 1) are the same for BAM, Conn_Abort, EndOfMsgACK, CTS and RTS.

https://github.com/DanielMartensson/Open-SAE-J1939/blob/9ab0d03f9eb7bc5dac09e8476d9239ce4e504825/Src/SAE_J1939/SAE_J1939-21_Transport_Layer/Transport_Protocol_Connection_Management.c#L26-L52