While we need to pass execution_type to fake_moveit_controller_manager.launch

I think it's not even used in the fake_moveit_controller_manager.launch, the argument is declared there but it's not used.

Also if the argument is removed in trajectory_execution.launch.xml I guess we can also remove it in the robot specific controller manager https://github.com/ros-planning/moveit/blob/e763f768e3cd252ecb933a26678c95d718de37ba/moveit_setup_assistant/templates/moveit_config_pkg_template/launch/moveit_controller_manager.launch.xml#L4

Codecov Report

Merging #2928 (e763f76) into master (f3ac607) will increase coverage by 0.02%.
The diff coverage is n/a.

❗ Current head e763f76 differs from pull request most recent head 6453670. Consider uploading reports for the commit 6453670 to get more accurate results

@@            Coverage Diff             @@
##           master    #2928      +/-   ##
==========================================
+ Coverage   61.33%   61.34%   +0.02%     
==========================================
  Files         373      373              
  Lines       31766    31766              
==========================================
+ Hits        19479    19485       +6     
+ Misses      12287    12281       -6     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3ac607...6453670. Read the comment docs.

@rhaschke Sorry to be nitpicking, but I think I'm missing something here. In my tests and also the panda_moveit_config the pass_all_args="true" is not needed since @mintar added the following code 5 months ago (see 781a82c):

https://github.com/ros-planning/moveit/blob/dc318f95f6e3c19cebbc0d14136277fb46749f78/moveit_setup_assistant/templates/moveit_config_pkg_template/launch/moveit_controller_manager.launch.xml#L3-L4

This can be seen by running roslaunch moveit_bug_2786_example_repo main.launch moveit_controller_manager:=fake and roslaunch moveit_bug_2786_example_repo main.launch while building the following example repository.

I think there is nothing wrong with #2928 but if you use pass_all_args="true" you can remove the following code:

https://github.com/ros-planning/moveit/blob/dc318f95f6e3c19cebbc0d14136277fb46749f78/moveit_setup_assistant/templates/moveit_config_pkg_template/launch/moveit_controller_manager.launch.xml#L3-L4

Personally, I liked @mintar more since it is more explicit and thus easier to see what is going on.

I only tested on ROS noetic, so it could be that it works differently on different ROS versions.

@rickstaa and @marioney, you are both right.
This line can be removed (which I did it for moveit/panda_moveit_config@52bc5c8):
https://github.com/ros-planning/moveit/blob/dc318f95f6e3c19cebbc0d14136277fb46749f78/moveit_setup_assistant/templates/moveit_config_pkg_template/launch/moveit_controller_manager.launch.xml#L3-L4

I am open to both, removing or keeping the line. I agree with Rick's point that it explicitly states why we need the argument.
My PR was mainly addressing issues with existing MoveIt configs that don't (yet) have this line - but that doesn't help if they don't recreate via MSA either ...
@v4hn: What's your opinion?

I prefer the implicit argument passing with your new patch and would remove the unused argument slot in the real execution manager again.
It's bike shedding to debate this a lot though. Feel free to push either solution.

@v4hn I think you are right, it leads to less code and the comment explains what is happening.