-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add runid #5553
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.
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
base: main
Are you sure you want to change the base?
Add runid #5553
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,7 @@ class BtActionNode : public BT::ActionNodeBase | |
| if (getInput("server_name", remapped_action_name)) { | ||
| action_name_ = remapped_action_name; | ||
| } | ||
| getInput("is_global", is_global_); | ||
| createActionClient(action_name_); | ||
|
|
||
| // Give the derive class a chance to do any initialization | ||
|
|
@@ -121,7 +122,8 @@ class BtActionNode : public BT::ActionNodeBase | |
| { | ||
| BT::PortsList basic = { | ||
| BT::InputPort<std::string>("server_name", "Action server name"), | ||
| BT::InputPort<std::chrono::milliseconds>("server_timeout") | ||
| BT::InputPort<std::chrono::milliseconds>("server_timeout"), | ||
| BT::InputPort<bool>("is_global", false, "Use RunID for initialization") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The configuration guide would need to be updated to mention this. Question: does this make more sense as a BT XML parameter for each and every node, or as a ROS configuration parameter applied to every node automatically from a yaml file? Is there a situation we'd want some nodes to do this and not others? |
||
| }; | ||
| basic.insert(addition.begin(), addition.end()); | ||
|
|
||
|
|
@@ -202,8 +204,20 @@ class BtActionNode : public BT::ActionNodeBase | |
| */ | ||
| BT::NodeStatus tick() override | ||
| { | ||
| bool needs_initialization_ = false; | ||
| // first step to be done only at the beginning of the Action | ||
| if (!BT::isStatusActive(status())) { | ||
| if (is_global_) { | ||
| std::string current_run_id = config().blackboard->get<std::string>("run_id"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should throw an exception if the run_id is not set but |
||
| if (current_run_id != last_run_id_) { | ||
| needs_initialization_ = true; | ||
| last_run_id_ = current_run_id; | ||
| } | ||
| } else { | ||
| if (!BT::isStatusActive(status())) { | ||
| needs_initialization_ = true; | ||
| } | ||
| } | ||
| if (needs_initialization_) { | ||
| // reset the flag to send the goal or not, allowing the user the option to set it in on_tick | ||
| should_send_goal_ = true; | ||
|
|
||
|
|
@@ -499,6 +513,10 @@ class BtActionNode : public BT::ActionNodeBase | |
|
|
||
| // Can be set in on_tick or on_wait_for_result to indicate if a goal should be sent. | ||
| bool should_send_goal_; | ||
|
|
||
| // Initialized to UINT64_MAX as a sentinel value to ensure the first tick always triggers | ||
| bool is_global_ {false}; | ||
| std::string last_run_id_; | ||
| }; | ||
|
|
||
| } // namespace nav2_behavior_tree | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,10 @@ | |
| #include <string> | ||
| #include <vector> | ||
|
|
||
| #include <boost/uuid/uuid.hpp> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to link this / package.xml the dependency? Please only include just the boost libraries required (not all) |
||
| #include <boost/uuid/uuid_generators.hpp> | ||
| #include <boost/uuid/uuid_io.hpp> | ||
|
|
||
| #include "geometry_msgs/msg/pose_stamped.hpp" | ||
| #include "nav2_behavior_tree/behavior_tree_engine.hpp" | ||
| #include "nav2_behavior_tree/ros_topic_logger.hpp" | ||
|
|
@@ -304,6 +308,10 @@ class BtActionServer | |
| // internal error tracking (IOW not behaviorTree blackboard errors) | ||
| uint16_t internal_error_code_; | ||
| std::string internal_error_msg_; | ||
|
|
||
| // To keep track of the execution number | ||
| boost::uuids::random_generator uuid_generator_; | ||
| std::string run_id_; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe |
||
| }; | ||
|
|
||
| } // namespace nav2_behavior_tree | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,10 @@ | |
| #include <string> | ||
| #include <vector> | ||
|
|
||
| #include <boost/uuid/uuid.hpp> | ||
| #include <boost/uuid/uuid_generators.hpp> | ||
| #include <boost/uuid/uuid_io.hpp> | ||
|
|
||
| #include "nav2_msgs/action/navigate_to_pose.hpp" | ||
| #include "nav2_behavior_tree/bt_action_server.hpp" | ||
| #include "nav2_ros_common/node_utils.hpp" | ||
|
|
@@ -179,7 +183,8 @@ bool BtActionServer<ActionT, NodeT>::on_configure() | |
| blackboard_->set<std::chrono::milliseconds>( | ||
| "wait_for_service_timeout", | ||
| wait_for_service_timeout_); | ||
|
|
||
| run_id_ = boost::uuids::to_string(uuid_generator_()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we ever actually use |
||
| blackboard_->set<std::string>("run_id", run_id_); // NOLINT | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -362,6 +367,8 @@ void BtActionServer<ActionT, NodeT>::executeCallback() | |
| return; | ||
| } | ||
|
|
||
| run_id_ = boost::uuids::to_string(uuid_generator_()); | ||
| blackboard_->set<std::string>("run_id", run_id_); | ||
| auto is_canceling = [&]() { | ||
| if (action_server_ == nullptr) { | ||
| RCLCPP_DEBUG(logger_, "Action server unavailable. Canceling."); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: should this be applied to non-action nodes or only the action nodes?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think this is also missing the other nodes that have the initialization procedure within their implementations themselves (not just the action server base node). I think that's a key part of the ticket #5036 but I can understand just starting in one spot to get it right before doing all the others!