Don't assume gripper controller for single joint control in MoveIt Setup Assistant #2555
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
We'd also like this backported to Humble |
sjahr
approved these changes
Nov 27, 2023
sjahr
added
backport-humble
sjahr
reviewed
Nov 27, 2023
moveit_setup_assistant/moveit_setup_controllers/src/ros2_controllers_config.cpp
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2555 +/- ##
==========================================
+ Coverage 50.52% 50.52% +0.01%
==========================================
Files 387 387
Lines 32099 32099
==========================================
+ Hits 16215 16216 +1
+ Misses 15884 15883 -1 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Sebastian Jahr <sebastian.jahr@tuta.io>
| @@ -192,7 +192,7 @@ bool ROS2ControllersConfig::GeneratedControllersConfig::writeYaml(YAML::Emitter& | |||
| emitter << YAML::Value; | |||
| emitter << YAML::BeginMap; | |||
| { | |||
| if (ci.joints_.size() != 1) | |||
| if (ci.type_ != "position_controllers/GripperActionController") | |||
There was a problem hiding this comment.
While the diff will be slightly larger - maybe swap these statements so that we aren't working in negatives?
if (ci.type_ == "position_controllers/GripperActionController")
{
emitter << YAML::Key << "joint" << YAML::Value << ci.joints_[0];
}
else
{
emitter << YAML::Key << "joints" << YAML::Value << ci.joints_;
}
There was a problem hiding this comment.
The default behavior is to use the "joints" parameter as only the gripper controller uses the singular "joint". Either putting the "default" or "positive" clause first would work, but I think I'll leave it as is for now.
mergify bot
pushed a commit
that referenced
this pull request
Nov 29, 2023
…tup Assistant (#2555) * For single joint controllers which are not gripper controllers, still output joints list * Use OR * Only check for GripperActionController Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> --------- Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> (cherry picked from commit 81094a6)
mergify bot
pushed a commit
that referenced
this pull request
Nov 29, 2023
…tup Assistant (#2555) * For single joint controllers which are not gripper controllers, still output joints list * Use OR * Only check for GripperActionController Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> --------- Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> (cherry picked from commit 81094a6)
henningkayser
pushed a commit
that referenced
this pull request
Dec 15, 2023
…tup Assistant (backport #2555) (#2560) * For single joint controllers which are not gripper controllers, still output joints list * Use OR * Only check for GripperActionController Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> --------- Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> (cherry picked from commit 81094a6) Co-authored-by: Forrest Rogers-Marcovitz <39061824+forrest-rm@users.noreply.github.com>
henningkayser
pushed a commit
that referenced
this pull request
Dec 15, 2023
…tup Assistant (backport #2555) (#2559) * For single joint controllers which are not gripper controllers, still output joints list * Use OR * Only check for GripperActionController Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> --------- Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> (cherry picked from commit 81094a6) Co-authored-by: Forrest Rogers-Marcovitz <39061824+forrest-rm@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The MoveIt Setup Assistant assumed any controller with a single joint was a gripper controller. This created a bug in the generated ros2_controllers yaml with parameter
jointinstead ofjoints.