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

Improve fixed wing trim throttle compensation & geneal refactoring #19623

Merged
merged 9 commits into from
Jul 12, 2022

Conversation

RomanBapst
Copy link
Contributor

@RomanBapst RomanBapst commented May 10, 2022

  1. Introduced trim throttle compensation based on vehicle weight. Weight can be specified via WEIGHT_BASE and WEIGHT_GROSS parameters.
  2. Cleaned up trim throttle compensation based on air density.
  3. Get rid of trim throttle update via position setpoint triplet. Navigator can command speed setpoints which will take effect if an airspeed estimate is available. Airspeed can either be measured by an airspeed sensor or obtained as a synthetic airspeed measurements (see ASPD_PRIMARY parameter).

Weight compensation
Introduce WEIGHT_BASE and WEIGHT_GROSS parameters which are the nominal weight (weight at which the vehicle was tuned) and the actual weight of the vehicle. The two parameters define a weight ratio which are used to adjust the minimum equivalent stall speed based on the relation between weight_ratio and stall speed.

$$V_{stall_{new}} = {V_{stall} \sqrt{n m \over M}} $$ where $m$ is the actual weight of vehicle, $M$ the weight of vehicle during tuning and $n$ the load factor

Test data / coverage
SITL

Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

I don't like it too much that the groundstation is allowed to set such a low-level control value as cruise throttle. And that currently cruise_airspeed and cruise_throttle aren't linked. Maybe we can bring the airspeed-less speed setting closer to the with-airspeed one going forward (doesn't have to be this PR). E.g. the user airspeeds setpoints even in airspeedless mode, they get passed to the FW Position controller that then translates it to throttle setpoints (e.g. assuming linear correlation) that are then applied in both cases, w and w/o airspeed.

@tstastny
Copy link

tstastny commented May 10, 2022

E.g. the user airspeeds setpoints even in airspeedless mode, they get passed to the FW Position controller that then translates it to throttle setpoints (e.g. assuming linear correlation) that are then applied in both cases, w and w/o airspeed.

this is the way

throttle setpoints

let's call it throttle trims -- the final setpoint is still determined with that plus feedback term from TECS

Copy link
Member

@Jaeyoung-Lim Jaeyoung-Lim left a comment

Choose a reason for hiding this comment

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

If I understand correctly, this would be a permanent change of the vehicle cruise throttle throughout the flight unless the vehicle has rebooted right?

Would there be a way that we can reset the cruise throttle to default?

@RomanBapst
Copy link
Contributor Author

RomanBapst commented May 10, 2022

@tstastny @sfuhrer @Jaeyoung-Lim Providing a short summary of things I have already discussed with some of you:

I will change this PR to enable speed changes via direct airspeed setpoints also for the airspeed-less case.
This allows the same API to be used and is generally cleaner.
There will be a mapping from airspeed setpoint to cruise throttle which TECS will use to adjust cruise throttle based on calibrated airspeed setpoints. The benefit is also that TECS will automatically scale cruise throttle based on air density as that is already part of the conversion from calibrated to true airspeed setpoint.

@RomanBapst RomanBapst force-pushed the pr-fw_set_cruise_throttle branch 2 times, most recently from 59e4f51 to 7f3ae51 Compare May 24, 2022 08:20
@RomanBapst RomanBapst changed the title Support cruise throttle changes via VEHICLE_CMD_DO_CHANGE_SPEED Support airspeed <-> trim throttle curve and variable vehicle weight May 24, 2022
Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

I think the general architecture looks good. The formulas would need to be checked for physical correctness. Could you update the RP description? Really cool would be a flow diagram showing the control flows (airspeed setpoint from user to FW_POS_C, trim limits/ trim_throttle at trim_airspeed are adapted by FW_POS_C based on air density, passed into TECS where the trim_throttle is mapped to the airspeed setpoint etc).
BTW, would maybe be cleaner to do all the throttle compensation/mapping inside TECS (thus also the air density scaling). ATM it's kind of hard to understand if a "throttle_trim" variable is linked to a param, scaled by air density, scaled by airspeed, scaled by weight etc.

@RomanBapst RomanBapst force-pushed the pr-fw_set_cruise_throttle branch from d0382ae to 53cb529 Compare May 25, 2022 06:08
@RomanBapst RomanBapst requested a review from priseborough May 25, 2022 07:36
@RomanBapst RomanBapst force-pushed the pr-fw_set_cruise_throttle branch from 56e45b6 to 1d26ab9 Compare June 10, 2022 11:06
@RomanBapst RomanBapst force-pushed the pr-fw_set_cruise_throttle branch from fc2a907 to d056a88 Compare June 19, 2022 10:31
@RomanBapst RomanBapst force-pushed the pr-fw_set_cruise_throttle branch from d056a88 to 89810fb Compare July 5, 2022 16:52
@RomanBapst
Copy link
Contributor Author

@tstastny @sfuhrer I did another pass here after some outdoor testing two weeks ago. The conclusion from the testing was that it makes more sense to invest efforts into obtaining a synthetic airspeed estimate and let TECS figure out the cruise throttle.
We did use the synthetic wind estimate of the airspeed selector and were quite happy with the results once the wind speed converged. The advantage of this is that the airspeed can also be used to scale the actuators which is important when a range of airspeed setpoints needs to be supported.

For the synthetic airspeed we still need to work on faster convergence, handling the case when we know that the wind estimate has not converged yet (e.g. right after transition) and potentially some tricks to speed up the convergence (e.g. fusing airspeed setpoints in the wind estimator when flying level).

I therefore removed the mapping from airspeed setpoint to trim throttle in this PR as it will become somewhat obsolete with the synthetic airspeed.
Please have another look at it.

Copy link

@tstastny tstastny left a comment

Choose a reason for hiding this comment

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

Looks nice, just some minor comments. Now that the airspeed -> throttle mapping is removed, you may want to update the PR description again though, just for future folks looking back

Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

Yes please update the tilte, description and possibly clean up the commits.
Also go through all the comments, there are some of me that are still valid from May. And close them when addressed/commented.
So with the mapping removed: for flying with variable airspeed setpoints we want to rely purely on the throttle integrator instead of modelling the drag?
I think generally your new approach via the synthetic airspeed seems more light weight, generic and doesn't require tuning (good!), but as you also say we prob need to put in some more effort into making it robust and finding reasonable safety margins for the airspeed less flight.

@RomanBapst RomanBapst changed the title Support airspeed <-> trim throttle curve and variable vehicle weight Improve fixed wing cruise throttle compensation & geneal refactoring Jul 7, 2022
@RomanBapst RomanBapst changed the title Improve fixed wing cruise throttle compensation & geneal refactoring Improve fixed wing trim throttle compensation & geneal refactoring Jul 7, 2022
Signed-off-by: RomanBapst <bapstroman@gmail.com>
- allow compensation based on vehicle weight (parameterized)
- use density for calculating trim throttle compensation instead of pressure
- removed parameter FW_THR_ALT_SCL

Signed-off-by: RomanBapst <bapstroman@gmail.com>
…oint triplet

- trim throttle is handled entirely by the position controller
- navigator should use speed setpoints
- added flag gliding_enabled in position setpoint to stills support gliding

Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
Signed-off-by: RomanBapst <bapstroman@gmail.com>
@RomanBapst RomanBapst force-pushed the pr-fw_set_cruise_throttle branch from abfbcd8 to 9691271 Compare July 7, 2022 10:09
@RomanBapst
Copy link
Contributor Author

@Jaeyoung-Lim Could you help me figure out if I didn't break gliding?

@RomanBapst
Copy link
Contributor Author

@tstastny @sfuhrer I think I addressed all of your inputs.

@RomanBapst
Copy link
Contributor Author

@Jaeyoung-Lim This is the commit which I hope preserves gliding. e86d2fa

Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

Looks good to me, my comment about cruising_throttle can be addressed in a follow up.

@mrivi mrivi merged commit a23cb11 into main Jul 12, 2022
@mrivi mrivi deleted the pr-fw_set_cruise_throttle branch July 12, 2022 07:07
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.

5 participants