Skip to content

MSA: write the default controller namespace#1515

Merged
AndyZe merged 4 commits intomoveit:mainfrom
AndyZe:andyz/msa_controllers_yaml
Aug 17, 2022
Merged

MSA: write the default controller namespace#1515
AndyZe merged 4 commits intomoveit:mainfrom
AndyZe:andyz/msa_controllers_yaml

Conversation

@AndyZe
Copy link
Copy Markdown
Member

@AndyZe AndyZe commented Aug 15, 2022 •
edited
Loading

Description

MoveIt Setup Assistant should write the controller namespace to the yaml file, like this:

  arm_controller:
    type: FollowJointTrajectory
    action_ns: follow_joint_trajectory
    joints:
      - J1
...
      - J6

Fixes #1514

@AndyZe AndyZe requested review from DLu and JafarAbdi August 15, 2022 21:11
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 15, 2022 •
edited
Loading

Codecov Report

Merging #1515 (92a37ba) into main (e89526d) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1515      +/-   ##
==========================================
+ Coverage   51.13%   51.16%   +0.03%     
==========================================
  Files         380      380              
  Lines       31755    31758       +3     
==========================================
+ Hits        16236    16246      +10     
+ Misses      15519    15512       -7
Impacted Files Coverage Δ
...etup_controllers/src/moveit_controllers_config.cpp 85.58% <100.00%> (+0.43%) ⬆️
...nning_scene_monitor/src/planning_scene_monitor.cpp 45.73% <0.00%> (+0.44%) ⬆️
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 76.43% <0.00%> (+1.08%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@AndyZe AndyZe changed the title Write the default controller namespace MSA: write the default controller namespace Aug 15, 2022
mikeferguson
mikeferguson reviewed Aug 16, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@mikeferguson mikeferguson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there must have been a recent regression - my generated config a few weeks ago (June 29) has these defined properly - https://github.com/mikeferguson/ubr_reloaded/blob/ros2/ubr1_moveit/config/moveit_controllers.yaml - aren't these actually "controller.parameters_" (which get exported about ten lines below?)

moveit_setup_assistant/moveit_setup_controllers/src/moveit_controllers_config.cpp Outdated
{
RCLCPP_ERROR_STREAM(*logger_, "Only a FollowJointTrajectory type of controller is supported, for now. "
"Update moveit_controllers.yaml.");
return false;
Copy link
Copy Markdown
Contributor

@mikeferguson mikeferguson Aug 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will actually break the ability to re-export if you have other controllers defined/updated (for instance, a gripper controller would cause this to fail to export).

Copy link
Copy Markdown
Member Author

@AndyZe AndyZe Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for mentioning that -- I forgot about grippers.

I simplified this to be identical to the state before 8ee767a

@mikeferguson
Copy link
Copy Markdown
Contributor

mikeferguson commented Aug 16, 2022

This PR: 8ee767a is relevant - it's where these became generic "parameters_"

@AndyZe AndyZe merged commit 066e862 into moveit:main Aug 17, 2022
@tylerjw tylerjw added the backport-humble Mergify label that triggers a PR backport to Humble label Oct 28, 2022
@mergify mergify bot mentioned this pull request Oct 28, 2022
Merged
mergify bot pushed a commit that referenced this pull request Oct 28, 2022
@mergify
MSA: write the default controller namespace (#1515)
5ddaf26 
* Write the default controller namespace

* Check for a sane controller type

* Revert the check for FollowJointTrajectory type

(cherry picked from commit 066e862)
tylerjw pushed a commit that referenced this pull request Oct 28, 2022
@mergify
MSA: write the default controller namespace (#1515) (#1651)
2042b82 
(cherry picked from commit 066e862)

Co-authored-by: AndyZe <zelenak@picknik.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikeferguson mikeferguson mikeferguson left review comments

@JafarAbdi JafarAbdi JafarAbdi approved these changes

@DLu DLu Awaiting requested review from DLu

Assignees

No one assigned

Labels

backport-humble Mergify label that triggers a PR backport to Humble

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MSA: default moveit_controllers.yaml is missing a few lines

4 participants

@AndyZe @mikeferguson @JafarAbdi @tylerjw