-
Notifications
You must be signed in to change notification settings - Fork 6
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
base: main
Are you sure you want to change the base?
Conversation
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 I'll attach wireshark traces or screenshots later. Regards, |
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. |
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 2: Certain Ethernet frames seem to be missing |
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: It provides incrementing data values, making log files easier to examine for missing frames. (it also is 29-bit CANID's like in J1939) |
Thanks! I'm going to test it again tomorrow with your new commits. |
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. |
I’m done with the features. Thanks for asking.
(For what it is worth, I did enjoy digging into the Unix interrupt scheme and understanding how to make the timeout work. I very much appreciate that you had interest in adding these features.)
On Aug 20, 2024, at 4:01 AM, Adriaan Nieß ***@***.***> wrote:
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.
—
Reply to this email directly, view it on GitHub<#32 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABEGGWDQF2LV5TQY3JNOFCDZSMAYDAVCNFSM6AAAAABMEQY5L6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJYGM2DMMJXHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
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.