Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

adding timeout for action client initialization #3029

Merged
merged 2 commits into from
Jun 22, 2022
Merged

adding timeout for action client initialization #3029

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa88917
adding timeout for action client initialization
vinnnyr Jun 21, 2022
56e276f
adding constant 1s timeout, catching exception
vinnnyr Jun 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion nav2_behavior_tree/include/nav2_behavior_tree/bt_action_node.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,12 @@ class BtActionNode : public BT::ActionNodeBase

// Make sure the server is actually there before continuing
RCLCPP_DEBUG(node_->get_logger(), "Waiting for \"%s\" action server", action_name.c_str());
action_client_->wait_for_action_server();
if (!action_client_->wait_for_action_server(server_timeout_)) {
Copy link
Contributor Author

@vinnnyr vinnnyr Jun 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I think I have to rethink this a small bit. The default server_timeout is 20ms, which is not a lot of time to wait by default.

When trying to launch the system:

[lifecycle_manager-3] [INFO] [1655777607.945443640] [lifecycle_manager_navigation]: Activating bt_navigator
[bt_navigator-2] [INFO] [1655777607.945732127] [bt_navigator]: Activating
[bt_navigator-2] [ERROR] [1655777607.968145727] [bt_navigatornavigate_through_poses_rclcpp_node]: "compute_path_through_poses" action server not available after waiting for 20 ms
[bt_navigator-2] [ERROR] [1655777607.976364703] []: Caught exception in callback for transition 13
[bt_navigator-2] [ERROR] [1655777607.976390295] []: Original error: Action server not available
[bt_navigator-2] [WARN] [1655777607.976438741] []: Error occurred while doing error handling.
[bt_navigator-2] [FATAL] [1655777607.976449793] [bt_navigator]: Lifecycle node bt_navigator does not have error state implemented
[lifecycle_manager-3] [ERROR] [1655777607.976858429] [lifecycle_manager_navigation]: Failed to change state for node: bt_navigator
[lifecycle_manager-3] [ERROR] [1655777607.976902588] [lifecycle_manager_navigation]: Failed to bring up all requested nodes. Aborting bringup.

Should I declare a new parameter for this timeout?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hesitant to have a new timeout since that might confuse people, even if well named - but maybe there's a good name that would be clear so its moot? wait_for_action_server_timeout server_initialization_timeout?

Otherwise, if we can't come up with a good & specific name, I think just hardcoding it at some value would be fine. I'd say 1s just to be nice and round. We have a few like that (only a few) in the stack where it didn't make much sense to parameterize it for various reasons and since they were just initialization operations, it wasn't super important for them to be configurable for 99.8% of users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say 1s just to be nice and round

I think I agree

RCLCPP_ERROR(
node_->get_logger(), "\"%s\" action server not available after waiting for %li ms",
action_name.c_str(), server_timeout_.count());
throw std::runtime_error("Action server not available");
}
}

/**
Expand Down