ros / robot_state_publisher

Allows you to publish the state of a robot (i.e the position of its base and all joints) via the "tf" transform library
http://www.ros.org/wiki/robot_state_publisher
BSD 3-Clause "New" or "Revised" License
87 stars 169 forks source link

publishTransforms and publishFixedTransforms are protected #165

Closed vatanaksoytezer closed 3 years ago

vatanaksoytezer commented 3 years ago

In ROS1 and ROS2 distros prior to Foxy, publishTransforms and publishFixedTransforms methods were public. Starting from Foxy, these two methods are now protected and I would need a helper class to expose these methods. Is there any reason or added functionality with this change?

clalancette commented 3 years ago

It's mostly a code cleanliness thing; those methods are internal to how the node works, so I didn't see a reason they should be exposed as public.

What exactly are you trying to accomplish with calling those methods from outside the class?

vatanaksoytezer commented 3 years ago

@clalancette thanks for the quick jump! I am in the process of porting xpp to ros2 which basically visualizes robots and terrains in Rviz in the goal of finally visualizing motion plans for legged robots. Particularly https://github.com/leggedrobotics/xpp/blob/master/xpp_vis/src/urdf_visualizer.cc#L73 uses these methods.

clalancette commented 3 years ago

Looking at that code, it seems to me that it something of a misuse of robot_state_publisher.

In particular, what robot_state_publisher does is to take sensor_msgs::msg::JointState messages as published to the joint_states topic, and the publish tf2 transforms based on those joint updates. So it looks to me that the UrdfVisualizer::StateCallback method should only publish a message to joint_states; robot_state_publisher will pick it up from there and publish the tf2 transforms properly. That is, something like:

UrdfVisualizer::UrdfVisualizer()
{
    ...

    joint_state_pub = this->create_publisher<sensor_msgs::msg::JointStates>("joint_states", 10);
}

void
UrdfVisualizer::StateCallback(const xpp_msgs::RobotStateJoint& msg)
{
    joint_state_pub->publish(msg.joint_state);
}
vatanaksoytezer commented 3 years ago

What you said makes sense and is reasonable to me. I can definitely do that in the port instead of exposing these methods. I am closing the issue since that is indeed a misuse and there is a more reasonable way of doing that without using these methods. Thanks a lot @clalancette!