Guided mode improvements

[rubenp02]

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:

• Review Contribution Guidelines.
• Review Code of Conduct.
• I have tested my changes.

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

[rubenp02]

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]

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]

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]

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]

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]

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

[DonLakeFlyer]

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]

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

[rubenp02]

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]

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]

• 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]

• 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]

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]

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.

[DonLakeFlyer]

So where does this stand now? My only last thing was always confirm on goto's.

[rubenp02]

I want to make a few improvements when I have the time, before the weekend most likely Update: I've been busy but I'll do this as soon as possible

[rubenp02]

It’d be great to fix the couple issues I mention in the first comment. Any ideas of what causes the issue when switching to GoTo from Auto, and what would be a good fix for the max distance fail?

[DonLakeFlyer]

• 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.

There is console logging you can turn for GuidedActionsControler which is useful to debug these sorts of things. Can't remember the name. Look in the qml and you'll find it.

[DonLakeFlyer]

• 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).

The mapIndicator is already plumbed through the confirmation function. You could plumb that all the way through executeAction as well. And then make the guided action firmware plugin call return bool for success/fail. Then if fail in the executeAction you can call mapIndicator.cancelled().

[rubenp02]

Just pushed some changes to make the confirmation always required by default and to show the loiter radius while editing it (not sure if it looks "native" enough within QGC - please tell me if it doesn't). I think I'll fix the issues we were talking about and then reopen this. Your idea of "tapping to edit" sounds good but maybe it should be part of a wider effort to improve actions as a whole and make them more consistent?

[DonLakeFlyer]

Your idea of "tapping to edit" sounds good but maybe it should be part of a wider effort to improve actions as a whole and make them more consistent?

yeah, that's all fine

I didn't know that and it's undesirable for my use case

What's the problem with this. Can you explain why it doesn't work for you?

[DonLakeFlyer]

show the loiter radius while editing it (not sure if it looks "native" enough within QGC - please tell me if it doesn't).

Could provide screen shots or a video so I can see what it looks like. Don't quite have time right now to get all this stuff up and running.

[rubenp02]

Could provide screen shots or a video so I can see what it looks like.

Sure, it looks like this:

It has the same size/radius/color/opacity as the map label bubbles, and I decided to place it above the gizmo so it's not covered with the finger on touchscreens.

[rubenp02]

What's the problem with this. Can you explain why it doesn't work for you?

A config of one of our UAVs has an inward pointing camera to continuously point to whatever its circling around, which requires a very large loiter radius. Users of our UAVs often use GoTo commands for navigation but also for this loitering, and then when they use another GoTo command to reposition the aircraft, they might not notice the huge radius and accidentally send it into a collision course with distant terrain/obstacles. We used to have an overlay to set WP_LOITER_RAD directly (the Mission Planner way) and it was a nightmare because that way the radius is persistent even between flights, and it would even affect the Takeoff mode loiter radius. This caused many issues.

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.

This should be pretty easy to do and maybe it could be the way to change the GoTo location without changing the radius. And when a new GoTo is sent it always uses the default radius. It would be great if it also made the altitude slider appear. I would still keep the individual Change Altitude action and the new Change Loiter Radius action this PR adds, though.

[rubenp02]

• 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).

The mapIndicator is already plumbed through the confirmation function. You could plumb that all the way through executeAction as well. And then make the guided action firmware plugin call return bool for success/fail. Then if fail in the executeAction you can call mapIndicator.cancelled().

Maybe I'm wrong but at that point wouldn't mapIndicator.actionConfirmed() have already been called? And thus the indicator icon will have already moved and will just disappear. This can be worked around but ideally it shouldn't happen in the first place.

[DonLakeFlyer]

The guided commands which can fail due to settings or whatnot should return bool up through the call to return sucess/fail. Then that be used to call mapIndicator.cancelled in actionConfirmed. Or maybe there should be a new mapIndicator.actionFailed signal as well. In case something different needs to happen between cancel and fail behavior. Does that work?

[DonLakeFlyer]

On the loiter radius saving, what you say makes me think that in general preserving the loiter radius as a setting across runs, vehicles, flights is probably a bad thing in general. Maybe it should be removed. What do you think?

[DonLakeFlyer]

@rubenp02 Updates? Want to get this in before we lock down for 5.0.

[rubenp02]

On the loiter radius saving, what you say makes me think that in general preserving the loiter radius as a setting across runs, vehicles, flights is probably a bad thing in general. Maybe it should be removed. What do you think?

I completely agree

[rubenp02]

@rubenp02 Updates? Want to get this in before we lock down for 5.0.

I was meaning to fix the two bugs but if this is going into feature lock then it's probably better to merge this before that and fix them in a future PR. To be clear they were existing bugs, not part of this PR.

[DonLakeFlyer]

I completely agree

Can you pull that out then?

As far as feature lock goes I'll hold off until we get your stuff through.

[DonLakeFlyer]

I'm going to merge this and the rest can go into another pull...