Application Layer calcs TP packets

[grivera90]

Hi!. I do somethings with your stack j1939 and I thing great job. Any way i thing i found a problem in Application Layer when calculating the packet to TP. I thing will be:

"j1939->this_ecu_tp_cm.number_of_packages = j1939->this_ecu_tp_cm.total_message_size % 7 > 0 ? j1939->this_ecu_tp_cm.total_message_size/7 + 1 : j1939->this_ecu_tp_cm.total_message_size/7; / Rounding up /"

I was do a j1939 bootloader and I see wrong calcs in the packets and i revised that.

What your things?

[DanielMartensson]

The reason why I did this "round" up is beacuse If you are going to send 9 bytes, then you need 2 packages, even if one package is 8 bytes of data. That means, you are sending 16 bytes, but you only need to send 9 bytes. It's impossible to send 9 bytes in one package that can only hold 8 bytes.

So this round up is a special way to make sure you always get enough of packages to transmitt your data.

[grivera90]

HI! I undestand the rounding but i was referring to the divition per 8 in a line of your code. When i transmit for example a binary firmware in packets of 1024 bytes the calcs is wrongs. I thing will be divide per 7 and not 8 according to the sae j1939 becouse the packets in TP contend 7 bytes of data leave the 8th byte to the counter packet.

[DanielMartensson]

Can you send me your code so I can have a look at it?

[grivera90]

Yes, sorry for the delay. Its a prototype very weak but i use to test a bootloader and assembly ethe packets here: my function: `ENUM_J1939_STATUS_CODES SAE_J1939_send_request_update_tp(J1939 *j1939, uint8_t DA) { ENUM_J1939_STATUS_CODES status = STATUS_SEND_OK; UINT bytes_read = 0; uint16_t crc = 0xFFFF;

if(0 != f_eof(&fp))
    return STATUS_SEND_ERROR;

/* num_packet / total_packets / 1024 bytes of data / crc16 */
packets_counts++;
for(uint8_t i = 0; i < 4; i++)
{
    j1939->this_ecu_tp_dt.data[i] = (uint8_t)(packets_counts >> i * 8);
    j1939->this_ecu_tp_dt.data[i + 4] = (uint8_t)(packets_1024kb >> i * 8);
}

printf("offset_file: %d\r\n", (int)offset_file);
res = f_read(&fp, &j1939->this_ecu_tp_dt.data[0] + 8, 1024, &bytes_read);

uint16_t len_packet_crc = bytes_read + 8;

crc = crc_16_calculator(j1939->this_ecu_tp_dt.data, len_packet_crc);
printf("crc: 0x%04X\r\n", crc);

j1939->this_ecu_tp_dt.data[bytes_read + 8] = (uint8_t)(crc >> 8); // crc hight
j1939->this_ecu_tp_dt.data[bytes_read + 8 + 1] = (uint8_t)crc;

printf("crch: 0x%02X | crcl: 0x%02X\r\n", j1939->this_ecu_tp_dt.data[bytes_read + 8], j1939->this_ecu_tp_dt.data[bytes_read + 8 + 1]);

offset_file += bytes_read;
res = f_lseek(&fp, offset_file);
printf("bytes_read: %d\r\n", bytes_read);

if(0 == bytes_read)
    return STATUS_SEND_ERROR;

if(1024 > bytes_read)
{
    end_of_file = true;
    printf("end of file with bytes_read: %d\r\n", bytes_read);
}

j1939->this_ecu_tp_cm.total_message_size = bytes_read + 8 + 2; // buffer de 1024 + crc16 + packets_count + packets.
j1939->this_ecu_tp_cm.number_of_packages = j1939->this_ecu_tp_cm.total_message_size % 7 > 0 ? j1939->this_ecu_tp_cm.total_message_size/7 + 1 : j1939->this_ecu_tp_cm.total_message_size/7; /* Rounding up */
j1939->this_ecu_tp_cm.PGN_of_the_packeted_message = PGN_PROPA;
j1939->this_ecu_tp_cm.control_byte = CONTROL_BYTE_TP_CM_RTS;
status = SAE_J1939_Send_Transport_Protocol_Connection_Management(j1939, DA);

return status;

}`

I changed the division to 7 because with 8 the calculations for the packages would be wrong. 1 byte is used for the packet counter and the other 7 for the data field. Based on an example of the application layer in the function "SAE_J1939_Response_Request_Component_Identification" for example I saw that you use 8 and not 7. When I tested in my application I noticed that the packets arrived wrong and were not completed (there were about 17 packets of 1024 bytes for example).

[grivera90]

see j1939->this_ecu_tp_cm.number_of_packages = j1939->this_ecu_tp_cm.total_message_size % 7 > 0 ? j1939->this_ecu_tp_cm.total_message_size/7 + 1 : j1939->this_ecu_tp_cm.total_message_size/7; /* Rounding up */

[DanielMartensson]

Hi.

It's not division. It's modulus. :)

I can explain what this does:

j1939->this_ecu_tp_cm.number_of_packages = j1939->this_ecu_tp_cm.total_message_size % 8 > 0 ? j1939->this_ecu_tp_cm.total_message_size/8 + 1 : j1939->this_ecu_tp_cm.total_message_size/8; /* Rounding up */

If you are going to send 8 bytes, then

 j1939->this_ecu_tp_cm.total_message_size = 8;

When you take j1939->this_ecu_tp_cm.total_message_size modulus 8 you will get 0 because it's no remainer. This means that the else-statement must be called. So the number of packages will be j1939->this_ecu_tp_cm.total_message_size / 8, which is exactly 1.

But assume that you want to send

 j1939->this_ecu_tp_cm.total_message_size = 14;

