Skip to content

Delete deprecated Panda config and launch files#98

Merged
henningkayser merged 9 commits intomoveit:ros2from
AndyZe:andyz/param_simplification
Mar 3, 2022
Merged

Delete deprecated Panda config and launch files#98
henningkayser merged 9 commits intomoveit:ros2from
AndyZe:andyz/param_simplification

Conversation

@AndyZe
Copy link
Copy Markdown
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
Copy Markdown
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 force-pushed the andyz/param_simplification branch from 94e8fdf to ea0f7bb Compare February 21, 2022 01:29
@AndyZe AndyZe changed the title Delete unused ompl xml files Delete unused Panda xml files Feb 21, 2022
@AndyZe
Copy link
Copy Markdown
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 requested a review from jliukkonen February 25, 2022 14:38
@AndyZe AndyZe changed the title Delete unused Panda xml files Delete unused Panda xml & yaml files Feb 25, 2022
Copy link
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

@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
# 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

4 participants