Skip to content

Commit

Permalink
[GripperController] Fix failing tests (#1210)
Browse files Browse the repository at this point in the history
  • Loading branch information
saikishor authored Jul 16, 2024
1 parent 0bb8880 commit b958da3
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 42 deletions.
14 changes: 8 additions & 6 deletions parallel_gripper_controller/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,19 @@ if(BUILD_TESTING)
find_package(controller_manager REQUIRED)
find_package(ros2_control_test_assets REQUIRED)

ament_add_gmock(test_load_parallel_gripper_action_controllers
test/test_load_parallel_gripper_action_controller.cpp
)
add_rostest_with_parameters_gmock(test_load_parallel_gripper_action_controllers
test/test_load_parallel_gripper_action_controller.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test/gripper_action_controller_params.yaml)
target_include_directories(test_load_parallel_gripper_action_controllers PRIVATE include)
ament_target_dependencies(test_load_parallel_gripper_action_controllers
controller_manager
ros2_control_test_assets
)

ament_add_gmock(test_parallel_gripper_controller
test/test_parallel_gripper_controller.cpp
)
add_rostest_with_parameters_gmock(test_parallel_gripper_controller
test/test_parallel_gripper_controller.cpp
${CMAKE_CURRENT_SOURCE_DIR}/test/gripper_action_controller_params.yaml)
target_include_directories(test_parallel_gripper_controller PRIVATE include)
target_link_libraries(test_parallel_gripper_controller
parallel_gripper_action_controller
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ controller_interface::CallbackReturn GripperActionController::on_init()
try
{
param_listener_ = std::make_shared<ParamListener>(get_node());
params_ = param_listener_->get_params();
}
catch (const std::exception & e)
{
Expand Down Expand Up @@ -245,6 +244,7 @@ controller_interface::CallbackReturn GripperActionController::on_configure(
RCLCPP_ERROR(logger, "Joint name cannot be empty");
return controller_interface::CallbackReturn::ERROR;
}
RCLCPP_INFO(logger, "Joint name is : %s", params_.joint.c_str());

return controller_interface::CallbackReturn::SUCCESS;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ parallel_gripper_action_controller:
joint: {
type: string,
read_only: true,
validation: {
not_empty<>: null
}
}
state_interfaces: {
type: string_array,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
test_gripper_action_position_controller:
ros__parameters:
joint: "joint1"

test_gripper_action_position_controller_empty_joint:
ros__parameters:
joint: ""
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

TEST(TestLoadGripperActionControllers, load_controller)
{
rclcpp::init(0, nullptr);

std::shared_ptr<rclcpp::Executor> executor =
std::make_shared<rclcpp::executors::SingleThreadedExecutor>();

Expand All @@ -32,12 +30,16 @@ TEST(TestLoadGripperActionControllers, load_controller)

ASSERT_NE(
cm.load_controller(
"test_gripper_action_position_controller", "position_controllers/GripperActionController"),
nullptr);
ASSERT_NE(
cm.load_controller(
"test_gripper_action_effort_controller", "effort_controllers/GripperActionController"),
"test_gripper_action_position_controller",
"parallel_gripper_action_controller/GripperActionController"),
nullptr);
}

int main(int argc, char ** argv)
{
::testing::InitGoogleMock(&argc, argv);
rclcpp::init(argc, argv);
int result = RUN_ALL_TESTS();
rclcpp::shutdown();
return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ using GoalHandle = rclcpp_action::ServerGoalHandle<GripperCommandAction>;
using testing::SizeIs;
using testing::UnorderedElementsAre;

void GripperControllerTest::SetUpTestCase() { rclcpp::init(0, nullptr); }
void GripperControllerTest::SetUpTestCase() {}

void GripperControllerTest::TearDownTestCase() { rclcpp::shutdown(); }
void GripperControllerTest::TearDownTestCase() {}

void GripperControllerTest::SetUp()
{
Expand All @@ -46,11 +46,13 @@ void GripperControllerTest::SetUp()

void GripperControllerTest::TearDown() { controller_.reset(nullptr); }

void GripperControllerTest::SetUpController()
void GripperControllerTest::SetUpController(
const std::string & controller_name = "test_gripper_action_position_controller",
controller_interface::return_type expected_result = controller_interface::return_type::OK)
{
const auto result =
controller_->init("gripper_controller", "", 0, "", controller_->define_custom_node_options());
ASSERT_EQ(result, controller_interface::return_type::OK);
controller_->init(controller_name, "", 0, "", controller_->define_custom_node_options());
ASSERT_EQ(result, expected_result);

std::vector<LoanedCommandInterface> command_ifs;
command_ifs.emplace_back(this->joint_1_cmd_);
Expand All @@ -62,7 +64,9 @@ void GripperControllerTest::SetUpController()

TEST_F(GripperControllerTest, ParametersNotSet)
{
this->SetUpController();
this->SetUpController(
"test_gripper_action_position_controller_no_parameters",
controller_interface::return_type::ERROR);

// configure failed, 'joints' parameter not set
ASSERT_EQ(
Expand All @@ -72,9 +76,9 @@ TEST_F(GripperControllerTest, ParametersNotSet)

TEST_F(GripperControllerTest, JointParameterIsEmpty)
{
this->SetUpController();

this->controller_->get_node()->set_parameter({"joint", ""});
this->SetUpController(
"test_gripper_action_position_controller_empty_joint",
controller_interface::return_type::ERROR);

// configure failed, 'joints' is empty
ASSERT_EQ(
Expand All @@ -86,7 +90,9 @@ TEST_F(GripperControllerTest, ConfigureParamsSuccess)
{
this->SetUpController();

this->controller_->get_node()->set_parameter({"joint", "joint_1"});
this->controller_->get_node()->set_parameter({"joint", "joint1"});

rclcpp::spin_some(this->controller_->get_node()->get_node_base_interface());

// configure successful
ASSERT_EQ(
Expand All @@ -98,29 +104,14 @@ TEST_F(GripperControllerTest, ConfigureParamsSuccess)
ASSERT_THAT(cmd_if_conf.names, SizeIs(1lu));
ASSERT_THAT(
cmd_if_conf.names,
UnorderedElementsAre(std::string("joint_1/") + hardware_interface::HW_IF_POSITION));
UnorderedElementsAre(std::string("joint1/") + hardware_interface::HW_IF_POSITION));
EXPECT_EQ(cmd_if_conf.type, controller_interface::interface_configuration_type::INDIVIDUAL);
auto state_if_conf = this->controller_->state_interface_configuration();
ASSERT_THAT(state_if_conf.names, SizeIs(2lu));
ASSERT_THAT(state_if_conf.names, UnorderedElementsAre("joint_1/position", "joint_1/velocity"));
ASSERT_THAT(state_if_conf.names, UnorderedElementsAre("joint1/position", "joint1/velocity"));
EXPECT_EQ(state_if_conf.type, controller_interface::interface_configuration_type::INDIVIDUAL);
}

TEST_F(GripperControllerTest, ActivateWithWrongJointsNamesFails)
{
this->SetUpController();

this->controller_->get_node()->set_parameter({"joint", "unicorn_joint"});

// activate failed, 'joint4' is not a valid joint name for the hardware
ASSERT_EQ(
this->controller_->on_configure(rclcpp_lifecycle::State()),
controller_interface::CallbackReturn::SUCCESS);
ASSERT_EQ(
this->controller_->on_activate(rclcpp_lifecycle::State()),
controller_interface::CallbackReturn::ERROR);
}

TEST_F(GripperControllerTest, ActivateSuccess)
{
this->SetUpController();
Expand Down Expand Up @@ -167,3 +158,12 @@ TEST_F(GripperControllerTest, ActivateDeactivateActivateSuccess)
this->controller_->on_activate(rclcpp_lifecycle::State()),
controller_interface::CallbackReturn::SUCCESS);
}

int main(int argc, char ** argv)
{
::testing::InitGoogleMock(&argc, argv);
rclcpp::init(argc, argv);
int result = RUN_ALL_TESTS();
rclcpp::shutdown();
return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ class GripperControllerTest : public ::testing::Test
void SetUp();
void TearDown();

void SetUpController();
void SetUpController(
const std::string & controller_name, controller_interface::return_type expected_result);
void SetUpHandles();

protected:
Expand Down

0 comments on commit b958da3

Please sign in to comment.