Skip to content

Optional nanosecond timestamp logging #592

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RikusW
Copy link

@RikusW RikusW commented Mar 10, 2025

Use timespec instead of timeval so timestamp values with nanosecond resolution can be displayed.

@marckleinebudde
Copy link
Member

marckleinebudde commented Mar 10, 2025

I think we're not allowed to change the output for the -l format. The tool canplayer relies on 6 decimal places:

candump any -f foo &
cangen vcan0 -I 2 -Li -Di -p 10 -g 100 -n 10
killall candump
canplayer -I ./foo

Otherwise this is a good enhancement for the hardware timestamps.

@hartkopp
Copy link
Member

Yes. The 6 decimal places are fix in the CAN log file format for 20+ years now. And having the maximum rate of CAN frames on the CAN bus in mind we are already far beyond what is needed as a timestamp precision.

Of course everyone might use a higher precision from timespec inside a SocketCAN application - but we have to stay with 6 decimal places in the log file format.

@RikusW
Copy link
Author

RikusW commented Mar 11, 2025

We are using MCP2518FD with 25ns resolution and are planning to correlate that to PTP time.
I've made the nanosecond format optional with microseconds being default.

@marckleinebudde
Copy link
Member

@RikusW Can you elaborate on you use case a but more, I have some proof of concept patches that implement a PTP clock for the mcp251xfd driver.

@RikusW
Copy link
Author

RikusW commented Mar 11, 2025

@marckleinebudde We will be using 8 CAN ports with all of them exactly in sync and will be doing the PTP part in userspace for now. I'm working on https://tsn.systems/en/products/basicsolution/tsn-basicsolution-octocan/
I had to remove timecounter_cyc2time since it introduces 100ms of jitter... currently using raw counter values.

@RikusW RikusW closed this Mar 11, 2025
@RikusW RikusW reopened this Mar 11, 2025
@hartkopp
Copy link
Member

I'm not generally against creating a new log file format flavor that supports a nanosecond resolution with a -N option.
But the highest impact to can-utils will be to make the consumers of those new log files , especially canplayer and log2asc aware of 9 digits and adapt themselves at runtime. These tools need to be converted until candump starts to provide such log files.

@RikusW
Copy link
Author

RikusW commented Mar 11, 2025

For now I just need to be able to view nanosecond timestamps, transmit/replay ability could be done at a later time even.
Adding us/ns autodetection should be easy enough, I'll have a look at that a bit later.

@hartkopp
Copy link
Member

Let's hold this PR in mind until it is completed with the other tools then.

@RikusW
Copy link
Author

RikusW commented Jul 12, 2025

I've re-based onto the newest master and added us/ns auto detection.
canplayer and log2asc was tested and log2long buffer was enlarged.
A potential canplayer sscanf buffer overflow bug was also fixed.

@RikusW RikusW changed the title Use timespec Optional nanosecond timestamp logging Jul 12, 2025
@RikusW
Copy link
Author

RikusW commented Jul 14, 2025

Squashed into a single commit

@RikusW
Copy link
Author

RikusW commented Jul 14, 2025

Looks like libc broke in some test setups...

@marckleinebudde
Copy link
Member

Looks like libc broke in some test setups...

yes - debian and ubuntu have some problems.

@RikusW
Copy link
Author

RikusW commented Jul 14, 2025

Looks like libc broke in some test setups...

yes - debian and ubuntu have some problems.

At least it is not my patch that is the problem here...

@marckleinebudde
Copy link
Member

ACK

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