Add headers in conversions #18
Conversation
Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds header support (frame_id and timestamp) to the conversion functions between NavMap data structures and ROS messages. The changes maintain backward compatibility by providing overloads that don't require header parameters.
Changes:
- Added header parameter support to all conversion functions (to_msg/from_msg for NavMap, NavMapLayer, and OccupancyGrid)
- Provided backward-compatible overloads for all conversion functions
- Updated tests to verify header propagation works correctly
- Updated documentation to describe the new header parameters
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| navmap_ros/include/navmap_ros/conversions.hpp | Added function declarations with header parameters and backward-compatible overloads with documentation |
| navmap_ros/src/navmap_ros/conversions.cpp | Implemented header handling in all conversion functions with backward-compatible overloads |
| navmap_ros/tests/test_conversions.cpp | Added comprehensive tests verifying header extraction and assignment in all conversion paths |
| navmap_ros/tests/test_navmap_io.cpp | Updated existing tests to use new header-aware conversion functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * | ||
| * @param[in] msg Input NavMapLayer message. | ||
| * @param[in,out] nm Destination NavMap (must already have navcels sized correctly). | ||
| * @param[in] header Header to assign to the resulting message. |
There was a problem hiding this comment.
The parameter direction in the documentation is incorrect. The header parameter should be marked as @param[out] since it's an output parameter that gets populated by the function, not an input parameter. The function extracts the header from the message and assigns it to this parameter.
| * @param[in] header Header to assign to the resulting message. | |
| * @param[out] header Header extracted from the message. |
| * using a regular triangular surface with shared vertices. | ||
| * | ||
| * @param[in] grid Input ROS OccupancyGrid (row-major, width×height, resolution and origin). | ||
| * @param[in] header Header to assign to the resulting message. |
There was a problem hiding this comment.
The parameter direction in the documentation is incorrect. The header parameter should be marked as @param[out] since it's an output parameter that gets populated by the function, not an input parameter. The function extracts the header from the grid and assigns it to this parameter.
| * @param[in] header Header to assign to the resulting message. | |
| * @param[out] header Header to assign to the resulting message. |
Signed-off-by: Francisco Martín Rico <fmrico@gmail.com>
1 participant
Hi,
This PR takes into account the header (frame_id + stamp) en in the conversion functions, maintaining backward compatibility.
Best