ros2 / ros1_bridge

ROS 2 package that provides bidirectional communication between ROS 1 and ROS 2
Apache License 2.0
452 stars 288 forks source link

on Dashing: f-Strings breaking support with Python 3.5 #305

Closed potentialdiffer closed 3 years ago

potentialdiffer commented 3 years ago

Bug report

Required Info:

Steps to reproduce issue

Compiling package from source on systems with Python 3.5, e.g. Debian 9, Raspbian Stretch ...

Expected behavior

Successful compilation.

Actual behavior

Failing compilation due to syntax error caused by f-Strings.

Traceback (most recent call last):
  File "bin/ros1_bridge_generate_factories", line 11, in <module>
    from ros1_bridge import generate_cpp
  File "/work/ros2_dashing/src/ros2/ros1_bridge/ros1_bridge/__init__.py", line 821
    f"message '{ros2_msg.package_name}/msg/{ros2_msg.message_name}' "
                                                                    ^
SyntaxError: invalid syntax
make[2]: *** [generated/get_factory.cpp] Error 1
make[1]: *** [CMakeFiles/ros1_bridge.dir/all] Error 2
make: *** [all] Error 2
---
Failed   <<< ros1_bridge [31.2s, exited with code 2]

Summary: 0 packages finished [34.2s]
  1 package failed: ros1_bridge
  1 package had stderr output: ros1_bridge

Additional information

We experience compilation issues as of 01b55faf451c7af08b4aa60c7fcc7e1c2b206a4b with f-Strings which where introduced in Python 3.6 and are therefore not supported with earlier Python versions. [1] REP-2000 however states Python 3.5 as dashing's minimum requirements. [2]

A workaround is to fix the package to ros1_bridge version 0.7.6. Dashing support ends this May. I am not sure how relevant this is, but wanted to document it anyway.

[1] https://www.python.org/dev/peps/pep-0498/ [2] https://www.ros.org/reps/rep-2000.html#dashing-diademata-may-2019-may-2021

clalancette commented 3 years ago

Thanks for the report. It looks like it should be easy enough to use one of the other string methods for Dashing so that this doesn't happen. Would you mind opening a pull request to the dashing branch fixing it?

clalancette commented 3 years ago

This was fixed by #306, so closing.