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.

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

Fixing bugs with action cancellations #918

Merged
merged 2 commits into from
Jul 11, 2019

Conversation

orduno
Copy link
Contributor

@orduno orduno commented Jul 9, 2019


Basic Info

Info Please fill out this column
Ticket(s) this addresses #911
Primary OS tested on Ubuntu 18.04
Robotic platform tested on Turtlebot3 gazebo

Description of contribution in a few bullet points

Made a few improvements to SimpleActionServer, mainly to address #911:

  • Check if the goal is active before canceling or aborting.
  • Cancel only if the client has requested so, otherwise abort.
  • Remove separate cancel and abort methods, instead have one terminate_goals which calls the appropriate method depending on the state of the goal.

Future work that may be required in bullet points

@orduno orduno added the 1 - High High Priority label Jul 9, 2019
@orduno orduno added this to the D Turtle Release milestone Jul 9, 2019
@orduno orduno self-assigned this Jul 9, 2019
@orduno orduno added this to In progress in Navigation 2 Kanban via automation Jul 9, 2019
Navigation 2 Kanban automation moved this from In progress to Reviewer approved Jul 10, 2019
Copy link
Contributor

@mhpanah mhpanah left a comment

Choose a reason for hiding this comment

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

Looks good.

@orduno orduno merged commit e63b853 into ros-navigation:master Jul 11, 2019
Navigation 2 Kanban automation moved this from Reviewer approved to Done Jul 11, 2019
@orduno orduno deleted the 911_action_server_cancel_bug branch July 11, 2019 18:11
@codecov
Copy link

codecov bot commented Jul 11, 2019

Codecov Report

Merging #918 into master will increase coverage by 8.66%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #918      +/-   ##
=========================================
+ Coverage   21.13%   29.8%   +8.66%     
=========================================
  Files         184     191       +7     
  Lines        9448    9934     +486     
  Branches     2313    3272     +959     
=========================================
+ Hits         1997    2961     +964     
+ Misses       6311    5162    -1149     
- Partials     1140    1811     +671
Impacted Files Coverage Δ
...v2_util/include/nav2_util/simple_action_server.hpp 18.1% <0%> (-16.52%) ⬇️
...rc/navigation2/nav2_motion_primitives/src/main.cpp 16.66% <0%> (-8.34%) ⬇️
...rc/navigation2/nav2_motion_primitives/src/spin.cpp 11.66% <0%> (-3.23%) ⬇️
...navigation2/nav2_motion_primitives/src/back_up.cpp 12.82% <0%> (-2.81%) ⬇️
src/navigation2/nav2_util/test/test_actions.cpp 50% <0%> (-0.5%) ⬇️
src/navigation2/nav2_util/src/lifecycle_node.cpp 62.5% <0%> (ø) ⬆️
...navigation2/nav2_bt_navigator/src/bt_navigator.cpp 0% <0%> (ø) ⬆️
...b_controller/dwb_controller/src/dwb_controller.cpp 0% <0%> (ø) ⬆️
...c/navigation2/nav2_world_model/src/world_model.cpp 0% <0%> (ø) ⬆️
...tion2/nav2_costmap_2d/src/costmap_2d_publisher.cpp 0% <0%> (ø) ⬆️
... and 47 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd16483...d1ac7b0. Read the comment docs.

mlherd pushed a commit to mlherd/navigation2 that referenced this pull request Jul 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - High High Priority
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants