ros/ros_comm

ros::init with no __name and name = "" is not tested in roscpp?

Closed this issue · 11 comments

I'd like to report a possible bug at roscpp initialization.
In roscpp initializer ros::init (const M_string &remappings, const std::string &name, uint32_t options=0), a node can be initialized even empty string is passed to name argument if remappings are passed through from command line arguments.
But if __name:=foobar is removed from remappings and name is empty, ros node can be registered with invalid name, which can occur unexpected errors.
I did experiment in some conditions. the result is followings:

remap_name node_name_empty test_group this_node::name this_node::namespace
true false false /test_node /
true false true /group1/test_node //group1
true true false /test_node /
true true true /group1/test_node //group1
false false false /test_node /
false false true /group1/test_node //group1
false true false / /
false true true //group1 //group1

In the last condition, namespace and name are the same and other nodes cannot resolve the node.

code for reproduce:

#include <ros/ros.h>
#include <std_msgs/String.h>
#include <map>

int main(int argc, char** argv) {
  bool remap_name = true;
  bool node_name_empty = true;
  std::map<std::string, std::string> remaps;
  if (remap_name) remaps["__name"] = "test_node";
  std::string node_name = "test_node";
  if (node_name_empty) node_name = "";
  ros::init(remaps, node_name, (uint32_t)0);

  ROS_INFO_STREAM("getName: " << ros::this_node::getName());
  ROS_INFO_STREAM("getNameSpace: " << ros::this_node::getNamespace());

  ros::NodeHandle nh;
  ros::Publisher pub = nh.advertise<std_msgs::String>("string", 1);

  std_msgs::String s;
  ros::Rate r(10);
  for(int i = 0; ros::ok(); ++i) {
    std::ostringstream os;
    os << "published string " << i;
    s.data = os.str();
    pub.publish(s);
    ros::spinOnce();
    r.sleep();
  }

  return 0;
}
<launch>
  <arg name="test_group" default="false" />
  <group ns="group1" if="$(arg test_group)">
    <node name="nstest_node" pkg="roscpp_nstest" type="nstest" output="screen" />
  </group>
  <node name="nstest_node" pkg="roscpp_nstest" type="nstest" output="screen" unless="$(arg test_group)" />
</launch>

In the last condition:

$ rostopic info /group1/string
Type: std_msgs/String

Publishers: 
 * //group1 (unknown address //group1)

Subscribers: None

In the first place, is it allowed to initialize node with empty string as name argument for `ros::init``?

Since node names are a graph resource name I would expect an empty string to be invalid.

@dirk-thomas Thank you for clarifying! OK. Then I'll create the pull request in which passing empty string to node_name is invalid.

(Could I wait other some mention about this issue for some days?)

@ros/ros_team What do you think?

Empty strings are definitely invalid names and should error either if passed directly or remapped to an empty value.

I second what @tfoote said.

@dirk-thomas @tfoote @wjwwood Thank you all for advice! I created pull requests.

close via bd3af70