Trackback flight controller lock up fix

[breadoven]

Should provide a fix for #11392 and #11007.

Prevents a position being set when the trackback array index is < 0 which can cause the flight controller to lock up in some instances.

It's an issue if trackback is initiated when the trackback store hasn't completely filled initially and then runs back to the first trackback point set. Once the trackback point store has filled and is wrapping around the problem goes away.

This issue affects all versions of INAV back to 6.0 when trackback was first introduced. It's not clear why it only seems to cause occasional FC lockups, likely just a result of undefined behaviour when trying to set a position with an invalid array index.

HITL testing shows the position is no longer set if the trackback array index is < 0.

[github-actions]

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

• maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

• maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.

---

This is an automated suggestion to help route contributions to the appropriate branch.

[qodo-code-review]

Review Summary by Qodo

Fix trackback flight controller lockup with array bounds validation

🐞 Bug fix

Walkthroughs

Description

• Prevents flight controller lockup by validating trackback array index before setting position
• Fixes issue where invalid array index caused undefined behavior during trackback initialization
• Improves trackback termination logic to correctly detect last waypoint
• Affects all INAV versions since trackback feature introduction in v6.0

Diagram

flowchart LR
  A["Waypoint Reached"] --> B["Decrement activePointIndex"]
  B --> C["Check if activePointIndex equals WrapAroundCounter"]
  C -->|Not Equal| D["Set Position with Valid Index"]
  C -->|Equal| E["End Trackback Safely"]
  D --> F["Continue Navigation"]
  E --> F

Loading

File Changes

1. src/main/navigation/rth_trackback.c 🐞 Bug fix +5/-4

Add array bounds validation for trackback position setting

• Moved calculateAndSetActiveWaypointToLocalPosition() call inside conditional check to prevent
 setting position with invalid array index
• Replaced array index comparison logic with direct equality check between activePointIndex and
 WrapAroundCounter
• Added else clause to safely set activePointIndex to -1 when trackback termination condition is
 met
• Improved code clarity with explanatory comment about trackback point termination

src/main/navigation/rth_trackback.c

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Trackback ends one loop late 🐞 Bug ✓ Correctness

Description

When the final trackback point is reached, the new logic sets activePointIndex to -1 but still
returns true, so the FSM remains in NAV_STATE_RTH_TRACKBACK for one extra iteration with
rthTrackbackActive still true. This can delay switching back to normal RTH and creates an
intermediate state where rthTrackbackActive=true while activePointIndex is invalid (<0).

Code

src/main/navigation/rth_trackback.c[R156-161]

+        // Last trackback point reached when activePointIndex = WrapAroundCounter so only set position when not equal
+        if (rth_trackback.activePointIndex != rth_trackback.WrapAroundCounter) {
+            calculateAndSetActiveWaypointToLocalPosition(getRthTrackBackPosition());
+        } else {
+            rth_trackback.activePointIndex = -1;  // if not already = -1 set to -1 to end trackback next iteration
        }

Evidence

rthTrackBackSetNewPosition() sets activePointIndex=-1 at the end condition but does not clear
rthTrackbackActive and still returns true; navigation’s RTH_TRACKBACK handler only exits this state
when rthTrackBackSetNewPosition() returns false, so the transition is necessarily delayed until the
next call.

src/main/navigation/rth_trackback.c[149-167]
src/main/navigation/rth_trackback.c[143-146]
src/main/navigation/navigation.c[1645-1658]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
When the last trackback point is reached, `rthTrackBackSetNewPosition()` sets `activePointIndex = -1` but still returns `true`. The RTH_TRACKBACK FSM state only exits when this function returns `false`, so trackback ends one iteration late and leaves a transient inconsistent state (`rthTrackbackActive == true` while `activePointIndex < 0`).

### Issue Context
This PR correctly avoids calling `getRthTrackBackPosition()` with a negative index, but the termination path should also drive an immediate state transition.

### Fix Focus Areas
- src/main/navigation/rth_trackback.c[149-167]
- src/main/navigation/navigation.c[1645-1658]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Add defensive index guard 🐞 Bug ⛯ Reliability

Description

getRthTrackBackPosition() still dereferences pointsList[activePointIndex] without validating bounds;
while this PR removes one known path, any unexpected state could still lead to undefined behavior.
Adding a lightweight bounds guard or debug assertion would make future regressions less likely to
lock up the FC.

Code

src/main/navigation/rth_trackback.c[R156-160]

+        // Last trackback point reached when activePointIndex = WrapAroundCounter so only set position when not equal
+        if (rth_trackback.activePointIndex != rth_trackback.WrapAroundCounter) {
+            calculateAndSetActiveWaypointToLocalPosition(getRthTrackBackPosition());
+        } else {
+            rth_trackback.activePointIndex = -1;  // if not already = -1 set to -1 to end trackback next iteration

Evidence

activePointIndex is explicitly set to -1 in multiple legitimate code paths, and
getRthTrackBackPosition() directly indexes pointsList using activePointIndex. The PR fixes the
specific call site in rthTrackBackSetNewPosition, but the helper remains inherently unsafe if called
with an invalid index.

src/main/navigation/rth_trackback.c[169-177]
src/main/navigation/rth_trackback.c[143-146]
src/main/navigation/rth_trackback.c[179-184]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`getRthTrackBackPosition()` indexes `pointsList[activePointIndex]` without validating that `activePointIndex` is in range. Even though the current PR prevents one specific call with `-1`, the helper remains unsafe if called from any other path when `activePointIndex` is negative or out of bounds.

### Issue Context
`activePointIndex` is legitimately set to `-1` when trackback is reset/canceled/ended.

### Fix Focus Areas
- src/main/navigation/rth_trackback.c[169-177]
- src/main/navigation/rth_trackback.c[143-146]
- src/main/navigation/rth_trackback.c[179-184]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[github-actions]

Test firmware build ready — commit 9d44fe5

Download firmware for PR #11393

223 targets built. Find your board's .hex file by name on that page (e.g. MATEKF405SE.hex). Files are individually downloadable — no GitHub login required.

Development build for testing only. Use Full Chip Erase when flashing.