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

soundplay_node no longer works on Hydro (v0.2.5) #33

Closed
piyushk opened this issue Feb 24, 2014 · 2 comments
Closed

soundplay_node no longer works on Hydro (v0.2.5) #33

piyushk opened this issue Feb 24, 2014 · 2 comments

Comments

@piyushk
Copy link
Contributor

piyushk commented Feb 24, 2014

I get the following error trying to run the code:

bwi@calculon:~$ rosrun sound_play soundplay_node.py 
Traceback (most recent call last):
  File "/opt/ros/hydro/lib/sound_play/soundplay_node.py", line 368, in <module>
    soundplay()
  File "/opt/ros/hydro/lib/sound_play/soundplay_node.py", line 304, in __init__
    rootdir = os.path.join(roslib.packages.get_pkg_dir('sound_play'),'sounds')
NameError: global name 'roslib' is not defined

I believe this change is responsible:
f5a1355#diff-c4ecd77ae058b18c549f3b287bd340b4R304

As far as I can tell, roslib is never imported, and that seems to be the only error. I'll submit a PR.

@jack-oquin
Copy link
Member

I believe roslib is no longer recommended for resolving package paths.

The preferred approach is:

import rospkg
rp = rospkg.RosPack()
try:
    pkg_path = rp.get_path(package)
except rospkg.ResourceNotFound:
    rospy.logwarn("unknown package: " + package + " (ignored)")

That requires this in the package.xml:

<run_depend>python-rospkg</run_depend>

ahendrix added a commit that referenced this issue Feb 26, 2014
now importing roslib. closes #33
@ahendrix
Copy link
Contributor

Tested and released in sound_play 0.2.6: ros/rosdistro#3292

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

3 participants