fix(velodyne): propagate sub-second part of timestamps to pointcloud timestamp

[mojomex]

PR Type

• Bug Fix

Related Links

• Found this bug when testing chore!: upgrade to Nebula v0.2.1 autowarefoundation/autoware#5275
• bug introduced in refactor(velodyne)!: combine Velodyne ROS wrappers into a single node #148 (here: decoder_wrapper.cpp:177)

Description

This PR fixes a bug where all Velodyne pointclouds have a cloud timestamp which is rounded down to to the nearest second.

Ths bug was introduced by mistakenly changing the following expression

/* from */ rclcpp::Time(packet.stamp).seconds()
/*   to */ packet.stamp.sec

Review Procedure

Confirm that cloud timestamps are correct (nanosec field increases by 100ms each scan).

Remarks

Tested with Autoware logging simulator:

ros2 topic echo --field header.stamp --csv /sensing/lidar/left/aw_points_ex

Before:

1585897255,0
1585897255,0
1585897255,0
1585897255,0
1585897256,0
1585897256,0
1585897256,0
1585897256,0
1585897256,0
1585897256,0
1585897256,0
1585897256,0
1585897256,0
1585897256,0
1585897257,0
1585897257,0

After:

1585897255,310290688
1585897255,360631040
1585897255,461410560
1585897255,561021440
1585897255,660529152
1585897255,761372928
1585897255,860926720
1585897255,960501248
1585897256, 61242112
1585897256,160895744
1585897256,260359168
1585897256,361232640
1585897256,460778240
1585897256,562444032
1585897256,661132800
1585897256,760673536
1585897256,860208384
1585897256,961116672
1585897257, 60684800
1585897257,160203520

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.

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 25.80%. Comparing base (9f706e3) to head (3af07cf).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
nebula_ros/src/velodyne/decoder_wrapper.cpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #201      +/-   ##
==========================================
- Coverage   29.65%   25.80%   -3.86%     
==========================================
  Files          99       99              
  Lines        8760     8748      -12     
  Branches     2173     2145      -28     
==========================================
- Hits         2598     2257     -341     
+ Misses       6116     6105      -11     
- Partials       46      386     +340     

Flag	Coverage Δ
differential	25.80% <75.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.

[knzo25]

@mojomex
Just to be clear, is this the bug related to the sample rosbag not working as expected?

[mojomex]

@knzo25 Correct. This causes concat to only output point clouds at 1 Hz, killing driving performance.

[knzo25]

nit:
checkAndHandleScanComplete accepts int32, is called using a double, and only uses it to call reset_pointcloud which uses double.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
20.0% Duplication on New Code

See analysis details on SonarCloud

[knzo25]

LGTM !
(tested that this fixes the error found in autowarefoundation/autoware#5275)