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

PR #58 is not compatible with previous version... #60

Merged
merged 1 commit into from
Feb 2, 2016

Conversation

k-okada
Copy link
Member

@k-okada k-okada commented Jan 26, 2016

we may need this for naoqi_driver to perform in same namespace,,, #58

@vrabaud
Copy link
Contributor

vrabaud commented Feb 1, 2016

@suryaambrose , any input ?

@suryaambrose
Copy link
Member

I don't really understand what you are trying to fix here, what is the problem actually ?

@k-okada
Copy link
Member Author

k-okada commented Feb 2, 2016

  1. The original problem was reported by prevent calling ros::init without name; use node_name command option instead of namespace #44, which could not
    set proper node name when no ROS namespace is found. (which usually not happens when we used nao or pepper robot)
  2. I thouhg this was fixed by https://github.com/ros-naoqi/naoqi_driver/pull/58/files, but it is not correct. this will set ros node name as <namespace>/naoqi_driver_node, because ::naoqi::ros_env::getPrefix() is already initialized at https://github.com/k-okada/naoqi_driver/blob/fix_58/src/naoqi_driver.cpp#L126

@suryaambrose
Copy link
Member

OK thanks for the explanation.
I wanted to test that today, but I really don't have time. However it seems good to me, so if it works as expected for you, go.

vrabaud added a commit that referenced this pull request Feb 2, 2016
PR #58 is not compatible with previous version...
@vrabaud vrabaud merged commit 4638a97 into ros-naoqi:master Feb 2, 2016
@k-okada k-okada deleted the fix_58 branch February 4, 2016 00:22
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.

3 participants