Skip to content

Commit 3bc364a

Browse files
fujitatomoyafujitatomoya
authored
call shutdown in LifecycleNode dtor to avoid leaving the device in unknown state (2nd) (#2528)
* Revert "Revert "call shutdown in LifecycleNode dtor to avoid leaving the device in un… (#2450)" (#2522)" This reverts commit 42b0b57. Signed-off-by: Tomoya Fujita <[email protected]> * lifecycle node dtor shutdown should be called only in primary state. Signed-off-by: Tomoya Fujita <[email protected]> * adjust warning message if the node is still in transition state in dtor. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]>
1 parent 22df1d5 commit 3bc364a

File tree

2 files changed

+160
-0
lines changed

2 files changed

+160
-0
lines changed

‎rclcpp_lifecycle/src/lifecycle_node.cpp

+20
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,26 @@ LifecycleNode::LifecycleNode(
152152

153153
LifecycleNode::~LifecycleNode()
154154
{
155+
auto current_state = LifecycleNode::get_current_state().id();
156+
// shutdown if necessary to avoid leaving the device in any other primary state
157+
if (current_state < lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED) {
158+
auto ret = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
159+
auto finalized = LifecycleNode::shutdown(ret);
160+
if (finalized.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED ||
161+
ret != rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS)
162+
{
163+
RCLCPP_WARN(
164+
rclcpp::get_logger("rclcpp_lifecycle"),
165+
"Shutdown error in destruction of LifecycleNode: final state(%s)",
166+
finalized.label().c_str());
167+
}
168+
} else if (current_state > lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED) {
169+
RCLCPP_WARN(
170+
rclcpp::get_logger("rclcpp_lifecycle"),
171+
"Shutdown error in destruction of LifecycleNode: Node still in transition state(%u)",
172+
current_state);
173+
}
174+
155175
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
156176
node_waitables_.reset();
157177
node_time_source_.reset();

‎rclcpp_lifecycle/test/test_lifecycle_node.cpp

+140
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,146 @@ TEST_F(TestDefaultStateMachine, bad_mood) {
447447
EXPECT_EQ(1u, test_node->number_of_callbacks);
448448
}
449449

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

0 commit comments

Comments
 (0)