Skip to content

Fix Broken isValid Check in Path Components Iterator#103

Merged
SteveMacenski merged 2 commits into
open-navigation:mainfrom
Jquinny:fix-path-components-iterator
Mar 28, 2026
Merged

Fix Broken isValid Check in Path Components Iterator#103
SteveMacenski merged 2 commits into
open-navigation:mainfrom
Jquinny:fix-path-components-iterator

Conversation

@Jquinny

@Jquinny Jquinny commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

This is a really really simple PR to fix the isValid check #102

closes #102

@SteveMacenski SteveMacenski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The only thing I'd like is for you to confirm to me that you tested before this PR and confirmed the issue in its use in the code. I trust this is a valid solution but I want to make sure there's not some reason I did that which may not be obvious.

Why is the test not failing? https://github.com/open-navigation/opennav_coverage/blob/main/opennav_coverage/test/test_utils.cpp

@Jquinny

Jquinny commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

The only thing I'd like is for you to confirm to me that you tested before this PR and confirmed the issue in its use in the code. I trust this is a valid solution but I want to make sure there's not some reason I did that which may not be obvious.

Why is the test not failing? https://github.com/open-navigation/opennav_coverage/blob/main/opennav_coverage/test/test_utils.cpp

The only thing I'd like is for you to confirm to me that you tested before this PR and confirmed the issue in its use in the code. I trust this is a valid solution but I want to make sure there's not some reason I did that which may not be obvious.

Why is the test not failing? https://github.com/open-navigation/opennav_coverage/blob/main/opennav_coverage/test/test_utils.cpp

Sounds good. I've added a quick iteration variable test to the current path components iterator test function. When running the tests with the old isValid check, I get the following from the test log file:

9: [ RUN      ] UtilsTests.TestPathComponentsIterator
9: /home/user/ros2_ws/src/opennav_coverage/opennav_coverage/test/test_utils.cpp:235: Failure
9: Expected equality of these values:
9:   i
9:     Which is: 5
9:   msg.swaths.size()
9:     Which is: 4
9: 
9: [  FAILED  ] UtilsTests.TestPathComponentsIterator (0 ms)

This highlights that the iteration loop is going one past the expected amount. Once I added back in the quick fix the test passed

@SteveMacenski SteveMacenski merged commit 40c2d84 into open-navigation:main Mar 28, 2026
6 of 7 checks passed
Jquinny added a commit to Jquinny/opennav_coverage that referenced this pull request Mar 30, 2026
…#103)

* fix: incorrect isValid check

* fix: add path component iteration index validation test
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.

Path Components Iterator Broken isValid() Check

2 participants