[Bug] Fixed-wing unable to disarm without GPS

[MaEtUgR]

Describe the bug

I went maiden flying my newly built Multiplex FunCub NG without GPS unit yet but with PX4 to handle Crossfire input, 7 PWM mixing (throttle, elevator, rudder, 2 ailerons, 2 flaps), gyro and attitude stabilization. GPS I'll build in later.
I armed, flew for a bit, landed, flipped the disarm switch and assumed the propeller will not turn but sometimes it unexpectedly started spinning again when the throttle stick was not at zero and in the end I had to disconnect the battery while it was still armed.

Note: It didn't always deny disarming. After some short flights it would work as expected.

To Reproduce

It's also reproducible in simulation, see steps and log below.

1. Fixed-wing with no GPS
   Sim: make px4_sitl gazebo-classic_plane and execute failure gps off to disable GPS.
2. Takeoff and fly in Stabilized, Acro, Manual
3. Land again
4. Try to disarm

Expected behavior

For safety reasons I expect a guaranteed way to disarm the vehicle after landing.

Additional context

From the log I found out what the chain of problems is:

• For fixed-wing unlike multicopter in Stabilized, Manual or Acro mode it's not allowed to disarm when the land detector doesn't declare the vehicle to be landed:
  

  PX4-Autopilot/src/modules/commander/Commander.cpp

  Line 641 in bcbae86

  if (!landed && !(mc_manual_thrust_mode && commanded_by_rc && _param_com_disarm_man.get())) {

• Land detector is not landed in the logs and after some inspection I'm pretty sure it's because it sees a horizontal velocity that is constantly above a certain threshold:
  

  PX4-Autopilot/src/modules/land_detector/FixedwingLandDetector.cpp

  Lines 114 to 115 in bcbae86

  	&& _velocity_xy_filtered < vel_xy_max_threshold
  	&& _velocity_z_filtered < _param_lndfw_vel_z_max.get()

• The estimator presumably publishes a best guess dead-reckon velocity but explicitly flags it as invalid using

  PX4-Autopilot/msg/VehicleLocalPosition.msg

  Line 9 in bcbae86

  bool v_xy_valid # true if vx and vy are valid

  The land detector ignores that flag and uses the values in vehicle_local_position.vx/vy if they are not NAN:
  

  PX4-Autopilot/src/modules/land_detector/FixedwingLandDetector.cpp

  Lines 78 to 80 in bcbae86

  	if (PX4_ISFINITE(val)) {
  	_velocity_xy_filtered = val;
  	}

Now as a solution the first thing that comes to mind is just respecting the validity flag for horizontal velocity in the fixed-wing land detector but we need to consider what happens if the velocity is invalid. In the current way the land detector is written the horizontal velocity filter just doesn't get updated if the velocity is not valid and that could work if it was never valid because the filter stays zero and that passes the threshold. But in the more rare case where the estimate was ever valid (like e.g. in my simulation log) the problem would persist and get the filter stuck above the threshold. If we just make the check pass whenever the estimate is not valid that could potentially potentially lead to false positives and premature auto disarm.

My suggestion is to:

• keep the disarming behavior in manual thrust modes consistent across fixed-wing and multicopter for the same safety reasons
• revise the fixed wing land detector to consider thrust level and rotational movement such that the horizontal movement check can be skipped when the estimate is invalid without any significant risk of false positives.

Flight Log

Real world: https://review.px4.io/plot_app?log=1874cf36-a219-4089-a25e-df600d6457c9

Reproduction in simulation: https://review.px4.io/plot_app?log=16393ecb-4698-4439-8705-1d5c67d93b9a

Software Version

main c024ea3

Flight controller

pixhawk 4 mini

Vehicle type

Fixed Wing

How are the different components wired up (including port information)

No response

Screenshot / Media

[sfuhrer]

Thanks for the detailed write up!

keep the disarming behavior in manual thrust modes consistent across fixed-wing and multicopter for the same safety reasons

I'm not yet so sure we should allow disarming in manual thrust modes even if not landed. After all, other than a MC, a FW can fly completely fine without thrust for a long while, so having the stick at 0 doesn't mean we're landed as it generally the case for MC.

revise the fixed wing land detector to consider thrust level and rotational movement such that the horizontal movement check can be skipped when the estimate is invalid without any significant risk of false positives.

Yes that one we clearly have to improve, and we really should share as much between the MC and FW land detector as possible 👍

[MaEtUgR]

@sfuhrer Thanks for having a look and your feedback!

FW can fly completely fine without thrust for a long while, so having the stick at 0 doesn't mean we're landed as it generally the case for MC

Yes on the gesture you're totally right, it's not safe at all on fixed-wing manual mode. I was thinking in terms of my arm/disarm switch which I'd not use in air but would give me a reliable way to turn it off compared to adding a kill-switch that I would then use as a disarm switch but doesn't allow arming again of course.

[MaEtUgR]

I started drafting land detector improvements on: https://github.com/PX4/PX4-Autopilot/tree/maetugr/fixed-wing-land-detector-improvements
Test logs of this version are here: #22668 (comment)