-
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 (attempt2) #27396
base: master
Are you sure you want to change the base?
Conversation
99bda29
to
d6b2584
Compare
9df66ef
to
25df1a3
Compare
0ed1d0b
to
85d1a5f
Compare
OK, I'll switch this to use the aux function in order to save flash space. |
0c53aac
to
1455cc4
Compare
def GripperReleaseOnThrustLoss(self): | ||
'''tests that gripper is released on thrust loss if option set''' | ||
|
||
self.context_push() |
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 retain this test - all we need to do is install_example_script_context
here for your new script
@MattKear is going to review logs from the original customer. |
I have not been able to find logs from the original customer, so I am not going to stand in the way of this going in. Thanks for pausing this to give me chance to look. |
Co-authored-by: Iampete1 <[email protected]>
1455cc4
to
1c5e783
Compare
rebased on master |
This is a replacement for PR #27339 and attempts to resolve the concerns raised with simply removing the option:
I've done some simple testing in SITL to confirm the arming check works
.. and to confirm that the new script does release the gripper on thrust loss or more accurately when these cases are true for a full second:
I tested by setting SIM_ENGINE_MUL = 0.6 which causes the quadcopter to lose altitude and hit the ground.