feat: Add subscriber plugins using pluginlib#198
feat: Add subscriber plugins using pluginlib#198Nosille wants to merge 8 commits intoRobotWebTools:ros2from
Conversation
|
Thank you for the PR. I'll try find some time to review it till the end of the week (ping me if I forget). My only request for now is to split it into 2 separate PRs, one that changes the classes structure and one that adds the pointcloud subscriber plugin. |
|
I finally found time to remove the pointcloud2 stuff. I will make a separate pull request after this one is complete. |
bjsowa
left a comment
There was a problem hiding this comment.
Sorry I couldn't find much time to review this. I generally like the idea of separating streamers and subscribers. I quickly skimmed through the code and the solution looks alright. My only concern is that the subscriber interface only works for streamers which expect a raw image. This does not fit with the ros_compressed streamer which never decodes the image.
I'll try to take go back to it later next week. For now, try to pass CI.
| @@ -0,0 +1,75 @@ | |||
| // Copyright (c) 2024-2025, The Robot Web Tools Contributors | |||
| find_package(tf2 REQUIRED) | ||
| find_package(tf2_ros REQUIRED) | ||
| find_package(tf2_sensor_msgs REQUIRED) | ||
| find_package(tf2_geometry_msgs REQUIRED) | ||
|
|
There was a problem hiding this comment.
Remove these lines
| find_package(tf2 REQUIRED) | |
| find_package(tf2_ros REQUIRED) | |
| find_package(tf2_sensor_msgs REQUIRED) | |
| find_package(tf2_geometry_msgs REQUIRED) |
There was a problem hiding this comment.
Done. These are pointcloud2 dependencies I overlooked.
| # ${avcodec_INCLUDE_DIRS} | ||
| # ${avformat_INCLUDE_DIRS} | ||
| # ${avutil_INCLUDE_DIRS} | ||
| # ${swscale_INCLUDE_DIRS} |
There was a problem hiding this comment.
You can safely remove these lines
| # ${avcodec_INCLUDE_DIRS} | |
| # ${avformat_INCLUDE_DIRS} | |
| # ${avutil_INCLUDE_DIRS} | |
| # ${swscale_INCLUDE_DIRS} |
| ament_target_dependencies(${PROJECT_NAME}_subscribers rclcpp tf2 tf2_ros tf2_sensor_msgs tf2_geometry_msgs) | ||
|
|
There was a problem hiding this comment.
We don't use ament_target_dependencies anymore since it's deprecated
| ament_target_dependencies(${PROJECT_NAME}_subscribers rclcpp tf2 tf2_ros tf2_sensor_msgs tf2_geometry_msgs) |
There was a problem hiding this comment.
Done. These are also pointcloud2 dependencies anyway.
|
|
||
| ament_target_dependencies(${PROJECT_NAME}_subscribers rclcpp tf2 tf2_ros tf2_sensor_msgs tf2_geometry_msgs) | ||
|
|
||
| target_link_libraries(${PROJECT_NAME}_subscribers |
There was a problem hiding this comment.
You don't need to link so many dependencies. These should be enough:
async_web_server_cpp::async_web_server_cpp
image_transport::image_transport
pluginlib::pluginlib
rclcpp::rclcpp
${sensor_msgs_TARGETS}
Public API Changes
Reorganized codebase to isolate subscribers from streamers.
Make subscribers an additional pluginlib class (base_class_type="web_video_server::SubscriberFactoryInterface">)
Created image_transport and pointcloud2 subscribers
Description
This is a big change that will probably take a little back and forth. If you are not interested let me know. The goal was to make it possible to subscribe to ros msg formats other than images and convert them to image streams. This was done in a plugin-able fashion. Existing image_transport based subscription capabilities were removed from the existing streamer plugins and replaced with a new subscriber plugin. A new pointcloud2_subscriber plugin was added that subscribes to pointcloud2 msgs, projects them into a 2d image (user selected viewing frame with parameters for height, width, and focal length), then sends the resulting image to any of the existing streamers. A somewhat silly subscriber plugin example/tutorial was created in the docs that converts a string msg into an image.