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

Guided mode improvements #12536

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

rubenp02
Copy link
Contributor

@rubenp02 rubenp02 commented Mar 4, 2025

Guided mode improvements

Description

Various improvements to Guided mode and its actions, mostly for ArduPilot.

Fix overlap between Go here and Orbit map items:

Added a visibility condition to the Orbit map item to hide it when the orbit is part of a Go To action, preventing both items from overlapping.

Fix Orbit and Go here map items disappearing:

Fixed an issue where the Orbit and Go here map items would disappear upon opening the map click drop panel. Also tweaked the formatting of the conditional expression that controls the panel's opening.

Restore previous Go here map item on cancellation:

Added logic to commit and revert coordinate changes for the Go here map item. Updated the Go To action handlers to utilize this logic, allowing the previous Go here item to be restored when a Go To action is cancelled.

Add property to check if vehicle in forward flight:

Added a new property in the Vehicle class to determine if the vehicle is in forward flight. This was previously done in GuidedActionsController.qml but it's useful enough to include in the main Vehicle class. The existing logic in the GuidedActionsController module has been updated to utilize this new property.

Support MAV_CMD_DO_REPOSITION radius parameter:

Added support for the radius parameter of the MAV_CMD_DO_REPOSITION command. When in forward flight, this parameter can be used to specify the radius of the orbit around the Go here position in Guided Mode. A Fly View setting has been added to control this radius.

Key changes:

  • Modified the guidedModeGotoLocation method across all firmware plugins to accept an optional forwardFlightLoiterRadius parameter, which sets the desired loiter radius during forward flight.
  • Added "Loiter Radius in Forward Flight" setting to the Guided Commands section of the FlyViewSettings to control the default radius.

Note: PX4 doesn't doesn't handle the radius parameter per the MAVLink spec and therefore doesn't support this functionality.

Add Go here circle visuals if in forward flight:

Added circle visuals to the Go here map item to show the orbit pattern that aircraft in forward flight will follow upon reaching the position.

This feature is already available in firmware that supports the ORBIT_EXECUTION_STATUS MAVLink message, so this is only shown for vehicles operating on firmware that lacks this support.

Add "Change Loiter Radius" action:

Added a new Guided mode action to change the radius of the orbit around the Go here position when in forward flight. This action integrates with the Go here circle visuals, allowing the new radius to be selected using the MapCircleVisuals interactive editing gizmo.

Add Go to confirmation option to Fly View settings:

Added a Fly View setting to require confirmation for the "Go to location" action when in Guided/Go To mode. The option is disabled by default to avoid constant confirmation requests, as accidental triggers are rare. However, a confirmation is always required if the vehicle is not already in Guided/Go To mode. Enabling this setting restores the previous behavior of always requiring confirmation.

Demo

guided-mode-improvements-demo.mp4

Checklist:

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

rubenp02 added 8 commits March 4, 2025 13:59
Added a visibility condition to the Orbit map item to hide it when the
orbit is part of a Go To action, preventing both items from overlapping.
Fixed an issue where the Orbit and Go here map items would disappear
upon opening the map click drop panel. Also tweaked the formatting of
the conditional expression that controls the panel's opening.
Added logic to commit and revert coordinate changes for the Go here map
item. Updated the Go To action handlers to utilize this logic, allowing
the previous Go here item to be restored when a Go To action is
cancelled.
Added a new property in the Vehicle class to determine if the vehicle is
in forward flight. This was previously done in
GuidedActionsController.qml but it's useful enough to include in the
main Vehicle class. The existing logic in the GuidedActionsController
module has been updated to utilize this new property.
Added support for the radius parameter of the MAV_CMD_DO_REPOSITION
command. When in forward flight, this parameter can be used to specify
the radius of the orbit around the Go here position in Guided Mode. A
Fly View setting has been added to control this radius.

Key changes:
- Modified the guidedModeGotoLocation method across all firmware plugins
  to accept an optional forwardFlightLoiterRadius parameter, which sets
  the desired loiter radius during forward flight.
- Added "Loiter Radius in Forward Flight" setting to the Guided Commands
  section of the FlyViewSettings to control the default radius.

Note: PX4 doesn't doesn't handle the radius parameter per the MAVLink
spec and therefore doesn't support this functionality.
Added circle visuals to the Go here map item to show the orbit pattern
that aircraft in forward flight will follow upon reaching the position.

This feature is already available in firmware that supports the
ORBIT_EXECUTION_STATUS MAVLink message, so this is only shown for
vehicles operating on firmware that lacks this support.
Added a new Guided mode action to change the radius of the orbit around
the Go here position when in forward flight. This action integrates with
the Go here circle visuals, allowing the new radius to be selected using
the MapCircleVisuals interactive editing gizmo.
Added a Fly View setting to require confirmation for the "Go to
location" action when in Guided/Go To mode. The option is disabled by
default to avoid constant confirmation requests, as accidental triggers
are rare. However, a confirmation is always required if the vehicle is
not already in Guided/Go To mode. Enabling this setting restores the
previous behavior of always requiring confirmation.
@rubenp02
Copy link
Contributor Author

rubenp02 commented Mar 4, 2025

A couple issues that remain (neither of them is a consequence of this PR, but maybe they should be fixed here) are:

  • At least in ArduPilot, when launching the Go To action while in Auto mode, the action's hide trigger quickly toggles, not sure why. The action still happens normally, but its cancel handler is called when it shouldn't. This makes the Go here map item disappear.
  • When the Go To action fails after confirmation (for example, when the destination is too far) the map item doesn't reflect this (it remains at the confirmed position).

@DonLakeFlyer
Copy link
Contributor

Sorry it's taking me so long on this. I'm prepping for a big testing trip in Zimbabwe in a few weeks. So I'm totally swamped right now.

@DonLakeFlyer
Copy link
Contributor

The option is disabled by default to avoid constant confirmation requests, as accidental triggers are rare.

I don't think this is a good idea. The concept of being in "Guided" mode is more of a QGC thing than a reality across firmwares. For example in PX4 when you Takeoff, and then takeoff is complete the vehicle goes into Hold flight mode. This is "Guided" mode for PX4. Which in turns means if you then do a goto, you will not be prompted to confirm. I get it's rare to get an accidental goto but this is a change in functionality from pervious version where someone might be used to getting a confirm and all of the sudden the vehicle just takes off in a direction right away. I think this should always require confirm without an option to turn off/on. Just like everything else.

@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Mar 12, 2025

The inFwdFlight thing is fixed wing only right? If so, I think it would be better to call that out and change it to 'fixedWingInFwdFlight' or something like that to make it more clear what this is. Or 'fwInFwdFlight'? When I first looked at this I was a little confused as to when it would be active.

@rubenp02
Copy link
Contributor Author

The inFwdFlight thing is fixed wing only right? If so, I think it would be better to call that out and change it to 'fixedWingInFwdFlight' or something like that to make it more clear what this is.

No, it's for any kind of forward flight (fixed-wing or VTOL if it currently is in forward flight). That's why I added it, since there already is a fixedWing Vehicle Q_PROPERTY.

@DonLakeFlyer
Copy link
Contributor

Yeah, I think of VTOL as a fixed wing as well. Anyway, leave it as is.

@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Mar 12, 2025

Testing PX4 sitl: I'm confused as to GoTo showing a loiter radius on the map for FW when flying PX4. Where does that radius come from if PX4 doesn't support radius on goto?

@DonLakeFlyer
Copy link
Contributor

Also since goto stores the loiter radius in settings so next time that's what you get. Should orbit do the same?

@rubenp02
Copy link
Contributor Author

I'm confused as to GoTo showing a loiter radius on the map for FW when flying PX4. Where does that radius come from if PX4 doesn't support radius on goto?

That's the orbit mode telemetry. PX4 seems to send that whenever it's on a loiter pattern. That's on master as well, and since APM doesn't support orbit mode at all, I decided to make this PR as kind of a fallback so that it also gets loiter visuals in Guided mode most of the time.

This PR also fixes an overlap between the GoTo and the Orbit visuals in PX4.

@rubenp02
Copy link
Contributor Author

Also since goto stores the loiter radius in settings so next time that's what you get. Should orbit do the same?

I didn't know that and it's undesirable for my use case, but I can add it as an option defaulted to true if you want. It could affect both goto and orbit.

@DonLakeFlyer
Copy link
Contributor

  • Added "Loiter Radius in Forward Flight" setting to the Guided Commands section of the FlyViewSettings to control the default radius.

It would be awesome if this worked a little more like Plan view. Where you can click on things to make them "interactive". Or in other words you click the Orbit or GoTo item in the map and the draggable radius things shows back up to change. Also not sure if the draggable thingies show the actual radius somewhere on the screen so you know how big you are making it? I know much more complicated, but also much nicer.

@rubenp02
Copy link
Contributor Author

  • Added "Loiter Radius in Forward Flight" setting to the Guided Commands section of the FlyViewSettings to control the default radius.

It would be awesome if this worked a little more like Plan view. Where you can click on things to make them "interactive". Or in other words you click the Orbit or GoTo item in the map and the draggable radius things shows back up to change. Also not sure if the draggable thingies show the actual radius somewhere on the screen so you know how big you are making it? I know much more complicated, but also much nicer.

I thought about adding an overlay when dragging that shows the current radius and I agree it would be very nice. As for the other things I'd say they would make it too easy to fat finger a loiter to somewhere you don't want.

@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Mar 12, 2025

For interactive you still would need a confirm slider to make anything actually happen and then you can cancel as well. So confirm slider displays when you go into interactive. Finger futz radius. Confirm/Cancel. I think that's doable with the current guided controller stuff.

@DonLakeFlyer
Copy link
Contributor

For interactive you still would need a confirm slider to make anything actually happen and then you can cancel as well. So confirm slider displays when you go into interactive. Finger futz radius. Confirm/Cancel. I think that's doable with the current guided controller stuff.

That said, all depends on how much effort you want to put into this. It's ok as is.

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.

2 participants