Skip to content

Missions Control fixes + Add Manual Throttle Launch Option - #2704

Merged
breadoven merged 1 commit into
iNavFlight:maintenance-10.xfrom
breadoven:abo_mission_tweaks_add_manual_launch
Aug 11, 2026
Merged

Missions Control fixes + Add Manual Throttle Launch Option#2704
breadoven merged 1 commit into
iNavFlight:maintenance-10.xfrom
breadoven:abo_mission_tweaks_add_manual_launch

Conversation

@breadoven

Copy link
Copy Markdown
Collaborator

Adds missing speed option for landing waypoints and restricts waypoint P1 and P2 to only +ve values where applicable.

@qodo-code-review

Copy link
Copy Markdown
Contributor

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Mission Control waypoint fixes and manual throttle launch option

🐞 Bug fix ✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Add UI + i18n strings for a new “Manual Throttle Launch” launch mode.
• Fix landing waypoint parameter labeling by exposing the missing speed (cm/s) field.
• Enforce positive-only P1/P2 inputs for waypoint types where negatives are invalid.
Diagram

graph TD
  A["Advanced Tuning tab"] --> B["Manual Launch toggle"] --> C["FW setting: nav_fw_launch_manual_throttle"]
  D["Mission Control tab"] --> E["Waypoint editor"] --> F["P1/P2 input validation"] --> G["Waypoint model (marker)"]
  E --> H["Landing param labels"]

  subgraph Legend
    direction LR
    _ui["UI"] ~~~ _logic["Client logic"] ~~~ _cfg["Config/Setting"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize waypoint parameter validation in the waypoint model
  • ➕ Guarantees correctness regardless of UI entry point (future UIs, imports, API updates)
  • ➕ Keeps UI thinner and reduces duplicated rules across controls
  • ➖ Requires touching more code paths (marker setters / mission update pipeline)
  • ➖ May need careful handling to preserve legacy behavior for existing missions
2. Define per-waypoint-type schemas (labels + constraints) in one table
  • ➕ Single source of truth for parameter names, units, and valid ranges
  • ➕ Easier to extend with new waypoint types without scattering conditionals
  • ➖ Larger refactor than this PR’s targeted fix
  • ➖ Up-front design effort to model constraints cleanly

Recommendation: The PR’s targeted UI-side sanitation is pragmatic and low-risk for immediate correctness. If waypoint constraints continue to expand, consider moving validation (and ideally labels/units) into a centralized per-waypoint-type schema or the waypoint model to avoid UI-specific rule drift.

Files changed (3) +19 / -2

Enhancement (2) +12 / -1
messages.jsonAdd i18n strings for Manual Throttle Launch +7/-1

Add i18n strings for Manual Throttle Launch

• Introduces new English UI strings for the Manual Throttle Launch toggle and its help text. Also removes trailing whitespace in an existing OSD help entry.

locale/en/messages.json

advanced_tuning.htmlExpose Manual Throttle Launch checkbox in Advanced Tuning +5/-0

Expose Manual Throttle Launch checkbox in Advanced Tuning

• Adds a new checkbox bound to 'nav_fw_launch_manual_throttle' with live updates and contextual help tooltip. Wires the label/help to the newly added i18n keys.

tabs/advanced_tuning.html

Bug fix (1) +7 / -1
mission_control.jsFix landing speed label and constrain invalid negative waypoint params +7/-1

Fix landing speed label and constrain invalid negative waypoint params

• Updates the landing waypoint parameter label mapping to include Speed (cm/s) for action 8. Adds input sanitation so P1 is forced positive for non-SET_HEAD actions and P2 is forced positive for POSHOLD_TIME before updating the selected marker and mission.

tabs/mission_control.js

@sonarqubecloud

Copy link
Copy Markdown

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. NaN written to waypoint 🐞 Bug ≡ Correctness
Description
The new Math.abs(Number(...)) normalization for #pointP1/#pointP2 writes "NaN" back into the input
when the value isn’t numeric, and then persists NaN into Waypoint P1/P2. When missions are
serialized, NaN is coerced to 0 via bitwise byte extraction, silently changing the transmitted/saved
parameter values.
Code

tabs/mission_control.js[R3140-3142]

+                if (selectedMarker.getAction() != MWNP.WPTYPE.SET_HEAD) {
+                    $('#pointP1').val(Math.abs(Number($('#pointP1').val())));
+                }
Evidence
The diff-added normalization writes the absolute value back into the DOM input and then stores it
into the waypoint without validating Number(...) is finite; because the inputs are type="text",
non-numeric content is possible. Waypoint serialization uses bitwise byte extraction which coerces
NaN to 0, so NaN parameters will be silently transmitted/saved as zero.

tabs/mission_control.js[3138-3159]
tabs/mission_control.html[477-482]
js/waypointCollection.js[260-286]
js/bitHelper.js[7-13]

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

### Issue description
`#pointP1` and `#pointP2` are text inputs; the new normalization uses `Math.abs(Number(value))` and writes the result back to the input. For any non-numeric value, `Number(value)` becomes `NaN`, `Math.abs(NaN)` remains `NaN`, and jQuery `.val(NaN)` displays `"NaN"`, which then gets stored in the `Waypoint` via `setP1/setP2`.

Downstream, waypoint parameters are serialized with bitwise operations (`BitHelper.lowByte/highByte`), which coerce `NaN` to `0`, so missions can be silently altered when sent/saved.

### Issue Context
This is a regression in behavior/UX introduced by the new “force positive” logic: previously invalid text could still be entered (and would become `NaN` in the model), but it would not be rewritten into the visible field as the literal string `"NaN"`.

### Fix Focus Areas
- tabs/mission_control.js[3138-3159]

### Suggested fix approach
- Parse once into a local `const v = Number($('#pointP1').val())` (and similarly for P2).
- Guard with `Number.isFinite(v)` (or at least `!isNaN(v)`), and if invalid:
 - revert the input to the previous valid value (e.g., `selectedMarker.getP1()` / `getP2()`), and **do not** update the waypoint, or
 - clamp to a safe default (e.g., `0`) and optionally show a validation warning.
- Only apply `Math.abs(v)` after the finite-number check.
- Optionally, consider converting the HTML inputs to `type="number"` with `min="0"` for actions where negatives are invalid (but keep SET_HEAD and JUMP repeat semantics intact).

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


Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread tabs/mission_control.js
Comment on lines +3140 to +3142
if (selectedMarker.getAction() != MWNP.WPTYPE.SET_HEAD) {
$('#pointP1').val(Math.abs(Number($('#pointP1').val())));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Nan written to waypoint 🐞 Bug ≡ Correctness

The new Math.abs(Number(...)) normalization for #pointP1/#pointP2 writes "NaN" back into the input
when the value isn’t numeric, and then persists NaN into Waypoint P1/P2. When missions are
serialized, NaN is coerced to 0 via bitwise byte extraction, silently changing the transmitted/saved
parameter values.
Agent Prompt
### Issue description
`#pointP1` and `#pointP2` are text inputs; the new normalization uses `Math.abs(Number(value))` and writes the result back to the input. For any non-numeric value, `Number(value)` becomes `NaN`, `Math.abs(NaN)` remains `NaN`, and jQuery `.val(NaN)` displays `"NaN"`, which then gets stored in the `Waypoint` via `setP1/setP2`.

Downstream, waypoint parameters are serialized with bitwise operations (`BitHelper.lowByte/highByte`), which coerce `NaN` to `0`, so missions can be silently altered when sent/saved.

### Issue Context
This is a regression in behavior/UX introduced by the new “force positive” logic: previously invalid text could still be entered (and would become `NaN` in the model), but it would not be rewritten into the visible field as the literal string `"NaN"`.

### Fix Focus Areas
- tabs/mission_control.js[3138-3159]

### Suggested fix approach
- Parse once into a local `const v = Number($('#pointP1').val())` (and similarly for P2).
- Guard with `Number.isFinite(v)` (or at least `!isNaN(v)`), and if invalid:
  - revert the input to the previous valid value (e.g., `selectedMarker.getP1()` / `getP2()`), and **do not** update the waypoint, or
  - clamp to a safe default (e.g., `0`) and optionally show a validation warning.
- Only apply `Math.abs(v)` after the finite-number check.
- Optionally, consider converting the HTML inputs to `type="number"` with `min="0"` for actions where negatives are invalid (but keep SET_HEAD and JUMP repeat semantics intact).

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

@github-actions

Copy link
Copy Markdown

Configurator test build ready — commit ff82c86

Download build artifacts for PR #2704

Available platforms (scroll to the Artifacts section at the bottom of the run page):

  • Windows x64 (ZIP, MSI) and x32 (ZIP, MSI)
  • macOS arm64 (ZIP, DMG) and x64 (ZIP, DMG)
  • Linux x64 (DEB, RPM, ZIP) and aarch64 (DEB, RPM, ZIP)

A GitHub login is required to download artifacts. Build is for testing only.

@breadoven
breadoven merged commit 19dd004 into iNavFlight:maintenance-10.x Aug 11, 2026
8 checks passed
@breadoven
breadoven deleted the abo_mission_tweaks_add_manual_launch branch August 11, 2026 10:49
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.

1 participant