fix(hw_interfaces): fix the lossy nanoseconds extraction

[xmfcx]

PR Type

• Bug Fix

Related Links

None.

Description

// old
packet.stamp.nanosec = static_cast<int>((now_nanosecs / 1000000000. - static_cast<double>(now_secs)) * 1000000000);
// new
packet.stamp.nanosec = static_cast<std::uint32_t>(now_nanosecs % 1000000000);

The first line of code you provided appears to have a bug related to the calculation of the nanoseconds portion of a timestamp. Here's the breakdown:

1. First Line Analysis:

   • packet.stamp.nanosec = static_cast<int>((now_nanosecs / 1000000000. - static_cast<double>(now_secs)) * 1000000000);
   • This line seems to be trying to calculate the nanoseconds part by taking the total nanoseconds (now_nanosecs), dividing it by 1 billion (to get the seconds part), and then subtracting the seconds part (now_secs) from it. This result is then multiplied by 1 billion to convert it back to nanoseconds.
   • The issue here is in the calculation method. When you divide now_nanosecs by 1 billion and then subtract now_secs, you might not get an accurate nanoseconds part because of potential floating-point precision errors. The calculation also unnecessarily converts values to a double and then back to an int, which can introduce additional precision errors.

2. Second Line Fix:

   • packet.stamp.nanosec = static_cast<std::uint32_t>(now_nanosecs % 1000000000);
   • This line simplifies the process by using the modulo operator (%). It effectively gets the remainder of now_nanosecs when divided by 1 billion, which directly gives the nanoseconds part without needing to convert to seconds first.
   • Using modulo is a common and more reliable method for extracting the smaller unit (nanoseconds in this case) from a larger time value (like total nanoseconds). It avoids the precision issues of floating-point arithmetic.

In summary, the first line has a potential precision issue due to unnecessary floating-point calculations and conversions, while the second line efficiently and accurately calculates the nanoseconds part using the modulo operation.

Review Procedure

Remarks

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

• Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

• Commits are properly organized and messages are according to the guideline
• (Optional) Unit tests have been written for new behavior
• PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

• All open points are addressed and tracked via issues or tickets

CI Checks

• Build and test for PR: Required to pass before the merge.

[xmfcx]

There were errors in 100~ nanoseconds range from the lossy conversion when I tested it.

[xmfcx]

https://www.twitch.tv/videos/1991133148?t=01h12m47s was found/fixed during this stream.

[codecov-commenter]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (c71a157) 8.27% compared to head (7aee61e) 3.96%.

Files	Patch %	Lines
.../nebula_hesai_hw_interfaces/hesai_hw_interface.cpp	0.00%	1 Missing ⚠️
...a_velodyne_hw_interfaces/velodyne_hw_interface.cpp	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #101      +/-   ##
========================================
- Coverage   8.27%   3.96%   -4.32%     
========================================
  Files         87      27      -60     
  Lines       8456    5950    -2506     
  Branches     853     463     -390     
========================================
- Hits         700     236     -464     
+ Misses      7180    5302    -1878     
+ Partials     576     412     -164     

Flag	Coverage Δ
differential	3.96% <0.00%> (?)
total	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mojomex]

Thank you @xmfcx for your PR, and for noticing the issue.
If you could add separators ' for every 3 0s to make it more readable as suggested in the comments below, it would be perfect!

[mojomex]

LGTM 👍