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

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

Closed
furushchev opened this issue Sep 8, 2016 · 11 comments
Closed

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

furushchev opened this issue Sep 8, 2016 · 11 comments

Comments

@furushchev
Copy link
Contributor

furushchev commented Sep 8, 2016

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.

@furushchev
Copy link
Contributor Author

furushchev commented Sep 8, 2016

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>

@furushchev
Copy link
Contributor Author

furushchev commented Sep 8, 2016

In the last condition:

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

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

Subscribers: None

@furushchev
Copy link
Contributor Author

furushchev commented Sep 8, 2016

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

@dirk-thomas
Copy link
Member

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

@furushchev
Copy link
Contributor Author

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

@furushchev
Copy link
Contributor Author

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

@dirk-thomas
Copy link
Member

@ros/ros_team What do you think?

@tfoote
Copy link
Member

tfoote commented Sep 9, 2016

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

@wjwwood
Copy link
Member

wjwwood commented Sep 10, 2016

I second what @tfoote said.

@furushchev
Copy link
Contributor Author

furushchev commented Sep 11, 2016

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

@furushchev
Copy link
Contributor Author

close via bd3af70

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

4 participants