14 is larger than 8 and each package requries 8 bytes. You have 14 bytes now, that means you need to have at least 2 packages as minimum because 2 packages are total 16 bytes. In this case, take 14 modulus 8, you will get 6.

6 = j1939->this_ecu_tp_cm.total_message_size % 8;

This indicates that the if-statement must be called. If you divide 14 / 8 you will get 1.75. That's close to 2 packages.

1.75 = j1939->this_ecu_tp_cm.total_message_size/8;

But we are using integers, is it will be this instead. Integers cannot hold decimals.

1 = j1939->this_ecu_tp_cm.total_message_size/8;

So I just add +1 insead

2 = j1939->this_ecu_tp_cm.total_message_size/8 + 1;

Done.

I changed the division to 7 because with 8 the calculations for the packages would be wrong. 1 byte is used for the packet counter and the other 7 for the data field. Based on an example of the application layer in the function "SAE_J1939_Response_Request_Component_Identification" for example I saw that you use 8 and not 7. When I tested in my application I noticed that the packets arrived wrong and were not completed (there were about 17 packets of 1024 bytes for example).

Yes, the first bytes is the package counter. But notice that when you are sending multiple packages, you are not sending e.g Component Identification, you are sending SAE_J1939_Send_Transport_Protocol_Connection_Management

[grivera90]

yes, its modulus but i was referring of other divition and of course the modulu too....I thinks that was the calcs to connection manaement was all the same in all cases. You say that my case is right ? becouse it works very well for me. I think that applicate to all config when use a TP in the stack

[DanielMartensson]

Well, if you are using your calculations, then you are doing it at the wrong way.

Yes. it's the same for all cases. No, your case is not correct. Do the math for example.

If you are sending e.g 8 bytes. That means you only need one package of data.

1 byte is used for the packet counter and the other 7 for the data field. Based on an example of the application layer in the function

When you are sending a multi package. Then you are sending a TP_CM package. It contains:

• Control byte (If we are going to broadcast the message to all ECUs or send only to one specific ECU)
• How many bytes we are going to send
• How many packages we are going to send
• The PNG code so the reciever can rebuild the package and use the data

If we are using a BAM message, that means we broadcast it to all ECUs with address 0xFF = 255. If we are using RTS message, then we waiting for a responce. RTS should be used when we only want to send to a specific ECU.

When we recieve back the CTS (Clear To Send) message from the transmitter, then we are going to send TP_DT (Data transfer message) and I think you are going to send a TP_DT directly? If yes, then you are not following the standard of SAE-j1939.

Se my documentation:

[grivera90]

Hi, I am using TP.DT but of course I am following the previous steps like the RTS from the originator ecu to the particular ecu (I use the PGN PropA) following the standard as indicated in the picture below:

My appreciation arose because once the CTS is received the originating ecu must start sending the TP.DT packets where as the standard indicates, 1 of the 8 bytes of data is the one that indicates the packet, the other 7 would be data. As I understand that in CAN 2.0 can only travel a message of 8 bytes we can not have 8 bytes of data plus 1 packet sequence, it would be 9 bytes. So I was thinking that when you calculate the data packets you should do it based on the 7 bytes of data and not 8. In your stack I only modified by 7 and then all the transport layer communication worked great. With that reasoning I only made that change leaving the stack to manage the RTS and CTS the calculations were perfect. I hope not to bother with the approach.

[DanielMartensson]

You mean like this?

https://github.com/DanielMartensson/Open-SAE-J1939/blob/4297cff44107e5278f120243cb9a611eafe8c42f/Src/SAE_J1939/SAE_J1939-21_Transport_Layer/Transport_Protocol_Data_Transfer.c#L84-L105

[DanielMartensson]

Have you read the last post about the Data Transfer function?

[grivera90]

Have you read the last post about the Data Transfer function?

Sorry! I'm holyday...pls, tomorrow see this. Ok?

[grivera90]

Hello again. Sorry for the delay. I saw your function and everything else in your stack and I haven't made any changes, except when calculating the %7 and /7 packets as I indicated above because I noticed that in large packets (maybe that's the way to see it better) the correct amount was not coming from the other side. So to start the process of sending over the PGN PorpA of the SAE j1939 multipackets is that I do the following:

j1939->this_ecu_tp_cm.total_message_size = bytes_read + 8 + 2; // buffer de 1024 + crc16 + packets_count + packets. j1939->this_ecu_tp_cm.number_of_packages = j1939->this_ecu_tp_cm.total_message_size % 7 > 0 ? j1939->this_ecu_tp_cm.total_message_size/7 + 1 : j1939->this_ecu_tp_cm.total_message_size/7; /* Rounding up */ j1939->this_ecu_tp_cm.PGN_of_the_packeted_message = PGN_PROPA; j1939->this_ecu_tp_cm.control_byte = CONTROL_BYTE_TP_CM_RTS; status = SAE_J1939_Send_Transport_Protocol_Connection_Management(j1939, DA);

I use everything just like your stack. I have only had to modify the calculation criteria because they are not sent correctly if one does it thinking in 8 units. I did the following reasoning together with a colleague:

8 bytes of data has the CAN payload. In the case of j1939 TP one byte is the packet counter and the other 7 are data/information. That is why in the standard it indicates that:

I understand that in the attribute "total_message_size" one should put the total of his message. And in the attribute "number_of_packages" the total of the packets and the packets are calculated based on the available data bytes without counting the counter byte. That means that it would be 7 and not 8.

But without the intention of making my collaboration too long, I wanted to share with you this change that we made with a colleague that made the transfer of packets by TP to be successful and as indicated by the standard without changing anything else in your stack or sending sequence, maintaining and respecting RTS and CTS.

[DanielMartensson]

Hi. Read the Sonceboz Document inside the documents folder. It explains everything.