Skip to content

Commit 0fc660d

Browse files
committed
fix: address reviewer comments , merge tests and add comment
1 parent 9ea1764 commit 0fc660d

2 files changed

Lines changed: 3 additions & 38 deletions

File tree

controller_manager/src/controller_manager.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,6 +1623,7 @@ controller_interface::return_type ControllerManager::configure_controller(
16231623
return controller_interface::return_type::ERROR;
16241624
}
16251625

1626+
// Remove any stale exported interfaces from a prior configure cycle before re-configuring.
16261627
cleanup_controller_exported_interfaces(*found_it);
16271628

16281629
try

controller_manager/test/test_load_controller.cpp

Lines changed: 2 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -221,57 +221,21 @@ TEST_P(TestLoadedControllerParametrized, can_not_start_finalized_controller)
221221
lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED, controller_if->get_lifecycle_state().id());
222222
}
223223

224-
TEST_P(TestLoadedControllerParametrized, inactive_controller_cannot_be_cleaned_up)
224+
TEST_P(TestLoadedControllerParametrized, configure_controller_requires_unconfigured_state)
225225
{
226226
const auto test_param = GetParam();
227227

228228
EXPECT_EQ(cm_->configure_controller(CONTROLLER_NAME_1), controller_interface::return_type::OK);
229229

230230
start_test_controller(test_param.strictness);
231-
232231
stop_test_controller(test_param.strictness);
233-
234232
ASSERT_EQ(
235233
lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE, controller_if->get_lifecycle_state().id());
236234

237-
std::shared_ptr<test_controller::TestController> test_controller =
238-
std::dynamic_pointer_cast<test_controller::TestController>(controller_if);
239-
size_t cleanup_calls = 0;
240-
test_controller->cleanup_calls = &cleanup_calls;
241-
// Configure from inactive state: rejected because controller is not unconfigured
242-
test_controller->simulate_cleanup_failure = true;
235+
// configure_controller must reject any state other than UNCONFIGURED
243236
EXPECT_EQ(cm_->configure_controller(CONTROLLER_NAME_1), controller_interface::return_type::ERROR);
244237
ASSERT_EQ(
245238
lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE, controller_if->get_lifecycle_state().id());
246-
EXPECT_EQ(0u, cleanup_calls);
247-
}
248-
249-
TEST_P(TestLoadedControllerParametrized, inactive_controller_cannot_be_configured)
250-
{
251-
const auto test_param = GetParam();
252-
253-
EXPECT_EQ(cm_->configure_controller(CONTROLLER_NAME_1), controller_interface::return_type::OK);
254-
255-
start_test_controller(test_param.strictness);
256-
257-
stop_test_controller(test_param.strictness);
258-
ASSERT_EQ(
259-
lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE, controller_if->get_lifecycle_state().id());
260-
261-
std::shared_ptr<test_controller::TestController> test_controller =
262-
std::dynamic_pointer_cast<test_controller::TestController>(controller_if);
263-
size_t cleanup_calls = 0;
264-
test_controller->cleanup_calls = &cleanup_calls;
265-
// Configure from inactive state: rejected because controller is not unconfigured
266-
test_controller->simulate_cleanup_failure = false;
267-
{
268-
ControllerManagerRunner cm_runner(this);
269-
EXPECT_EQ(
270-
cm_->configure_controller(CONTROLLER_NAME_1), controller_interface::return_type::ERROR);
271-
}
272-
ASSERT_EQ(
273-
lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE, controller_if->get_lifecycle_state().id());
274-
EXPECT_EQ(0u, cleanup_calls);
275239
}
276240

277241
INSTANTIATE_TEST_SUITE_P(

0 commit comments

Comments
 (0)