Skip to content
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

Disable ALTHOLD on user motor stop request #3298

Merged

Conversation

Projects
None yet
3 participants
@shellixyz
Copy link
Collaborator

commented May 30, 2018

Before if the user puts the throttle stick bellow mincheck when ALTHOLD (all nav states using NAV_CTL_ALT) is enabled and the MOTOR_STOP feature is enabled the motor is stopped while ALTHOLD still tried to maintain altitude. This can quickly lead to a stall.

With this patch ALTHOLD is paused when the throttle is bellow mincheck (motor stopped if the MOTOR_STOP feature is enabled or at minthrottle otherwise) and resumed when the user throttles up again.

It allows to start gliding just by putting the throttle stick to 0 and start maintaining altitude again by increasing throttle above mincheck

@@ -99,12 +99,14 @@ typedef enum {
COMPASS_CALIBRATED = (1 << 8),
ACCELEROMETER_CALIBRATED= (1 << 9),
PWM_DRIVER_AVAILABLE = (1 << 10),
HELICOPTER = (1 << 11)
HELICOPTER = (1 << 11),
USER_MOTOR_STOP = (1 << 12) // User asked for stopping motor (rcData[THROTTLE] < mincheck)

This comment has been minimized.

Copy link
@digitalentity

digitalentity May 31, 2018

Member

Wouldn't it be easier to allow mixer expose status of motor?
I.e. getMotorStatus with possible values of MOTOR_RUNNING, MOTOR_STOPPED_USER, MOTOR_STOPPED_AUTO?

This comment has been minimized.

Copy link
@shellixyz

shellixyz May 31, 2018

Author Collaborator

Good idea. I added a new getMotorStatus function in mixer.c and removed the state flag USER_MOTOR_STOP

@shellixyz shellixyz force-pushed the shellixyz:disable_althold_on_zero_throttle branch 2 times, most recently from 1c12c9d to 1a79c43 May 31, 2018

setupAltitudeController();
setDesiredPosition(&navGetCurrentActualPositionAndVelocity()->pos, posControl.actualState.yaw, NAV_POS_UPDATE_Z); // This will reset surface offset
}

return NAV_FSM_EVENT_SUCCESS;
if (motorStatus != MOTOR_STOPPED_USER) return NAV_FSM_EVENT_SUCCESS; // user asked to restart the motor, resume ALTHOLD

This comment has been minimized.

Copy link
@digitalentity

digitalentity May 31, 2018

Member

I believe when we exit the "pause" we should reset the altitude setpoint - hence setDesiredPosition should be called here, before switching to "in-progress" state

@@ -424,7 +424,7 @@ void applyFixedWingPitchRollThrottleController(navigationFSMStateFlags_t navStat
rollCorrection += posControl.rcAdjustment[ROLL];
}

if (isPitchAdjustmentValid && (navStateFlags & NAV_CTL_ALT)) {
if (isPitchAdjustmentValid && (navStateFlags & NAV_CTL_ALT) && (getMotorStatus() != MOTOR_STOPPED_USER)) {

This comment has been minimized.

Copy link
@digitalentity

digitalentity May 31, 2018

Member

Why checking for motor-stop here? When ALTHOLD is paused NAV_CTL_ALT is not set anyway.

This comment has been minimized.

Copy link
@shellixyz

shellixyz May 31, 2018

Author Collaborator

True ! It is a reminiscence of old code. Fixed

@digitalentity
Copy link
Member

left a comment

Look ok now. Some testing required before we can merge this.

@digitalentity

This comment has been minimized.

Copy link
Member

commented May 31, 2018

Once we have full featured TECS with airspeed we'll have to revisit this - TECS will be able to pitch down to maintain airspeed so it will have to be running at all times.

@shellixyz

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 2, 2018

Please don't merge this yet I need to change the way it is working to be able to do the same for the upcoming 3D CRUISE mode.

@shellixyz shellixyz force-pushed the shellixyz:disable_althold_on_zero_throttle branch 2 times, most recently from d685f7b to 0b0d04d Jun 4, 2018

@shellixyz

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 4, 2018

I modified this PR. I think it is much better now, more polyvalent. No need to modify the ALTHOLD states and it will be working with 3D cruise mode without changing anything to it either.

I also took the opportunity to simplify/deduplicate code in applyFixedWingNavigationController()

Tested and now ready to merge.

@shellixyz shellixyz force-pushed the shellixyz:disable_althold_on_zero_throttle branch from 0b0d04d to 3705a05 Jun 12, 2018

@shellixyz shellixyz force-pushed the shellixyz:disable_althold_on_zero_throttle branch from 3705a05 to f046554 Jun 13, 2018

@digitalentity
Copy link
Member

left a comment

LGTM

@digitalentity digitalentity added this to the 2.0 milestone Jun 15, 2018

@fiam fiam merged commit 3e18c56 into iNavFlight:development Jun 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.