tier4 / nebula

A universal LiDAR and radar driver for ROS 2, supporting Hesai, Velodyne, Robosense and Continental sensors.
https://tier4.github.io/nebula/
Apache License 2.0
55 stars 54 forks source link

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

Closed mojomex closed 2 months ago

mojomex commented 2 months ago

PR Type

Related Links

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.

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

CI Checks

codecov[bot] commented 2 months ago

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 :warning:
Additional details and impacted files ```diff @@ 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](https://app.codecov.io/gh/tier4/nebula/pull/201/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | Coverage Δ | | |---|---|---| | [differential](https://app.codecov.io/gh/tier4/nebula/pull/201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | `25.80% <75.00%> (?)` | | | [total](https://app.codecov.io/gh/tier4/nebula/pull/201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

knzo25 commented 2 months ago

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

mojomex commented 2 months ago

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

sonarcloud[bot] commented 2 months ago

Quality Gate Passed 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