inav
inav copied to clipboard
iNavFlight
[Multirotor only] Land: Improvment to check that the airframe is not accelerating
The current verification using an average between the 3 axes of the Gyro doesn't seem to be very functional for some users... And for me it never worked, my multirotor never disarms after Land. Now I present another method to make the substitution. In this new method that I am presenting, the Earth-Frame Acceleration calculated by the INS will be used to calculate the X, Y and Z normalization, and filter this normalization at 1Hz, simple. Now my multirotor always trips after a Land.
Only thing I'd say about this is it's obviously more important to avoid false landing detection than failed landing detection, especially on multirotors. So this may improve landing detection reliability for multirotors but does it also increase the likelihood of false detection/disarms ? How much margin is there on the numbers ?
Also was posControl.actualState.velXY removed because it causes problems ? I'd have thought if you've landed there should be minimal XY velocity. Does it just need the threshold tweaking if it does cause problems rather than removing it altogether ?
And out of interest ... what values do you get for accel_stationary for a fixed wing when flying in smooth air vs static on the ground and the same for a multirotor ?
So this may improve landing detection reliability for multirotors but does it also increase the likelihood of false detection/disarms ?
I promise you that I will test this over and over again, in the meantime i can leave it as experimental
Also was
posControl.actualState.velXYremoved because it causes problems ?
get_accel_ef_length() does almost the same thing as this variable, the only difference is that I use the z-axis subtracted at 1G, along with x and y, and all this filtered at 1Hz. So it doesn't make sense to keep both at the same time.
what values do you get for accel_stationary for a fixed wing when flying in smooth air vs static on the ground and the same for a multirotor ?
you use the averageAbsGyroRates() < 2.0f check for both multirotor and fixed-wing aircraft. So I use get_accel_ef_length() <= LAND_DETECTOR_ACCEL_MAX also for both vehicles is basically the same as you did...
I really appreciate the enhancements you've added to verify that the Land has been completed... But what I'm trying to propose with this PR is that using a 1Hz filtered Earth-Frame 3D acceleration normalization is more reliable than using a simple average between the 3 axes of the Gyro, that's all... I'm not pointing out that your model is incorrect, not really! Because, of course, you can even guide me on how to do it the right way!
The averageAbsGyroRates was a pretty basic method of checking for movement so anything that improves on it is welcome especially if it doesn't work for some multirotors. It worked for me but generally took longer than it should have so certainly wasn't optimal in any way. Interesting to see if this works better.
I tested it with two different size quads. And also did a ground test with a plane in FS. It appears to require the same lack of motion from both craft, as with the current method. Although it appears to detect MC landing a little faster, both after an RTH land, and when the MC has touched down in Angle/Horizon modes.
I would say its very unlikely to disarm in flight with either craft.. Even holding the fixed wing as still as I could in my hand wasn't enough to trip the landing detector... I had to hold it on a motionless surface. Which is what I had to do with the current method.
This function has shown to consistently reduce the time taken for auto disarm to occur on un-level ground and in windy conditions, when compared to the current gyro method... While not appearing to increase the chances of false detection.
This function has shown to consistently reduce the time taken for auto disarm to occur on un-level ground and in windy conditions, when compared to the current gyro method... While not appearing to increase the chances of false detection.
Thanks for the Jetrell tests! When I get to my house, I'm going to rework this PR so that this new approach is only used in Multirotors. Using this approach of mine, and the current iNav approach (averaging the 3 axes of the gyro) on fixed-wing aircraft is totally wrong... You know what I'm going to bring to the Land of fixed-wing aircraft in the future 💋🔥😉
I'm still using this with the Multirotor only change.. No false disarms have occurred. And its still consistently faster, comparing back to back with the current disarm detection method. Even manually landing in Airmode with the motors spinning @3%, and with strong winds blowing across the props.. It still disarms very quickly, compared to it taking up to 30 seconds with the present method.
@breadoven Have you had a chance to compare it?
This works really well, I've tested it too... For me this could already be merged!
The problem with this is it's all about detection safety margins which are currently set on the conservative side to avoid false detection.
I've just tested the current detection but with the gyro limit set at 4 degs/s (vs current 2 degs/s) and a vertical velocity limit set the same as this PR (100cm/s vs the current value of 25 cm/s, so much higher which makes a difference in itself) and it also detects landings much more quickly or in fact within the minimum time possible, i.e. a couple of seconds. Hard to know if 4 degs/s would cause false detections, I'm guessing not unless the multirotor is very stable during a descent (throttle needs to be low to detect a manual landing so it will always be descending if there's no ground to stop it). However, this testing was on a flat surface, would need testing on a bit of a slope to see how that affects things.
@Jetrell I assume you mean this PR improves things for manual landings (or emergency landings) only. It shouldn't make any difference for RTH or WP landings given they use a different method for detection.
I assume you mean this PR improves things for manual landings (or emergency landings) only. It shouldn't make any difference for RTH or WP landings given they use a different method for detection.
Just landing in Acro, Angle or Horizon modes, then lowering the throttle and allowing it to auto disarm.
None of my quads are super stable in the landing descent phase.. Even when I've set them to a low 0.5m/s descent rate.. They still get a small amount of prop-wash action. So I haven't encountered a false detection as yet... Its hard to say whether a larger MC would have any issues.
The problem with this is it's all about detection safety margins which are currently set on the conservative side to avoid false detection.
I've just tested the current detection but with the gyro limit set at 4 degs/s (vs current 2 degs/s) and a vertical velocity limit set the same as this PR (100cm/s vs the current value of 25 cm/s, so much higher which makes a difference in itself) and it also detects landings much more quickly or in fact within the minimum time possible, i.e. a couple of seconds. Hard to know if 4 degs/s would cause false detections, I'm guessing not unless the multirotor is very stable during a descent (throttle needs to be low to detect a manual landing so it will always be descending if there's no ground to stop it). However, this testing was on a flat surface, would need testing on a bit of a slope to see how that affects things.
@Jetrell I assume you mean this PR improves things for manual landings (or emergency landings) only. It shouldn't make any difference for RTH or WP landings given they use a different method for detection.
how is Land detected in multirotors in RTH and WP mode?
When landing after an RTH or a mission... I've tested it on hard level ground and on soft un-level ground, when the quad has slightly fallen to one side.. Under both situations, it detects landing within 5 seconds.
On one test, it even landed on the edge of a piece of board, and was balancing there. It too detected the landing. While before, that type of condition would cause the quad to increase and decrease the motor RPM's to balance itself, all the while never detecting a landing. And eventually tilting so far, that the props struck the ground.
But as @breadoven stated. The most important thing, is that we don't get a false detection on a very stable MC. Anybody have one of those?
@Jetrell Can you test this new commit?
@JulioCesarMatias I'm not really sure why you decided to make all these changes which I assume you haven't tested because it's obvious just looking at the code they won't work, e.g. navigation.c line 2788 :
if (isFlightDetected()) {
resetLandingDetector();
} else if (isLandingDetected()) {
ENABLE_STATE(LANDING_DETECTED);
}
isFlightDetected is always going to be true in flight so it will never even try to detect a landing. It will however try to detect a landing when flight is NOT detected which could be after you've landing OR before you've even taken off (that is the reason for having landingDetectorIsActive, to avoid landing detection before you've taken off). Obviously if isFlightDetected is false it might indicate you've landed but it wasn't really intended to be used for this since it's quite broad in the way it initially detects a flight has started.
As regards the use of acceleration in general to check for craft movement, how does this work when you have constant or zero velocities ?
I'm going to revert this to the last commit, and modify it so that the Land detector works in manual and automatic modes... it doesn't make sense to me that it works only in manual flight
Testing the current landing detector logic again with a 5" quad on sloping ground with an increased Gyro limit of 4 degs/s and vertical velocity limit of 100 cm/s and it detects landing within 3s. This is with the quad in Horizon mode so it balances with the rear arms only on the ground until landing is detected . Increasing the vertical velocity limit helps because vertical velocity increases when on the ground due to prop wash effects and this can take several seconds to reduce below 0.5 m/s. However it seems doubling the Gyro margin makes a big difference, the current 2 degs/s seems to be right on the limit of movement for a 5" quad so it keeps resetting the landing timer.
Perhaps the best solution would be a sensitivity setting to adjust the detection criteria accordingly. Adjust to optimise detection speed whilst avoiding false landings.
1 - Replace the verification of the Gyro axes, with the acceleration in Earth-Frame filtered at 1Hz (Much better).
2 - Always check the acceleration condition in Earth-Frame to validate the Land, regardless if it is in RTH, WP or Emergency.
3 - New intervals (aka "extra delay") added... 1 second in Angle mode, and 3 seconds in AirMode mode.
4 - Added macros to make things clearer.
Afraid I still don't really see the point of this change especially the last commits which appear to break the current functionality. Seems to be trying to fix something that isn't broken, just needs tweaking instead, i.e. detection limits need better optimising for multicopters. This would be better achieved by adjusting the limits and adding a detection sensitivity setting.
As for manual landing detection on a multirotor the only important factor is vertical velocity. The other parameters XY velocity and gyro rates are only there as basic safety backup checks. These could be replaced with the acceleration based check in this PR if need be but there seems little point in the other changes.
new commit
Now the logic is as before... Two changes only:
1 - Swap the gyro average for the filtered EF acceleration
2 - Instead of using a fixed time of 2 seconds as extra time... Use 1 second in Angle mode, and 3 seconds in AirMode.
I think this is OK now. Would be useful to test on a MC though with the minimum setting for nav_manual_climb_rate (0.1 m/s) and Poshold with throttle stick low to check what it does during a slow descent.
What is the reason for the 1s and 3s delay ?
in angle mode the deceleration of the engines is faster, so there will be less vibrations in the IMU, so I judge 1 second with a good minimum time, I believe that it is not necessary more than that... And for AirMode it's the same thing , however, on the contrary...
after merging this PR >> https://github.com/iNavFlight/inav/pull/8427
It made this one useless... Although I totally disagree with adding one more configuration parameter (which no one will use), and also using a simple average of the 3 axes of the gyro...
Not sure why you closed this, you just needed to resolve the conflicts as required,