search

PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.46k stars 13.5k forks source link

Support mission mode without Mission for FW #649

Closed julianoes closed 10 years ago

julianoes commented 10 years ago

Switching to AUTO MISSION currently leads to a dive if no mission has been set.

thomasgubler commented 10 years ago

In HIL (at least a few days ago) the system behaved as expected

julianoes commented 10 years ago

Indeed this is working fine in HIL. Maybe the logs could give any clues...

sjwilks commented 10 years ago

This reminds me of what happens sometimes as it uses baro for altitude which ends up being 20+ meters lower than the actual altitude depending on conditions. On Feb 10, 2014 6:10 PM, "julianoes" notifications@github.com wrote:

Indeed this is working fine in HIL. Maybe the logs could give any clues...

Reply to this email directly or view it on GitHubhttps://github.com/PX4/Firmware/issues/649#issuecomment-34655807 .

DrTon commented 10 years ago

Maybe something bad with altitude_is_relative flag?

julianoes commented 10 years ago

@sjwilks: could you reproduce this on your flight yesterday? If not, I think, we can close it.

DrTon commented 10 years ago

BTW, it should be the same as switching to LOITER, because if mission was not set nav_state will be LOITER. @julianoes, can you try to repeat this on bench? This diving can be very dangerous, I think.

julianoes commented 10 years ago

@DrTon: the thing is that we haven't been able to reproduce this in HIL. Maybe I could try to reproduce it on the bench, you're right.

DrTon commented 10 years ago

Does your HIL use baro offset and non-zero ground level?

sjwilks commented 10 years ago

Sorry I did not test this. Weather permitting I will try this weekend. On Feb 13, 2014 4:12 PM, "Anton Babushkin" notifications@github.com wrote:

Does your HIL use baro offset and non-zero ground level?

Reply to this email directly or view it on GitHubhttps://github.com/PX4/Firmware/issues/649#issuecomment-34987075 .

LorenzMeier commented 10 years ago

Setting mission mode without waypoint set makes the copter rise constantly (max vertical velocity) on beta. This kind of sounds familiar, just a different sign 8).

LorenzMeier commented 10 years ago

@DrTon Could you maybe have a quick look a this? Remo and I (I flew today) have the same symptoms.

julianoes commented 10 years ago

@LorenzMeier: I assume you are describing the case where it climbs to NAV_MIN_ALT which might be set to 50m by default.

LorenzMeier commented 10 years ago

50m AGL?

DrTon commented 10 years ago

AGL means nothing in mountains :) Above home altitude.

DrTon commented 10 years ago

BTW, is it good behavor? Why not to simply loiter at current altitude? For MC it should be ok, maybe required only for FW?

LorenzMeier commented 10 years ago

I think we need to change the 50m default at least. @thomasgubler @julianoes What do you think?

Or do we need rotary and fixed wing parameters for this?

DrTon commented 10 years ago

Hm, I think we need min alt for MC too to avoid situation when user switches from MANUAL to AUTO while landed. Holding zero altitude will be dangerous. But for MC it can be e.g. 5m by default, not 50m. Adding separate parameter doen't look very good for me: we will have two parameters with the same meaning. But it looks like we have no another choice...

kroimon commented 10 years ago

Can't we just set the parameter to 5m in rc.mc_defaults?

DrTon commented 10 years ago

I thought about this - not good idea. User may forget (or don't want) to set SYS_AUTOCONFIG=1 when switching from FW to MC configuration or on first run.

LorenzMeier commented 10 years ago

That doesn't sound like a bad choice 8)

LorenzMeier commented 10 years ago

True. I could implement a "only overwrite if default" param set command. Would that help?

DrTon commented 10 years ago

Ok, then I will add another parameter. At least it will be safe when switching from FW to MC config.

kroimon commented 10 years ago

@LorenzMeier: Sounds like a great idea! Because using two separate parameters requires setting the correct platform type, too...

DrTon commented 10 years ago

Hm, maybe, but still, if user changed it for FW he will not get good value for MC. So separate param is better :)

kroimon commented 10 years ago

On another note: There are other NAV_* params that get set in rc.fw_defaults. These would need to be changed back too when changing from MC to FW or back...

DrTon commented 10 years ago

For MC it's solved now, NAV_MIN_ALT not used anymore (https://github.com/PX4/Firmware/blob/beta/src/modules/navigator/navigator_main.cpp#L1062): Also, if landed -> switch to READY if LOITER requested (if land detector not triggered yet it can cause falling from 1-2m if switching to LOITER immediately after takeoff, I think it's not too dangerous). If not landed -> keep current altitude.

julianoes commented 10 years ago

@thomasgubler and I did some more investigations on this today: The observation is that "sometimes" loiter (whether mission without mission, loiter or RTL) lets fixedwing dive pretty scary.

This happens even though airspeed is working fine and the altitude setpoint is correct. Our theory is now that the groundspeed undershoot is calculated incorrectly in the case of loiter (see here: https://github.com/PX4/Firmware/blob/beta/src/modules/fw_pos_control_l1/fw_pos_control_l1_main.cpp#L716) Since the distance to the loiter WP is used to calculate the groundspeed undershoot, this most probably leads to a huge "desired airspeed" which then makes the tecs controller dive in order to gain velocity.

The current proposed workaround is to set groundspeed undershoot to 0 in loiter: 75c6706278119e50ce56ecbb1620025a9b9b936d 909e138182000350df19d47367d6f68d0a6b7fd4

This hopefully solves this case for now, testing has yet to prove it.

On a side note, I also think that the plane should never be allowed to dive like this, even if it can't reach a waypoint because of strong wind. It should rather be pushed away by the wind than crash, right?

LorenzMeier commented 10 years ago

@julianoes @thomasgubler I agree on the dive part. I'm not sure if this is a parameter tuning issue or the TECS implementation is suboptimal in that case. Do you guys have a log of the dive? Does the system go full throttle on dive? If it does, we need to limit the groundspeed undershoot to a reasonable value. If it doesn't go full throttle, then we might need to do something about this part.

I think your workaround is sensible, but it would be great to understand the issue fully here, so that we do not run into it again in a different flight mode.

julianoes commented 10 years ago

@LorenzMeier: Yes we do have a log of a dive, I've already had a thorough look at it. We do immediately get full throttle during the dive. Basically, it should stop pitching down if it's already at full throttle. I think I also read comments in the code in that direction, maybe it's something that has been commented out.

The workaround has been flown successfullly today on the X5, no dive issues so far!

julianoes commented 10 years ago

The current dives are mostly tuning issues, therefore closing.