Skip to content

Commit c1a01fc

Browse files
fujitatomoyafujitatomoya
authored
revert call shutdown in LifecycleNode destructor (Iron) (ros2#2559)
* Revert "lifecycle node dtor shutdown should be called only in primary state. (ros2#2543)" This reverts commit dceb612. Signed-off-by: Tomoya Fujita <[email protected]> * Revert "call shutdown in LifecycleNode dtor to avoid leaving the device in un… (ros2#2450) (ros2#2490)" This reverts commit f41a353. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]>
1 parent 47adc8f commit c1a01fc

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
@@ -146,34 +146,6 @@ LifecycleNode::LifecycleNode(
146146

147147
LifecycleNode::~LifecycleNode()
148148
{
149-
auto current_state = LifecycleNode::get_current_state().id();
150-
// shutdown if necessary to avoid leaving the device in any other primary state
151-
if (current_state < lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED) {
152-
if (node_base_->get_context()->is_valid()) {
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-
} else {
164-
// TODO(fujitatomoya): consider when context is gracefully shutdown before.
165-
RCLCPP_DEBUG(
166-
rclcpp::get_logger("rclcpp_lifecycle"),
167-
"Context invalid error in destruction of LifecycleNode: Node still in transition state(%u)",
168-
current_state);
169-
}
170-
} else if (current_state > lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED) {
171-
RCLCPP_WARN(
172-
rclcpp::get_logger("rclcpp_lifecycle"),
173-
"Shutdown error in destruction of LifecycleNode: Node still in transition state(%u)",
174-
current_state);
175-
}
176-
177149
// release sub-interfaces in an order that allows them to consult with node_base during tear-down
178150
node_waitables_.reset();
179151
node_time_source_.reset();
@@ -184,7 +156,6 @@ LifecycleNode::~LifecycleNode()
184156
node_timers_.reset();
185157
node_logging_.reset();
186158
node_graph_.reset();
187-
node_base_.reset();
188159
}
189160

190161
const char *

‎rclcpp_lifecycle/test/test_lifecycle_node.cpp‎

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

0 commit comments

Comments
 (0)