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

FW position control add takeoff minimum pitch #11373

Closed
wants to merge 1 commit into from

Conversation

dagar
Copy link
Member

@dagar dagar commented Feb 4, 2019

This is currently passed through via mavlink, but I don't think it makes sense to set per mission. This is a vehicle parameter.

@@ -1309,7 +1309,7 @@ FixedwingPositionControl::control_takeoff(const Vector2f &curr_pos, const Vector
_parameters.throttle_max, // XXX should we also set runway_takeoff_throttle here?
_parameters.throttle_cruise,
_runway_takeoff.climbout(),
radians(_runway_takeoff.getMinPitch(pos_sp_curr.pitch_min, 10.0f, _parameters.pitch_limit_min)),
Copy link
Contributor

Choose a reason for hiding this comment

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

comment only: additional benefit is removal of hidden hard-coded values like this 10 degree thing...

@Antiheavy
Copy link
Contributor

@dagar I've reviewed the code and as far as I can tell this looks good. One question: a long time ago you made a change that allowed the vehicle to bypass the Takeoff pitch/throttle behavior when switching to Takeoff Waypoint in the middle of flight. is that preserved? or maybe not even necessary anymore?

bresch
bresch previously approved these changes Feb 5, 2019
Copy link
Member

@bresch bresch left a comment

Choose a reason for hiding this comment

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

Lgtm

@dagar
Copy link
Member Author

dagar commented Feb 11, 2019

Testing this in SITL is actually a bit difficult. Anyone interested in hand (or catapult) launch with gazebo?

@Antiheavy
Copy link
Contributor

One thing I am still confused about: will this takeoff minimum pitch be valid until FW_CLMBOUT_DIFF or until MIS_TAKEOFF_ALT? or is it both (essentially whichever is greater)?

@Antiheavy
Copy link
Contributor

Testing this in SITL is actually a bit difficult.

can you put an unreasonable value into QGC's Takeoff Waypoint dialog and verify bad things happen without this PR and that okay thing happen with this PR, i.e. the value is ignored? I don't have my SITL set up with me today, but can try tomorrow.
image

@Antiheavy
Copy link
Contributor

Testing this in SITL is actually a bit difficult.

actually you are right, this is hard to test in SITL. I don't know if it is due to the way the SITL model works, or if there is actually something else going on. I can't get the vehicle to behave differently during takeoff no matter what "Pitch" value I place in the TakeoffWP param. This doesn't match our experience in real flight test. Might have to go do some real flight tests on this.

@dagar dagar force-pushed the pr-fw_takeoff_pitch_simple branch from e27f085 to 3f5e3b9 Compare April 13, 2019 16:49
@dagar
Copy link
Member Author

dagar commented Apr 13, 2019

Testing as part of #11845

@Antiheavy
Copy link
Contributor

We finally got out to flight test this PR. It works! It was tested on a lightly customized branch of 1.8.x firmware so it may need re-testing as part of 1.9.x, but the logic works.

Fixed wing flight log:
https://review.px4.io/plot_app?log=68ef0917-824e-446b-9aff-dc8e93fcf64c

4 short flights with FW_TKO_PITCH_MIN set to different values. You can see the vehicle's takeoff and climbout pitch behavior changes according to the parameter values. It is also successfully ignoring the default value set in QGC (15 degrees).
image

@LorenzMeier
Copy link
Member

Closing this in favor of #11845

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Parameter request: "minimum takeoff pitch"
4 participants