LVR Update #46
LVR Update #46
Conversation
| find_package(mesh_msgs REQUIRED) | ||
| find_package(resource_retriever REQUIRED) |
There was a problem hiding this comment.
This is missing as a ROS dependency in package.xml
(missing dep might be hidden because another pkg installs this)
There was a problem hiding this comment.
Out of curiosity, for what functionality do we depend on this pkg here?
There was a problem hiding this comment.
We only linked against it in the CMakeLists. However, it was not used anywhere in the code. So I have removed it.
| // example: | ||
| // "vertices" -> Channel<float> | ||
| std::cout << *mesh_buffer << std::endl; | ||
|
|
||
| // "mesh_navigatio_tutorials/maps/floor_is_lava.h5" |
There was a problem hiding this comment.
I suspect this logging statement should not be in here. It breaks the flow of the multi line comments with the example. Also, if we want to do logging here, we should probably use ROS logs.
| // example: | |
| // "vertices" -> Channel<float> | |
| std::cout << *mesh_buffer << std::endl; | |
| // "mesh_navigatio_tutorials/maps/floor_is_lava.h5" | |
| // example: | |
| // "vertices" -> Channel<float> | |
| // "mesh_navigatio_tutorials/maps/floor_is_lava.h5" |
There was a problem hiding this comment.
I think it is a quite helpful print. So I changed the cout to RCLCPP_INFO_STREAM
| // is not working anyway | ||
| // // Open IO | ||
| // hdf5_map_io::HDF5MapIO map_io(m_mapFilePath->getFilename()); | ||
|
|
||
| // Add label with faces list | ||
| map_io.addOrUpdateLabel(results[0], results[1], faces); | ||
| // // Add label with faces list | ||
| // map_io.addOrUpdateLabel(results[0], results[1], faces); |
There was a problem hiding this comment.
I'd be fine with fully removing these lines of code. I usually prefer having less clutter over seeing how things were done in the past. If we implement e.g. color/texture loading later and some of the commented out code is doing something non-trivial, we can always look at the ROS1 version in git, for example.
The same applies for the two larger comment blocks above in this file.
There was a problem hiding this comment.
I removed all the larger comment blocks
I think I've considered everything on the list |
This is a reaction to the upcoming LVR update (3.0.0)
Loading/Writing HDF5
The HDF5 structure has changed slightly over time so that the mesh_tools paper description of the data got wrong. I changed parts of it back so that it gets more like the papers again. It also makes things easier to handle since vertex attributes are now seperated from the actual mesh geometry inside the HDF5 (as it was described in the mesh_tools paper).
Cleanup
I have also removed packages for which we have no tests and which are currently not used elsewhere:
Testing instructions
This PR works together with: