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

FwPositionControl: don't do takeoff help for vtol #10048

Merged
merged 1 commit into from
Apr 30, 2019
Merged

Conversation

RomanBapst
Copy link
Contributor

@RomanBapst RomanBapst commented Jul 26, 2018

  • takeoff help is used for fixed wings, it increases the altitude setpoint
    after a launch. A vtol does not need this as it's already sufficiently
    high up in the air.

Signed-off-by: Roman bapstroman@gmail.com

@RomanBapst RomanBapst requested review from dagar and bresch July 26, 2018 07:32
@RomanBapst
Copy link
Contributor Author

If anybody has been wondering why in the gazebo standard vtol simulation the vehicle climbed like crazy after a front transition just to descend again later...

@dagar
Copy link
Member

dagar commented Jul 26, 2018

This helper function is only used in altitude and position control modes, although I don't see why we aren't using it everywhere.

@dagar
Copy link
Member

dagar commented Jul 27, 2018

  1. On second thought I don't think we should change do_takeoff_help() for VTOL (or at least not specifically for VTOL). All it's doing is forcing a climb in altitude or position control modes if you're close to the ground after takeoff.

  2. This doesn't fix VTOL standard climb after transition. See comparison below.

master (vtol standard CI test)

image

PR (vtol standard CI test)

@dagar
Copy link
Member

dagar commented Jul 27, 2018

It's likely something to do with proper TECS initialization (or lack thereof).

image

Copy link
Member

@dagar dagar left a comment

Choose a reason for hiding this comment

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

On second thought I don't think we should change do_takeoff_help() for VTOL (or at least not specifically for VTOL). All it's doing is forcing a climb in altitude or position control modes if you're close to the ground after takeoff.

@RomanBapst
Copy link
Contributor Author

@dagar I think I'm already a step ahead. I was implementing front transition using TECS and discovered this behavior. With current master you don't see this because I assume TECS is only activated after transition.
I currently put TECS into no-airspeed mode during the transition and give it a predefined throttle ramp up.
Like this we can have it control altitude during the front transition and it's states (except the throttle states) will be initialized after transition. In SITL I get smooth pitch and throttle setpoints and we can avoid having to hack altitude controllers for the transition into the vtol module. I'm not entirely sure how it will work in practice but it's worth a shot. I'm currently testing in SITL and will push a PR most likely today.

@bresch
Copy link
Member

bresch commented Jul 30, 2018

@RomanBapst Why not just setting FW_CLIMBOUT_DIFF to zero for a VTOL if it is just used for FW takeoff and landing?

@stale
Copy link

stale bot commented Jan 20, 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.

@stale
Copy link

stale bot commented Feb 4, 2019

Closing as stale.

@stale stale bot closed this Feb 4, 2019
@RomanBapst RomanBapst reopened this Apr 24, 2019
@stale stale bot removed the Admin: Wont fix label Apr 24, 2019
@RomanBapst
Copy link
Contributor Author

@dagar I think this should be fixed for the release because as is it's broken and it causes the plane to increase altitude after transition.

Since the FW position controller does not run during hover mode the ground reference altitude is not set correctly, it's actually set as soon as the vehicle goes into transition mode.
That means that currently for VTOL we always get an increase in altitude after transition no matter how far you are from the ground. I've confirmed that this does not happen if you set the climb-out diff parameter to 0. But that is not the solution as this parameter is used in other parts of the code, not just for the takeoff help.
Additionally, even if we fix the ground reference we still only allow takeoff help for a duration of 10 seconds after the vehicle goes in air. Practically this means that the takeoff help is anyway useless for VTOL as usually the transition to FW takes place after that.

Let me know what you think.

@dagar
Copy link
Member

dagar commented Apr 24, 2019

Alright, can we start by getting it rebased?

@RomanBapst
Copy link
Contributor Author

@dagar No, if we agree how to do it I will quickly do a PR. I just reopened this as a placeholder.

- takeoff help is used for fixed wings, it increases the altitude setpoint
after a launch. A vtol does not need this as it's already sufficiently
high up in the air.

Signed-off-by: Roman <bapstroman@gmail.com>
@RomanBapst
Copy link
Contributor Author

@dagar We will test this in the afternoon.

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.

Makes sense to me.

@RomanBapst RomanBapst assigned RomanBapst and unassigned RomanBapst Apr 27, 2019
@RomanBapst RomanBapst requested a review from a team April 27, 2019 17:21
@RomanBapst
Copy link
Contributor Author

@PX4/testflights Please test on the deltaquad. The vehicle should not climb noticeably after the front transition (assuming you don't command it to climb).

@dagar
Copy link
Member

dagar commented Apr 30, 2019

I've given this a quick test in SITL to verify the behavior. @PX4/testflights please still test this as a VTOL sanity check for the release.

@dagar dagar merged commit 2bd8b51 into master Apr 30, 2019
@dagar dagar deleted the pr-takeoff_help branch April 30, 2019 06:45
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.

4 participants