search

peteflorence / nanomap_ros

NanoMap: Fast, Uncertainty-Aware Proximity Queries with Lazy Search of Local 3D Data
https://www.youtube.com/watch?v=zWAs_Djd_hA
Other
176 stars 57 forks source link

Remove PCL dependency (hopefully) #6

Open bzcheeseman opened 6 years ago

bzcheeseman commented 6 years ago

This makes some assumptions (documented in pcl.h) that are not necessarily true, but I just don't know.

Happy to chat more about design decisions, etc - this was just a quick first attempt at removing the PCL dependency.

Also - I was having a lot of trouble setting up catkin on my machine so I added a docker build that will hopefully help with testing. Just build as normal then you have to run catkin_make within docker (as below)

docker run -it --rm <nanomap image> bash
catkin_make
peteflorence commented 6 years ago

Thank you for your PR! Looking forward to reviewing when I get a chance!

Best, Pete

peteflorence commented 6 years ago

Hey @bzcheeseman, this is very nice. I appreciate the attention to detail in dealing with Eigen nicely, etc.

Also I am a big fan of docker, --every project I've done since this one lives in a docker container, so the docker stuff is great.

Can you let me know what configuration you used to test this? Do you know if standard ROS point cloud publishing satisfies the assumption stated on line 57 of pcl.h?

bzcheeseman commented 6 years ago

@peteflorence I used the tests you provided - which is not that helpful because it doesn't fill the point cloud with anything. As far as I could tell the pcl::PointXYZ is contiguous in memory but I can't say for 100% certain. I was hoping you might have some ideas to test with non-empty point clouds (I don't have anything physical set up to test this on at the moment)

peteflorence commented 6 years ago

@bzcheeseman OK yes what we should do is test with a rosbag of data -- that will contain real ros PointCloud2s getting published and we can see how this branch deals with performing Knn queries on them. I can maybe also find a place to make a rosbag available for testing.

And yes, the unit tests are not currently very useful for testing point clouds!