Skip to content

Commit

Permalink
revert call shutdown in LifecycleNode destructor (#2557)
Browse files Browse the repository at this point in the history 
* Revert "LifecycleNode shutdown on dtor only with valid context. (#2545)"

This reverts commit d8d83a0.

Signed-off-by: Tomoya Fujita <[email protected]>

* Revert "call shutdown in LifecycleNode dtor to avoid leaving the device in unknown state (2nd) (#2528)"

This reverts commit 3bc364a.

Signed-off-by: Tomoya Fujita <[email protected]>

---------

Signed-off-by: Tomoya Fujita <[email protected]>
  • Loading branch information
@fujitatomoya
fujitatomoya authored Jun 11, 2024
1 parent 7c09688 commit f4923c6
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 170 deletions.
30 changes: 0 additions & 30 deletions rclcpp_lifecycle/src/lifecycle_node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,37 +152,8 @@ LifecycleNode::LifecycleNode(

LifecycleNode::~LifecycleNode()
{
auto current_state = LifecycleNode::get_current_state().id();
// shutdown if necessary to avoid leaving the device in any other primary state
if (current_state < lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED) {
if (node_base_->get_context()->is_valid()) {
auto ret = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
auto finalized = LifecycleNode::shutdown(ret);
if (finalized.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED ||
ret != rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS)
{
RCLCPP_WARN(
rclcpp::get_logger("rclcpp_lifecycle"),
"Shutdown error in destruction of LifecycleNode: final state(%s)",
finalized.label().c_str());
}
} else {
// TODO(fujitatomoya): consider when context is gracefully shutdown before.
RCLCPP_DEBUG(
rclcpp::get_logger("rclcpp_lifecycle"),
"Context invalid error in destruction of LifecycleNode: Node still in transition state(%u)",
current_state);
}
} else if (current_state > lifecycle_msgs::msg::State::PRIMARY_STATE_FINALIZED) {
RCLCPP_WARN(
rclcpp::get_logger("rclcpp_lifecycle"),
"Shutdown error in destruction of LifecycleNode: Node still in transition state(%u)",
current_state);
}

// release sub-interfaces in an order that allows them to consult with node_base during tear-down
node_waitables_.reset();
node_type_descriptions_.reset();
node_time_source_.reset();
node_parameters_.reset();
node_clock_.reset();
Expand All @@ -191,7 +162,6 @@ LifecycleNode::~LifecycleNode()
node_timers_.reset();
node_logging_.reset();
node_graph_.reset();
node_base_.reset();
}

const char *
Expand Down
140 changes: 0 additions & 140 deletions rclcpp_lifecycle/test/test_lifecycle_node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -447,146 +447,6 @@ TEST_F(TestDefaultStateMachine, bad_mood) {
EXPECT_EQ(1u, test_node->number_of_callbacks);
}


TEST_F(TestDefaultStateMachine, shutdown_from_each_primary_state) {
auto success = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
auto reset_key = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;

// PRIMARY_STATE_UNCONFIGURED to shutdown
{
auto ret = reset_key;
auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
auto finalized = test_node->shutdown(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
}

// PRIMARY_STATE_INACTIVE to shutdown
{
auto ret = reset_key;
auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
auto configured = test_node->configure(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
ret = reset_key;
auto finalized = test_node->shutdown(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
}

// PRIMARY_STATE_ACTIVE to shutdown
{
auto ret = reset_key;
auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
auto configured = test_node->configure(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
ret = reset_key;
auto activated = test_node->activate(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
ret = reset_key;
auto finalized = test_node->shutdown(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
}

// PRIMARY_STATE_FINALIZED to shutdown
{
auto ret = reset_key;
auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
auto configured = test_node->configure(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
ret = reset_key;
auto activated = test_node->activate(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
ret = reset_key;
auto finalized = test_node->shutdown(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
ret = reset_key;
auto finalized_again = test_node->shutdown(ret);
EXPECT_EQ(reset_key, ret);
EXPECT_EQ(finalized_again.id(), State::PRIMARY_STATE_FINALIZED);
}
}

TEST_F(TestDefaultStateMachine, test_shutdown_on_dtor) {
auto success = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
auto reset_key = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;

bool shutdown_cb_called = false;
auto on_shutdown_callback =
[&shutdown_cb_called](const rclcpp_lifecycle::State &) ->
rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn {
shutdown_cb_called = true;
return rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
};

// PRIMARY_STATE_UNCONFIGURED to shutdown via dtor
shutdown_cb_called = false;
{
auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
EXPECT_EQ(State::PRIMARY_STATE_UNCONFIGURED, test_node->get_current_state().id());
EXPECT_FALSE(shutdown_cb_called);
}
EXPECT_TRUE(shutdown_cb_called);

// PRIMARY_STATE_INACTIVE to shutdown via dtor
shutdown_cb_called = false;
{
auto ret = reset_key;
auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
auto configured = test_node->configure(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
EXPECT_FALSE(shutdown_cb_called);
}
EXPECT_TRUE(shutdown_cb_called);

// PRIMARY_STATE_ACTIVE to shutdown via dtor
shutdown_cb_called = false;
{
auto ret = reset_key;
auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
auto configured = test_node->configure(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
ret = reset_key;
auto activated = test_node->activate(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
EXPECT_FALSE(shutdown_cb_called);
}
EXPECT_TRUE(shutdown_cb_called);

// PRIMARY_STATE_FINALIZED to shutdown via dtor
shutdown_cb_called = false;
{
auto ret = reset_key;
auto test_node = std::make_shared<rclcpp_lifecycle::LifecycleNode>("testnode");
test_node->register_on_shutdown(std::bind(on_shutdown_callback, std::placeholders::_1));
auto configured = test_node->configure(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(configured.id(), State::PRIMARY_STATE_INACTIVE);
ret = reset_key;
auto activated = test_node->activate(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(activated.id(), State::PRIMARY_STATE_ACTIVE);
ret = reset_key;
auto finalized = test_node->shutdown(ret);
EXPECT_EQ(success, ret);
EXPECT_EQ(finalized.id(), State::PRIMARY_STATE_FINALIZED);
EXPECT_TRUE(shutdown_cb_called); // should be called already
}
EXPECT_TRUE(shutdown_cb_called);
}

TEST_F(TestDefaultStateMachine, lifecycle_subscriber) {
auto test_node = std::make_shared<MoodyLifecycleNode<GoodMood>>("testnode");

Expand Down

0 comments on commit f4923c6

Please sign in to comment.