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

Name change of internal rosapi/src/rosapi to rosapi/src/ros_api to remov... #170

Closed
wants to merge 2 commits into from

Conversation

jackpien
Copy link

@jackpien jackpien commented Feb 7, 2015

...e runtime errors that occur from using catkin_make. Check out - http://answers.ros.org/question/113671/catkin-package-cannot-find-own-message-type-python/ - for reference


I build with catkin_make on Indigo. Before this change, rb_server always launched with "unable to find params" error w. After doing some investigations, apparently there's something with catkin which prevents it from resolving the "rosapi" internal module name used within the rosapi package. So changed the internal module name to ros_api to fix. Apparently, this problem doesn't happen with rosbuild which is why it probably wasn't a problem for anyone else prior to catkin.

@jihoonl
Copy link
Member

jihoonl commented Feb 8, 2015

Ah.. Good catch. It was not noticeable because we have not used rosapi as python library so far.
I see what causes this problem though.

The problem is that name conflict between rosapi script, in scripts directory, and rosapi python library module, in src directory. So it is not a problem when rosapi script is started as a ros node.

I would suggest to rename rosapi script to rosapi_node instead of renaming rosapi python module as ros_api. Because keeping consistent in python library would make more sense. In this case, we only need to rename rosapi script as rosapi_node and update launch file in rosbridge_server.

Any thoughts? @rctoris

@rctoris
Copy link
Contributor

rctoris commented Feb 10, 2015

I agree with @jihoonl 's suggestion of changing the node name.

@jackpien
Copy link
Author

Sorry for late reply on my thoughts. The reason why I didn't want to change the actual node name is because then any outside dependencies on calling the node as "rosapi" will have to change to "rosapi_node" where as if we just change the name of the internal python script, then no outside dependencies, uses of the "rosapi" node has to change and everything works as normal.

@jihoonl
Copy link
Member

jihoonl commented Feb 17, 2015

Yeah I agree that having backward compatibility is important.

But I think consistency of library module name is more important than keeping executable script name.
These are the reasons I would like not to change library name.

  • most ros python packages uses library name same as package name. So we better rename package name if we change library name.
  • Library name change might mess up with deb install space which might require apt-get dist-upgrade to user.. It also gives headaches as maintainer
  • Lastly, I think it is not a big deal to renaming executable script as an user perspective too. It is one line of change in roslaunch file.

@jackpien
Copy link
Author

Sorry again for chiming in late on this pull request. Please make the name change to rosapi_node then. Totally up to you guys.

@jihoonl
Copy link
Member

jihoonl commented Mar 17, 2015

Addressing with #176

@jihoonl jihoonl closed this Mar 17, 2015
rctoris added a commit that referenced this pull request Mar 17, 2015
rename rosapi script to rosapi_node to address #170
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