-
Notifications
You must be signed in to change notification settings - Fork 734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
image_view: consistent image_transport #876
image_view: consistent image_transport #876
Conversation
this->declare_parameter<std::string>("transport", std::string("raw")); | ||
std::string transport = this->get_parameter("transport").as_string(); | ||
// TransportHints does not actually declare the parameter | ||
this->declare_parameter<std::string>("image_transport", std::string("raw")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE: if we back port this - we probably need to revert the change in name of this parameter
@@ -75,13 +75,15 @@ ExtractImagesNode::ExtractImagesNode(const rclcpp::NodeOptions & options) | |||
auto node_base = this->get_node_base_interface(); | |||
std::string topic = node_base->resolve_topic_or_service_name("image", false); | |||
|
|||
this->declare_parameter<std::string>("transport", std::string("raw")); | |||
// TransportHints does not actually declare the parameter | |||
this->declare_parameter<std::string>("image_transport", "raw"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here - renamed parameter for consistency with image_transport specification - I've noted both instances of this in my Jazzy: Documentation ticket so we can mention the changed parameter name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
linters
- image_view.cpplint whitespace/line_length [2] (/__w/image_pipeline/image_pipeline/src/image_view/src/video_recorder_node.cpp:79)
All of these nodes already have the proper remapping support - but image_transport parameter support was scattered
All of these nodes already have the proper remapping support - but image_transport parameter support was scattered