COVESA / Open1722

Open-source implementation of the IEEE 1722 protocol, for streaming audio/video, tunneling CAN/LIN messages and enabling remote access to peripheral bus systems.
BSD 3-Clause "New" or "Revised" License
17 stars 5 forks source link

Aggregation of multiple CAN messages in 1722 frame #20

Open adriaan-niess opened 1 month ago

adriaan-niess commented 1 month ago

Based on comment from @JLPLabs we should consider adding support to aggregate multiple CAN frames into a single Ethernet frame.

Adriaan, You'll see a 3rd commit on this PR.

NOTE PLEASE LET ME KNOW: a) if this is more than you want, b) if there is a better / more idiomatic approach I should be taking.

This last change can be summarized as:

  • LISTENER

    • updated one line in acf-can-listener so that the acf_pdu pointer is moved through the ethernet frame. (right now there is logic that seems to imply that was the desired behavior, but the pointer wasn't actually being updated).
  • TALKER

    • added a --count COUNT flag to the command line. This allows the user to require COUNT number of CAN frames to be packed into one Ethernet frame.
    • used the variable that already existed, num_acf_msgs for handling the count logic.

Originally posted by @JLPLabs in https://github.com/COVESA/Open1722/issues/18#issuecomment-2248463892

adriaan-niess commented 1 month ago

@JLPLabs, At first glance I think it would be a little bit more flexible to provide a timeout (longest possible time allowed to buffer a CAN message) to the talker application instead of a fixed number of CAN messages to be aggregated. Then the talker would either wait until

For the default setting we could set the timeout simply to zero which would imply no buffering at all.

But maybe you had a very specific reason why you did it the way you did?

JLPLabs commented 1 month ago

Adriaan,

I like those ideas! And thank you for asking if I had a specific reason. I do, but it is perhaps a little selfish...

I do like the idea of a max number of CAN frames per Ethernet frame, as there may be use cases where the user (particularly while doing initial development, which is me right now!) prefers looking at only a handful of CAN frames in an Ethernet frame while analyzing protocol behavior.

I'm going to take the liberty of combining your idea with mine... I hope you are OK with that.

Let's say the default behavior of talker is "full buffering"-- unless the user indicates otherwise, we'll pack as many CAN frames into the Ethernet frame as we can before sending it.

If the user doesn't want "full buffering" then they can use one or both of --timeout and/or --count.

Here are behaviors other than default:

  1. --count 1 -- today's behavior of talker (we send the Ethernet frame as soon as we get CAN frame).

  2. --timeout 0 -- also gives today's behavior.

  3. --count 5 -- do some packing, but keep it user-friendly when doing manual inspection of frames

  4. --timeout 400 -- pack only "bursty" traffic

Formal Definitions

Interaction

The Ethernet frame is sent once any of these are true:

--notes-- [1] This "only" gives the user 1.19 hours between messages... which for nearly all practical vehicle use is OK. (I could imagine some diagnostics done by a product engineer -- where they filter traffic looking for a very specific and rare event and they want all of theses rare events in one Ethernet frame and 1.19 hours isn't enough). For this case the engineer should create their own application?? Otherwise the tradeoff could be to measure <time> in ms, and which means we can no longer require packing to be done for just "bursts" of traffic.

adriaan-niess commented 1 month ago

Hey, this looks very well thought out to me. Big thanks! Unfortunately I can only give you a short reply, because I'm in a little bit of hurry. Some thoughts:

JLPLabs commented 1 month ago

Sorry for my delayed response. I think your logic for timing makes sense. I'll start to work on updated code with this definition for --timeout:

adriaan-niess commented 1 month ago

Hi,

don't worry take your time. Only thing I'd change is to set the default timeout to 0 instead of 2^32-1 because then it could happen that a single CAN frame gets delayed for a very long time. I fear some users could interpret this as a bug if they don't study the documentation first.

If you need any help or feel stuck at some point let me know.

Regards Adriaan