Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

RFC: Add support for timestamps#77

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
poeschel wants to merge5 commits intosocketcan-rs:develop
base:develop
Choose a base branch
Loading
frompoeschel:timestamps

Conversation

poeschel
Copy link
Contributor

@poeschelpoeschel commentedOct 1, 2024
edited
Loading

This pull-request adds support for receiving can frames with timestamps from the kernel.
It tries to solve issue#22. It shows, that the concept works. See the examples.
I chose to provide new socket types, which provide a tuple as a return type, whenread_frame'ing from the socket. This tuple contains aCanFrame and aOption<SystemTime>. While providing these Sockets requires a lot of boilerplate code for implementing the Traits, I think this way feels very natural from a socketcan-rs user perspective.
There are however some points I would like to discuss:

  1. Is this way a valid aproach to provide timestamps? What do you think?
  2. The return type with the tuple:(CanFrame, Option<SystemTime>) I got this from the TODO inside of src/socket.rs. Maybe it is better to provide a named struct here? Something likeCanFrameTimestamp that then contains theCanFrame and theSystemTime.
  3. I was searching for a proper rust type to return the timestamp. The linux kernel hasstruct timespec on the C side. The nix crate hasTimeSpec for this, but I think returning a nix crate type here is maybe not the right thing? What do we have in std? Well, there isDuration,Instant andSystemTime. From these three,SystemTime felt the best, but I am not sure.
  4. For now I did choose to return theSystemTime in anOption, because, well is there something that can go wrong while obtaining the timestamp? Maybe when we do not have acmsg? Can this fail? What to do, when this fails? I don't know. For now, this is an Option. Is this a good idea?
  5. Please pay some attention to the not-very-rust-like unsafe block. I did this to cast theIoSliceMut, that I get fromrecvmsg into alibc::canframe without copying bytes around. Copying of the bytes does happen when constructing aCanFrame::from(libc_f) a little later. Is this a feasible way here? Can anybody come up with something better? Maybe something more idiomatic rust?
  6. In the same unsafe block I useptr::from_ref which is not accepted by the current CI. I know this. This can also be done by another pointer-casting-dereference-dance, which looks very ugly. Maybe when anybody comes up with a better solution for 5., this can be eliminated along the way? If not, I would opt to raiserust-version and CI to 1.76 which bringsstd::ptr::from_ref out of nightly. I added a patch raising the rust-version as test. This can be removed if there are any objections.
  7. Naming:CanSocketTimestamp andCanFdSocketTimestamp. Does this sound right?
  8. I introduceopen_with_timestamp with an enumTimestampingMode, to choose between hardware and software timestamping. Any comments on this?open_addr is doingTimestampingMode::Software as a default. Is this sane?

I could only test software timestamps so far. I try to set up something to test hardware timestamps as well.

vslynko, tuna-f1sh, and damszew reacted with thumbs up emoji
@poeschelpoeschelforce-pushed thetimestamps branch 3 times, most recently fromb890db0 toc5e31bbCompareOctober 4, 2024 14:46
@poeschelpoeschel marked this pull request as draftOctober 7, 2024 14:40
@poeschelpoeschel marked this pull request as ready for reviewOctober 8, 2024 06:11
@jr1221
Copy link

If you need someone to test this PR for stabilty or performance please let me know.

@poeschelpoeschelforce-pushed thetimestamps branch 2 times, most recently fromdbadfc4 toc10b400CompareOctober 11, 2024 08:17
@poeschel
Copy link
ContributorAuthor

poeschel commentedOct 11, 2024
edited
Loading

If you need someone to test this PR for stabilty or performance please let me know.

Thank you for this offer. I am quite certain, that this works and it will be as performant as everything else in socketcan-rs. You're welcome to test it and give feedback
What's more interesting to me at the moment is:

  1. Has this as a concept a chance of beeing merged and
  2. From a socketcan-rs crate user point of view, is this in a shape that is good to use. Is the new API offered here something that is appealing to use? Can I improve something in this direction?
tuna-f1sh reacted with thumbs up emoji

@tuna-f1sh
Copy link

@fpagliughi this seems like a good stab at timestamp support and closing#22 after 3 years! If this was rebased to 3.5 release, what do you think? Or do you have other ideas/WIP?

poescheland others added4 commitsMarch 5, 2025 12:04
This adds the possibility to retrieve a timestamp from the kernel whenreceiving can frames.The way this works is you have to open a special Socket, calledCanSocketTimestamp that yields a SystemTime along with the CanFrameinstead of only the CanFrame. Have a look at the examples.
Co-authored-by: Sven Bachmann <dev@mcbachmann.de>
Co-authored-by: Sven Bachmann <dev@mcbachmann.de>
@poeschelpoeschelforce-pushed thetimestamps branch 2 times, most recently from358257f to982d4f0CompareMarch 5, 2025 12:15
from_ref is used in the timestamping code and supported since Rust1.76.0
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@poeschel@jr1221@tuna-f1sh

[8]ページ先頭

©2009-2025 Movatter.jp