Skip to content

Commit 0f9604d

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#2491)
* 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 4fb589e commit 0f9604d

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
@@ -133,6 +133,22 @@ LifecycleNode::LifecycleNode(
133133

134134
LifecycleNode::~LifecycleNode()
135135
{
136+
// shutdown if necessary to avoid leaving the device in unknown state
137+
if (LifecycleNode::get_current_state().id() !=
138+
lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED)
139+
{
140+
auto ret = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
141+
auto finalized = LifecycleNode::shutdown(ret);
142+
if (finalized.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED ||
143+
ret != rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS)
144+
{
145+
RCLCPP_WARN(
146+
rclcpp::get_logger("rclcpp_lifecycle"),
147+
"Shutdown error in destruction of LifecycleNode: final state(%s)",
148+
finalized.label().c_str());
149+
}
150+
}
151+
136152
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
137153
node_waitables_.reset();
138154
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
@@ -435,6 +435,146 @@ TEST_F(TestDefaultStateMachine, bad_mood) {
435435
EXPECT_EQ(1u, test_node->number_of_callbacks);
436436
}
437437

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

0 commit comments

Comments
 (0)