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

fix ros_location_find with Python 3 #270

Merged
merged 1 commit into from
Sep 17, 2020
Merged

Conversation

seanyen
Copy link
Contributor

@seanyen seanyen commented Sep 16, 2020

Since Python 3, the subprocess.Popen has a text/non-text mode which decides the data type to use for communicate.

Source: https://docs.python.org/3.8/library/subprocess.html#subprocess.Popen.communicate

If streams were opened in text mode, input must be a string. Otherwise, it must be bytes.

And without extra conversion, the rosfindpath.py will fail with the following trackback:

Traceback (most recent call last):
  File "c:\opt\ros\noetic\x64\bin\\rosfindpath.py", line 82, in <module>
    sys.exit(findpathmain(sys.argv[1:]))
  File "c:\opt\ros\noetic\x64\bin\\rosfindpath.py", line 74, in findpathmain
    rosdir = os.path.normpath(os.path.sep.join([package_dir, reldir]))
TypeError: sequence item 0: expected str instance, bytes found

This pull request is try to use decode() to convert it back to str to make it type correct.

This fix was motivated by the issue reported here: ms-iot/ROSOnWindows#248

@seanyen
Copy link
Contributor Author

seanyen commented Sep 16, 2020

cc @dirk-thomas @sloretz This is ready for review and merge. Thanks!

@dirk-thomas dirk-thomas changed the title [Windows][Noetic] Enable Python3 support fix ros_location_find with Python 3 Sep 17, 2020
@dirk-thomas
Copy link
Member

Thanks for the fix.

@dirk-thomas dirk-thomas merged commit ff219ab into ros:noetic-devel Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants