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

LUA example Glide into wind #27413

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

Conversation

agising
Copy link

@agising agising commented Jun 28, 2024

As a response to issue #6145.
A LUA script example enables the requested feature and shows yet an other example of how to build LUA-scripts.
Script will do the following:

  • If set up according to instructions, plane will turn into wind via RC-override as soon as FS_LONG has triggered GLIDE (FBWA).

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

I'm not a LUA expert, but this seemed nicely written to me.

I'm a little concerned by your use case.

You appear to have started off by triggering off the AFS system, which has the concept of a dual-failure, where you lose both your GCS and your GPS, which seems entirely sensible.

What this script currently does is take an aircraft which has lost its GCS connection and crashes it. That aircraft might be entirely capable of navigating itself closer to home where it might pick up its GCS again. To give you some indication of why this is important.... if someone trips over an network cable at the GCS (or the GCS software crashes) your aircraft turns into an unguided missile, where it could happily loiter until the problem was resolved.

I think it is possible that the authorities are simply really, really unhappy with the concept of an aircraft flying autonomously with no visibility from the ground control operators, and that the ground-risk where the vehicle is flying is considered negligible. If that is the case then this needs to be clearly stated at the top of the LUA script. I would also point out that the vehicle isn't always out at sea - it has to come home at some point, and it would be rather unfortunate for it to "give up" on final approach to landing as it comes in on the wrong side of a (very!) directional antenna or whatnot.

libraries/AP_Scripting/examples/glide_into_wind.lua Outdated Show resolved Hide resolved
libraries/AP_Scripting/examples/glide_into_wind.lua Outdated Show resolved Hide resolved
libraries/AP_Scripting/examples/glide_into_wind.lua Outdated Show resolved Hide resolved
-- GLIDE_WIND_ENABL is enabled, look for triggers
-- Monitor time since last gcs heartbeat
if last_seen == gcs:last_seen() then
link_lost_for = link_lost_for + looptime
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding times is usually a pattern considered harmful as things go really wonky when the numbers wrap.

It's not a problem here, but a pattern we do try to avoid.

I can't see why you haven't calculated link_lost_for where needed by subtracting gcs:last_seen() from the current time. That does mean finding a time in the same "frame" as gcs:last_seen(), but I think you'll find the current time routines will return that.

Copy link
Author

Choose a reason for hiding this comment

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

I agree that adding up time like this builds bigger and bigger errors and in general is not a good solution.

The reason is that I cannot properly find the time, it baffles me but I'm not the only one. There is GNSS dependent solution to extract time from the GNSS signals, but does not make sense either.
The highest resolution of os.time() is seconds.

For this use, I think it is ok. As a general solution for checking how much time has elapsed it is not a good practice.

RC_ROLL:set_override(1500)
send_to_gcs(_INFO, 'LUA: Gliding into wind, no more steering')
end
-- Do not override again until link has been recovered, set flag to false
Copy link
Contributor

Choose a reason for hiding this comment

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

You haven't explained the reason for this anywhere, in the PR or (preferably) in a comment at the top of the script.

Copy link
Author

@agising agising Sep 7, 2024

Choose a reason for hiding this comment

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

You are right, I shall explain that and put it in the top comment of the script. I will commit an updated version later today.
Except that I don't like RC-override in general, there are 2-3 reasons:

  1. If there is RC pilot in range, the pilot can control pitch and rudder but not roll as long as the channel is overridden. This is confusing and can be stressful and has the potential of locking up the mind of the pilot.
  2. At touch down we want the roll attitude to be level.
  3. (I'm not 100% sure this is actually a problem/risk) Since the plane glides in FBWA with 0 throttle, it is likely close to stall speed after some time. Close to stall speed the roll response is smaller which could lead to bigger control error and hence bigger control outputs. During low speed this would further slow down the plane and also risk stalling the plane.

There is a problem in the current implementation:
If the RC pilot wants to cancel the ongoing glide into wind before the script says 'no more steering' and disables the flag override_enable, he/she flips flight mode and gets full control, ok. Later when the RC-pilot returns to FBWA, and the GCS link has still not recovered, the script again will try to steer into wind.
I will present an update version later today where this is adressed.

edit: typo

Copy link
Contributor

Choose a reason for hiding this comment

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

Except that I don't like RC-override in general, there are 2-3 reasons:

So now I'm curious.

An alternate formulation of this script would have all of the same detection logic for entering the glide-into-wind failsafe logic, but would avoid using RC overrides to achieve the desired heading.

Instead, it would change the vehicle into GUIDED mode and use some mavlink guided-mode binding to achieve the desired heading (vehicle:update_target_location if nothing else)

This would have several advantages

  • it makes it clear to the poor sod holding the RC transmitter that they're in an abnormal operating condition via the mode-change announcement.
  • no RC overrides going on!
  • recovery is via a mode-change to FBWA, not via a change to some other mode and then back again.
  • simpler logic when in the failsafe condition

Comment on lines 267 to 268
-- Parse flight mode
function parse_flight_mode(flight_mode_num)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a little strange; you could just do the comparison against the mode_XXX value instead of converting to a string and then comparing...

Copy link
Author

Choose a reason for hiding this comment

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

Completely agree! Will update tonight

-- Init parameters
FS_GCS_ENABL = bind_param('FS_GCS_ENABL') -- Is set to 1 if GCS lol should trigger FS after FS_LONG_TIMEOUT
FS_LONG_TIMEOUT = bind_param('FS_LONG_TIMEOUT') -- FS long timeout in seconds
AFS_GCS_TIMEOUT = bind_param('AFS_GCS_TIMEOUT') -- Doc: The time (in seconds) of persistent data link loss before GCS failsafe occurs. Not sure though
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unused, and your instructions wouldn't make this parameter visible to the user.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! In an earlier version of the script I used it.. I will evaluate if it should be used or removed. Update to come later tonight.

@timtuxworth
Copy link
Contributor

timtuxworth commented Sep 5, 2024

I'm not a LUA expert, but this seemed nicely written to me.

I'm a little concerned by your use case.

You appear to have started off by triggering off the AFS system, which has the concept of a dual-failure, where you lose both your GCS and your GPS, which seems entirely sensible.

What this script currently does is take an aircraft which has lost its GCS connection and crashes it.

I think it is possible that the authorities are simply really, really unhappy with the concept of an aircraft flying autonomously with no visibility from the ground control operators, and that the ground-risk where the vehicle is flying is considered negligible.

In Canada, MAAC (Model Aircraft Association of Canada) has the following guidance when flying in Controlled Airspace (which is different from general flying, but still an important use case for some flyers):

_Appendix D – MAAC RPAS Flight Termination Options
As a matter of MAAC policy, all RPAS operated in controlled airspace must have an operable “flight termination” system or design criteria that can reasonably be expected to terminate the flight with minimal delay in the event of a control link failure. _

IMO the authorities (this was negotiated by MAAC with Transport Canada), are more concerned with air risk than ground risk.

@LupusTheCanine
Copy link

In Canada, MAAC (Model Aircraft Association of Canada) has the following guidance when flying in Controlled Airspace (which is different from general flying, but still an important use case for some flyers):

_Appendix D – MAAC RPAS Flight Termination Options As a matter of MAAC policy, all RPAS operated in controlled airspace must have an operable “flight termination” system or design criteria that can reasonably be expected to terminate the flight with minimal delay in the event of a control link failure. _

IMO the authorities (this was negotiated by MAAC with Transport Canada), are more concerned with air risk than ground risk.

Terminating flight with minimal delay would be achieved by means of diving at -90°.It would absolutely minimize air risk 😀.

That it may go through somebody's roof or head with significant energy apparently doesn't concern Transport Canada as much 😀.

agising and others added 3 commits September 7, 2024 14:10
@agising
Copy link
Author

agising commented Sep 7, 2024

I'm a little concerned by your use case.

You appear to have started off by triggering off the AFS system, which has the concept of a dual-failure, where you lose both your GCS and your GPS, which seems entirely sensible.

What this script currently does is take an aircraft which has lost its GCS connection and crashes it. That aircraft might be entirely capable of navigating itself closer to home where it might pick up its GCS again. To give you some indication of why this is important.... if someone trips over an network cable at the GCS (or the GCS software crashes) your aircraft turns into an unguided missile, where it could happily loiter until the problem was resolved.

I think it is possible that the authorities are simply really, really unhappy with the concept of an aircraft flying autonomously with no visibility from the ground control operators, and that the ground-risk where the vehicle is flying is considered negligible. If that is the case then this needs to be clearly stated at the top of the LUA script. I would also point out that the vehicle isn't always out at sea - it has to come home at some point, and it would be rather unfortunate for it to "give up" on final approach to landing as it comes in on the wrong side of a (very!) directional antenna or whatnot.

I've understood that for many this use case is somewhat disturbing. But let's look at it the script as a small improvement of the already implemented, accepted and used FS_LONG_ACTION called GLIDE, implemented by means of FBWA throttle=0.

This script improves this behaviour by adjusting the heading towards wind with the purpose of minimising risk for people on ground and risk for damage on the plane by means of decreasing the ground speed. Gliding into wind also increases the (small) chances of not colliding with threes and stuff since the approach will be steeper relative to ground.

And apparently some kind of termination is required/desired from Canadian authorities at least.
In the specific use case this was developed for, we are likely to not have enough battery to return to home, we need to stay on location for as long as possible. We are also not allowed to continue flight for long without a functional control link.

@agising
Copy link
Author

agising commented Sep 7, 2024

Thank you for looking into this and providing feedback. I've accepted change requests and tried to follow your recommendations. I also corrected a case where the script did not cancel properly when the pilot toggled modes.
I replaced the nested if statements with a state machine that also made pilot take-over much more robust and the script easier to follow.
I've tried to explain both why Glide is a viable fail safe, and that this script can enhance that fail safe action by decreasing ground speed by steering into wind.
I argued for only steering until heading is into wind and then stop steering. I also suggest a setting that will steer the plane all the way down, although I do not recommend it (wrote that in the comment too).

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

This should be a single commit, titled something along hte lines of `AP_Scripting: add glide-into-wind example.

Please don't take my musing on RC as "must be done" - just suggesting ways this could be done better. I loathe RC overrides being used by mechanical systems :-)

I expect this current implementation is well-tested?

libraries/AP_Scripting/examples/glide_into_wind.lua Outdated Show resolved Hide resolved
libraries/AP_Scripting/examples/glide_into_wind.lua Outdated Show resolved Hide resolved
RC_ROLL:set_override(1500)
send_to_gcs(_INFO, 'LUA: Gliding into wind, no more steering')
end
-- Do not override again until link has been recovered, set flag to false
Copy link
Contributor

Choose a reason for hiding this comment

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

Except that I don't like RC-override in general, there are 2-3 reasons:

So now I'm curious.

An alternate formulation of this script would have all of the same detection logic for entering the glide-into-wind failsafe logic, but would avoid using RC overrides to achieve the desired heading.

Instead, it would change the vehicle into GUIDED mode and use some mavlink guided-mode binding to achieve the desired heading (vehicle:update_target_location if nothing else)

This would have several advantages

  • it makes it clear to the poor sod holding the RC transmitter that they're in an abnormal operating condition via the mode-change announcement.
  • no RC overrides going on!
  • recovery is via a mode-change to FBWA, not via a change to some other mode and then back again.
  • simpler logic when in the failsafe condition

@peterbarker peterbarker self-assigned this Sep 25, 2024
@agising agising requested a review from peterbarker November 20, 2024 08:11
@agising
Copy link
Author

agising commented Nov 20, 2024

Sorry, I did not mean to request a review yet. I'm new this git workflow. Working on the code today

@agising
Copy link
Author

agising commented Nov 21, 2024

@peterbarker, now I'm ready for review :)
I agree with you, RC override is not nice in general. But I also avoid having scripts settings flight modes if possible because there is always a risk that the script takes control when you don't want it to if you do not manage to handle all corner cases.
I'm grateful for your comments and your arguing. I'm also more satisfied with the new implementation using set_target_location instead of RC override. It feels more clean and robust.
It works very well in SITL, I have not been able to test this version in flight though.

The key safety things that must always work in this script are:

  • That the operator should be able to recover by sending a new goto point
  • That the operator or RC pilot should be able to recover by changing flight mode
  • Eventually, the drone must GLIDE to ground if link is lost (given that the parameters are set for it) - if not a RC recovers the plane.

The change from RC override to target location brought a lot of changes. These are the main changes:

  • I did rewrite and replace the RC override with ´vehicle:set_target_location´. This could not at all be done line by line.
  • I did remove one parameter not needed any more.
  • I added logic to detect if the operator sent an other target location
  • I added protection to not stay in GUIDED if not reaching the target heading within 15'000ms
  • Since this method is using GNSS, I'm skipping adjusting heading if I get no location data.
  • Typos and misspellings

Any comments appreciated!

@LupusTheCanine
Copy link

IMHO heading change is a much more robust way of handling failsafe state as it depends on the minimal set of capabilities needed.

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.

4 participants