Skip to content

Commit f41a353

Browse files
mergify[bot]fujitatomoya
mergify[bot]
and
fujitatomoya
authored
call shutdown in LifecycleNode dtor to avoid leaving the device in un… (ros2#2450) (ros2#2490)
* call shutdown in LifecycleNode dtor to avoid leaving the device in unknown state. Signed-off-by: Tomoya Fujita <[email protected]> * add test to verify LifecycleNode::shutdown is called on destructor. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit 04ea0bb) Co-authored-by: Tomoya Fujita <[email protected]>
1 parent f80980b commit f41a353

File tree

2 files changed

+156
-0
lines changed

2 files changed

+156
-0
lines changed

‎rclcpp_lifecycle/src/lifecycle_node.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,22 @@ LifecycleNode::LifecycleNode(
146146

147147
LifecycleNode::~LifecycleNode()
148148
{
149+
// shutdown if necessary to avoid leaving the device in unknown state
150+
if (LifecycleNode::get_current_state().id() !=
151+
lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED)
152+
{
153+
auto ret = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
154+
auto finalized = LifecycleNode::shutdown(ret);
155+
if (finalized.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED ||
156+
ret != rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS)
157+
{
158+
RCLCPP_WARN(
159+
rclcpp::get_logger("rclcpp_lifecycle"),
160+
"Shutdown error in destruction of LifecycleNode: final state(%s)",
161+
finalized.label().c_str());
162+
}
163+
}
164+
149165
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
150166
node_waitables_.reset();
151167
node_time_source_.reset();

‎rclcpp_lifecycle/test/test_lifecycle_node.cpp‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,146 @@ TEST_F(TestDefaultStateMachine, bad_mood) {
414414
EXPECT_EQ(1u, test_node->number_of_callbacks);
415415
}
416416

417+
418+
TEST_F(TestDefaultStateMachine, shutdown_from_each_primary_state) {
419+
auto success = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
420+
auto reset_key = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
421+
422+
// PRIMARY_STATE_UNCONFIGURED to shutdown
423+
{
424+
auto ret = reset_key;
425+
auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
426+
auto finalized = test_node->shutdown(ret);
427+
EXPECT_EQ(success, ret);
428+
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
429+
}
430+
431+
// PRIMARY_STATE_INACTIVE to shutdown
432+
{
433+
auto ret = reset_key;
434+
auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
435+
auto configured = test_node->configure(ret);
436+
EXPECT_EQ(success, ret);
437+
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
438+
ret = reset_key;
439+
auto finalized = test_node->shutdown(ret);
440+
EXPECT_EQ(success, ret);
441+
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
442+
}
443+
444+
// PRIMARY_STATE_ACTIVE to shutdown
445+
{
446+
auto ret = reset_key;
447+
auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
448+
auto configured = test_node->configure(ret);
449+
EXPECT_EQ(success, ret);
450+
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
451+
ret = reset_key;
452+
auto activated = test_node->activate(ret);
453+
EXPECT_EQ(success, ret);
454+
EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
455+
ret = reset_key;
456+
auto finalized = test_node->shutdown(ret);
457+
EXPECT_EQ(success, ret);
458+
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
459+
}
460+
461+
// PRIMARY_STATE_FINALIZED to shutdown
462+
{
463+
auto ret = reset_key;
464+
auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
465+
auto configured = test_node->configure(ret);
466+
EXPECT_EQ(success, ret);
467+
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
468+
ret = reset_key;
469+
auto activated = test_node->activate(ret);
470+
EXPECT_EQ(success, ret);
471+
EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
472+
ret = reset_key;
473+
auto finalized = test_node->shutdown(ret);
474+
EXPECT_EQ(success, ret);
475+
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
476+
ret = reset_key;
477+
auto finalized_again = test_node->shutdown(ret);
478+
EXPECT_EQ(reset_key, ret);
479+
EXPECT_EQ(finalized_again.id(), State::PRIMARY_STATE_FINALIZED);
480+
}
481+
}
482+
483+
TEST_F(TestDefaultStateMachine, test_shutdown_on_dtor) {
484+
auto success = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
485+
auto reset_key = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
486+
487+
bool shutdown_cb_called = false;
488+
auto on_shutdown_callback =
489+
[&shutdown_cb_called](const rclcpp_lifecycle::State &) ->
490+
rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn {
491+
shutdown_cb_called = true;
492+
return rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
493+
};
494+
495+
// PRIMARY_STATE_UNCONFIGURED to shutdown via dtor
496+
shutdown_cb_called = false;
497+
{
498+
auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
499+
test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
500+
EXPECT_EQ(State::PRIMARY_STATE_UNCONFIGURED, test_node->get_current_state().id());
501+
EXPECT_FALSE(shutdown_cb_called);
502+
}
503+
EXPECT_TRUE(shutdown_cb_called);
504+
505+
// PRIMARY_STATE_INACTIVE to shutdown via dtor
506+
shutdown_cb_called = false;
507+
{
508+
auto ret = reset_key;
509+
auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
510+
test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
511+
auto configured = test_node->configure(ret);
512+
EXPECT_EQ(success, ret);
513+
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
514+
EXPECT_FALSE(shutdown_cb_called);
515+
}
516+
EXPECT_TRUE(shutdown_cb_called);
517+
518+
// PRIMARY_STATE_ACTIVE to shutdown via dtor
519+
shutdown_cb_called = false;
520+
{
521+
auto ret = reset_key;
522+
auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
523+
test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
524+
auto configured = test_node->configure(ret);
525+
EXPECT_EQ(success, ret);
526+
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
527+
ret = reset_key;
528+
auto activated = test_node->activate(ret);
529+
EXPECT_EQ(success, ret);
530+
EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
531+
EXPECT_FALSE(shutdown_cb_called);
532+
}
533+
EXPECT_TRUE(shutdown_cb_called);
534+
535+
// PRIMARY_STATE_FINALIZED to shutdown via dtor
536+
shutdown_cb_called = false;
537+
{
538+
auto ret = reset_key;
539+
auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
540+
test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
541+
auto configured = test_node->configure(ret);
542+
EXPECT_EQ(success, ret);
543+
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
544+
ret = reset_key;
545+
auto activated = test_node->activate(ret);
546+
EXPECT_EQ(success, ret);
547+
EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
548+
ret = reset_key;
549+
auto finalized = test_node->shutdown(ret);
550+
EXPECT_EQ(success, ret);
551+
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
552+
EXPECT_TRUE(shutdown_cb_called); // should be called already
553+
}
554+
EXPECT_TRUE(shutdown_cb_called);
555+
}
556+
417557
TEST_F(TestDefaultStateMachine, lifecycle_subscriber) {
418558
auto test_node = std::make_shared<MoodyLifecycleNode<GoodMood>>("testnode");
419559

0 commit comments

Comments
 (0)