Skip to content

Conversation

@RasmusLar
Copy link
Contributor

Basic Info

Info Please fill out this column
Ticket(s) this addresses N/A
Primary OS tested on Ubuntu
Robotic platform tested on GTest
Does this PR contain AI generated software? Yes (GH Copilot)

Description of contribution in a few bullet points

  • While writing unit tests for Remove unnecessary sqrt calculations #4942, fixed discrepancy in goal checker orientation, which was checking for < tolerance instead of <= tolerance, as all the other limit checks are.
  • Reduced tolerance time for the progress checker unit tests to 0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.
  • Added edge case unit tests

Description of documentation updates required from your changes

  • I think no documentation update is needed.

Description of how this change was tested

  • Only unit tests

Future work that may be required in bullet points

  • Other performance improvements that I can see, reduced copying values from const references

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

- Fixed discrepancy in goal checker orientation, which was checking for
< tolerance instead of <= tolerance, as all the other limit checks are.
- Reduced tolerance time for the progress checker unit tests to
0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.

Signed-off-by: Rasmus Larsson <[email protected]>
@RasmusLar RasmusLar force-pushed the fix/progressCheckerTests branch from e7335af to c7bb826 Compare March 15, 2025 07:45
@codecov
Copy link

codecov bot commented Mar 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
nav2_controller/plugins/simple_goal_checker.cpp 100.00% <100.00%> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

Thanks!

@SteveMacenski SteveMacenski merged commit c7a5cc5 into ros-navigation:main Mar 17, 2025
11 checks passed
Nils-ChristianIseke pushed a commit to Nils-ChristianIseke/navigation2 that referenced this pull request Mar 21, 2025
- Fixed discrepancy in goal checker orientation, which was checking for
< tolerance instead of <= tolerance, as all the other limit checks are.
- Reduced tolerance time for the progress checker unit tests to
0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.

Signed-off-by: Rasmus Larsson <[email protected]>
Signed-off-by: Nils-ChristianIseke <[email protected]>
SteveMacenski pushed a commit that referenced this pull request Apr 15, 2025
- Fixed discrepancy in goal checker orientation, which was checking for
< tolerance instead of <= tolerance, as all the other limit checks are.
- Reduced tolerance time for the progress checker unit tests to
0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.

Signed-off-by: Rasmus Larsson <[email protected]>
SteveMacenski added a commit that referenced this pull request Apr 15, 2025
* Add short delay before lookup transform in polygon test (#4939)

* Add short delay before lookup transform

Signed-off-by: mini-1235 <[email protected]>

* Fix linting

Signed-off-by: mini-1235 <[email protected]>

---------

Signed-off-by: mini-1235 <[email protected]>

* Populate stamped command message with now timestamp, if velocity timed out. (#4959)

Signed-off-by: Georg Flick <[email protected]>

* Integrate ClearCostmapExceptRegion and ClearCostmapAroundRobot Services into the API (#4962)

* Add ClearCostmapExceptRegion and ClearCostmapAroundRobot services to BasicNavigator API

Signed-off-by: BCKSELFDRIVEWORLD <[email protected]>

* typo ament_flake8

Signed-off-by: BCKSELFDRIVEWORLD <[email protected]>

* type fix ament_flake

Signed-off-by: BCKSELFDRIVEWORLD <[email protected]>

---------

Signed-off-by: BCKSELFDRIVEWORLD <[email protected]>

* Fix urls in node hybrid (#4973)

Signed-off-by: mattbooker <[email protected]>

* Use main function to replace global variable in gtest. (#4978)

Signed-off-by: ChenYing Kuo <[email protected]>

* nav2_behavior_tree: fix input port parsing error in AreErrorCodesPresent (#4986)

The getInput method does not support std::set<uint16_t> parsing. So,
let's replace the type of the input port by std::vector<int> which is
supported, and convert the list to a std::set<uint16_t>.

This commit fixes issue #4985.

Signed-off-by: Dylan De Coeyer <[email protected]>

* Change to goal checker orientation for yaw angle (#4988)

- Fixed discrepancy in goal checker orientation, which was checking for
< tolerance instead of <= tolerance, as all the other limit checks are.
- Reduced tolerance time for the progress checker unit tests to
0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.

Signed-off-by: Rasmus Larsson <[email protected]>

* Declare_parameter_if_not_declared in docking navigator (#5023)

Signed-off-by: Alberto Tudela <[email protected]>

* bumping to 1.3.6 for release

Signed-off-by: Steve Macenski <[email protected]>

---------

Signed-off-by: mini-1235 <[email protected]>
Signed-off-by: Georg Flick <[email protected]>
Signed-off-by: BCKSELFDRIVEWORLD <[email protected]>
Signed-off-by: mattbooker <[email protected]>
Signed-off-by: ChenYing Kuo <[email protected]>
Signed-off-by: Dylan De Coeyer <[email protected]>
Signed-off-by: Rasmus Larsson <[email protected]>
Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: mini-1235 <[email protected]>
Co-authored-by: Georg Flick <[email protected]>
Co-authored-by: Burak Can Kaya <[email protected]>
Co-authored-by: Matthew Booker <[email protected]>
Co-authored-by: ChenYing Kuo (CY) <[email protected]>
Co-authored-by: DylanDeCoeyer-Quimesis <[email protected]>
Co-authored-by: RasmusLar <[email protected]>
Co-authored-by: Alberto Tudela <[email protected]>
stevedanomodolor pushed a commit to stevedanomodolor/navigation2 that referenced this pull request Apr 29, 2025
- Fixed discrepancy in goal checker orientation, which was checking for
< tolerance instead of <= tolerance, as all the other limit checks are.
- Reduced tolerance time for the progress checker unit tests to
0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.

Signed-off-by: Rasmus Larsson <[email protected]>
Signed-off-by: stevedanomodolor <[email protected]>
SakshayMahna pushed a commit to SakshayMahna/navigation2 that referenced this pull request Jun 8, 2025
- Fixed discrepancy in goal checker orientation, which was checking for
< tolerance instead of <= tolerance, as all the other limit checks are.
- Reduced tolerance time for the progress checker unit tests to
0.1 seconds, to reduce test runtime from ~17 to ~7 seconds.

Signed-off-by: Rasmus Larsson <[email protected]>
Signed-off-by: Sakshay Mahna <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants