melodic: 'param' unexpected in 'include' in move_group.launch

[gavanderhoorn]

Description

roslaunch complains about a param element inside an include element in move_group.launch with every MoveIt configuration package generated by the MSA on Melodic.

Your environment

• ROS Distro: Melodic
• OS Version: Ubuntu 18.04
• binary, apt: 1.0.8

Steps to reproduce

Generate a MoveIt config pkg using the current MSA on Melodic.

It'll include the following move_group.launch:

https://github.com/ros-planning/moveit/blob/dc5b4102a87de91fa41b2806db6655a3ec44c541/moveit_setup_assistant/templates/moveit_config_pkg_template/launch/move_group.launch#L48-L52

[gavanderhoorn]

Reason I first open an issue is because this was introduced in #1893 in master (here), and then got overwritten by #2127 (here). As Noetic basically gets master, it's not a problem there.

#2127 moves those two params back to the lines later on in move_group.launch (as children of move_group the node), which makes more sense to me.

Perhaps there was a reason they got moved to be args on the included planning_pipeline.launch.xml, which I'm not aware of.

As melodic-devel doesn't have #2127, moving them back could lead to (further) regressions?

[v4hn]

crap.

I guess this is related to the pilz_planner providing custom capabilities (namely the Sequence capabilities.

However, these are now handled in the move_group node now by looking for additional capabilities in the pipeline namespaces, so there is no reason to have the args with the planning pipeline in any way and they should be below with the move_group node.

Could you create a pull-request please?

[gavanderhoorn]

I'm happy to move them back, but I don't see the same code in Melodic @v4hn:

https://github.com/ros-planning/moveit/blob/dc5b4102a87de91fa41b2806db6655a3ec44c541/moveit_ros/move_group/src/move_group.cpp#L110-L179

I believe that was also introduced in #2127, which Melodic doesn't have.

[v4hn]

You are right, moving the parameters down is not enough to solve the issue, though I would be happier. :-) The pilz planner PR moved the responsibility to set the capabilities parameter into the `*_planning_pipeline.launch.xml` and that's the current situation in melodic now. I don't like that at all, but I also did not review the original PR, so I can hardly complain... Changing it once again is also unnecessary breakage. Users have to regenerate their configurations for noetic anyway and it's different there. So I would say, let's turn the broken `<param>` tags you found into `<arg>`s ([which they were intended to be](https://github.com/ros-planning/moveit/blob/melodic-devel/moveit_setup_assistant/templates/moveit_config_pkg_template/launch/planning_pipeline.launch.xml#L9-L12)) and keep the logic that sets the capabilities parameter in melodic. Does that work for you?

[gavanderhoorn]

Yeah, I already did that locally to make things work again, so I can certainly PR that.

Things like this make me wonder whether any of the (active) maintainers actually use the MSA. It's not difficult to spot this, roslaunch is pretty vocal about it -- not to mention the capability management doesn't work correctly like this.

[v4hn]

Things like this make me wonder whether any of the (active) maintainers actually use the MSA.

Obviously you do. Last time I checked you were a maintainer and you are always active in comments. ;) On a more serious note, the nature of the MSA is such that you won't use it in your everyday workflow. For my own projects I use it something like twice a year, though I also get feedback at times from colleagues who use it. Automated tests for the MSA (even just for the backend and without writing GUI tests) would help a lot with this, but somebody would have to write them.

It's not difficult to spot this, `roslaunch` is pretty vocal about it

So thanks for bringing it up and counteracting [the Bystander effect](https://psycnet.apa.org/record/1968-08862-001) of FOSS projects!

[felixvd]

On a more serious note, the nature of the MSA is such that you won't use it in your everyday workflow. For my own projects I use it something like twice a year.

Same for me. It's easy to miss regressions like this in MSA, especially on legacy branches.

[gavanderhoorn]

It's easy to miss regressions like this in MSA, especially on legacy branches.

the regression was actually on master. It just got fixed there almost accidentally by an unrelated follow-up PR.

[gavanderhoorn]

Closing as #2729 got merged.