Repository navigation
Fix mixed coordinate units in Dijkstra grid indexing - #1461
Open
huxiaoyi-ovo wants to merge 2 commits into
Open
huxiaoyi-ovo wants to merge 2 commits into
huxiaoyi-ovo wants to merge 2 commits into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference issue
Fixes #1318.
What does this implement/fix?
DijkstraPlanner.calc_index()previously subtracted the map's world-coordinate origin (min_x,min_y, in metres) fromNode.xandNode.y, even though these node coordinates are already zero-based grid indices (in cells). This mixes units and produces an incorrect nonzero index for the grid-origin cell whenever the world origin is offset.Use the direct row-major index
node.y * self.x_width + node.x. The previous offset was constant for a map, so the fix corrects the index definition without changing Dijkstra's search ordering or planned paths.A focused regression test checks two maps with different nonzero world origins and non-unit resolutions (
0.5and2.0m/cell), including:(0, 0)and index0;y * width + x;width * height - 1.This also addresses the maintainer's earlier request for an offset-origin, non-unit-resolution regression test on #1360 (closed without merge).
Additional information
Validated against current upstream
master(cdd0cc888802b584c2d654ca85e3c5460973487d) using fork-only draft PR #2:PathPlanning/Dijkstra/dijkstra.pyandtests/test_dijkstra.py, with no new dependencies.CheckList
AI assistance was used to review the issue history and prepare the minimal fix and regression test; the final branch was checked against upstream, and the full fork CI passed.