Rename "service_name" input port for action nodes in nav2_tree_nodes.xml #3370
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Classes inheriting from nav2_behavior_tree::BtActionNode use "server_name" as input port. The "service_name" input port is reserved to sub-classes of nav2_behavior_tree::BtServiceNode. There seemed to be a confusion in the nav2_tree_nodes.xml file.
7 tasks
SteveMacenski
requested changes
Jan 17, 2023
| <input_port name="server_timeout">Server timeout</input_port> | ||
| </Action> | ||
|
|
||
| <Action ID="CancelControl"> | ||
| <input_port name="service_name">Service name to cancel the controller server</input_port> | ||
| <input_port name="server_name">Server name to cancel the controller server</input_port> |
There was a problem hiding this comment.
This file does not change the name in the actual BT nodes, just in the representation in Groot. Are you only saying that the names are actually server_name in the BT nodes themselves and only the visual representation in Groot is wrong?
There was a problem hiding this comment.
I'm a little unclear from your main PR comment
There was a problem hiding this comment.
Yes, this is right. Only the representation in Groot was wrong. But the impact is that the behavior trees generated with Groot used the wrong input port names, leading to runtime errors.
There was a problem hiding this comment.
OK - just needed that clarification
DylanDeCoeyer-Quimesis
changed the title
SteveMacenski
approved these changes
Jan 18, 2023
SteveMacenski
pushed a commit
that referenced
this pull request
Feb 10, 2023
…3370) Classes inheriting from nav2_behavior_tree::BtActionNode use "server_name" as input port. The "service_name" input port is reserved to sub-classes of nav2_behavior_tree::BtServiceNode. There seemed to be a confusion in the nav2_tree_nodes.xml file.
SteveMacenski
added a commit
that referenced
this pull request
Feb 10, 2023
* Std::string doesn't have such constructor, therefore wrong exception happens when waiting for action server is over (#3333) * Update bt_action_node.hpp * Update bt_cancel_action_node.hpp * Update bt_action_node.hpp * Update bt_cancel_action_node.hpp * Do only consider enabled costmap plugins/filters in the isCurrent() method of a LayeredCostmap (#3356) * Do only consider enabled costmap plugins/filters in the isCurrent() method of a LayeredCostmap When clearing entirely a costmap (see the `clear_entirely_*` service), its layers are reset. This sets their `current_` attribute to False, until they are updated (`updateCosts()`). However, for disabled plugins/filters, the `updateCosts()` is bypassed so that the `current_` attribute is never reset to True, and the costmap never becomes "current" again; because of that the controller_server used to get stuck in an endless loop (see `while (!costmap_ros_->isCurrent())` in `ControllerServer::computeControl()`). This patch fixes that by adding a condition for not considering disabled plugins/filters in the `LayeredCostmap::isCurrent()`. * Add forgotten Layer::isEnabled() definition * Export ompl include directories from nav2_smac_planner. (#3358) * Update default w_smooth to 2M (#3341) * Fix nav2_velocity_smoother for sim time (#3360) * Fix nav2_velocity_smoother for sim time * Update nav2_velocity_smoother/src/velocity_smoother.cpp Co-authored-by: Steve Macenski <stevenmacenski@gmail.com> * catch general exceptions (#3373) * catch general exceptions * Update controller_server.cpp * nav2_tree_nodes: rename "service_name" input port for action nodes (#3370) Classes inheriting from nav2_behavior_tree::BtActionNode use "server_name" as input port. The "service_name" input port is reserved to sub-classes of nav2_behavior_tree::BtServiceNode. There seemed to be a confusion in the nav2_tree_nodes.xml file. * Fixing Theta* test failures (#3380) * adding printouts since can't see locally * one more to isolate * request in bounds * remove prints * Fix SpeedController (#3378) * Fix SpeedController * Using blackboard get() method that does not throw error * Share odom_smoother on the blackboard Signed-off-by: Borong Yuan <yuanborong@hotmail.com> * Fix SpeedController linting and test errors Signed-off-by: Borong Yuan <yuanborong@hotmail.com> Signed-off-by: Borong Yuan <yuanborong@hotmail.com> * node_name change during initialization (#3387) * Added missing conditions to nav2_tree_nodes (#3401) * Add WouldAControllerRecoveryHelp and WouldAPlannerRecoveryHelp to nav2_tree_nodes * Add WouldAControllerRecoveryHelp and WouldAPlannerRecoveryHelp...... to nav2_tree_nodes * Update nav2_tree_nodes.xml --------- Co-authored-by: blanker <you@example.com> * Fix multi-robot launch issues (#3383) * Fix multi-robot launch issues: * Use full name of composable nodes container * Correct and update BT plugins and error codes list * Fix and update local and global costmap used plugins parameters * Do not publish voxel map for multi-robot case * Update controller_server.cpp * Update nav2_smoother.cpp --------- Signed-off-by: Borong Yuan <yuanborong@hotmail.com> Co-authored-by: Andrii Maistruk <71632363+andriimaistruk@users.noreply.github.com> Co-authored-by: milidam <milidam@users.noreply.github.com> Co-authored-by: Marc Alban <marcalban@gmail.com> Co-authored-by: Alexey Merzlyakov <60094858+AlexeyMerzlyakov@users.noreply.github.com> Co-authored-by: HovorunB <87417416+HovorunB@users.noreply.github.com> Co-authored-by: DylanDeCoeyer-Quimesis <102609575+DylanDeCoeyer-Quimesis@users.noreply.github.com> Co-authored-by: Borong Yuan <yuanborong@hotmail.com> Co-authored-by: Austin Greisman <92941098+austin-InDro@users.noreply.github.com> Co-authored-by: muttistefano <mutti.stefano.jp@gmail.com> Co-authored-by: blanker <you@example.com>
andrewlycas
pushed a commit
to StratomInc/navigation2
that referenced
this pull request
Feb 23, 2023
…os-navigation#3370) Classes inheriting from nav2_behavior_tree::BtActionNode use "server_name" as input port. The "service_name" input port is reserved to sub-classes of nav2_behavior_tree::BtServiceNode. There seemed to be a confusion in the nav2_tree_nodes.xml file.
hyunseok-yang
pushed a commit
to lge-ros2/navigation2
that referenced
this pull request
Mar 6, 2023
* Std::string doesn't have such constructor, therefore wrong exception happens when waiting for action server is over (ros-navigation#3333) * Update bt_action_node.hpp * Update bt_cancel_action_node.hpp * Update bt_action_node.hpp * Update bt_cancel_action_node.hpp * Do only consider enabled costmap plugins/filters in the isCurrent() method of a LayeredCostmap (ros-navigation#3356) * Do only consider enabled costmap plugins/filters in the isCurrent() method of a LayeredCostmap When clearing entirely a costmap (see the `clear_entirely_*` service), its layers are reset. This sets their `current_` attribute to False, until they are updated (`updateCosts()`). However, for disabled plugins/filters, the `updateCosts()` is bypassed so that the `current_` attribute is never reset to True, and the costmap never becomes "current" again; because of that the controller_server used to get stuck in an endless loop (see `while (!costmap_ros_->isCurrent())` in `ControllerServer::computeControl()`). This patch fixes that by adding a condition for not considering disabled plugins/filters in the `LayeredCostmap::isCurrent()`. * Add forgotten Layer::isEnabled() definition * Export ompl include directories from nav2_smac_planner. (ros-navigation#3358) * Update default w_smooth to 2M (ros-navigation#3341) * Fix nav2_velocity_smoother for sim time (ros-navigation#3360) * Fix nav2_velocity_smoother for sim time * Update nav2_velocity_smoother/src/velocity_smoother.cpp Co-authored-by: Steve Macenski <stevenmacenski@gmail.com> * catch general exceptions (ros-navigation#3373) * catch general exceptions * Update controller_server.cpp * nav2_tree_nodes: rename "service_name" input port for action nodes (ros-navigation#3370) Classes inheriting from nav2_behavior_tree::BtActionNode use "server_name" as input port. The "service_name" input port is reserved to sub-classes of nav2_behavior_tree::BtServiceNode. There seemed to be a confusion in the nav2_tree_nodes.xml file. * Fixing Theta* test failures (ros-navigation#3380) * adding printouts since can't see locally * one more to isolate * request in bounds * remove prints * Fix SpeedController (ros-navigation#3378) * Fix SpeedController * Using blackboard get() method that does not throw error * Share odom_smoother on the blackboard Signed-off-by: Borong Yuan <yuanborong@hotmail.com> * Fix SpeedController linting and test errors Signed-off-by: Borong Yuan <yuanborong@hotmail.com> Signed-off-by: Borong Yuan <yuanborong@hotmail.com> * node_name change during initialization (ros-navigation#3387) * Added missing conditions to nav2_tree_nodes (ros-navigation#3401) * Add WouldAControllerRecoveryHelp and WouldAPlannerRecoveryHelp to nav2_tree_nodes * Add WouldAControllerRecoveryHelp and WouldAPlannerRecoveryHelp...... to nav2_tree_nodes * Update nav2_tree_nodes.xml --------- Co-authored-by: blanker <you@example.com> * Fix multi-robot launch issues (ros-navigation#3383) * Fix multi-robot launch issues: * Use full name of composable nodes container * Correct and update BT plugins and error codes list * Fix and update local and global costmap used plugins parameters * Do not publish voxel map for multi-robot case * Update controller_server.cpp * Update nav2_smoother.cpp --------- Signed-off-by: Borong Yuan <yuanborong@hotmail.com> Co-authored-by: Andrii Maistruk <71632363+andriimaistruk@users.noreply.github.com> Co-authored-by: milidam <milidam@users.noreply.github.com> Co-authored-by: Marc Alban <marcalban@gmail.com> Co-authored-by: Alexey Merzlyakov <60094858+AlexeyMerzlyakov@users.noreply.github.com> Co-authored-by: HovorunB <87417416+HovorunB@users.noreply.github.com> Co-authored-by: DylanDeCoeyer-Quimesis <102609575+DylanDeCoeyer-Quimesis@users.noreply.github.com> Co-authored-by: Borong Yuan <yuanborong@hotmail.com> Co-authored-by: Austin Greisman <92941098+austin-InDro@users.noreply.github.com> Co-authored-by: muttistefano <mutti.stefano.jp@gmail.com> Co-authored-by: blanker <you@example.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Classes inheriting from nav2_behavior_tree::BtActionNode use "server_name" as input port. The "service_name" input port is reserved to sub-classes of nav2_behavior_tree::BtServiceNode. There seemed to be a confusion in the nav2_tree_nodes.xml file.
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Future work that may be required in bullet points
For Maintainers: