Rally Points: enable rally points always, independent of RTL_TYPE set…

[sfuhrer]

Describe problem solved by this pull request
This PR is a follow-up of #12696, where safe landing points (aka rally points) where enabled.

With the above mentioned PR, safe landing points were only considered if RTL_TYPE is set to 3 (which is a new rtl_type). After giving it some more thoughts and getting feedback from others I want to change that with this PR: safe landing points are now always considered and chosen when closer then the default option (see below), plus with RTL_TYPE=3 home and mission landing are always considered. In more detail for the different RTL settings:

• RTL_TYPE = 0 (home): vehicle lands whatever is closest: home or nearest safe landing point (if set)
• RTL_TYPE = 1(landing): vehicle lands whatever is closest: mission landing( if set) or nearest safe landing point (if set) or home. It only considers home if mission landing is not set.
• (RTL_TYPE = 2(reverse): same as 1, but now if mission landing is chosen, it follows the mission path in reverse)
• RTL_TYPE = 3 (home/landing): vehicle lands whatever is closest: home, mission landing (if set) or nearest safe landing point (if set). Other than 1 and 2 it always considers home and mission landing (if set), as well as the potential safe landing points.

EDIT: RTL_TYPE=2 is now unchanged from current master, so it won't consider rally points for the rtl type

For me tpye 3 makes most sense for big missions (potentially VTOL/ BVLOS), where the start and landing point are not on the same place.

Test data / coverage
SITL tested.

[sfuhrer]

@hamishwillee FYI

[hamishwillee]

@sfuhrer This does not quite capture our discussion.
My general point was that home and rally points are synonymous - if you've defined a rally point then then you want it to be considered alongside home. However types 1, 2 exist to say "I still prefer a mission landing pattern if one is defined" and that should be obeyed first.

So basically the desired behaviour is exactly the same as currently, but wherever it says "goto/fallback to home" it should say "goto/fallback to closest of home or rally point".

It should look like this - with change in italic w.r.t. current definitions:

• 0: Return to closest safe point (home or rally point) via direct path
• 1: Return to a planned mission landing, if available, via direct path, else return to to the closest safe point (home or rally point) via direct path
• 2: Return to a planned mission landing, if available, using the mission path (fast-forward), else fast-reverse the mission path and then fly to the closest safe point (home or rally point). If no mission is defined fly directly to the closest safe point (home or rally point).
• 3: Return via direct path to closest destination: home, start of mission landing pattern or safe point. If the destination is a mission landing pattern, follow the pattern to land.

[sfuhrer]

@sfuhrer This does not quite capture our discussion.
My general point was that home and rally points are synonymous - if you've defined a rally point then then you want it to be considered alongside home. However types 1, 2 exist to say "I still prefer a mission landing pattern if one is defined" and that should be obeyed first.

So basically the desired behaviour is exactly the same as currently, but wherever it says "goto/fallback to home" it should say "goto/fallback to closest of home or rally point".

It should look like this - with change in italic w.r.t. current definitions:

* 0: Return _to closest safe point (home or rally point)_ via direct path

* 1: Return to a planned mission landing, if available, via direct path, else return to _to the closest safe point (home or rally point)_ via direct path

* 2: Return to a planned mission landing, if available, using the mission path _(fast-forward)_, else fast-reverse the mission path and then fly _to the closest safe point (home or rally point). If no mission is defined fly directly to the closest safe point (home or rally point)_.

* 3: Return via direct path to closest destination: home, _start of_ mission landing pattern or safe point. _If the destination is a mission landing pattern, follow the pattern to land._

From my perspective it makes more sense to always consider safe landing points, independent of RTL_TYPE (I know, changed my mind there lately:)). Or what would be the reason that you want to discard them for types 1 and 2 when the mission contains a landing? If you don't want to land on safe landing points then just don't put them in. And if you keep the RTL_TYPE to 1 or 2 and don't put in a mission landing, then you'd have the same behavior as you propose (lands at home or safe landing point, whatever is closest).

Let's ask for a second opinion: @julianoes @DanielePettenuzzo @bkueng what are your thoughts?

[julianoes]

@sfuhrer is there a point of having a rally point if it is not used by RTL? So, is there a mechanism to use a rally point through a command instead of RTL?

From a terminology point of view I don't like the naming RTL_TYPE and the fact that RTL would not go back to launch (Return to Launch) but rather somewhere else. I guess this one boils down to RTL meaning Return To Launch and not Return to Land.

[sfuhrer]

@sfuhrer is there a point of having a rally point if it is not used by RTL? So, is there a mechanism to use a rally point through a command instead of RTL?

From a terminology point of view I don't like the naming RTL_TYPE and the fact that RTL would not go back to launch (Return to Launch) but rather somewhere else. I guess this one boils down to RTL meaning Return To Launch and not Return to Land.

@julianoes no, there is now way to use it beside the RTL.

Agree the this is no longer a proper RTL (launch), but just want to point out that it currently is neither (you already have the option of choosing the mission landing instead of home now).
The question is of course if that affects this PR (and the current implementation of rally points), or if we could get this in like it is now (with the meaning Return to Land).

[Antiheavy]

From a terminology point of view I don't like the naming RTL_TYPE and the fact that RTL would not go back to launch (Return to Launch) but rather somewhere else. I guess this one boils down to RTL meaning Return To Launch and not Return to Land.

This terminology around Return Mode has been a debate in the past. The variables are named RTL which is refered to in the code in different places as return to launch or return to land. There is also the concept of return to home which is what often happens in reality. The solution is to always refer to the various return modes as "Return Mode" in documentation. This way there is flexibility to what the behavior is. By only using the word "Return" and no other words, it can mean anything such as Return to Launch/Land/Home/Ground/SafePoint/RallyPoint, etc

[Antiheavy]

For me tpye 3 makes most sense for big missions (potentially VTOL/ BVLOS), where the start and landing point are not on the same place.

Please keep in mind fixed wing vehicles. For fixed wings, takeoff and landing are almost always different locations (different enough where it matters to not land into a tree/fence/automobile/person!)

[julianoes]

@Antiheavy thanks for your comments. I agree about the naming, and hence I don't like the param name RTL_TYPE but I'm probably too late to change that.
Anyway, do you therefore agree with the proposed change?

[bkueng]

Looks good from my side. Do we need to update the param description as well?

[sfuhrer]

Looks good from my side. Do we need to update the param description as well?

Yes, good point. Done that now, please review. Also @hamishwillee please.

[hamishwillee]

1. What is the benefit of ignoring home as a possible landing destination? ie. why would you choose to go to a rally point that was defined but not to home? Also the wording little odd it implies "home is not a safe point".
2. The path taken is important - ie direct.
3. I like the term "mission landing pattern"

So depending on your answer to 1 - one of:

• @value 1 Return to closest safe point (start of mission landing pattern, rally point, or home), via direct path. If safe point is a landing pattern then land.

• @value 1 Return to closest safe point other than home (mission landing pattern or rally point), via direct path. If no mission landing or rally points are defined return home via direct path.

[hamishwillee]

Or to put it another way, why do we need both valy 1 and value 2?

[sfuhrer]

Home should only be considered for value 1 if no landing or rally point is defined because with a VTOL/FW it's safer. It the user doesn't set a rally point, then 1 results in the same behavior as is currently implemented.

Your wording sounds better, changed that (for value 1)

[sfuhrer]

Or to put it another way, why do we need both valy 1 and value 2?

You mean between TYPE 1 and 2? That it follows the mission, either forward (mission landing) or inverse (home).

[hamishwillee]

Same question as above for home vs rally points. Will affect how this is worded so will wait on your response.

[Antiheavy]

Suggest:
@value 2 Same as @value 1, but return via mission path (for return to mission landing), via the reverse mission path (for return to home) or via direct path (for return to rally point).

[hamishwillee]

Well this is a little complicated, because what is the safe point for the mission case? The mission landing pattern or the closest waypoint? Also in current implementation behaviour differs if you're on a mission already or just flying around.
My assumption is that if you're in a mission you will always want to use this path, but perhaps that is wrong?

I can help write this, but can you confirm the assumptions?

1. On a mission then fast forward to end or reverse to back, irrespective of rally points?
2. Not on a mission then go to closest of rally points or nearest waypoint.
3. If on mission path do fast forward or reverse depending on whether mission landing
4. If no rally points or mission defined, go to home via direct path.
5. If I reverse mission path, currently I then fly to home after takeoff position. Should I go to a rally point if it is closer than home at this point?

[Antiheavy]

I agree about the naming, and hence I don't like the param name RTL_TYPE but I'm probably too late to change that.

This confusion is bound to come up again in the future. However, it would be a ton of work and probably impact lots of people to go back and change all "RTL_" to "RTN_" or whatever the new param short hand would be... also many code comments should be updated to be more specific on the true behavior (return to home, return to landing pattern, return to ....).... this work is not in the scope of this PR I think.

[Antiheavy]

Anyway, do you therefore agree with the proposed change?

I've had to think about it for a while, but I still have concerns on how this impacts fixed wing behavior and generally impacts anyone choosing RTL_TYPE 1 and possibly RTL_TYPE 2. Sorry for the long post, here are my concerns:

Summary: I'm not sure this change is appropriate for RTL_TYPE 1 and RTL_TYPE 2

concerns for RTL_TYPE 1:
RTL_TYPE 1 adds a requirement to mission feasibility checker for there to be a landing pattern in the mission. This is intended so that the vehicle (e.g. a fixed wing or VTOL) will follow a very specific Return Mode behavior and land using the pre-planned landing pattern (this is how auto-landing fixed wings must work). The fact that PX4 "falls back" to the standard Return to Home behavior if a landing pattern is missing is really intended as a last resort and in-truth is kind of a "hack" to cover for a fundamental flaw in PX4 documented here: #12473 Allowing Rally Points to function with RTL_TYPE 1 basically defeats the purpose of RTL_TYPE 1. Especially given my concerns with fixed wing Rally Points described below.

concerns for RTL_TYPE 2:
RTL_TYPE 2 is all about following the mission path. This may typically be used in the case of flying in hills/mountains or places with obstacles or no-fly areas. All the same concerns for RTL_TYPE 1 apply here except that a landing pattern is not required by mission feasibility checker. @acfloria developed RTL_TYPE 2 so he may have other thoughts here. In many scenario's I can envision, Safe Points would essentially override the intent of RTL_TYPE 2, although I can see where someone may want to use them if placed in very careful locations along the mission path.

concerns for fixed wing Rally Points:
Currently there is no safe option for fixed wings to auto-land at the Home point or at Rally Points. For this reason, the fixed wing default RTL_LAND_DELAY parameter allows the vehicle to loiter around Home indefinitely (ref: https://github.com/PX4/Firmware/blob/master/ROMFS/px4fmu_common/init.d/rc.fw_defaults#L36). I assume the same RTL_LAND_DELAY behavior occurs at a Rally Point and a fixed wing vehicle would just loiter indefinitely (maybe I shouldn't assume...)? This leads to the potentially inconsistent behavior where the vehicle might actually land using the landing pattern or it might just loiter around Home or the Rally Point(s). If fixed wing users set RTL_LAND_DELAY to an actual time, the behavior is the vehicle lands in a seemingly random direction, usually very far away from the Home or Rally Point which defeats the purpose and is unsafe anyway (except when Rally Points are used as "ditch points" which is a valid use case).

[sfuhrer]

@hamishwillee @Antiheavy I've pushed a new version, now it doesn't consider rally points for RTL_TYPE=2. Figured out how it was implemented before didn't really go well with this RTL_TYPE, and as you two are anyway more reluctant to use rally points in this case I thought I just as well can take it out. Actually I don't see why somebody would want to set rally points if he sets this option, so I agree with you.

So for RTL_TYPE =2 this PR doesn't change anything. I've thus also reverted the description.

[sfuhrer]

Anyway, do you therefore agree with the proposed change?

I've had to think about it for a while, but I still have concerns on how this impacts fixed wing behavior and generally impacts anyone choosing RTL_TYPE 1 and possibly RTL_TYPE 2. Sorry for the long post, here are my concerns:

Summary: I'm not sure this change is appropriate for RTL_TYPE 1 and RTL_TYPE 2

@Antiheavy I understand your concerns for FW. At the same time I think it is the users responsibility to know what it means if he sets a rally point. I don't see how somebody just sets a safe landing point somewhere without need and without being aware what it implies. And if he doesn't set a safe landing point then nothing changes with this PR.
The reason that I'm in favor of enabling safe landing points for TYPE=1 is that if we don't, then we don't cover the following case:

• VTOL
• big mission with the aircraft flying multiple km away from home
• same start and landing point
• user defines landing pattern
• user defines safe landing points some distance from home, such that the vehicle will land there if it RTLs (as it can't reach home in MC mode)
• user still wants to use the mission landing if close to home, as landing this way is safer than at home (and this is what separates this RTL_TYPE from TYPE=3, as there it also considers home as a safe landing point)

I see that for FW this is probably not as urgent, as there I anyway doubt the usefulness of rally point. But then again, the user should be aware of that and not just set them.

About your other question about the action of a FW at a safe landing point: it is the same as at home, so it will loiter.

[hamishwillee]

@sfuhrer I'm OK with this now. The rules are clear so someone can do the right thing if they want. It is a bit harder to document than they would have been with my approach :-).

I'm still not convinced on the behaviour because types 1, 2 were created on the assumption that for FW you ALWAYS want to do a mission landing. That's really always, even if you you're really close to home (or a rally point). But as long as the rules are clear it is not broken.

About your other question about the action of a FW at a safe landing point: it is the same as at home, so it will loiter.

To be annoyingly pedantic, it will do whatever the return parameters say, which by default for FW is loiter.

[sfuhrer]

@sfuhrer I'm OK with this now. The rules are clear so someone can do the right thing if they want. It is a bit harder to document than they would have been with my approach :-).

I'm still not convinced on the behaviour because types 1, 2 were created on the assumption that for FW you ALWAYS want to do a mission landing. That's really always, even if you you're really close to home (or a rally point). But as long as the rules are clear it is not broken.

About your other question about the action of a FW at a safe landing point: it is the same as at home, so it will loiter.

To be annoyingly pedantic, it will do whatever the return parameters say, which by default for FW is loiter.

Okay thanks. What's your opinion @Antiheavy ? Could you live with how it would be done with this PR? Otherwise we would need to either change the behavior depending on whether it is a VTOL or FW, or then add another parameter (or yet another other RTL_TYPE). But this then gets really messy from my point of view.

[sfuhrer]

@bkueng @julianoes this is ready to be merged from my perspective. Let's get is in and then think about how to reduce the dm_reads necessary (#13536).

[bkueng]

2 suggestions, feel free to merge.