Skip to content

use binary_state rather than default state#5370

Merged
SteveMacenski merged 7 commits intoros-navigation:mainfrom
olaghattas:fix/binary_reset_beh
Jul 22, 2025
Merged

use binary_state rather than default state#5370
SteveMacenski merged 7 commits intoros-navigation:mainfrom
olaghattas:fix/binary_reset_beh

Conversation

@olaghattas
Copy link
Contributor

@olaghattas olaghattas commented Jul 18, 2025 •
edited
Loading

Basic Info

Info Please fill out this column
Ticket(s) this addresses Related to #5368
Primary OS tested on Ubuntu
Robotic platform tested on Hello Robot Stretch (hardware)
Does this PR contain AI generated software? No
Was this PR description generated by AI software? No

Description of contribution in a few bullet points

  • Fixed a bug in BinaryFilter::resetFilter() where the filter state was incorrectly restored to default_state_ rather than the last known binary_state_ which caused incorrect behavior when the costmap was cleared via Behavior Tree.
  • The issue and fix were previously described in #5368, which has now been closed.

Description of documentation updates required from your changes

None

Description of how this change was tested

  • Deployed and tested on the physical Hello Robot Stretch platform in an environment with a binary costmap filter. Verified that:
    • After a costmap reset or clear operation, the filter retained the correct binary_state_ instead of reverting to the default_state_.
    • The robot consistently classified its position relative to the filter mask correctly, regardless of where it started.

Future work that may be required in bullet points

None

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
  • Should this be backported to current distributions? If so, tag with backport-*.

@mergify
Copy link
Contributor

mergify bot commented Jul 18, 2025

@olaghattas, 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).

@olaghattas olaghattas force-pushed the fix/binary_reset_beh branch from 26ba889 to 5657c8b Compare July 18, 2025 21:09
@mergify
Copy link
Contributor

mergify bot commented Jul 18, 2025

@olaghattas, 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).

@olaghattas olaghattas force-pushed the fix/binary_reset_beh branch 2 times, most recently from d9bbbba to 3c57579 Compare July 19, 2025 05:23
@mergify
Copy link
Contributor

mergify bot commented Jul 19, 2025

@olaghattas, 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).

@codecov
Copy link

codecov bot commented Jul 19, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
...stmap_2d/plugins/costmap_filters/binary_filter.cpp 94.66% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

@olaghattas
Copy link
Contributor Author

olaghattas commented Jul 19, 2025

@SteveMacenski I’ve addressed all the errors, but the Lint / ament_mypy (pull_request) check is still failing even though the issue doesn’t seem to originate from nav2_costmap_2d. Do you have any insight on how to resolve this?

@SteveMacenski
Copy link
Member

SteveMacenski commented Jul 21, 2025

@leander-dsouza mypy is acting up once more :(

@SteveMacenski SteveMacenski linked an issue Jul 21, 2025 that may be closed by this pull request
BinaryFilter resets unexpectedly during stable mask input #5368
Closed
SteveMacenski
SteveMacenski reviewed Jul 21, 2025
View reviewed changes
hello-ola added 5 commits July 21, 2025 16:30
@hello-ola
use binary_state rather than default state
4606766 
Signed-off-by: hello-ola <oghattas@hello-robot.com>
@hello-ola
fix test
9664c4d 
Signed-off-by: hello-ola <oghattas@hello-robot.com>
@hello-ola
fix test and comment
4e02103 
Signed-off-by: hello-ola <oghattas@hello-robot.com>
@hello-ola
linting fix
90194a3 
Signed-off-by: hello-ola <oghattas@hello-robot.com>
@hello-ola
remove changestate from resetFilter
be298d5 
Signed-off-by: hello-ola <oghattas@hello-robot.com>
@olaghattas olaghattas force-pushed the fix/binary_reset_beh branch from 2bc3f52 to be298d5 Compare July 21, 2025 23:30
@hello-ola
test fix by pub value in resetfilter method
e1508c4 
Signed-off-by: hello-ola <oghattas@hello-robot.com>
SteveMacenski
SteveMacenski approved these changes Jul 22, 2025
View reviewed changes
@SteveMacenski
Update nav2_costmap_2d/plugins/costmap_filters/binary_filter.cpp
031b992 
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
@SteveMacenski SteveMacenski merged commit 38c5f00 into ros-navigation:main Jul 22, 2025
11 of 13 checks passed
@leander-dsouza leander-dsouza mentioned this pull request Jul 22, 2025
Merged
8 tasks
@leander-dsouza
Copy link
Contributor

leander-dsouza commented Jul 23, 2025

@leander-dsouza mypy is acting up once more :(

I have proposed a fix for this at #5382 :)

SakshayMahna pushed a commit to SakshayMahna/navigation2 that referenced this pull request Aug 8, 2025
@olaghattas @hello-ola
@SteveMacenski @SakshayMahna
use binary_state rather than default state (ros-navigation#5370)
9c2d317 
* use binary_state rather than default state

Signed-off-by: hello-ola <oghattas@hello-robot.com>

* fix test

Signed-off-by: hello-ola <oghattas@hello-robot.com>

* fix test and comment

Signed-off-by: hello-ola <oghattas@hello-robot.com>

* linting fix

Signed-off-by: hello-ola <oghattas@hello-robot.com>

* remove changestate from resetFilter

Signed-off-by: hello-ola <oghattas@hello-robot.com>

* test fix by pub value in resetfilter method

Signed-off-by: hello-ola <oghattas@hello-robot.com>

* Update nav2_costmap_2d/plugins/costmap_filters/binary_filter.cpp

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

---------

Signed-off-by: hello-ola <oghattas@hello-robot.com>
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
Co-authored-by: hello-ola <oghattas@hello-robot.com>
Co-authored-by: Steve Macenski <stevenmacenski@gmail.com>
@olaghattas olaghattas mentioned this pull request Aug 14, 2025
Merged
8 tasks
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BinaryFilter resets unexpectedly during stable mask input

4 participants

@olaghattas @SteveMacenski @leander-dsouza @hello-ola