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

make subprocess output a string instead of bytes, needed in python 3.7 #113

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

lucasw
Copy link

@lucasw lucasw commented Mar 22, 2022

Running roswtf resulted in this exception on Ubuntu 20.04 and ros noetic both in the apt released version and current source here:

running tf checks, this will take a second...
... tf checks complete
Traceback (most recent call last):
  File "<string>", line 224, in _roswtf_main
  File "/opt/ros/noetic/lib/python3/dist-packages/openni2_launch/wtf_plugin.py", line 114, in roswtf_plugin_online
    error_rule(r, r[0](ctx), ctx)
  File "/opt/ros/noetic/lib/python3/dist-packages/openni2_launch/wtf_plugin.py", line 85, in sensor_notfound
    devices = _device_notfound_subproc(
  File "/opt/ros/noetic/lib/python3/dist-packages/openni2_launch/wtf_plugin.py", line 58, in _device_notfound_subproc
    for i in df.split('\n'):
TypeError: a bytes-like object is required, not 'str'
a bytes-like object is required, not 'str'

Adding text=True to the subprocess call looks to be the correct thing to do in newer python versions:

https://docs.python.org/3/library/subprocess.html#subprocess.CompletedProcess.stdout

https://stackoverflow.com/questions/5902485/can-i-have-subprocess-call-write-the-output-of-the-call-to-a-string

@lucasw lucasw force-pushed the subprocess_wtf_python3_7 branch from db7b370 to 9ad6eb7 Compare December 26, 2022 17:07
@130s
Copy link
Member

130s commented Feb 17, 2023

Thank you for the contribution.

Although I won't have a room to take a closer look nor test myself, I'm inclined to merging (and depend on the OSS community to test). In a few days I might merge unless we'll receive objections.

@adivardi
Copy link

@130s
sorry to at you, but since it is approved already it would be nice to merge. This PR fixes the package for python3 and is blocking any usage of roswtf with this package installed.

@130s 130s merged commit 2282d9b into ros-drivers:ros1 Oct 25, 2023
@130s
Copy link
Member

130s commented Oct 25, 2023

Thanks for the headsup. Merged, made 1.6.1 release for ros1 branch #133, and made a request to build an installer package for ROS Noetic ros/rosdistro#38806.

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