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_env.hpp] delete std::getenv(ROS_NAMESPACE) #81

Closed

Conversation

kochigami
Copy link
Contributor

@kochigami kochigami commented Feb 14, 2017

Hi,
I encountered the same issue as ros-naoqi/pepper_robot#35.
My environment is ubuntu 16.04, ros kinetic, naoqi 2.4.3.28.
I use source of naoqi_driver and pepper_robot package.

If there are any comments, please let me know.

roslaunch pepper_bringup pepper_full.launch network_interface:=enp0s25
rosnode list

=>

/pepper_robot/pepper_robot/camera/bottom_rectify_color
/pepper_robot/pepper_robot/camera/camera_nodelet_manager
/pepper_robot/pepper_robot/camera/depth_metric
/pepper_robot/pepper_robot/camera/depth_metric_rect
/pepper_robot/pepper_robot/camera/depth_rectify_depth
/pepper_robot/pepper_robot/camera/depth_registered_hw_metric_rect
/pepper_robot/pepper_robot/camera/depth_registered_metric
/pepper_robot/pepper_robot/camera/depth_registered_rectify_depth
/pepper_robot/pepper_robot/camera/depth_registered_sw_metric_rect
/pepper_robot/pepper_robot/camera/front_rectify_color
/pepper_robot/pepper_robot/camera/ir_rectify_ir
/pepper_robot/pepper_robot/camera/points_xyzrgb_hw_registered
/pepper_robot/pepper_robot/camera/points_xyzrgb_sw_registered
/pepper_robot/pepper_robot/camera/register_depth_front
/pepper_robot/pepper_robot/naoqi_driver_node (This node name is modified)
/pepper_robot/pepper_robot/pose/pose_controller
/pepper_robot/pepper_robot/pose/pose_manager
/rosout

From #73, I learned that we can set ROS_NAMESPACE as a environmental variable, but in this case, we have to change the codes to use std::getenv(ROS_NAMESPACE).
(I didn't know how to change them...)

src/ros_env.hpp Outdated
@@ -89,8 +89,7 @@ static void setMasterURI( const std::string& uri, const std::string& network_int
remap["__master"] = uri;
remap["__ip"] = ::naoqi::ros_env::getROSIP(network_interface);
// init ros without a sigint-handler in order to shutdown correctly by naoqi
const char* ns_env = std::getenv("ROS_NAMESPACE");
ros::init( remap, (ns_env==NULL)?(std::string("naoqi_driver_node")):(::naoqi::ros_env::getPrefix()) , ros::init_options::NoSigintHandler );
ros::init(remap, std::string("naoqi_driver_node"), ros::init_options::NoSigintHandler);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, we should still consider the namespace and the prefix option.
The prefix option is meant to be set once the naoqi driver is loaded as a naoqi module.
When loaded externally (as a ros module), we should still have the option of namespacing. Have a look at this one: http://wiki.ros.org/Remapping%20Arguments

Since we anyway use the remapping options, we can then also set the __ns field.
Does this make sense what I am saying?
We could

@kochigami kochigami force-pushed the avoid-ros-invalid-name-exception branch from 0a074af to d38fed4 Compare February 15, 2017 08:17
@kochigami
Copy link
Contributor Author

Thank you very much for your advice.
As discussion in ros-naoqi/pepper_robot#35 ,
I was stuck when using ROS_NAMESPACE as a namespace.
Sorry for my bad pull-request.

case1: use ROS_NAMESPACE (something wrong)
case2: use value in launch file
case3: command line remapping

case1.

export ROS_NAMESPACE=test
roslaunch naoqi_driver naoqi_driver.launch network_interface:=enp0s25 

rosnode list
=>

/rosout
/test/test (I can't follow what is happening...)

case2.

unset ROS_NAMESPACE
roslaunch naoqi_driver naoqi_driver.launch network_interface:=enp0s25 

rosnode list
=>

/naoqi_driver_node
/rosout

case3.

unset ROS_NAMESPACE
roslaunch naoqi_driver naoqi_driver.launch network_interface:=enp0s25 namespace:=pepepe

rosnode list
=>

/pepepe
/rosout

@kochigami
Copy link
Contributor Author

kochigami commented Feb 17, 2017

I'm very sorry.
I just misunderstood the function of ROS_NAMESPACE.
Now I think we can use prefix and namespace if my understanding is correct.
My environment is ubuntu 14.04, ros indigo, Naoqi 2.4.3.8.

case1: prefix: default, ROS_NAMESPACE: x

unset ROS_NAMESPACE
roslaunch naoqi_driver naoqi_driver.launch network_interface:=eth1
rosnode list
=> /naoqi_driver_node etc

case2: prefix: "test", ROS_NAMESPACE: x

unset ROS_NAMESPACE
roslaunch naoqi_driver naoqi_driver.launch network_interface:=eth1 namespace:=test
rosnode list
=> /test etc

case3: prefix: default, ROS_NAMESPACE: "pepepe"

export ROS_NAMESPACE=pepepe
roslaunch naoqi_driver naoqi_driver.launch network_interface:=eth1
rosnode list
=> /pepepe/naoqi_driver_node etc

case4: prefix: "test", ROS_NAMESPACE: "pepepe"

export ROS_NAMESPACE=pepepe
roslaunch naoqi_driver naoqi_driver.launch network_interface:=eth1 namespace:=test
rosnode list
=> /pepepe/test etc

@kochigami kochigami closed this Apr 12, 2017
@kochigami
Copy link
Contributor Author

I intended to create this and ros-naoqi/pepper_robot#37 for ros-naoqi/pepper_robot#35.
I was a little bit sad to know these were skipped, but anyway I closed them.

@Karsten1987
Copy link
Contributor

This PR looks good to me? Why is this closed? Maybe you can cross-reference with the other open PRs for not duplicating work.

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

Successfully merging this pull request may close these issues.

2 participants