Skip to content
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

[ROS2]: image_transport does not resolve base topic in correct namespace #187

Open
devrite opened this issue May 5, 2021 · 0 comments
Open

Comments

@devrite
Copy link

devrite commented May 5, 2021

Hi everyone,

I think I came across a bug yesterday (using foxy, but I gues it is in rolling too) as I just tried the way image_transport resolves the base topic manually to verify this (see below). So this should affect any ROS2 version and not only foxy.

If you inspect the following excerpt of the camerapublisher:

CameraPublisher::CameraPublisher(
rclcpp::Node * node,
const std::string & base_topic,
rmw_qos_profile_t custom_qos)
: impl_(std::make_shared<Impl>(node))
{
// Explicitly resolve name here so we compute the correct CameraInfo topic when the
// image topic is remapped (#4539).
std::string image_topic = rclcpp::expand_topic_or_service_name(base_topic,
node->get_name(), node->get_namespace());
std::string info_topic = getCameraInfoTopic(image_topic);
auto qos = rclcpp::QoS(rclcpp::QoSInitialization::from_rmw(custom_qos), custom_qos);
impl_->image_pub_ = image_transport::create_publisher(node, image_topic, custom_qos);
impl_->info_pub_ = node->create_publisher<sensor_msgs::msg::CameraInfo>(info_topic, qos);

It uses the expand mechanism but only using the node namespace. ROS2 Nodes feature the ability to create sub-nodes with an associated sub-namespace. If you look into the official templates of create a subscriber or publisher they will use the function resolve_topic_name'), which does all this sub-namespace handling correctly.

I suggest we switch to this or as similar function or call node->get_effective_namespace()instead.

I will try to provide a merge request for the latter today.

BR, Markus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant