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 parameter #9243

Closed
wants to merge 1 commit into from

Conversation

dagar
Copy link
Member

@dagar dagar commented Apr 4, 2018

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

@Antiheavy

@Antiheavy
Copy link
Contributor

Antiheavy commented Apr 5, 2018

Agree.

Once this goes in place, then QGC could hide the takeoff pitch value in the waypoint editor which is a big flight safety improvement. People fat figure the wrong pitch into that field and cause the vehicle to crash on launch -- i've done it a couple times and I've seen others do it too.

This also makes Takeoff mode actually useful for PX4 Fixed Wing since the pitch can be set appropriately for each vehicle, instead of some previously hard-coded value. Was it hard coded to 10 degrees previously for Takeoff mode? actually... is that being fixed here?

@Antiheavy
Copy link
Contributor

original issue post here: #6787

@dagar dagar requested a review from RomanBapst April 5, 2018 02:15
@dagar dagar force-pushed the pr-fw_takeoff_pitch branch from 9bed1d2 to 3814a9c Compare April 12, 2018 03:59
@dagar dagar force-pushed the pr-fw_takeoff_pitch branch from 3814a9c to 5b9b2cc Compare May 17, 2018 23:27
@dagar dagar added this to the Release v1.8.0 milestone May 22, 2018
@dagar dagar force-pushed the pr-fw_takeoff_pitch branch from 5b9b2cc to b5afbb9 Compare May 23, 2018 00:36
@dagar dagar force-pushed the pr-fw_takeoff_pitch branch from b5afbb9 to 22e605c Compare June 13, 2018 04:50
@dagar
Copy link
Member Author

dagar commented Jun 13, 2018

Conflicts resolved and rebased on master.

@Antiheavy
Copy link
Contributor

@dagar we can help with flight testing a branch that has this. FYI @Kjkinney

@Antiheavy
Copy link
Contributor

we should make a scan for all launch mode features for both mission takeoff and QGC button takeoff behaviors. e.g. how does this fit in #9858 (comment)

@Antiheavy
Copy link
Contributor

Antiheavy commented Jul 12, 2018

@philipoe @tstastny @ryanjAA @acfloria your input would be appreciated for this fixed wing related PR. original feature request here: #6787

@dagar dagar force-pushed the pr-fw_takeoff_pitch branch 3 times, most recently from aa9d1fc to 8bdb9cd Compare July 16, 2018 05:00
@Antiheavy
Copy link
Contributor

can we add this to the Fixed Wing project board?
https://github.com/PX4/Firmware/projects/22

@philipoe
Copy link
Contributor

I totally agree that this needed to be changed/fixed. I did a quick review and found no issues with the code changes related to the pitch min parameter. However, there is also a lot of variable name changes mixed into this one commit that i thus did not review...

@dagar
Copy link
Member Author

dagar commented Aug 24, 2018

Rebased on master (and simplified).

@dagar dagar force-pushed the pr-fw_takeoff_pitch branch from b425b3b to 0b3be0e Compare February 4, 2019 16:52
@dagar
Copy link
Member Author

dagar commented Feb 4, 2019

Simpler version of this PR without the other FW position controller changes. #11373

@Antiheavy
Copy link
Contributor

@dagar is this PR overcome by the newer #11845 ?

@stale
Copy link

stale bot commented Aug 30, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@Antiheavy
Copy link
Contributor

Hmmmm. @dagar this is the version we’ve been flying for months. Works great.

@stale
Copy link

stale bot commented Nov 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@hamishwillee
Copy link
Contributor

@RomanBapst This still looks sensible. Any reason why it hasn't gone in?

@stale stale bot removed the stale label May 27, 2020
@RomanBapst
Copy link
Contributor

@hamishwillee Yes, looks sensible. I'll have to rebase it and merge.

@ryanjAA
Copy link
Contributor

ryanjAA commented May 27, 2020

This will be good to have. No reason to really ever change takeoff pitch per mission unless testing something. I have enjoyed an accidental 35 degree full flap takeoff instead of a 25 deg test (that should be essentially impossible unless very deliberate) which this will help users not do.

@hamishwillee
Copy link
Contributor

Thanks @RomanBapst - I hope soon :-0

@Antiheavy
Copy link
Contributor

I have enjoyed an accidental 35 degree full flap takeoff instead of a 25 deg test (that should be essentially impossible unless very deliberate) which this will help users not do.

guess what happens when someone thinks they are typing in their takeoff altitude into the pitch field! yep, done it.... that is why we rolled in something nearly identical to this PR a year ago.

@hamishwillee
Copy link
Contributor

Ping @RomanBapst - rebasing and merging reminder :-)

@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Oct 4, 2020
@Antiheavy
Copy link
Contributor

This (or something similar) should really go into to upstream. We’ve been running this PR for well over a year now. Many 1000’s of flights. As both I and @ryanjAA have pointed out there is a safety concern about the current requirement to type in a Takeoff pitch value into each mission. Are there know users this would negatively impact? @dagar @RomanBapst

@stale stale bot removed the stale label Oct 4, 2020
@ryanjAA
Copy link
Contributor

ryanjAA commented Oct 7, 2020

I agree. Takeoff pitch really should be set either as a parameter or harder to access mission setting since it shouldn’t be easily accessible to the point of accidentally changing it. I never change it unless it’s a test but I do like knowing I see it when planning a mission as it validates the planning is correct (although I don’t need to see cruise speed and that’s a parameter so it’s just getting used to it I suppose). This does make sense to have as parameter or possibly viewable in qgc mission planning so if there are negative reasons not known maybe we approach this from the qgc side and make it simply harder to change than a simple text box but one way or another too easy of access isn’t ideal.

@LorenzMeier
Copy link
Member

Closing as stale.

@LorenzMeier LorenzMeier deleted the pr-fw_takeoff_pitch branch January 10, 2021 17:51
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"
7 participants