Skip to content

Commit 51849b3

Browse files
nbbrooksclaude
andauthored
Support parallel_gripper_action_controller in Setup Assistant (#3752) (#3773)
* Support parallel_gripper_action_controller in Setup Assistant moveit_resources migrated the gripper from position_controllers/ GripperActionController (Humble/Iron) to parallel_gripper_action_controller/ GripperActionController (Jazzy and newer). Both are single-joint GripperActionControllers that take a `joint` parameter. Teach moveit_setup_controllers to recognize the new type: - ros2_controllers_config: emit the `joint` (singular) parameter for either gripper controller type, instead of only the old one (the old check misclassified the new type and emitted a `joints` list). - ros2_controllers: offer the new type in getAvailableTypes(). - test_controllers (ParsePanda): accept either gripper type so the test is distro-agnostic (the panda config now ships the Jazzy+ type). * Address review: guard gripper joint index, add panda gripper output test - ros2_controllers_config: only write the singular `joint` parameter when the gripper controller actually has a joint, so joints_[0] is never indexed out of bounds for a gripper with an empty joint list. - test_controllers: add OutputPandaGripperUsesSingularJoint, which generates the panda ros2_controllers.yaml and asserts the GripperActionController is emitted with the singular `joint` key (not the `joints` list) -- coverage the gripper-less OutputFanuc test could not provide. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0a18b73 commit 51849b3

3 files changed

Lines changed: 40 additions & 4 deletions

File tree

moveit_setup_assistant/moveit_setup_controllers/include/moveit_setup_controllers/ros2_controllers.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ class ROS2Controllers : public Controllers
7272

7373
std::vector<std::string> getAvailableTypes() const override
7474
{
75-
return { "joint_trajectory_controller/JointTrajectoryController", "position_controllers/GripperActionController" };
75+
// position_controllers/GripperActionController is used on Humble/Iron;
76+
// parallel_gripper_action_controller/GripperActionController on Jazzy+.
77+
return { "joint_trajectory_controller/JointTrajectoryController", "position_controllers/GripperActionController",
78+
"parallel_gripper_action_controller/GripperActionController" };
7679
}
7780

7881
std::string getDefaultType() const override

moveit_setup_assistant/moveit_setup_controllers/src/ros2_controllers_config.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,20 @@ bool ROS2ControllersConfig::GeneratedControllersConfig::writeYaml(YAML::Emitter&
192192
emitter << YAML::Value;
193193
emitter << YAML::BeginMap;
194194
{
195-
if (ci.type_ != "position_controllers/GripperActionController")
195+
// A GripperActionController commands a single joint, so it takes a
196+
// `joint` parameter instead of the `joints` list. Humble/Iron ship it
197+
// as position_controllers/GripperActionController; Jazzy and newer use
198+
// parallel_gripper_action_controller/GripperActionController.
199+
const bool is_gripper_controller = ci.type_ == "position_controllers/GripperActionController" ||
200+
ci.type_ == "parallel_gripper_action_controller/GripperActionController";
201+
if (!is_gripper_controller)
196202
{
197203
emitter << YAML::Key << "joints" << YAML::Value << ci.joints_;
198204
}
199-
else
205+
else if (!ci.joints_.empty())
200206
{
207+
// GripperActionController controls a single joint; guard against an
208+
// empty joint list so we never index joints_[0] out of bounds.
201209
emitter << YAML::Key << "joint" << YAML::Value << ci.joints_[0];
202210
}
203211

moveit_setup_assistant/moveit_setup_controllers/test/test_controllers.cpp

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,12 @@ TEST_F(ControllersTest, ParsePanda)
9292

9393
const ControllerInfo& ci2 = controllers[1 - offset];
9494
EXPECT_EQ("panda_hand_controller", ci2.name_);
95-
EXPECT_EQ("position_controllers/GripperActionController", ci2.type_);
95+
// Humble/Iron use position_controllers/GripperActionController; Jazzy and
96+
// newer use parallel_gripper_action_controller/GripperActionController.
97+
// Accept either so the test is distro-agnostic.
98+
EXPECT_TRUE(ci2.type_ == "position_controllers/GripperActionController" ||
99+
ci2.type_ == "parallel_gripper_action_controller/GripperActionController")
100+
<< "Unexpected gripper controller type: " << ci2.type_;
96101
EXPECT_EQ(1u, ci2.joints_.size());
97102

98103
/*
@@ -122,6 +127,26 @@ TEST_F(ControllersTest, OutputFanuc)
122127
}
123128
}
124129

130+
// Fanuc has no gripper, so OutputFanuc doesn't exercise the gripper branch of
131+
// ros2_controllers.yaml generation. Panda has a GripperActionController, which
132+
// must be written with the singular `joint` key (not the `joints` list). A full
133+
// YAML equivalence check isn't possible here because the panda gripper config
134+
// carries extra parameters (effort/velocity interfaces) the Setup Assistant does
135+
// not generate, so assert on the gripper parameter shape directly.
136+
TEST_F(ControllersTest, OutputPandaGripperUsesSingularJoint)
137+
{
138+
config_data_->preloadWithFullConfig("moveit_resources_panda_moveit_config");
139+
config_data_->get<moveit_setup::controllers::ControlXacroConfig>("control_xacro")->loadFromDescription();
140+
generateFiles<ROS2ControllersConfig>("ros2_controllers");
141+
142+
YAML::Node generated = YAML::LoadFile(output_dir_ / "config/ros2_controllers.yaml");
143+
ASSERT_TRUE(generated["panda_hand_controller"]) << "generated config missing panda_hand_controller";
144+
const YAML::Node params = generated["panda_hand_controller"]["ros__parameters"];
145+
ASSERT_TRUE(params) << "panda_hand_controller missing ros__parameters";
146+
EXPECT_TRUE(params["joint"]) << "gripper controller must emit the singular 'joint' parameter";
147+
EXPECT_FALSE(params["joints"]) << "gripper controller must not emit the 'joints' list";
148+
}
149+
125150
TEST_F(ControllersTest, AddDefaultControllers)
126151
{
127152
// only preload urdf and srdf

0 commit comments

Comments
 (0)