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

Ubuntu focal/ROS noetic compatibility #30

Closed
heuristicus opened this issue Apr 29, 2021 · 7 comments
Closed

Ubuntu focal/ROS noetic compatibility #30

heuristicus opened this issue Apr 29, 2021 · 7 comments

Comments

@heuristicus
Copy link
Owner

Our lab has recently moved to ubuntu 20.04/ROS noetic so I'm trying to set up our spot payload with that.

With the package set up as it currently is, I was unable to run spot_ros, due to various issues, some of which I have fixed in #29, which I did not encounter when we were still using melodic.

With those fixes I was still not able to run it, getting this error, or similar:

Traceback (most recent call last):
  File "/home/spot/spot_ws/devel/lib/spot_driver/spot_ros.py", line 15, in <module>
    exec(compile(fh.read(), python_script, 'exec'), context)
  File "/home/spot/spot_ws/src/spot_ros/spot_driver/scripts/spot_ros.py", line 33, in <module>
    from spot_wrapper import SpotWrapper
ImportError: cannot import name 'SpotWrapper' from 'spot_wrapper' (/home/spot/spot_ws/devel/.private/spot_driver/lib/spot_driver/spot_wrapper.py)

To me this seemed to be related to the set up of the python package, so I went ahead and created a branch which includes a modified package structure which works on noetic and which I believe is closer to the usual ros python package structure. You can find it at https://github.com/ori-drs/spot_ros/tree/python-package-structure-noetic.

Can anyone else share their experience of using this repository on noetic/focal?

@dniewinski
Copy link
Contributor

Hm. Could have sworn this was working. I'll work on this today and hopefully have an answer for you

@dniewinski
Copy link
Contributor

@civerachb-cpr

@civerachb-cpr
Copy link
Collaborator

It appears that the way Python3 handles relative imports has changed between 3.6 and 3.8 (or at least the issue is replicable with those versions; I have 3.6.9 on my Melodic dev machine, and 3.8.5 in my Noetic VM).

At this point it may be worthwhile to have separate melodic & noetic branches and merge @heuristicus' noetic branch into that. I tried the python-package-structure-noetic fork on my Melodic dev machine, and unfortunately it doesn't work on 18.04.

@heuristicus
Copy link
Owner Author

heuristicus commented May 7, 2021

I'm not super familiar with the guts of python but if you compare https://docs.python.org/3.6/reference/import.html and https://docs.python.org/3.7/reference/import.html#package-relative-imports, it looks like the relative imports change was introduced somewhere in 3.7. I suspect that if we remove the changes I made to the import lines but maintain the package structure changes, it would work on melodic as well. Since there are still a few years before melodic is end-of-life it might be good to have a noetic and melodic branch.

Alternatively, and I don't know if this is a good idea, it should be possible to switch how imports work depending on which version of python is running, something like

import sys
if sys.version_info < (3, 7):
    from ros_helpers import *
    from spot_wrapper import SpotWrapper
else:
    from .ros_helpers import *
    from .spot_wrapper import SpotWrapper

This would save having to maintain two branches.

If you're interested I could update my branch to use this mechanism and make a PR.

@civerachb-cpr
Copy link
Collaborator

Aha! I think the problem I was having on 18.04 was 2 things, both in the modified scripts/spot_ros file:

Original:

#!/bin/env python

Changed:

#!/usr/bin/env python3
  1. On Ubuntu 18.04 there is no /bin/env command (at least not by default). But /usr/bin/env appears to exist in both 18.04 and 20.04, so using that should fix the path issue
  2. On 18.04 the python command defaults to Python2, not Python3. Explicitly stating Python3 should load the correct version.

Applying those changes to your fork should enable everything to work correctly in 18.04 and 20.04. I am able to bring up the driver on my machine, but I don't have access to Spot hardware right now, so I can't verify that everything works. But Python doesn't throw a fit anymore immediately upon starting, which is a good sign.

@dniewinski if you're able, can you try checking out the python-package-structure-noetic branch of https://github.com/ori-drs/spot_ros.git, changing the first line of spot_driver/scripts/spot_ros as I described above, and making sure it actually communicates correctly with BD's driver?

@heuristicus
Copy link
Owner Author

Ah, good catch.

When using catkin_install_python the shebang line is automatically changed on all the scripts that are being installed, according to the python version when the environment is configured (see http://docs.ros.org/en/noetic/api/catkin/html/howto/format2/installing_python.html). That's an additional layer of potential weirdness in terms of python versions. I think explicitly specifying python 3 bypasses this though.

During our conversion of other code we initially explicitly specified that python 3 should be used, but since some of our code still needs to run melodic using catkin_install_python was very useful and so we went back to just using python. I think this is why I mistakenly switched it to use just python instead of python3.

@heuristicus
Copy link
Owner Author

Since #32 has been merged, I will close this issue. Thanks for the help in making this 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

No branches or pull requests

3 participants