Fix Unordered Point Clouds

[exo-core]

Fixes

Description of the changes:

Fixed the wrong size information in published point clouds (see #227 and #163).

Required before submitting a Pull Request:

• [x] I reviewed CONTRIBUTING.md
• [x] I built my changes locally
• [x] I tested my changes with a device
• [ ] I changed both the ROS1 and ROS2 branches

I tested changes on:

• [ ] Windows
• [x] Linux
• [x] ROS1
• [ ] ROS2

[christian-rauch]

For my understanding, does this essentially "reshape" the unstructured point cloud (list of points) into a structured point cloud (grid of points in the image domain)?

[exo-core]

For my understanding, does this essentially "reshape" the unstructured point cloud (list of points) into a structured point cloud (grid of points in the image domain)?

Yes. Actually the correct width and height values are already being set in lines 669/670 and 727/728, respectively, but the call to pcd_modifier.resize(n) resets the point cloud back to n times 1.

[christian-rauch]

For my understanding, does this essentially "reshape" the unstructured point cloud (list of points) into a structured point cloud (grid of points in the image domain)?

Yes. Actually the correct width and height values are already being set in lines 669/670 and 727/728, respectively, but the call to pcd_modifier.resize(n) resets the point cloud back to n times 1.

I see. In this case, you could also move all of: https://github.com/microsoft/Azure_Kinect_ROS_Driver/blob/c0742b9e470c9e688d796029f10cb52e1a763a4a/src/k4a_ros_device.cpp#L727-L730 to after the pcd_modifier.resize(point_count); so that setting point_cloud->height and point_cloud->width does not appear twice.

[christian-rauch]

Also... can you squash your commits? :-) Your third commit reverts your second commit. Such commits should not appear at all in the git history. And I think you can also squash your last commit into the first commit as they touch the same things.

[v4hn]

Thank you for tracking this down @exo-core ! It works.

@christian-rauch It's rather annoying that this is not the default behavior upstream yet. Can't you just squash-commit and maybe add your own cleanup commit?

[christian-rauch]

Thank you for tracking this down @exo-core ! It works.

@christian-rauch It's rather annoying that this is not the default behavior upstream yet. Can't you just squash-commit and maybe add your own cleanup commit?

Which commit? I don't have a commit in this PR and I also cannot squash or change other people's PRs.

[exo-core]

I guess we are a little spoiled from Gitlab, which has a "squash commits" option in the merge menu... (no clue what kind of back magic it does in the background)

I'm quite busy, finalizing my Ph.D. thesis at present, but I'll try to squeeze in a some time next week to update my PRs. For this one it should be straightforward. #240 seems to need some more thought and discussion though...

[christian-rauch]

I'm quite busy, finalizing my Ph.D. thesis at present

Good luck with your thesis writeup :-)

[exo-core]

I guess the PR should be fine now ;-)

Good luck with your thesis writeup :-)

Thanks!

[CalaW]

The process died after modifying and compiling the package in accordance with this commit. However I published ordered point clouds by simply remove the following line:

pcd_modifier.resize(point_count);

I am not familiar with point cloud representation in ROS, so I'm wondering if it's a valid solution.

[v4hn]

I guess we are a little spoiled from Gitlab, which has a "squash commits" option in the merge menu

The same is true for github. Also, unless the pr author explicitly disabled it, repository authors are able to push (and force-push) to remote branches that belong to pull-requests and can thus clean up history themselves. Even if the PR author disabled it, it's straight-forward to open a new PR for it (but a bit more work).

I'm quite busy, finalizing my Ph.D. thesis at present

Hang in there and congrats on getting that far! :-)

[exo-core]

The process died after modifying and compiling the package in accordance with this commit. However I published ordered point clouds by simply remove the following line:

pcd_modifier.resize(point_count);

I am not familiar with point cloud representation in ROS, so I'm wondering if it's a valid solution.

Thanks for pointing this out. I was able to get a camera in the lab today and could reproduce the issue.

[exo-core]

Ok, so turned out the initial resize wasn't save to remove after all, since resizing at the end invalidated the iterators. Guess, skipping the resize entirely isn't a good idea either, so I just moved the iterator initializations behind the resize operation.

Sorry, I didn't run detailed tests before; it seemed trivial and I did not have a camera at hand...