Refactor MoveIt Simple Controller Manager to use generate param library #2580
Conversation
| make_default: { | ||
| type: bool, | ||
| default_value: false, | ||
| description: "This param was originally named default. Can't use default with generate_param_lib as default is a C++ keyword. Marking a controller as | ||
| default makes MoveIt prefer this controller when multiple options are available.", | ||
| validation: { | ||
| not_empty<>: [] | ||
| } | ||
| } |
There was a problem hiding this comment.
I also had to rename this parameter. Originally, it was called default but default is a C++ keyword and won't work with how generate param library initializes variables to store the parameter values
/home/abishalini/ws_moveit2/build/moveit_simple_controller_manager/moveit_simple_controller_manager_parameters/include/moveit_simple_controller_manager_parameters.hpp:73:18: error: expected unqualified-id before ‘default’
73 | bool default = false;
| ^~~~~~~
There was a problem hiding this comment.
Do we need to update any config files in the moveit_resources repos or studio?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2580 +/- ##
==========================================
+ Coverage 50.74% 50.78% +0.04%
==========================================
Files 392 388 -4
Lines 32553 32358 -195
==========================================
- Hits 16517 16430 -87
+ Misses 16036 15928 -108 ☔ View full report in Codecov by Sentry. |
| @@ -63,6 +63,11 @@ class FollowJointTrajectoryControllerHandle | |||
|
|
|||
| // TODO(JafarAbdi): Revise parameter lookup | |||
There was a problem hiding this comment.
I think this comment can be removed or updated
| @@ -97,6 +98,19 @@ bool FollowJointTrajectoryControllerHandle::sendTrajectory(const moveit_msgs::ms | |||
| return true; | |||
| } | |||
|
|
|||
| void FollowJointTrajectoryControllerHandle::setPathTolerance(double /*path_tolerance*/) | |||
| { | |||
There was a problem hiding this comment.
Let's print a warning that they're not doing anything at the moment. Why do we need them in the first place?
| { | ||
| goal_template_.goal_time_tolerance = rclcpp::Duration::from_seconds(goal_time_tolerance); | ||
| } | ||
|
|
||
| // TODO(JafarAbdi): Revise parameter lookup | ||
| // void FollowJointTrajectoryControllerHandle::configure(XmlRpc::XmlRpcValue& config) |
| default_value: 0.0, | ||
| description: "Parameter to be set if using FollowJointTrajectory" | ||
| } | ||
| # __map_joints: |
There was a problem hiding this comment.
Why are these ones commented out?
Abishalini
force-pushed
the
pr-moveit-simple-controller-manager-parameters
branch
from
2ba1143
to
edaaac1
Compare
|
This pull request is in conflict. Could you fix it @Abishalini? |
|
This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete. |
github-actions
bot
added
the
stale
|
Seems like this PR resolves the issue of nested map parameters |
github-actions
bot
removed
the
stale
|
@Abishalini do you think you could complete this to get it merged? |
|
This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete. |
github-actions
bot
added
the
stale
|
This pull request is in conflict. Could you fix it @Abishalini? |
github-actions
bot
removed
the
stale
Description
Also resolves #2390
generate_parameter_librarycurrently doesn't support nested maps in the YAML - PickNikRobotics/generate_parameter_library#151So I haven't implemented the
goal_toleranceandpath_toleranceparameters in this PR. But we can now set thegoal_time_tolerancein the YAML file forfollow_joint_trajectory_controller.I will add a TODO/open another issue to implement the nested map parameters (unless I figure out a way to avoid nested maps)