fix(navigator): do not report the end of a mission as a storage failure [1.18] - #28537
Merged
Merged
Conversation
…re (PX4#28464) * fix(navigator): do not report the end of a mission as a storage failure getNonJumpItem() walks forward past items it skips, including a DO_JUMP that has no repeats left. When such a DO_JUMP is the last item in the mission the walk steps one index beyond the end and calls loadMissionItemFromCache(), which short circuits on its own bounds check without ever touching the datamanager. The caller then took the read failure path and told the operator: "Waypoint could not be read." (mavlink_log_critical) "Waypoint {1} could not be read from storage" (events, Log::Error) so a mission that simply ran to completion reported a Critical storage error. A mission of the form [WP, WP, WP, DO_JUMP(repeat N)] hits this on its final pass, and the look ahead for the next position setpoint takes the same path, so it can repeat. Check the index against the mission bounds inside the loop, exactly as the function already does for the index it is entered with, and return the same quiet PX4_ERROR that an out of range index produces there. Callers already treat that as "no further item". Genuine datamanager failures still report as before. Assisted-by: Claude:claude-fable-5 Signed-off-by: Saibernard Yogendran <bernie97@seas.upenn.edu> * test(navigator): cover traversal past the mission bounds over an exhausted DO_JUMP Two regression tests for getNonJumpItem entered at a DO_JUMP whose repeats are used up. Skipping it walks the index one step outside the mission, forward off the end and backward in front of the start. Both must report PX4_ERROR the same quiet way an out of range entry index does, instead of taking the storage failure path. On unfixed code the forward case reproduces the false "Waypoint could not be read" critical error that a mission ending in an exhausted DO_JUMP publishes on its final pass. Assisted-by: Claude:claude-fable-5 Signed-off-by: Saibernard Yogendran <bernie97@seas.upenn.edu> --------- Signed-off-by: Saibernard Yogendran <bernie97@seas.upenn.edu> (cherry picked from commit bd0b15d)
github-actions
Bot
added
kind:bug
Contributor
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 0 byte (0 %)]px4_fmu-v6x [Total VM Diff: -8 byte (-0 %)]Updated: 2026-09-04T02:53:23 |
dakejahl
approved these changes
Sep 4, 2026
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
2 participants
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.
Backport of #28464 to
release/1.18, adapted for the branch.Skipping an exhausted DO_JUMP at the end of a mission stepped one index past the end, and the operator saw "Waypoint could not be read." at critical severity for a mission that had simply finished. The code change is the same as on main. The two new tests are written against this branch's test fixture.