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

drive_on_heading: return TIMEOUT error code when exceeding time allowance #4836

Merged
merged 1 commit into from
Jan 10, 2025
Merged

drive_on_heading: return TIMEOUT error code when exceeding time allowance #4836

merged 1 commit into from
Jan 10, 2025

Conversation

DylanDeCoeyer-Quimesis
Copy link
Contributor

Until now, the NONE error code was returned when exceeding the time allowance. Let's return the more appropriate TIMEOUT error code instead.

Basic Info

Info Please fill out this column
Ticket(s) this addresses /
Primary OS tested on Ubuntu
Robotic platform tested on /
Does this PR contain AI generated software? No

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:

  • Check that any new parameters added are updated in docs.nav2.org
  • Check that any significant change is added to the migration guide
  • Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

@DylanDeCoeyer-Quimesis
nav2_behaviors: drive_on_heading: return TIMEOUT error code when exce…
348e6a2 
…eding time allowance

Until now, the NONE error code was returned when exceeding the time
allowance. Let's return the more appropriate TIMEOUT error code instead.

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 9, 2025

@DylanDeCoeyer-Quimesis, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

@SteveMacenski
Copy link
Member

LGTM, thanks for the catch. Our CI is looking like its having issues cloning the repo right now, will merge once I get that sorted

@codecov Codecov
Copy link

codecov bot commented Jan 10, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
...nclude/nav2_behaviors/plugins/drive_on_heading.hpp 93.24% <100.00%> (ø)

@SteveMacenski SteveMacenski merged commit 78f60ee into ros-navigation:main Jan 10, 2025
11 checks passed
CihatAltiparmak pushed a commit to CihatAltiparmak/navigation2 that referenced this pull request Jan 10, 2025
@DylanDeCoeyer-Quimesis @CihatAltiparmak
nav2_behaviors: drive_on_heading: return TIMEOUT error code when exce…
0f175c4 
…eding time allowance (ros-navigation#4836)

Until now, the NONE error code was returned when exceeding the time
allowance. Let's return the more appropriate TIMEOUT error code instead.

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
Signed-off-by: CihatAltiparmak <cihataltiparmak1@gmail.com>
masf7g pushed a commit to quasi-robotics/navigation2 that referenced this pull request Jan 17, 2025
@DylanDeCoeyer-Quimesis @masf7g
nav2_behaviors: drive_on_heading: return TIMEOUT error code when exce…
bbe5418 
…eding time allowance (ros-navigation#4836)

Until now, the NONE error code was returned when exceeding the time
allowance. Let's return the more appropriate TIMEOUT error code instead.

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
SteveMacenski pushed a commit that referenced this pull request Feb 5, 2025
@DylanDeCoeyer-Quimesis @SteveMacenski
nav2_behaviors: drive_on_heading: return TIMEOUT error code when exce…
83652b3 
…eding time allowance (#4836)

Until now, the NONE error code was returned when exceeding the time
allowance. Let's return the more appropriate TIMEOUT error code instead.

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
SteveMacenski added a commit that referenced this pull request Feb 5, 2025
@SteveMacenski @DylanDeCoeyer-Quimesis
@ajtudela @mikeferguson @tonynajjar @rayferric @RBT22 @ewak @doisyg @mini-1235
Jazzy sync 5: Feb 4, 2025 (#4902)
0aee198 
* nav2_smac_planner: handle corner case where start and goal are on the same cell (#4793)

* nav2_smac_planner: handle corner case where start and goal are on the same cell

This case was already properly handled in the smac_planner_2d, but it
was still leading to an A* backtrace failure in the smac_planner_hybrid
and smac_planner_lattice. Let's harmonize the handling of this case.

This commit fixes issue #4792.

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>

* nav2_smac_planner: use goal orientation when path is made of one point

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>

* nav2_smac_planner: publish raw path also when start and goal are on the same cell

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>

* nav2_smac_planner: add corner case to unit tests

Add a plan where the start and goal are placed on the same cell.

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>

---------

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>

* creating auto-transition option for nav2_util::LifecycleNode (#4804)

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>

* Fix trajectory generation bug in docking controller (#4807)

* Fix trajectory in docking controller

Signed-off-by: Alberto Tudela <ajtudela@gmail.com>

* Ceil and remove resolution

Signed-off-by: Alberto Tudela <ajtudela@gmail.com>

* Update nav2_docking/opennav_docking/src/controller.cpp

Co-authored-by: Steve Macenski <stevenmacenski@gmail.com>
Signed-off-by: Alberto Tudela <ajtudela@gmail.com>

* Update nav2_docking/opennav_docking/src/controller.cpp

Co-authored-by: Steve Macenski <stevenmacenski@gmail.com>
Signed-off-by: Alberto Tudela <ajtudela@gmail.com>

---------

Signed-off-by: Alberto Tudela <ajtudela@gmail.com>
Co-authored-by: Steve Macenski <stevenmacenski@gmail.com>

* graceful_controller: implement iterative selection of control points (#4795)

* initial pass at iterative

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* add v_angular_min_in_place

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* add orientation filter, fix remaining TODOs

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* try to increase coverage, fixup minor test issues

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* address review comments

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* review comments

* update defaults
* rename to in_place_collision_resolution

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* revert change in default

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

---------

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* fix bug in use of v_angular_min_in_place (#4813)

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* publish motion target as pose (#4839)

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* nav2_behaviors: drive_on_heading: return TIMEOUT error code when exceeding time allowance (#4836)

Until now, the NONE error code was returned when exceeding the time
allowance. Let's return the more appropriate TIMEOUT error code instead.

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>

* fix bug in orientation filtering (#4840)

* fix bug in orientation filtering

some global planners output all zeros for orientation, however
the plan is in the global frame. when transforming to the local
frame, the orientation is no longer zero. Instead of comparing
to zero, we simply check if all the orientations in the middle
of the plan are equal

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* account for floating point error

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

---------

Signed-off-by: Michael Ferguson <mfergs7@gmail.com>

* Adapt GoalUpdater to update goals as well (#4771)

* Add IsStoppedBTNode

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* add topic name + reformat

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix comment

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix abs

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* remove log

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* add getter functions for raw twist

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* remove unused code

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* use odomsmoother

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix formatting

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* update groot

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* Add test

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* reset at success

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* FIX velocity_threshold_

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* Fix stopped Node

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* Add tests  to odometry_utils

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix linting

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* Adapt goalUpdater to modify goals as well

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix formatting

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fixes

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* change name of msg

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* Make input goals be Goals again for compatibility

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* Revert "fix"

This reverts commit 8303cdc.

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* refactoring

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* ament

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* ignore if no timestamps

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* facepalm

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* update groot nodes

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* Use PoseStampedArray

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fixes

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* use geometry_msgs

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix import

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* use geometry_msgs

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* more fixes

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* .

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* revert

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* .

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* add common_interfaces

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* use PoseStampedArray

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* reformat

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* revert

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* revert

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix warn msg

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix test

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* improve

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix format

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* change to info

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

---------

Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>

* fix simple smoother failing during final approach (#4817)

* new test case for end of path approach

Signed-off-by: rayferric <63957587+rayferric@users.noreply.github.com>

* modify tests to match the more permissive smoother policy

Signed-off-by: rayferric <63957587+rayferric@users.noreply.github.com>

* implement steve's suggestions

Signed-off-by: rayferric <63957587+rayferric@users.noreply.github.com>

---------

Signed-off-by: rayferric <63957587+rayferric@users.noreply.github.com>

* Add acc limit consideration to avoid overshooting in RotationShimController (#4864)

* Add acc limit consideration to avoid overshoot in RotationShimController

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

* Add acceleration limit tests to RotationShimController

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

---------

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

* Fix flaky ness of opennav_docking test_docking_server (#4878) (#4879)

Call publish() (odom -> base_link tf) at startup to kick things off and
spin 10 times(1 second) TF, so that it has a chance to propogate to the
docking_server so that it will accept an action request.

Previously it was only spinning once, hoping the timer would fire and
call publish fast enough for it to propogate to the docking_server
so that it is able to accept the first 'dock_robot' action request

Signed-off-by: Mike Wake <macwake@gmail.com>

* [BtActionNode] [BtServiceNode] clear between calls (#4887)

Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>

* dwb_critics flaky test - lineCost coordinates must be within costmap (#4889)

(#4884)

There is no protection/checks in the pathway from lineCost to
costmap_2d::getIndex(mx, my) for grid coordinates that exceed
the of bounds of the allocated costmap. (presumably for speed)

This test was triggering an off by one error attempting to
read the the 2500 byte costmap at byte 2503

costmap size 50x50.

getIndex(3, 50)
= my * size_x_ + mx;
= 50 * 50 + 3;
= 2503

Signed-off-by: Mike Wake <macwake@gmail.com>

* Add option to use open-loop control with Rotation Shim (#4880)

* Initial implementation

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

* replace feedback param with closed_loop

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

* Reset last_angular_vel_ in activate method

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

* Add closed_loop parameter to dynamicParametersCallback

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

* Add tests

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

* Override reset function

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

---------

Signed-off-by: RBT22 <rozgonyibalint@gmail.com>

* Fix unstable test in nav2 util (#4894)

* Fix unstable test in nav2 util

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>

* Fix linting

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>

* Change 5s to 1s

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>

---------

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>

* Update bt2img syntax and bt pics (#4900)

Signed-off-by: Maurice-1235 <mauricepurnawan@gmail.com>

* bumping to 1.3.5

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>

* Revert "Adapt GoalUpdater to update goals as well (#4771)"

This reverts commit 55d7387.

---------

Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
Signed-off-by: Alberto Tudela <ajtudela@gmail.com>
Signed-off-by: Michael Ferguson <mfergs7@gmail.com>
Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>
Signed-off-by: rayferric <63957587+rayferric@users.noreply.github.com>
Signed-off-by: RBT22 <rozgonyibalint@gmail.com>
Signed-off-by: Mike Wake <macwake@gmail.com>
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
Signed-off-by: Maurice-1235 <mauricepurnawan@gmail.com>
Co-authored-by: DylanDeCoeyer-Quimesis <102609575+DylanDeCoeyer-Quimesis@users.noreply.github.com>
Co-authored-by: Alberto Tudela <ajtudela@gmail.com>
Co-authored-by: Michael Ferguson <mfergs7@gmail.com>
Co-authored-by: Tony Najjar <tony.najjar.1997@gmail.com>
Co-authored-by: Ray Ferric <63957587+rayferric@users.noreply.github.com>
Co-authored-by: Balint Rozgonyi <43723477+RBT22@users.noreply.github.com>
Co-authored-by: ewak <ewak@users.noreply.github.com>
Co-authored-by: Guillaume Doisy <doisyg@users.noreply.github.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: mini-1235 <58433591+mini-1235@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SteveMacenski SteveMacenski SteveMacenski approved these changes

Assignees
No one assigned
Labels
approved-waiting-ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DylanDeCoeyer-Quimesis @SteveMacenski