search

lishen2 / isotp-c

An implementation of the ISO-TP (ISO15765-2) CAN protocol in C
MIT License
189 stars 70 forks source link

Handling of Unexpected arrival issue #7

Closed sgetzpeng closed 2 years ago

sgetzpeng commented 4 years ago

Hi folks, I have traced the code and have a doubt on handling of unexpected arrival. In this implementation, link->receive_protocol_result will be assigned to ISOTP_PROTOCOL_RESULT_UNEXP_PDU when ink->receive_status is ISOTP_RECEIVE_STATUS_INPROGRESS and FF and SF is received, it compliant Table 18 ISO-15765-2.
But when CF is received and ink->receive_status != link->receive_status(i.e., the link is in segmented transmission state and CF is received), the implementation treated it an unexpected arrival, but according to Table 18 ISO-15765-2, the sender should just skip the PDU silently.

On another hand, when FC reception, the sender should accept the FC depends on how many CF has been sent. For example, the sender needs to sent 3 CF then waits for an FC, if the FC is coming after 1 CF transmission, then the sender need ignore the FC silently.

Please correct me if i'm wrong. Thanks /Sgetz

dnrelectronica commented 4 years ago

Hi Sgetz,

Have you solved already your issue you mentioned above here??

I have also some problems with the library, but the guy 'lishen2' is not responding at all on any issue.. too bad. The library seems to be ok for 90% but i have some questions about how the library will behave with lets say 3 senders and 1 receiver.

What happens is sender A starts sending a multiframe packet and in the middle of the CF frames sender B starts with sending his multiframe packet. How does the library handle this. From the lib i can see the FF of sender B will result in a UNEX_PDU because it is in receiving state of sender A. But there is missing a 'break;' statement i thing in the if statement!! line 314 in code.

Since the break is missing the library is calling isotp_receive_first_frame() function while it is in the middle of receiving message of sender A!!! This is not good in my opinion. The FF message of the sender B should be ignored and sender B should be notified it has to wait, because another node is sending its message.. try later on..

Maybe i am totally wrong, but this is my question.

I hope you or someone can help me out of this issue. Please let me know where i can find the specification of the ISOTP. It would be helpfull to understand how it is handling multiple messages sending on the buss the same point in time. And don't explain me how arbitration works, i know that.. but this is about how the receiver is handling a new message from sender B while it is in the middle of receiving message from sender A.

kind regards,

Corne Doggen.

sgetzpeng commented 4 years ago

Hi @dnrelectronica, Sorry for late reply because of Chinese new year :sparkles: :sparkles: :sparkles: My reponse is as following:

Have you solved already your issue you mentioned above here??

Actually, I have not apply the library in my system, I only referenced the head file for the ISOTP message payload and the link structure. The reason I didn't use it is the library doesn't have any OS utility such as thread creation, semaphore, timer, etc.(Or I do not fully understand how to use it).

What happens is sender A starts sending a multiframe packet and in the middle of the CF frames sender B starts with sending his multiframe packet. How does the library handle this. From the lib i can see the FF of sender B will result in a UNEX_PDU because it is in receiving state of sender A. But there is missing a 'break;' statement i thing in the if statement!! line 314 in code.

Since the break is missing the library is calling isotp_receive_first_frame() function while it is in the middle of receiving message of sender A!!! This is not good in my opinion. The FF message of the sender B should be ignored and sender B should be notified it has to wait, because another node is sending its message.. try later on..

You have brought out the limitation of the library, because the library doesn't use pthread or other multi-thread library, it seems not to handle multi-node communication. Second, your expected behavior of sender B conflicts ISO15765-2. According to the standard, different node(i.e., the sender A and sender B) should act independently. It means that the receiver should handler different state machine for sender A and sender B. So the FF of sender B should not be treated as UNEX_PDU, instead, send B should wait for receiver's response within Bs time (5 seconds).

Maybe i am totally wrong, but this is my question.

I hope you or someone can help me out of this issue. Please let me know where i can find the specification of the ISOTP. It would be helpfull to understand how it is handling multiple messages sending on the buss the same point in time. And don't explain me how arbitration works, i know that.. but this is about how the receiver is handling a new message from sender B while it is in the middle of receiving message from sender A.

I have got a list about CAN documentation in someone's github repository, but unfortunately I can't find it anymore, you can try to seek it, or mail to me.

kind regards,

Corne Doggen.

dnrelectronica commented 4 years ago

hi sgetzpeng,

Thank you so much for your information in your last comment above. I still think the library should be extended with a multi threaded ' receiver' option in such a case multiple nodes can send there multiframe messages simultaniously over the can network. I guess in automotive this IS already implemented some way, since in a car you have multiple nodes like ECU, light controllers, gas trottle etc... all these send information at the same time on the CAN buss, and i assume these messages are not all < 8 bytes right?? so they need a solution for multiframe messages. is OBDII for example using iso-tp under the hood?? or J1939?

sgetzpeng commented 4 years ago

Hi Corne Doggen(@dnrelectronica),

hi sgetzpeng,

Thank you so much for your information in your last comment above. I still think the library should be extended with a multi threaded ' receiver' option in such a case multiple nodes can send there multiframe messages simultaniously over the can network.

It's fine if you thought the effort is worthy, and be aware of the issue(although it's not a regular case) that i mentioned in the top of the thread.

I guess in automotive this IS already implemented some way, since in a car you have multiple nodes like ECU, light controllers, gas trottle etc... all these send information at the same time on the CAN buss, and i assume these messages are not all < 8 bytes right?? so they need a solution for multiframe messages. is OBDII for example using iso-tp under the hood?? or J1939?

I would say there are lots of CAN messages larger than 8-bytes. But I'm not sure if ODB-2's network layer is ISO-TP or not, but J1939 is. Another key point is you must ensure all devices on the same CAN bus support ISO-TP.

/Sgetz

lishen2 commented 4 years ago

Hi gays, I am finally here. This library is design to run on bare metal, so it can't use OS specific functions.

@dnrelectronica, the library use mulit-instance to handle multiple-peer. You should manually route different CAN packages to different instance according to it's ID. Anything else I can help?

dnrelectronica commented 4 years ago

This library is design to run on bare metal, so it can't use OS specific functions.

@dnrelectronica, the library use mulit-instance to handle multiple-peer. You should manually route different CAN packages to different instance according to it's ID. Anything else I can help?

In my opinion the library should create an instance on the fly , realtime, dynamically. If the message is handled you can free the memory again.

@lishen2 Please can you react on this?

lishen2 commented 4 years ago

The function library can run in an environment with an operating system. You need to implement user-implemented functions (such as isotp_user_send_can). If multiple threads access the protocol stack function concurrently, locks is required. In fact, I used this library to implement the testing tool under windows.

It should not be a problem for multiple instances in multiple connections, and the TCP protocol stack will also allocate a control block for each connection. If you need to establish 1000 ISOTP connections at the same time, you really need to generate 1000 instances (I doubt if there is such an application scenario).

This protocol stack may have two unsuitable places for your usage scenario: 1. The cache area is statically allocated (suitable for embedded environment), and there may be memory waste for a large number of connections. 2. Data reception is passive. No notification is received after the complete isotp message is received. Query is required. But both can be improved.

lishen2 commented 2 years ago

This library is design to run on bare metal, so it can't use OS specific functions.

  • what do you mean with that???

@dnrelectronica, the library use mulit-instance to handle multiple-peer. You should manually route different CAN packages to different instance according to it's ID. Anything else I can help?

  • So, for every different CAN ID i need to make an instance? If i have 1000 message ID's i need to create 1000 instances with each having their own buffers, this would take a lot of RAM!

In my opinion the library should create an instance on the fly , realtime, dynamically. If the message is handled you can free the memory again.

@lishen2 Please can you react on this?

You are absolute right.