Update compress_depth_image_transport to ROS2#27
Update compress_depth_image_transport to ROS2#27mjcarroll merged 5 commits intoros-perception:ros2-develfrom
Conversation
mjcarroll
left a comment
There was a problem hiding this comment.
Looks very similar to what I did, I just updated the logging macros.
| { | ||
| ROS_ERROR("%s", e.what()); | ||
| return sensor_msgs::Image::Ptr(); | ||
| RCUTILS_LOG_ERROR(e.what()); |
There was a problem hiding this comment.
I switched these to RCLCPP_ERROR per @mikaelarguedas suggestion.
There was a problem hiding this comment.
I did that for the code encapsulated in classes but I doubt about doing it in the codec file that contains helper functions. We can modify the interface to include a node shared ptr but looks a bit weird to me to do that for "decode" or "encode" functions.
There was a problem hiding this comment.
Yeah, I'm still a little unclear on what the best practice is here, especially since logging requires passing either a Node or Logger around.
There was a problem hiding this comment.
On gazebo_ros_pkgs sometimes we create a logger on the fly, i.e.
auto logger = rclcpp::get_logger("compressed_depth_image_transport");
Maybe here it makes sense to have a static logger.
There was a problem hiding this comment.
Thanks @chapulina. I've changed the macros in 05bd4d5
* Ignore other packages for now. * Update to new plugin API. * Add support for parameters. * Address reviewer feedback. * Switch to RCLCPP loggers. * Update compress_depth_image_transport to ROS2 (#27) * Port compress_depth_image_transport to ROS2 * Specifiers final for methods * Use rcutils logging * Implement parameters for encoding * Use RCLCPP_* macros instead of rcutils * Update theora_image_transport to ROS2 (#28) * Port theora plugin to ROS2 * Implement network options from ROS1 into rmw_qos_profile_t * Improve comment to clarify the use of dynamic reconfigure in ROS1 * Use RCLCPP logging instead of RCUTILS * Addressed reviewer comments
Mostly a clone of #26. I think that the TODO applies here too: