search

moveit / moveit_task_constructor

A hierarchical multi-stage manipulation planner
https://moveit.github.io/moveit_task_constructor
BSD 3-Clause "New" or "Revised" License
176 stars 150 forks source link

moveit default_velocity_scaling_factor is ignored #210

Open JonasTietz opened 4 years ago

JonasTietz commented 4 years ago

Hello, I noticed that the default_velocity_scaling_factor is ignored when planning trajectories with the pipeline planner, which is suprising at least. Especially if you tuned the joint_limits.yaml to be save at the default_velocity_scaling_factor.

rhaschke commented 4 years ago

Configurable scaling factors were introduced in https://github.com/ros-planning/moveit/pull/1890 after the corresponding MTC code was written. In MTC, the scaling factors are configured as (constant) properties here: https://github.com/ros-planning/moveit_task_constructor/blob/d3b878a31c63a5ceaead481b9c536cff929a9941/core/src/solvers/planner_interface.cpp#L45-L49

A pull request replacing this by a lookup of default scaling factors from ROS parameters is highly welcome. I'm not yet sure how to pass the move_group's namespace to access the correct parameters though.

v4hn commented 4 years ago

I discussed this with @JonasTietz yesterday and was wondering whether we want to support this in MTC at all. You are right, we can add support for this to the PlannerInterface classes, but doing so will ignore all custom stages and that might be more surprising than leaving it out everywhere.

We could also tackle this by making time parameterization an explicit parameter in each stage, additionally supporting different algorithms for that. I'm not sure I like the associated overhead though and it would add more parameters if you want to auto compute a "correct" dynamics profile for some stages, but allow the stage to compute their own profile if they know better (pouring is a good example for this :))

Lastly, we could also add default properties for this similar to the marker_ns property.

rhaschke commented 4 years ago

Actually, I like the idea of adding time parameterization as an explicit, configurable property of stages. Some stages, e.g. the Merger or merger Connect stages, don't have time parameterization (or it is hardcoded for a specific method). But I agree, that only configuring this for PlannerInterface classes, might be not sufficient.