search

Earthwings / annotate

Create 3D labelled bounding boxes in RViz
BSD 3-Clause "New" or "Revised" License
161 stars 22 forks source link

Bugfix: Using Time(double) to take double point timestamp #24

Open ameysutavani opened 1 year ago

ameysutavani commented 1 year ago

The **iter_timestamp is in seconds (of type double) but ros::Time::fromNSec(uint64_t) expects nanoseconds (of type uint64_t) which caused segfault in analyzePoints. Bug fixed by using ros::Time(double) to get point_time.

ameysutavani commented 1 year ago

@Earthwings Thanks for this awesome tool. Issuing this PR to fix a small bug I noticed during my testing.

Earthwings commented 1 year ago

The **iter_timestamp is in seconds (of type double) but ros::Time::fromNSec(uint64_t) expects nanoseconds (of type uint64_t) which caused segfault in analyzePoints. Bug fixed by using ros::Time(double) to get point_time.

Thanks for looking into this and providing a patch!

I think there are different ways of specifying point timestamps in use. The pointcloud you're working with uses offsets in seconds relative to the cloud's header stamp, right? In that case negative values are possible, which would indeed crash.

To get that case correct, I'd expect the code to construct the point time as a combination of the offset value and the cloud stamp. Your patch seems to lack the latter part yet, doesn't it?

Finally I think both variants should be supported: For small time values assume relative time to the cloud header stamp, otherwise assume absolute time.

ameysutavani commented 1 year ago

Hi @Earthwings . The point cloud that I am working with has point times NOT relative to the header stamp. Each point records time from the same global clock as the header.