Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buffering multiple can #32

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

JLPLabs
Copy link

@JLPLabs JLPLabs commented Aug 7, 2024

I believe the buffer behavior works as indicated in the "usage", but it is NOT ready for release. Why? The code has instrumentation 'fprintf()' in place to help me confirm behavior.

I'm releasing it to allow you to confirm the behavior is what you expect.

Usage: acf-can-talker [OPTION...]
            [ifname] dst-mac-address/dst-nw-address:port [can ifname]

acf-can-talker -- a program designed to send CAN messages to a remote CAN bus
                  over Ethernet using Open1722. The default behavior is to
                  send one CAN frame per Ethernet frame. However, a buffer can
                  be used that gives the opportunity to pack more than one.

OPTIONS

      --count=COUNT          Set count of CAN messages per Ethernet frame
      --timeout=TIME         Set time to wait for CAN messages to arrive
  -t, --tscf                 Use TSCF
  -u, --udp                  Use UDP
  can ifname                 CAN interface (set to STDIN by default)
  dst-mac-address            Stream destination MAC address (If Ethernet)
  dst-nw-address:port        Stream destination network address and port (If
                             UDP)
  ifname                     Network interface (If Ethernet)
  -?, --help                 Give this help list
      --usage                Give a short usage message

BUFFERING

  acf-can-talker, by default, packs only one CAN frame into an Ethernet frame.
  Use the --timeout and/or --count options to change this behavior.

  --timeout <time>   where <time> is an integer followed by units,
                     e.g., '1s', or '400us'. Allowable units: 's', 'ms', 'us'.
                     The Ethernet frame will be sent <time> after arrival
                     of the first CAN message.

  --count <count>    where <count> is the max number of CAN frames allowed in
                     an Ethernet frame.

  If both buffering options are presented, then whichever occurs first will
  trigger the sending of the Ethernet frame.

  In all cases, when the Ethernet frame is full it is sent.

EXAMPLES
  acf-can-talker eth0 aa:bb:cc:ee:dd:ff

    (tunnel transactions from STDIN to a remote CAN bus over Ethernet)


  acf-can-talker --count 10 eth0 aa:bb:cc:ee:dd:ff

    (as above, but send Ethernet frame as soon as we have 10 CAN frames)


  acf-can-talker --timeout 400ms eth0 aa:bb:cc:ee:dd:ff

    (as above, but send Ethernet frame when 400 msec have passed since
     arrival of first CAN frame)


  acf-can-talker -u 10.0.0.2:17220 vcan1

    (tunnel transactions from can1 interface to a remote CAN bus over IP)


  candump can1 | acf-can-talker -u 10.0.0.2:17220

    (another method to tunnel transactions from vcan1 to a remote CAN bus)

@adriaan-niess
Copy link
Member

Hi John,

thanks for all the work! I'll probably need a few days to review your code and test it a little bit more. On first glance, the way the timeout works seems to be correct.

However I've also encountered a few bugs, Unfortunately I did not have time to do a deeper investigation yet. I tried to run cangen vcan0 -g 500 and cangen vcan0 -g 750 in parallel (generating CAN frames in cycles of 500ms and 750ms) and in the talker I set a timeout of 500ms acf-can-talker lo aa:bb:cc:dd:ee:ff vcan0 --timout 500ms. The expected behavior would be that half the Ethernet frames would contain two CAN frames and the other half only one CAN frame. Here I've encountered some strange behavior, probably because some resources are accessed at the same time from normal program flow and from an interrupt context without using a mutex.

I'll attach wireshark traces or screenshots later.

Regards,
Adriaan

@JLPLabs
Copy link
Author

JLPLabs commented Aug 8, 2024

Thanks for your feedback! It help me show an issue by creating dropped messages in my test environment. I demonstrated dropped frames for two cangen's talking at 50ms and 75ms.

I've fixed the dropped messages by reordering the steps in my main loop for packing Ethernet. But to be clear, the fix was only proven out in my test environment, which, admittedly, is not the same as what you suggested in you note. For example I use candump piped into the talker and I'm using UDP.

Tomorrow I should be able to recreate your exact suggestion.

@adriaan-niess
Copy link
Member

adriaan-niess commented Aug 9, 2024

Here are a few more details. Didn't have time to test with your new commit, so the problems might not exist anymore. I run the CAN talker as described above with 2x cangen in parallel.

Problem 1: was that occasionally the wrong AVTP subtype was set. Might be a mutual exclusion problem because of a receive or timer interrupt.

problem_1

Problem 2: Certain Ethernet frames seem to be missing

problem_2

@nayakned nayakned closed this Aug 9, 2024
@nayakned nayakned reopened this Aug 9, 2024
@JLPLabs
Copy link
Author

JLPLabs commented Aug 12, 2024

I tried your setup and the talker program seems to work correctly. (I actually have two VM's talking over a private ethernet network).

The version of talker that I last posted had errors in the loop. That is fixed. It also has a cleaner output (again, just for debugging) that shows how many CAN frames are loaded into each Ethernet frame.

You may like to use:
$ cangen vcan0 -g 500 -e -I f004 -L 8 -D i

It provides incrementing data values, making log files easier to examine for missing frames. (it also is 29-bit CANID's like in J1939)

@adriaan-niess
Copy link
Member

Thanks! I'm going to test it again tomorrow with your new commits.

@adriaan-niess
Copy link
Member

Hi, looks good! Everything seems to work now as expected. Are you planning on doing more changes to the code or is the feature now done from your perspective?

Maybe you've notice, in the meantime we did some changes to the main branch in parallel like adding support for CAN-FD so there are some conflicts now. But that's not really a problem, I can resolve these conflicts for you as long as your branch is in a working state. To do that I would branch your branch so that the commit history stays intact.

@JLPLabs
Copy link
Author

JLPLabs commented Aug 20, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants