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

Plane: tailsitter disk theory gain scaling #15719

Merged
merged 7 commits into from
Dec 22, 2020

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented Nov 4, 2020

Inspired by #15613 this adds a disk theory based method for scaling tailsitter control surface gains. Requires that one set the disk loading of the vehicle with a new param. The disk loading equates how much airflow comes from the props vs the airspeed, if only half your control surface is behind the prop the disk loading should be set to half the true value ect. Lower disk loading = lower airflow from prop = less gain scaling.

Hopefully this will be a significant improvement over the current methods.

Needs testing in SITL and on real vehicles.

This also re-instates the <1 values of the throttle bases 'boost' scaling, not sure when that got lost.

Copy link
Contributor

@kd0aij kd0aij left a comment

Choose a reason for hiding this comment

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

Interesting idea; would this require an airspeed sensor to work properly?

Looks like this overrides rather than disables the existing two methods; could move the test to the top to potentially save a few cpu cycles.

@@ -334,7 +334,8 @@ void QuadPlane::tailsitter_speed_scaling(void)
if (is_zero(throttle)) {
spd_scaler = tailsitter.throttle_scale_max;
} else {
spd_scaler = constrain_float(hover_throttle / throttle, 1.0f, tailsitter.throttle_scale_max);
spd_scaler = constrain_float(hover_throttle / throttle, tailsitter.gain_scaling_min, tailsitter.throttle_scale_max);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is inside an if ((spd_scaler >= 1.0f) && ...
so this change is non-functional/unnecessary

Copy link
Member Author

Choose a reason for hiding this comment

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

We will hit this of the the first method is not enabled

Copy link
Contributor

@kd0aij kd0aij Nov 4, 2020

Choose a reason for hiding this comment

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

I was assuming that tailsitter.gain_scaling_min is < 1
Is that not guaranteed?
Also, this block is supposed to boost gain, not reduce it.
and I was wrong, the change does change behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah: 0 < gain_scaling_min < 1. It must reduce throttle, it does on stable.

spd_scaler = constrain_float(hover_throttle / throttle, 0, tailsitter.throttle_scale_max);

Scaling to less than 1 is required to get it working well at high throttle, especially on vectored.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, that change came in with commit 95da094, in April 2019.
Probably wasn't tested adequately on TVBS; IIRC we didn't think this option was very useful at the time.
But this means the bitmask value name is misleading as are the comments which say "boost gains at low throttle"

Copy link
Member Author

Choose a reason for hiding this comment

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

We also seems to have lost all scaling of thrust vectoring in the same commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the reasoning was that thrust vectoring would not be dependent on airspeed in the same way as control surfaces.

Copy link
Member Author

Choose a reason for hiding this comment

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

We should use throttle based scaling on them all the time, I will update this PR.

@robustini
Copy link
Contributor

Interesting idea; would this require an airspeed sensor to work properly?

I believe it is imperative, or it could never do this scaling.
Obviously airspeed not behind the propeller disc, but installed normally as always recommended.
Thanks @IamPete1, I try on the sim and I leave you my feedback.

@IamPete1
Copy link
Member Author

IamPete1 commented Nov 4, 2020

Yes, this does use a airspeed estimate, I have not tested how robust it is to it being wrong. Hopefully it will be less susceptible than direct airspeed scaling.

@robustini
Copy link
Contributor

I start with the first feedback: now perform a perfect QRTL with Q_ANGLE_MAX at 4000, before it was disastrous!

@robustini
Copy link
Contributor

I'm noticing however that hovering throttle remains the sticking point during plane->copter transitions.
Leaving the hover learn active on the sim it detects a hovering at 0.4, but if I switch to Q_Stabilize with the throttle stick in the center the tailsitter goes down, and the transitions are bad.
I then manually set it to 0.47 (the right value) and everything works fine.
This PR fixes the problem of high PIDs at high speeds, but it would take more than making the throttle hovering point so critical.

@robustini
Copy link
Contributor

Testing this pull request.
Obviously it is I who in QStab stress it properly, making it come out of the flight envelope by exaggerating in removing throttle, to test its recovery.

https://youtu.be/O47h2lk6SBE

@robustini
Copy link
Contributor

@IamPete1 what do you think of integrating scaling also based on barometric altitude?
I mean when the tailsitter going down quickly, like I do in the video.
This way scaling might even work better at that critical stage.

@IamPete1
Copy link
Member Author

IamPete1 commented Nov 5, 2020

@robustini we could totally add that, however we would have to make the assumption that the vehicle was tuned at sea level, so it would require a re-tune. (infact you could just work out the scaling and manually correct the gain). Once enabled is should be a good improvement over a significant altitude change, doubt it would make any difference over a few 100m.

I think I will add it as another option

@IamPete1 IamPete1 requested a review from kd0aij November 5, 2020 19:07
@IamPete1 IamPete1 marked this pull request as ready for review November 5, 2020 22:56
@IamPete1
Copy link
Member Author

IamPete1 commented Nov 5, 2020

I have done a fair bit of playing about in realflight, this seems to be at least as good as the exiting methods, I was able to get higher gains. Using the airspeed estimate it could be susceptible to it being wrong, but less than plane flight modes because we tend to be pointing up rather than directly into it.

I have tested on both vectored and none vectored. Not tested on copter tailsitters because the control surface are generally much smaller so would not show any issues as much.

ArduPlane/tailsitter.cpp Outdated Show resolved Hide resolved
ArduPlane/tailsitter.cpp Outdated Show resolved Hide resolved
ArduPlane/tailsitter.cpp Outdated Show resolved Hide resolved
@kd0aij
Copy link
Contributor

kd0aij commented Nov 26, 2020

@IamPete1 I see the same elevon oscillation problems with the Cat tailsitter on this branch as on master.
What physics frame rates are you getting in RF8, and what do you think is the minimum for a valid simulation for the Cat?

@IamPete1
Copy link
Member Author

@kd0aij You can still get it to oscillate but I found it much better than other methods. You can tune it in with the disk loading param, higher value should make it better at higher airspeeds. I have been running at 250 to 400 hz. I actually don't think the frame rate effects the level of tune you can get that much, but it does have a big effect what the values actually are.

@kd0aij
Copy link
Contributor

kd0aij commented Nov 26, 2020

I'm beginning to think my sitl/RF8 config is broken because neither the Cat nor the SkyCat (the SkyCat is very sluggish in roll and yaw) seem to behave at all like you and Marco describe. I did try running SITL on my Linux box with gigabit ethernet from it to the Windows laptop, but the oscillation was the same even though the frame rate increased to 300Hz.

@kd0aij
Copy link
Contributor

kd0aij commented Nov 29, 2020

Well, today the Cat flies OK with RF8 physics at 250Hz and "reasonable" PID gains. No idea what's changed.

@Hwurzburg
Copy link
Collaborator

Hwurzburg commented Dec 17, 2020

Tested this rebased to master today on my S-800 TVBS...the control surface scaling was off, but I needed the corrected tilt scaling (back to stable?) for the TVBS to fly safely in QLOITER moves....also, getting pitch tuned was impossible on master for the TVBS....
I set pitch tune back to defaults and test flew with this PR....was in control in QLOITER all the time, although both FX and VTOL axes need better tuning, it felt safe enough to do a complete VTOL->FW-VTOL LAND mission and was well behaved thru the mission including Loitering to the VTOL land point and landing...if nothing else the tilt gain scaling fix needs to be in master ASAP

@robustini
Copy link
Contributor

With Realflight I have not encountered any problems with this feature.

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Dec 22, 2020
@tridge tridge merged commit 6f92e62 into ArduPilot:master Dec 22, 2020
@IamPete1 IamPete1 deleted the disk_theory_gain_scailing branch December 22, 2020 00:01
@Hwurzburg Hwurzburg removed the WikiNeeded needs wiki update label May 31, 2021
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.

6 participants