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

Copter: fix compilation when drift mode is disabled #12098

Merged

Conversation

peterbarker
Copy link
Contributor

I have only validated that this compiles.

This should be NFC (apart from actually compiling...).

I think there's a wider discussion to be had on when we should allow these mode switches. Depending on how aggressively it is tuned, popping the vehicle into several other modes may cause it to leap off the ground here.

e.g.:

STABILIZE> mode guided
STABILIZE> Got MAVLink msg: COMMAND_ACK {command : 11, result : 0}
GUIDED> Mode GUIDED
rc 3 2000
GUIDED> param set FS_GCS_ENABLE 0
GUIDED> arm throttle
GUIDED> APM: Arming motors
Got MAVLink msg: COMMAND_ACK {command : 400, result : 0}
ARMED

GUIDED> mode loiter
GUIDED> Got MAVLink msg: COMMAND_ACK {command : 11, result : 0}
LOITER> Mode LOITER
APM: EKF2 IMU0 in-flight yaw alignment complete
APM: EKF2 IMU1 in-flight yaw alignment complete
height 15
height 25
SIM_VEHICLE: Keyboard Interrupt received ...
SIM_VEHICLE: Killing tasks

@peterbarker peterbarker force-pushed the pr/copter-fix-drift-disable-compilation branch from b645e77 to 4dca818 Compare August 21, 2019 06:17
@peterbarker
Copy link
Contributor Author

... I should note that our autotest suite covers getting a failure back in the case this is checking.

@rmackay9
Copy link
Contributor

rmackay9 commented Aug 21, 2019

maybe we can just change drift mode to return true to the "has_manual_throttle" method?

EDIT: to answer my own question, it's tricky to just change Drift's has-manual-throttle to return true because we have these lines in mode.cpp's exit_mode() method. I think this will not calculate the I-term correctly. So I guess in short my suggestion would need to be checked very carefully.

// smooth throttle transition when switching from manual to automatic flight modes
if (old_flightmode->has_manual_throttle() && !new_flightmode->has_manual_throttle() && motors->armed() && !ap.land_complete) {
    // this assumes all manual flight modes use get_pilot_desired_throttle to translate pilot input to output throttle
    set_accel_throttle_I_from_pilot_throttle();
}

@OXINARF OXINARF added the Copter label Aug 21, 2019
Copy link
Member

@OXINARF OXINARF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I think there's a wider discussion to be had on when we should allow these mode switches. Depending on how aggressively it is tuned, popping the vehicle into several other modes may cause it to leap off the ground here.

@peterbarker I want your ok_to_enter/enter methods idea back! 🙂

@peterbarker
Copy link
Contributor Author

peterbarker commented Aug 21, 2019 via email

@tridge tridge merged commit 0ce3cd0 into ArduPilot:master Aug 26, 2019
@peterbarker peterbarker deleted the pr/copter-fix-drift-disable-compilation branch September 3, 2019 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants