-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix SpeedController #3378
Fix SpeedController #3378
Conversation
* Using blackboard get() method that does not throw error * Share odom_smoother on the blackboard Signed-off-by: Borong Yuan <yuanborong@hotmail.com>
@borongyuan, please properly fill in PR template in the future. @SteveMacenski, use this instead.
|
Check CI. This PR has linting, test coverage, and XML invalidation errors. You can ignore the |
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.
Otherwise after CI is happy with your changes, this all looks good to me
Signed-off-by: Borong Yuan <yuanborong@hotmail.com>
Thanks for the fix! |
* 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>
* 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>
* 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>
* 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>
Signed-off-by: Borong Yuan yuanborong@hotmail.com
Basic Info
Description of contribution in a few bullet points
The SpeedController is broken since (#2271).
There are two get() methods in blackboard. One of them throws an error if
val
doesn't exist. Obviouslygoal
andgoals
cannot exist on blackboard at the same time. So we need to use another get() method like in GoalUpdatedController.Now the BT Navigator already has an instance of OdomSmoother, we can't create another one in SpeedController, otherwise we won't get the correct data. A simple fix is to share odom_smoother on blackboard.
Description of documentation updates required from your changes
Since odom_smoother is shared from BT Navigator, we cannot set
filter_duration
in SpeedController. So I removed this parameter. The default value of 0.3 works well in most cases, so this should not affect usage. But after these changes are confirmed, we need to update the documentation.Future work that may be required in bullet points
For Maintainers: