BerkeleyAutomation / FogROS2

An Adaptive and Extensible Platform for Cloud and Fog Robotics Using ROS 2
https://berkeleyautomation.github.io/FogROS2
Apache License 2.0
172 stars 29 forks source link

Circular import when trying to launch #27

Closed mjd3 closed 2 years ago

mjd3 commented 2 years ago

Describe the bug When trying to run ros2 launch, it appears there is a circular import causing it to crash. Potentially due to multiple modules named launch?

To Reproduce Steps to reproduce the behavior:

  1. Follow docker instructions to build docker image (using mjd3/code-cleanup branch)
  2. Run ros2 launch inside container Stack trace:
    Failed to load entry point 'launch': cannot import name 'LaunchContext' from partially initialized module 'launch.launch_context' (most likely due to a circular import) (/home/root/fog_ws/install/lib/python3.10/site-packages/launch/launch_context.py)
    Traceback (most recent call last):
    File "/opt/ros/rolling/bin/ros2", line 33, in <module>
    sys.exit(load_entry_point('ros2cli==0.18.0', 'console_scripts', 'ros2')())
    File "/opt/ros/rolling/lib/python3.10/site-packages/ros2cli/cli.py", line 50, in main
    add_subparsers_on_demand(
    File "/opt/ros/rolling/lib/python3.10/site-packages/ros2cli/command/__init__.py", line 237, in add_subparsers_on_demand
    extension = command_extensions[name]
    KeyError: 'launch'

Expected behavior Should instead output the possible next arguments, not crash.

vmayoral commented 2 years ago

Thanks for raising this up @mjd3, problem was previously described at https://github.com/BerkeleyAutomation/FogROS2/issues/15#issue-1196892468 and yesterday again at the end of my comment at https://github.com/BerkeleyAutomation/FogROS2/issues/19#issuecomment-1099446348. Particularly, note:

Remove duplicated code in FogROS2 launch extensions fogros2_launch (ping @jeffi and @KeplerC, there's currently lots of duplicated code and we should only include the extensions)

In a nutshell, the extension of the ROS 2 launch file system has been done by dumping the launch package re-building the package as an overlay (and re-extending the CLI tooling with the same verbs and subverbs). This obviously creates a clash. I'm pretty sure that you'll observe more errors as well if you just type ros2 in the CLI, as again this dives deep into how ROS 2 extensions work.

The conflicting package is https://github.com/BerkeleyAutomation/FogROS2/tree/mjd3/code-cleanup/fogros2_launch (which if you observe, has lots of duplicates when compared to https://github.com/ros2/launch/tree/master/launch). We should not be duplicating all of this. Instead, we should only be specializing whatever classes are needed (I think it's only just one class, right @KeplerC?).

So what's needed is simply to clean up https://github.com/BerkeleyAutomation/FogROS2/tree/mjd3/code-cleanup/fogros2_launch, remove all duplicates with respect launch and leave it as a simple set of extensions (classes that inherit launch abstractions and specialize them for the purpose of FogROS2). Let me know if that helps.

Also, these changes would need to be brought to all branches for things to work fine (including main and humble).

vmayoral commented 2 years ago

Added this to the list we're keeping at https://github.com/BerkeleyAutomation/FogROS2/issues/15 since this is also a major roadblock for release.

mjd3 commented 2 years ago

This should be closed by #29