ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
10.23k stars 16.82k forks source link

AP 3.4: TECS target speed should never cause pitch down after ARSPD_FBW_MIN has been reached #3357

Open marcmerlin opened 8 years ago

marcmerlin commented 8 years ago

short video clip shows what's going on: https://www.youtube.com/watch?v=OeT0yXTfnlg

ARSPD_FBW_MIN is 14 or 50kph, stall prevention is on (and should have been active in FBWA) Video shows I'm flying in FBWA at 46kph, which is a bit weird since stall prevention should prevent that? (arguably bug #1)

At the 6sec offset, speed is 44kph (real stall speed is below that, so plane flies fine). I hit the altitude deck of 70m, so GUIDED takes over. So far so good, except maybe my AS too low for stall prevention. In exactly 1 second, GUIDED brings the speed back to 50kph with an altitude of 66m That's still ok. But then, it goes downhill fast... GUIDED banks pretty hard, keeps descending and getting more airspeed, altitude plummets, AS climbs to 80kph. I take over (remove GUIDED) when altitude reaches 26m (40m drop in a few mere seconds), and barely save the plane from crashing (that's when the OSD disappears due to an unrelated OSD bug)

This was quite scary, GUIDED that's supposed to help me not hit the ground, almost drove me into it at high speed (80kph) which would have shattered the plane to pieces.

Droneshare still busted after almost 2 months, so here's the datalfash: http://marc.merlins.org/tmp/6_GUIDED_stall_prevention_near_crash.BIN

magicrub commented 8 years ago

thanks for the report.

Questions:

marcmerlin commented 8 years ago

Yes, there is an AS sensor (3DR one, digital, pin 65). The params are in the dataflash. I can see them if I open in apmplanner. Is there an easy way to open the dataflash and spit out just the params in a text file?

marcmerlin commented 8 years ago

I forgot to add that GUIDED has worked correctly all other times. It takes over every time I reach 70m and flies back up. This is the only time it's lost so much altitude and apparently was on its way to a high speed crash. At least I'm happy I had the altitude deck at 70m and not 50m :)

WickedShell commented 8 years ago

Easiest way to extract params if you have the pymavtools installed is "mavparams.py FOO.BIN > FOO_params.txt" or any other file extension you prefer

On Sun, Dec 20, 2015 at 11:15 AM, Marc MERLIN notifications@github.com wrote:

I forgot to add that GUIDED has worked correctly all other times. It takes over every time I reach 70m and flies back up. This is the only time it's lost so much altitude and apparently was on its way to a high speed crash. At least I'm happy I had the altitude deck at 70m and not 50m :)

— Reply to this email directly or view it on GitHub https://github.com/diydrones/ardupilot/issues/3357#issuecomment-166142132 .

magicrub commented 8 years ago

yeah, I can grab the params from the bin, I was being lazy. I'll take a look later.

On Sun, Dec 20, 2015 at 10:27 AM, WickedShell notifications@github.com wrote:

Easiest way to extract params if you have the pymavtools installed is "mavparams.py FOO.BIN > FOO_params.txt" or any other file extension you prefer

On Sun, Dec 20, 2015 at 11:15 AM, Marc MERLIN notifications@github.com wrote:

I forgot to add that GUIDED has worked correctly all other times. It takes over every time I reach 70m and flies back up. This is the only time it's lost so much altitude and apparently was on its way to a high speed crash. At least I'm happy I had the altitude deck at 70m and not 50m :)

— Reply to this email directly or view it on GitHub < https://github.com/diydrones/ardupilot/issues/3357#issuecomment-166142132> .

— Reply to this email directly or view it on GitHub https://github.com/diydrones/ardupilot/issues/3357#issuecomment-166144837 .

marcmerlin commented 8 years ago

No worries, happy to make it easier for you bfg6_3.4.param.txt

marcmerlin commented 8 years ago

Just to be clear, this is what you needed, right?

magicrub commented 8 years ago

Yes, and time. Time is a precious resource. On Dec 29, 2015 3:05 AM, "Marc MERLIN" notifications@github.com wrote:

Just to be clear, this is what you needed, right?

— Reply to this email directly or view it on GitHub https://github.com/diydrones/ardupilot/issues/3357#issuecomment-167766127 .

marcmerlin commented 8 years ago

As per @tridge's analysis of my mavlink, it looks like throttle nudge and 60% power at the time geofence kicked in, caused guided mode to pitch down big time to pick up a target airspeed of something like 37m/s, which explains why the plane flew towards the ground towards a crash.

As a side note, 37m/s is 133kph which my plane will not ever be able to reach, so such a target airspeed with throttle nudge will cause the plane to dive towards the ground without ever reaching that speed.

I do share blame for haivng ARSPED_FBW_MAX at 40, I should set it at 30, but at the same time, throttle nudge causing a target airspeed of 37m/s with a ARSPED_FBW_MIN of 14, sounds excessive :)

gmorph commented 8 years ago

ARMING_CHECK was 2030 according to your param file - that's weird. Not related to the issue - just weird. The THROTTLE_NUDGE allows you to go up to ARSPD_FBW_MAX which in your case is 40m/s. So 37m/s is perfectly possible if you had your throttle a long way up.

marcmerlin commented 8 years ago

@gmorph for ARMING_CHECK, see https://github.com/diydrones/ardupilot/issues/3291 Without 2030, my plane will often fail to arm and give: DEBUG 2015-10-30T08:54:46.593 APM: Text Message rx'd "PreArm: GPS horiz error 5.4" DEBUG 2015-10-30T08:58:00.927 APM: Text Message rx'd "PreArm: GPS speed error 1.5" Also: http://ardupilot.com/forum/viewtopic.php?f=113&t=13999

Obviously it's not good in some way, but I never got a solution for it.

For THROTTLE_NUDGE, @tridge and I discussed this last night, and with throttle at 60%, it's only a small nudge over 50%, so we still thought that having a target speed of 37 out of 40m/s was very excessive. Also, target airspeed is something to attain when the vehicle is flying level, not something to get by sacrificing altitude and crashing into the ground, especially when a geofence has triggered and prime directive should be to climb back up to the RTL altitude. Only once that altitude has been reached would it be reasonable to try and reach some target airspeed.

In other words, only trying to reach the FBW_MIN airspeed should ever be a reason to pitch down, since pitching down is usually better than stalling out of the sky.

Sounds fair?

marcmerlin commented 8 years ago

As a side note, for ARSPD_FBW_MAX, I would have been happy to have this set to 0, or undefined (can you do this)? I didn't want the flight controller to limit my max speed due to anything else than throttle_max and what my motor/prop combination would do. http://plane.ardupilot.com/wiki/arduplane-parameters/#maximum_airspeed_arduplanearspd_fbw_max doesn't make it clear if I can set it to 0, so in the meantime I set it to 40, which was high enough to ensure it would not limit my speed

marcmerlin commented 8 years ago

Summary of all potential issues: 1) I cannot think of a good reason to ever pitch down to reach a target airspeed (in any mode, not just guided) unless we're below ARSPD_FBW_MIN and stall prevention is enabled. @tridge verified that the AP3.4 code did pitch down towards the ground and would have hit it to reach a desired airspeed due to throttle nudge.

2) With ARSPED_FBW_MIN at 14 and ARSPED_FBW_MAX at 40, getting a target airspeed of 37m/s because of throttle nudge and throttle only 10% over center, seems off somewhat

3) Document whether you can have ARSPD_FBW_MAX be undefined so that you have a minimum airspeed, but not a maximum one, and potentially support no max airspeed if that's reasonable.

Thanks :)

WickedShell commented 8 years ago

1) If you are evenly weighing speed and altitude or heavily weighing airspeed then thats a totally valid solution as far as I can see.

3) If ARSPD_FBW_MAX is undefined how would you know how much to bump the airspeed by if you have throttle nudging enabled?

marcmerlin commented 8 years ago

@WickedShell fair point on #3, in the happy little world in my head :) it would just vary throttle towards the max throttle limit instead of working based on airspeed. This may not be desirable in all cases maybe, but for me, "more throttle" = "more amps to the motor" and whatever airspeed I get out of it, is what I get. Having the user know what max airspeed should be doesn't seem super desirable in my opinion because it really depends on a variety of factors, which could even vary on what battery you put in for that flight, or the fact that you just changed the prop to a slightly different one after the previous crash :) This is a separate issue from this one though. If you think it's worth discussing, I can split it into a separate issue. Things may just also be as they should, and 3) should be ignored, which would be fine if that's the case.

magicrub commented 8 years ago

Bottom line was ARSPED_FBW_MAX = 40 was un-achievable by your aircraft and is thus configured wrong. Simply changing that to an appropriate value for your airframe solves the problem and lets the flight controller control your aircraft properly.

On Tue, Feb 2, 2016 at 2:31 PM, Marc MERLIN notifications@github.com wrote:

@WickedShell https://github.com/WickedShell fair point on #3 https://github.com/diydrones/ardupilot/issues/3, in the happy little world in my head :) it would just vary throttle towards the max throttle limit instead of working based on airspeed. This may not be desirable in all cases maybe, but for me, "more throttle" = "more amps to the motor" and whatever airspeed I get out of it, is what I get. Having the user know what max airspeed should be doesn't seem super desirable in my opinion because it really depends on a variety of factors, which could even vary on what battery you put in for that flight, or the fact that you just changed the prop to a slightly different one after the previous crash :) This is a separate issue from this one though. If you think it's worth discussing, I can split it into a separate issue. Things may just also be as they should, and 3) should be ignored, which would be fine if that's the case.

— Reply to this email directly or view it on GitHub https://github.com/diydrones/ardupilot/issues/3357#issuecomment-178864999 .

marcmerlin commented 8 years ago

@magicrub, sorry I can't agree here :) See 1) and 2) again above. 2) says that having a target airspeed of 37m/s with only a 10% throttle nudge seems wrong 1) says clearly that I cannot see a good reason to ever pitch down to achieve an airspeed other than above stall. Altitude is life, diving towards the ground to reach some TECS calculated airspeed that isn't necessary for flight, cannot be the right course of action (arguably, ever), regardless of user input. Having this happen while in a low altitude fence trigger is just an even worse situation, but it's just negative icing on the cake

WickedShell commented 8 years ago

1) Airspeed is life, altitude is insurance, only in helicopters is altitude life. :)

On Tue, Feb 2, 2016 at 3:53 PM, Marc MERLIN notifications@github.com wrote:

@magicrub https://github.com/magicrub, sorry I can't agree here :) See 1) and 2) again above. 2) says that having a target airspeed of 37m/s with only a 10% throttle nudge seems wrong 1) says clearly that I cannot see a good reason to ever pitch down to achieve an airspeed other than above stall. Altitude is life, diving towards the ground to reach some TECS calculated airspeed that isn't necessary for flight, cannot be the right course of action (arguably, ever), regardless of user input. Having this happen while in a low altitude fence trigger is just an even worse situation, but that's negative icing on the cake

— Reply to this email directly or view it on GitHub https://github.com/diydrones/ardupilot/issues/3357#issuecomment-178873435 .

magicrub commented 8 years ago

You do have a good point about not pitching down unless to prevent a stall

On Tue, Feb 2, 2016 at 2:54 PM, Marc MERLIN notifications@github.com wrote:

@magicrub https://github.com/magicrub, sorry I can't agree here :) See 1) and 2) again above. 2) says that having a target airspeed of 37m/s with only a 10% throttle nudge seems wrong 1) says clearly that I cannot see a good reason to ever pitch down to achieve an airspeed other than above stall. Altitude is life, diving towards the ground to reach some TECS calculated airspeed that isn't necessary for flight, cannot be the right course of action (arguably, ever), regardless of user input. Having this happen while in a low altitude fence trigger is just an even worse situation, but that's negative icing on the cake

— Reply to this email directly or view it on GitHub https://github.com/diydrones/ardupilot/issues/3357#issuecomment-178873435 .

magicrub commented 8 years ago

I just duplicated this issue with https://github.com/diydrones/ardupilot/issues/3582 but specific to the one task

marcmerlin commented 8 years ago

Well, unless I analysed the logs wrong, this bug now crashed and destroyed my plane :( http://ardupilot.com/forum/viewtopic.php?f=115&t=15302 (in APM 3.5) Sigh, weeks of building gone...

WickedShell commented 8 years ago

(Replying here because I don't have a forum account). Looking at that my first thought is that pitching down was absolutely correct, and you're pitch loop looks like it never stabilizes, and is not good at handling a large upset in demand. The airspeed gets as low as 7.5 m/s which is far outside of the expected/tolerated range there so it was asking for 12 m/s. As soon as it got that it started looking to kill off the descent and for a climb (admittedly not as fast as I'd have expected it to).

Also worth noting you had more throttle available in FBWA then the autopilot ever had during the crash sequence. (I'm not terribly experienced with STICK_MIXING but the neutral input with FBWA mixing seems like that wouldn't be helping with the demands).

I'm not the best log analysis source so I'll bow to others on that aspect, but once it got the desired airspeed it started trying to fix it as far as I can see.

marcmerlin commented 8 years ago

@WickedShell, the plane was actually already pitching down when I was in MANU since I was coming in for landing. If GUIDED wasn't using full throttle, that's just plain bad. Stick mixing ought to be irrelevant here, if it's losing altitude and/or speed and throttle is available, it should use it (EDIT: it was using 80% throttle due to how I configured things, and that's fine, 80% is almost full thrust anyway). I didn't see speed getting as low as 7.5m/s when I looked (hopefully you didn't look at ground speed), because my plane cannot fly that slow and it never stalled, but it got slow, no question. I think one issue that probably didn't help is that I believe I had full flaps on, and that flaps were still on when guided mode kicked in (flaps are not controlled by ardupilot due to how hard it is to have AP control crow flaps on 4 channels). But even in that worst case, AP still had full motor, aileron, and elevator control. Flaps also cause pitch up by default, and AP was pitching down to gain ever more airspeed, like in my other near crash (that one did not involve flaps).

tridge commented 8 years ago

hi Marc, I've now completed an analysis of the crash, which I will post on the forums

marcmerlin commented 8 years ago

@tridge thanks for the ping, I failed to notice that the web board was not sending me any notification when you were replying there.

magicrub commented 8 years ago

Can someone post a link to the analysis please so I can follow along. On Mar 23, 2016 8:07 AM, "Marc MERLIN" notifications@github.com wrote:

@tridge https://github.com/tridge thanks for the ping, I failed to notice that the web board was not sending me any notification when you were replying there.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/diydrones/ardupilot/issues/3357#issuecomment-200385565

WickedShell commented 8 years ago

http://ardupilot.com/forum/viewtopic.php?f=115&t=15302

Same spot Marc posted previously :) On Mar 23, 2016 9:24 AM, "Tom Pittenger" notifications@github.com wrote:

Can someone post a link to the analysis please so I can follow along. On Mar 23, 2016 8:07 AM, "Marc MERLIN" notifications@github.com wrote:

@tridge https://github.com/tridge thanks for the ping, I failed to notice that the web board was not sending me any notification when you were replying there.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub < https://github.com/diydrones/ardupilot/issues/3357#issuecomment-200385565>

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/diydrones/ardupilot/issues/3357#issuecomment-200421736