-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
Copter: Remove Payload release on thrust loss detection #27339
Copter: Remove Payload release on thrust loss detection #27339
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. "Randomly" dropping payload is a serious concern.
It would need to check if this wasn't included in a recent releases
I have talked to Pete about this and the application was for a drone that dragged a line. If the line got snagged the drone would crash. This RELEASE_GRIPPER_ON_THRUST_LOSS would mean that anytime the aircraft hit maximum thrust and started reducing in altitude it would release. The interesting thing is that it would only work if the drone was leant over by less than 15 degrees. So that would mean the aircraft would need to be pulled down almost vertically. I wonder how this could prevent a crash. |
@@ -96,7 +96,12 @@ void Copter::crash_check() | |||
} | |||
} | |||
|
|||
// check for loss of thrust and trigger thrust boost in motors library | |||
// thrust_loss_check - returns true when the aircraft detects for 1 second that it is: | |||
// above 90% thrust, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// above 90% thrust, | |
// above 90% demanded thrust or high thrust saturation, |
|
||
#if AP_GRIPPER_ENABLED | ||
if (copter.option_is_enabled(FlightOption::RELEASE_GRIPPER_ON_THRUST_LOSS)) { | ||
gripper.release(); | ||
} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't just remove this. The would need to be a arming check if the option is set and the feature is gone.
I think we could potentially close this PR and instead focus on this updated version #27396 |
I'm going to go ahead and close this because @lthall has approved the other PR |
This PR removes a potentially very dangerous "feature" added in this PR #19890.
The previous PR will release the payload any time the aircraft uses more than 90% thrust for a second while descending under control and less than 15 degrees lean angle.
The thrust loss detection is designed to be a sub check of further dependent checks like the motor out test. It is a check that is expected to happen in normal flight.
This feature above looks like a sensible check to enable but is expected to result in random releases of the payload during landings on low battery for example. The aircraft is heavily loaded because it has a payload, the aircraft is descending and it is normal for the industry to specify their aircraft maximum take-off at 80% thrust on a full battery.
Releasing payloads in an unplanned and automated way, over landing zones or on approach is literally a risk of serious or fatal injury to ground crew.
I have tried to improve the comments on thrust_loss_check() to make it clearer what this check is doing.
We may also be able to improve the output message to the user to be clearer or more appropriate. It is currently "Potential Thrust Loss (%d)" and a motor. However, it does not require a motor lost so it will say motor 1 when no motor is lost I think.
The name is also misleading. Perhaps the name "Thrust Limited Detection" would be better.
We could also reduce the likelihood of this going off by changing the check for decent to make it "less than commanded decent" in alt_hold based modes.