Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream point cloud utility functions #53

Closed
chapulina opened this issue Oct 28, 2021 · 4 comments · Fixed by #138
Closed

Upstream point cloud utility functions #53

chapulina opened this issue Oct 28, 2021 · 4 comments · Fixed by #138
Assignees

Comments

@chapulina
Copy link
Contributor

I borrowed some point cloud utilities from ROS's point_cloud2_iterator in #87. That's just a quick search-and-replace port without giving it much thought. I'd like to move the functions to ign-msgs, cleaning them up and adding proper tests, to make sure they work as intended.

I'd also like to take a look at https://gitlab.com/ApexAI/point_cloud_msg_wrapper, presented at ROS World, and see if it's wiser to use that approach instead.

@mabelzhang
Copy link
Collaborator

So I don't know if you saw in #133, but I changed the point clouds so that each science data type is now published as a float array, instead of wrapping into a field in the point cloud.

That is advantageous in several ways:

  1. Each science data can be a separate topic name in the drop-down list
  2. Float arrays are easy to read, do not require complicated PointCloud2 unpacking
  3. Saves bandwidth. The point cloud only specifies the position of the data, not he data themselves, and therefore do not need to hog up the bandwidth each time one science data type is published. The PointCloud2 message takes up a lot more bandwidth than float arrays and is something we can omit publishing all the time to save memory.
  4. More modular to add new science data types.

With that, we only need to pack xyz and nothing else in the point cloud.

@chapulina
Copy link
Contributor Author

chapulina commented Oct 29, 2021

I don't know if you saw in #133

Oh I hadn't looked into it yet!

we only need to pack xyz and nothing else in the point cloud.

Got it. So the iterator is still used, but only for position. We're still using the iterator, which I'm not confident about until we add proper tests...

Each science data can be a separate topic name in the drop-down list
More modular to add new science data types.

+1 for keeping the science data separate from each other

The point cloud only specifies the position of the data, not he data themselves, and therefore do not need to hog up the bandwidth each time one science data type is published

Does this mean that we may be changing the contents of the science data without changing their positions?

Float arrays are easy to read

Yup, agreed. We just need to be careful because there's implicit coupling between the float arrays and the point cloud indices.

@chapulina
Copy link
Contributor Author

chapulina commented Nov 1, 2021

Related to this upstream issue: gazebosim/gz-sim#1156

@mabelzhang
Copy link
Collaborator

Does this mean that we may be changing the contents of the science data without changing their positions?

No. The publish frequency can be different between the positions and the actual science magnitudes though. The positions can be published once in a long while (say, only when the view port is changed enough to warrant a republish), while the data magnitudes can go on and off as the user toggles the drop-down list and unsubscribes, to save bandwidth. They're unlikely to visualize all the data types at once, as it'll be so cluttered they won't be able to see anything.

@chapulina chapulina transferred this issue from another repository Nov 2, 2021
@chapulina chapulina self-assigned this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants