Skip to content

Commit 6737773

Browse files
fujitatomoyafujitatomoya
authored
revert call shutdown in LifecycleNode destructor (Humble) (ros2#2560)
* Revert "lifecycle node dtor shutdown should be called only in primary state. (ros2#2544)" This reverts commit 595badb. Signed-off-by: Tomoya Fujita <[email protected]> * Revert "call shutdown in LifecycleNode dtor to avoid leaving the device in un… (ros2#2450) (ros2#2491)" This reverts commit 0f9604d. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]>
1 parent 32f1961 commit 6737773

File tree

2 files changed

+0
-169
lines changed

2 files changed

+0
-169
lines changed

‎rclcpp_lifecycle/src/lifecycle_node.cpp‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -133,34 +133,6 @@ LifecycleNode::LifecycleNode(
133133

134134
LifecycleNode::~LifecycleNode()
135135
{
136-
auto current_state = LifecycleNode::get_current_state().id();
137-
// shutdown if necessary to avoid leaving the device in any other primary state
138-
if (current_state < lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED) {
139-
if (node_base_->get_context()->is_valid()) {
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-
} else {
151-
// TODO(fujitatomoya): consider when context is gracefully shutdown before.
152-
RCLCPP_DEBUG(
153-
rclcpp::get_logger("rclcpp_lifecycle"),
154-
"Context invalid error in destruction of LifecycleNode: Node still in transition state(%u)",
155-
current_state);
156-
}
157-
} else if (current_state > lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED) {
158-
RCLCPP_WARN(
159-
rclcpp::get_logger("rclcpp_lifecycle"),
160-
"Shutdown error in destruction of LifecycleNode: Node still in transition state(%u)",
161-
current_state);
162-
}
163-
164136
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
165137
node_waitables_.reset();
166138
node_time_source_.reset();
@@ -171,7 +143,6 @@ LifecycleNode::~LifecycleNode()
171143
node_timers_.reset();
172144
node_logging_.reset();
173145
node_graph_.reset();
174-
node_base_.reset();
175146
}
176147

177148
const char *

‎rclcpp_lifecycle/test/test_lifecycle_node.cpp‎

Lines changed: 0 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -435,146 +435,6 @@ 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-
578438
TEST_F(TestDefaultStateMachine, lifecycle_subscriber) {
579439
auto test_node = std::make_shared<MoodyLifecycleNode<GoodMood>>("testnode");
580440

0 commit comments

Comments
 (0)