Add: parachute timeout parameter #27500
Conversation
| @@ -104,6 +108,7 @@ class AP_Parachute { | |||
| AP_Int16 _servo_off_pwm; // PWM value to move servo to when shutter is deactivated | |||
| AP_Int16 _alt_min; // min altitude the vehicle should have before parachute is released | |||
| AP_Int16 _delay_ms; // delay before chute release for motors to stop | |||
| AP_Int16 _timeout; // loss of control timeout value in seconds | |||
There was a problem hiding this comment.
If its in seconds I think it should be a float.
There was a problem hiding this comment.
I have corrected it. What do you think about the range from 0.5 to 5 seconds?
|
This looks pretty good to me. Could you squash the commits together? |
Sztosik
force-pushed
the
parachute-check-trigger
branch
2 times, most recently
from
37a96a7
to
fcff383
Compare
There was a problem hiding this comment.
This looks good to me. @Sztosik could you confirm that you've tested this somehow? I'm pretty sure it does but with such an important subsystem we should be sure. Perhaps set the timeout to a really large number (like 10 seconds) so that it is really obvious that the parameter is having an effect.
I tested it on a Hexsoon EDU450 witch Cube Orange+. I removed the propellers, switched to AltHold mode, armed the drone and raised it, then changed attitudes. Unfortunately, this does not cause the parachute to deploy (I performed an identical test on the stable version and it works fine). Now I'm trying to investigate it. |
I would guess its the baro altitude check preventing deployment if not falling. |
I'm not sure, |
Never mind, you have right @IamPete1 |
I figured out that value of |
I think this time out fix is worth having as it is, you can always come back with the additional changes in a new PR. |
| @@ -83,6 +83,14 @@ const AP_Param::GroupInfo AP_Parachute::var_info[] = { | |||
| // @User: Standard | |||
| AP_GROUPINFO("OPTIONS", 7, AP_Parachute, _options, AP_PARACHUTE_OPTIONS_DEFAULT), | |||
|
|
|||
| // @Param: TIMEOUT | |||
| // @DisplayName: Parachute timeout | |||
| // @Description: Triggers the parachute if the loss of control lasts for the time specified by this parameter | |||
There was a problem hiding this comment.
Need to mention that it's copter only.
There was a problem hiding this comment.
maybe we should make the parameters only appear for copter..?
There was a problem hiding this comment.
@rmackay9 how to achieve it? I've tried with APM_BUILD_COPTER_OR_HELI but automatic checks failed.
ArduCopter/crash_check.cpp
Outdated
| #define CRASH_CHECK_TRIGGER_SEC 2 // 2 seconds inverted indicates a crash | ||
| #if PARACHUTE == ENABLED | ||
| // CRASH_CHECK_TRIGGER_SEC should be grater than CHUTE_TIMEOUT, to prevent disarming without deploying parachute | ||
| #define CRASH_CHECK_TRIGGER_SEC parachute.get_chute_timeout() + 1.0f |
There was a problem hiding this comment.
this define is not doing what you want, as it is used in a multiplication, you need parantheses around the define
#define CRASH_CHECK_TRIGGER_SEC (parachute.get_chute_timeout() + 1.0)
| @@ -86,7 +92,7 @@ void Copter::crash_check() | |||
| // we may be crashing | |||
| crash_counter++; | |||
|
|
|||
| // check if crashing for 2 seconds | |||
| // check if crashing for time defined by CRASH_CHECK_TRIGGER_SEC seconds | |||
| if (crash_counter >= (CRASH_CHECK_TRIGGER_SEC * scheduler.get_loop_rate_hz())) { | |||
There was a problem hiding this comment.
the multiply in this line ends up multiplying by 1.0, not the users value
tridge
force-pushed
the
parachute-check-trigger
branch
from
03cde35
to
d8a189f
Compare
|
@Sztosik I've pushed fixes for the build issue and multiply to the PR |
Replace
PARACHUTE_CHECK_TRIGGER_SECconst withCHUTE_TIMEOUTparameter.The value of
PARACHUTE_CHECK_TRIGGER_SECset to a fixed value may in some cases cause premature parachute deployment.