Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,23 @@
"label": "Return altitude",
"optional": true,
"enableWhen": "_planeRtlAltFact && _planeRtlAltFact.value >= 0"
},
{
"param": "RTL_RADIUS",
"label": "Loiter radius",
"optional": true
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

RTL_RADIUS is marked optional but the control has no showWhen/enableWhen guard. If the param is missing on a given firmware version, getParameterFact(..., false) will yield null and the generated LabelledFactTextField can still be interactive, leading to runtime errors when editing finishes. Consider adding a guard based on controller.parameterExists(-1, "RTL_RADIUS") (or a fact-exists binding) to hide/disable this control when the parameter is unavailable.

Suggested change
"optional": true
"optional": true,
"showWhen": "controller.parameterExists(-1, \"RTL_RADIUS\")"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The existing RTL_ALTITUDE entries in this same section use optional: true without any parameterExists guard. These are standard ArduPlane params present in all firmware versions — optional is just a defensive pattern matching surrounding code. Adding showWhen only to the new entries would be inconsistent.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The existing RTL_ALTITUDE entries in this same section use optional: true without any parameterExists guard. These are standard ArduPlane params present in all firmware versions — optional is just a defensive pattern matching surrounding code. Adding showWhen only to the new entries would be inconsistent.

},
{
"control": "label",
"label": "0 = use Waypoint Loiter Radius (WP_LOITER_RAD), negative = counter-clockwise",
"indent": true,
"smallFont": true
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

The helper label under RTL loiter radius is unconditional. If RTL_RADIUS is not present (it’s optional above), this explanatory text will still show without the associated control. Consider adding the same showWhen/visibility condition used for the RTL radius control so the note only appears when the parameter exists.

Suggested change
"smallFont": true
"smallFont": true,
"showWhen": "controller.parameterExists(-1, 'RTL_RADIUS')"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same reasoning — the label is tied to the RTL_RADIUS control which will always exist on ArduPlane.

},
{
"param": "RTL_AUTOLAND",
"label": "Auto land after RTL",
"optional": true,
"control": "combobox"
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

RTL_AUTOLAND is marked optional but has no showWhen/enableWhen guard. If the param is missing, the generated combobox may still be focusable/clickable while its fact is null, which can trigger QML runtime errors on activation. Consider gating visibility/enabled state on controller.parameterExists(-1, "RTL_AUTOLAND") (or an equivalent fact-exists binding).

Suggested change
"control": "combobox"
"control": "combobox",
"showWhen": "controller.parameterExists(-1, \"RTL_AUTOLAND\")"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same reasoning as above — consistent with existing pattern.

}
]
},
Expand Down
3 changes: 3 additions & 0 deletions tools/generators/common/controls.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def render_label(
*,
text: str = "",
warning: bool = False,
small_font: bool = False,
tr_context: str = "",
) -> str:
"""Render a static ``QGCLabel`` (no fact binding)."""
Expand All @@ -98,6 +99,8 @@ def render_label(
lines.append(f"{indent} wrapMode: Text.WordWrap")
lines.append(f"{indent} Layout.fillWidth: true")
lines.append(f"{indent} Layout.preferredWidth: 0")
if small_font:
lines.append(f"{indent} font.pointSize: ScreenTools.smallFontPointSize")
if warning:
lines.append(f"{indent} color: qgcPal.warningText")
lines.append(f"{indent}}}")
Expand Down
3 changes: 3 additions & 0 deletions tools/generators/config_qml/page_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class ControlDef:
firstEntryIsAll: bool = False # bitmask: first entry is "all" toggle
toggleCheckbox: ToggleCheckboxDef | None = None # toggleCheckbox: custom checked/onClicked
indent: bool = False # indent control with left margin
smallFont: bool = False # label: use small font size


@dataclass
Expand Down Expand Up @@ -168,6 +169,7 @@ def load_page_def(json_path: Path) -> PageDef:
firstEntryIsAll=ctrl_data.get("firstEntryIsAll", False),
toggleCheckbox=parse_toggle_checkbox(ctrl_data.get("toggleCheckbox")),
indent=ctrl_data.get("indent", False),
smallFont=ctrl_data.get("smallFont", False),
))
repeat_data = sec_data.get("repeat")
repeat_def = None
Expand Down Expand Up @@ -339,6 +341,7 @@ def _apply_indent(qml: str) -> str:
indent,
text=ctrl.label,
warning=ctrl.warning,
small_font=ctrl.smallFont,
tr_context=tr_context,
)
if ctrl.showWhen:
Expand Down
Loading