velodyne_pointcloud: transform.cc loses device frame_id

[jack-oquin]

@gabor-meszaros found this error in transform.cc

// clear input point cloud to handle this packet
inPc_.points.clear();
inPc_.width = 0;
inPc_.height = 1;
inPc_.header.frame_id = scanMsg->header.frame_id;
std_msgs::Header header;
header.stamp = scanMsg->packets[next].stamp;
pcl_conversions::toPCL(header, inPc_.header);

The toPCL overwrites the inPc_header.frame_id with the header variable's frame_id which is an empty string by default.

It appears to have been caused by the PCL 1.7 fix @wjwwood created, and it may affect more packages than just this one. The other pcl_conversion updates in this package all look correct.

See the #11 discussion.

[wjwwood]

The problem is that I should have done this first:

std_msgs::Header header;
pcl_conversions::fromPCL(inPc_.header, header);
header.stamp = scanMsg->packets[next].stamp;
pcl_conversions::toPCL(header, inPc_.header);

[wjwwood]

I would usually catch that scenario, but I missed it this time. This is an example of why it would be so difficult to automate the changes required for pcl-1.7, as others had suggested we do.

Sorry for any of y'alls time this consumed.

[jack-oquin]

No problem, William. I just wanted you to be aware of that particular pitfall.

[wjwwood]

Examples of other places I did this (correctly):

https://github.com/wjwwood/navigation/commit/ae4203638c63e3c100ffc04b4c52be86fa7ef699#L2R318 https://github.com/wjwwood/navigation/commit/d8a477c73c27edc64da3da8169f512a148e84a6d#L2R332

For reference.

[jack-oquin]

Why not just change the sequence from this:

inPc_.header.frame_id = scanMsg->header.frame_id;
std_msgs::Header header;
header.stamp = scanMsg->packets[next].stamp;
pcl_conversions::toPCL(header, inPc_.header);

to this?

std_msgs::Header header;
header.stamp = scanMsg->packets[next].stamp;
header.frame_id = scanMsg->header.frame_id;
pcl_conversions::toPCL(header, inPc_.header);

In this case, scanMsg points to a normal velodyne_msgs/VelodyneScan ROS message.

[wjwwood]

You can do that too, I was a bit thrown that scanMsg->packets[next] wasn't followed by .header. So I wasn't sure if your message had a frame_id.

[jack-oquin]

It's a tricky situation. Each packet has its own timestamp, to minimize smearing during the transform due to the vehicle moving while the device turns. The resulting point cloud gets the timestamp and frame_id of the entire scan, which reflects the last packet it includes.

[jack-oquin]

fix committed in 5b15e7c685098fb1b22c5331ab63eb9197e93a5b

[jack-oquin]

fix released today, version 1.1.1