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

Delete deprecated Panda config and launch files #98

Merged
merged 9 commits into from
Mar 3, 2022

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Oct 13, 2021

All of the parameters from the Panda ompl_planning_pipeline.launch.xml have been moved to demo.launch.py so this config file is not used at all. For clarity, I think we should delete it.

Also delete fake_controllers.yaml which is not needed at all in ROS2. The ros2_control Fake system replaced it.

@henningkayser
Copy link
Member

All of the parameters from the Panda ompl_planning_pipeline.launch.xml have been moved to demo.launch.py so this config file is not used at all. For clarity, I think we should delete it.

Makes sense. With that, you could technically delete all launch xml files since those are not used in ROS 2 at all.

It looks like the Fanuc package hasn't really been ported to ROS2 at all, so I also deleted the same xml file there.

In this case it makes sense to keep the files, imo. Otherwise you would have to check out an old branch for porting it.

@AndyZe AndyZe changed the title Delete unused ompl xml files Delete unused Panda xml files Feb 21, 2022
@AndyZe
Copy link
Member Author

AndyZe commented Feb 21, 2022

@henningkayser I rebased this, reset the Fanuc changes, and grepped to double-check that I caught all unused Panda xml files.

@AndyZe AndyZe changed the title Delete unused Panda xml files Delete unused Panda xml & yaml files Feb 25, 2022
Copy link

@jliukkonen jliukkonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the look of it, there are many more ROS1 launch files for the Panda arm. Are all of the remaining, old .launch files with XML compatible with ROS2? It looks like, weirdly enough, ROS2 does still support the XML style launch files and even adds new keywords. https://docs.ros.org/en/galactic/How-To-Guides/Launch-files-migration-guide.html

To be clear, I don't have any issues with this PR, just wondering if even more files can/should be removed.

@AndyZe
Copy link
Member Author

AndyZe commented Feb 28, 2022

I think we should delete those old launch files now. If nobody pipes up with a strong opinion soon, I'll do so.

@AndyZe
Copy link
Member Author

AndyZe commented Feb 28, 2022

I'm not really sure why they've been kept around so long. I guess it was for convenience when porting to ROS2.

@AndyZe AndyZe changed the title Delete unused Panda xml & yaml files Delete deprecated Panda xml & yaml files Feb 28, 2022
@AndyZe AndyZe changed the title Delete deprecated Panda xml & yaml files Delete deprecated Panda config and launch files Feb 28, 2022
Copy link

@jliukkonen jliukkonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@henningkayser henningkayser merged commit f2cd2a0 into moveit:ros2 Mar 3, 2022
@@ -79,7 +79,7 @@ def generate_launch_description():

# Trajectory Execution Functionality
moveit_simple_controllers_yaml = load_yaml(
"moveit_resources_panda_moveit_config", "config/panda_controllers.yaml"
"moveit_resources_panda_moveit_config", "config/panda_moveit_controllers.yaml"
Copy link
Contributor

@DLu DLu Mar 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean panda_ros2_controllers.yaml?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(panda_moveit_controllers.yaml doesn't exist)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I guess I forget to rename the file. It's currently named panda_controllers.yaml. Will make a fixup PR right now.

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.

None yet

4 participants