Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various Landing Pattern fixes and add final approach speed #12238

Conversation

rubenp02
Copy link
Contributor

@rubenp02 rubenp02 commented Dec 19, 2024

Various Landing Pattern fixes and add final approach speed

Description

Fix landing item scanning for ArduPilot fixed wing:

Addressed issues with scanning and interpreting landing sequences for fixed wing ArduPilot aircraft. Previously, landing sequences uploaded to ArduPilot fixed wing aircraft and then downloaded back weren't being reinterpreted as QGC LandingComplexItems. The system to detect a landing sequence and translate it back to a LandingComplexItem wasn't working correctly due to multiple assumptions about mission item parameters that weren't accurate for ArduPilot.

The changes include adjusting parameter checks for NAV_LAND, NAV_LOITER_TO_ALT, NAV_WAYPOINT, and DO_LAND_START commands to align with ArduPilot's implementation, without modifying the existing logic for other firmwares.

Fix waypoint glide slope heading calculations:

When not using the "Use loiter to altitude" option in the Landing Pattern mission item, the heading of the glide slope still took into account the old loiter radius. Adjusted the QML code to change the loiter radius to 0 in landing patterns that use a waypoint as the approach start, which in turns lead to correct heading calculations.

Ideally, the _recalcFromHeadingAndDistanceChange, _recalcFromRadiusChange and _recalcFromCoordinateChange should be refactored to account for the fact that a landing pattern can use a waypoint instead of a loiter. However, this fix works just as well and is much simpler.

Fix boolean Fact interpretation in QGCCheckBox:

Addressed an issue where the values of boolean Facts where the QVariant was of type int were not being correctly interpreted to drive the checked status of the QGCCheckBox. This was a problem with boolean Facts loaded from JSON where the default was "false", in which case the checkbox didn't work correctly.

Add final approach speed to the Landing Pattern:

Introduced an option for setting an airspeed for the final approach in the Landing Pattern editor, almost identical to the one found in standard waypoints, for the purposes of energy management before the glide slope.

It works by inserting a DO_CHANGE_SPEED mission element right after the DO_LAND_START, in such a way that the speed is also changed in the event of an RTL. The option is unchecked by default.

Fix VTOL landing item scanning for PX4:

Fixes #12266 (fix by @anaam-wingxpand).

Checklist:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@DonLakeFlyer DonLakeFlyer left a comment

Choose a reason for hiding this comment

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

Looks good in general. I made some comments which need looking at. I also want to take a second look more in depth once those get addressed

src/MissionManager/LandingComplexItem.cc Outdated Show resolved Hide resolved
src/MissionManager/LandingComplexItem.cc Outdated Show resolved Hide resolved
src/MissionManager/LandingComplexItem.cc Outdated Show resolved Hide resolved
src/MissionManager/LandingComplexItem.cc Outdated Show resolved Hide resolved
src/PlanView/FWLandingPatternEditor.qml Show resolved Hide resolved
src/FactSystem/FactControls/FactCheckBox.qml Show resolved Hide resolved
@rubenp02 rubenp02 force-pushed the feature/add-landing-pattern-final-approach-speed branch 2 times, most recently from e4092d6 to 85a78bd Compare December 30, 2024 09:28
@rubenp02 rubenp02 force-pushed the feature/add-landing-pattern-final-approach-speed branch from 85a78bd to 3a3a036 Compare December 30, 2024 15:51
@DonLakeFlyer
Copy link
Contributor

@rubenp02 Can you make sure #12266 is taken care of as well?

Addressed issues with scanning and interpreting landing sequences for
fixed wing ArduPilot aircraft. Previously, landing sequences uploaded to
ArduPilot fixed wing aircraft and then downloaded back weren't being
reinterpreted as QGC LandingComplexItems. The system to detect a landing
sequence and translate it back to a LandingComplexItem wasn't working
correctly due to multiple assumptions about mission item parameters that
weren't accurate for ArduPilot.

The changes include adjusting parameter checks for NAV_LAND,
NAV_LOITER_TO_ALT, NAV_WAYPOINT, and DO_LAND_START commands to align
with ArduPilot's implementation, without modifying the existing logic
for other firmwares.
When not using the "Use loiter to altitude" option in the Landing
Pattern mission item, the heading of the glide slope still took into
account the old loiter radius. Adjusted the QML code to change the
loiter radius to 0 in landing patterns that use a waypoint as the
approach start, which in turns lead to correct heading calculations.

Ideally, the _recalcFromHeadingAndDistanceChange,
_recalcFromRadiusChange and _recalcFromCoordinateChange should be
refactored to account for the fact that a landing pattern can use a
waypoint instead of a loiter. However, this fix works just as well and
is much simpler.
Addressed an issue where the values of boolean Facts where the QVariant
was of type int were not being correctly interpreted to drive the
checked status of the QGCCheckBox. This was a problem with boolean Facts
loaded from JSON where the default was "false", in which case the
checkbox didn't work correctly.
Introduced an option for setting an airspeed for the final approach in
the Landing Pattern editor, almost identical to the one found in
standard waypoints, for the purposes of energy management before the
glide slope.

It works by inserting a DO_CHANGE_SPEED mission element right after the
DO_LAND_START, in such a way that the speed is also changed in the event
of an RTL. The option is unchecked by default.
@rubenp02 rubenp02 force-pushed the feature/add-landing-pattern-final-approach-speed branch from 3a3a036 to a8f6c00 Compare January 2, 2025 09:10
@DonLakeFlyer DonLakeFlyer merged commit 23c0b68 into mavlink:master Jan 3, 2025
8 checks passed
@DonLakeFlyer
Copy link
Contributor

Thanks so much for this one @rubenp02

@rubenp02 rubenp02 deleted the feature/add-landing-pattern-final-approach-speed branch January 4, 2025 17:59
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.

VTOL Landing Pattern gets split into multiple waypoints
2 participants