fw_att_control: schedule trims for airspeed and flap deployment #8937

tstastny · 2018-02-22T12:27:02Z

Currently, there are only one set of actuator trims for the FW controller, and rate integrators are relied on to retrim in various off-nominal conditions. Depending on the platform and the extent of the flight envelope in which one needs to operate, performance can be improved by scheduling trims via airspeed, then only relying on the integrator for smaller corrections. This is especially the case for landing configurations where flaps will e.g. change the required elevator trim. This PR incorporates scheduling of trims for these cases, but not changing the nominal operation for users who do not need these additions.

Additions:

Adds trim settings for minimum and maximum airspeed and bi-linearly interpolates with the airspeed (nominal) trim.
Adds a delta-trim for flap deployment, adding this increment whenever flaps are deployed.

Notes:

These trims are applied in all modes *except manual, where only the nominal trims are applied.
If the trims for vmin and vmax are both zero for a given actuator - the nominal trim is applied

tstastny · 2018-02-22T12:28:35Z

FYI - @acfloria and I will be going to test these changes in a flight test tomorrow - will upload flight logs afterwards. But if there are any major change recommendations that could be made prior to this - I could try to incorporate them before the flight test (sorry for short notice).

dagar · 2018-02-22T14:56:09Z

src/modules/fw_att_control/fw_att_control_params.c

+ * @decimal 2
+ * @increment 0.01
+ */
+PARAM_DEFINE_FLOAT(DTRIM_PITCH_FLPS, 0.0f);


Prefix with FW_

dagar · 2018-02-22T14:57:45Z

CI failure is code style (http://ci.px4.io:8080/blue/organizations/jenkins/Firmware/detail/PR-8937/1/pipeline). If you install astyle you can check or fix this locally (make check_format, make format).

acfloria · 2018-02-24T15:48:19Z

We did some test flight today (manual and auto mission. Due to the strong wind we were not able to tune and test the airspeed dependent trims in flight.

The airspeed dependent trims have been bench-tested and it worked. If it is required for merging we could also upload those logs.

bresch · 2018-02-26T13:56:35Z

src/modules/fw_att_control/FixedwingAttitudeControl.cpp

+				}
+
+				/* add trim increment if flaps are deployed  */
+				trim_roll += _flaps_applied * _parameters.dtrim_roll_flaps;


Do you really need a trim on roll since the flaps should be quite symmetric?

Ideally - yes the flaps should be a symmetric effect, but can be (depending on how large the flap setting is) that there is some non-symmetric aerodynamic behavior from the platform geometry (or potentially just simply the servos not going to exactly the same place). I'm not opposed to removing it though - as normally, as you say, it should be fine only to trim out the elevator. Thoughts?

I would say that we could leave it like that; even if I think that directly trimming the servos of the flaps would be cleaner, it might be easier do trim them this way.
If we see that it's not useful, we can still remove it later.

bresch · 2018-02-26T13:59:44Z

@tstastny Really good idea! Hope that you would have test flight data soon. Why not having that on manual mode? Safety?

tstastny · 2018-02-26T17:00:35Z

@bresch, please see the comment/logs from @acfloria - we went out to fly on Saturday.

We did some test flight today (manual and auto mission. Due to the strong wind we were not able to tune and test the airspeed dependent trims in flight.

No issues on the general mode switching etc side of things. Unfortunately could not directly trim out the scheduling.. as it was way too turbulent and windy to tune.. but I have also confirmed on the bench that for the given airspeeds and flap settings the trims go where they are suppose to. (I could upload a log of this as well if needed).

On not putting it in manual - yes, for safety, I think it's better not to be resetting the i/o trims for special conditions, and then have an FMU failure where these trims are now stuck in the pass through. Rather - the pilot should always have the same effect in manual as what would happen in failsafe. This is perhaps partially also more of a philosophical opinion, however. I think manual should be what it claims -- direct actuator control. All other additions just aid in stabilized or any other controlled modes.

bresch

lgtm

tstastny · 2018-03-01T10:17:47Z

@dagar any other comments, or ok to merge?

dagar · 2018-03-04T23:10:57Z

src/modules/fw_att_control/FixedwingAttitudeControl.cpp

+				float trim_yaw = _parameters.trim_yaw;
+
+				if (airspeed < _parameters.airspeed_trim) {
+					trim_roll = math::gradual(airspeed, _parameters.airspeed_min, _parameters.airspeed_trim,


It's fine for now, but we should make an effort to do this kind of thing when the actual data updates (orb_update) rather than every iteration.

dagar · 2018-03-04T23:13:59Z

src/modules/fw_att_control/FixedwingAttitudeControl.cpp

+	param_get(_parameter_handles.trim_yaw_vmax, &(_parameters.trim_yaw_vmax));
+
+	if (fabsf(_parameters.trim_yaw_vmin) < 0.01f && fabsf(_parameters.trim_yaw_vmax) < 0.01f) {
+		_parameters.trim_yaw_vmin = _parameters.trim_yaw;


I don't think you should mess with the param "value" like this. Put the check where you're adding the trim rather than trying to pass through the regular trim.

dagar · 2018-03-04T23:16:27Z

A couple minor things, but overall looks good. I don't really like the discrepancy between TRIM_{ROLL, PITCH, YAW}, and now these FW specific values. I'm wondering if we should bring TRIM_{ROLL, PITCH, YAW} into FW for consistency. They're already effectively FW parameters, but it gets a bit weird because the IO also needs them.

This definitely doesn't feel right (although we don't need to fix it here). https://github.com/PX4/Firmware/blob/master/src/drivers/px4io/px4io.cpp#L1145-L1185

tstastny · 2018-03-05T04:39:54Z

I don't think you should mess with the param "value" like this. Put the check where you're adding the trim rather than trying to pass through the regular trim.

@dagar - I attempted moving the param checks as you requested - however it's overflowing (flash) on px4fmu-v2_default. ideas on how to solve this? or should i revert back to how it was?

dagar · 2018-03-05T04:48:51Z

Is it rebased on master?

tstastny · 2018-03-05T04:51:45Z

Is it rebased on master?

yes

dagar · 2018-03-05T05:18:48Z

Alright, let me see what else I can find. There are a couple of larger cleanup PRs on the go that will provide some space, but they'll take a bit longer to merge.

dagar · 2018-03-05T06:20:31Z

Can you rebase again?

dagar · 2018-03-07T05:48:52Z

src/modules/fw_att_control/FixedwingAttitudeControl.cpp

+				float trim_yaw = _parameters.trim_yaw;
+
+				if (airspeed < _parameters.airspeed_trim) {
+					if (fabsf(_parameters.trim_roll_vmin) >= 0.01f || fabsf(_parameters.trim_roll_vmax) >= 0.01f) {


Isn't 0 safe here?

the idea with this check is that the trims should be monotonically increasing or decreasing. so if someone leaves them both zero - it effectively just means that whatever the nominal trim was set to is the only trim for all airspeeds (the standard case up until now). this is mostly just to not effect anyone's current setup if they have trims they like that arent scheduled, and maybe don't notice the new parameters. if either of the new trims (vmin or vmax) are set to another value, we are assuming the user specifically did this because they wanted to - and then it is their responsibility to make sure the values make sense from that point.

to maybe explain better - think of the case that someone doesnt know these trims exist.. and they have a nominal trim of 0.15 on the elevator, if we dont have this check to disable the new trims -- then the controller would be reducing the elevator trim to 0 in both minimum and maximum airspeed cases -- which i have a hard time thinking of a plane that exhibits this non monotonic trimming behavior

I'm simply suggesting skipping the checks and always calling the gradual. The result should be the same, and it's easier to read, less code, and negligible computationally.

If we skip the checks, then the unset trim vmin and vmax will be used in the gradual. That is what I'm trying to avoid. The checks are necessary regardless of if we skip the gradual, or, when the checks show trim vmax and vmin disabled, redefining the vmax and vmin trims as equal to the nominal trim and proceeding through the gradual with a flat line to interpolate on.

@dagar I understand your concern if this was only one parameter in the condition. The point here is that it takes **both max and min params to be set to 0 for them not to be used. the only case where this would be an issue, is if someone actually wanted min=0, max=0, and nom=anything else. But I would argue (as i did previously) that this will *never be the case for anyone properly using airspeed dependent trims. they should be, in the nominal operating region, monotonic - which means the 0 - some value - 0 case will not happen. otherwise -- if someone who has one trim value set (only the nominal) doesnt want to even bother with airspeed dependent trims, leaving the min and max at the default of zero means it wont affect him/her.

this does *not prohibit someone from using trim_pitch_max = 0, so long as trim_pitch_min is set to anything else that makes sense (i.e., anything besides 0 -- if trim_pitch_min=0 as well.. then trim_pitch_nom should also=0 -- again the monotonic argument).

if (_parameters.trim_roll_vmin < 0.01f) {
_parameters.trim_roll_vmin = _parameters.trim_roll;
}

also -- just to be clear / make sure we are on the same page -- @bresch forgot the second condition in this if statement in his comment -- i.e. I originally had it like this:

if ( fabsf(_parameters.trim_roll_vmin) < 0.01f && fabsf(_parameters.trim_roll_max) < 0.01f ) { _parameters.trim_roll_vmin = _parameters.trim_roll; _parameters.trim_roll_vmax = _parameters.trim_roll; }

If put up where the parameters are defined.

Yes that works, it's just unintuitive. So you set the max to 0 and it's working as expected, later you decide to kill the min trim because you didn't want it for whatever reason, but now your max has also gone away? I don't mean to be so difficult here, I'm trying to anticipate all the various ways things are misused, and won't' necessarily make sense until reading the code.

What if we applied these relative to the main TRIM? TRIM_X is your baseline, then FW_TRIM_X_VMIN/FW_TRIM_X_VMAX are some amount of additional trim applied linearly as you approach the min/max airspeeds.

roll min = TRIM_ROLL + FW_TRIM_R_VMIN roll trim = TRIM_ROLL roll max = TRIM_ROLL + FW_TRIM_R_VMAX

sure. this can also work. maybe also more robust to people not understanding exactly what to expect. then we get rid of the checks all together, and just assume that if someone sets the min/max trim deltas different from zero -- they should know what they are doing.

if these are now added deltas to the nominal trims, like the ones for flaps -- should we then change names to "DTRIM" (or anything similar)?

Great, yes whichever name you think makes the most sense. DTRIM or something mentioning additional trim.

dagar · 2018-03-16T05:03:18Z

Looks good to me. Let's make sure it works and get this in.

tstastny · 2018-03-16T17:52:29Z

Logs here: https://review.px4.io/plot_app?log=6bfbeda8-00da-478b-8ff8-857f2da2474e

Couldn't figure out how to do flaps with the gamepad. But set some elevator max, nom, and min trims according to airspeed and flew around in SITL stabilized. Note I killed all integrators to make sure the effect was directly visible -- the deviations about the main curve (which follows the airspeed trend) are simply either me given wobbly signals on the elevator stick, or any damping effects from the rate controller. So seems to work as expected.

Airspeed:

Elevator:

The flap trim code hasn't, however, changed from the last flight test we did on the previous commit. The trim increments were appropriately applied with the given flap settings during that test.

dagar reviewed Feb 22, 2018

View reviewed changes

dagar added Admin: Enhancement (improvement) 💡 Fixed Wing 🛩️ labels Feb 22, 2018

tstastny force-pushed the fw_trim_scheduling branch from a6de262 to 2396e3e Compare February 22, 2018 21:06

bresch reviewed Feb 26, 2018

View reviewed changes

bresch previously approved these changes Feb 27, 2018

View reviewed changes

dagar reviewed Mar 4, 2018

View reviewed changes

tstastny dismissed bresch’s stale review via e7ad668 March 5, 2018 03:28

tstastny force-pushed the fw_trim_scheduling branch from 2396e3e to e7ad668 Compare March 5, 2018 03:28

tstastny force-pushed the fw_trim_scheduling branch 2 times, most recently from a28b588 to 06e875e Compare March 5, 2018 19:42

dagar reviewed Mar 7, 2018

View reviewed changes

tstastny force-pushed the fw_trim_scheduling branch from 06e875e to cb18209 Compare March 12, 2018 13:23

fw_att_control: schedule trims for airspeed and flap deployment

fff19a0

tstastny force-pushed the fw_trim_scheduling branch from fd84a25 to fff19a0 Compare March 16, 2018 00:50

dagar approved these changes Mar 16, 2018

View reviewed changes

dagar merged commit 52e5e0d into PX4:master Mar 16, 2018

tstastny deleted the fw_trim_scheduling branch March 20, 2018 09:54

mrpollo mentioned this pull request Jun 5, 2018

v1.8 Changelog scratchpad #9593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fw_att_control: schedule trims for airspeed and flap deployment #8937

fw_att_control: schedule trims for airspeed and flap deployment #8937

tstastny commented Feb 22, 2018 •

edited by AuterionWrikeBot

Loading

tstastny commented Feb 22, 2018

dagar Feb 22, 2018

dagar commented Feb 22, 2018

acfloria commented Feb 24, 2018

bresch Feb 26, 2018

tstastny Feb 26, 2018

bresch Feb 26, 2018

bresch commented Feb 26, 2018

tstastny commented Feb 26, 2018

bresch left a comment

tstastny commented Mar 1, 2018

dagar Mar 4, 2018

dagar Mar 4, 2018

dagar commented Mar 4, 2018 •

edited

Loading

tstastny commented Mar 5, 2018

dagar commented Mar 5, 2018

tstastny commented Mar 5, 2018

dagar commented Mar 5, 2018

dagar commented Mar 5, 2018

dagar Mar 7, 2018

tstastny Mar 7, 2018

tstastny Mar 7, 2018

dagar Mar 9, 2018

tstastny Mar 9, 2018

tstastny Mar 15, 2018

tstastny Mar 15, 2018

dagar Mar 15, 2018

tstastny Mar 15, 2018

dagar Mar 15, 2018

dagar commented Mar 16, 2018

tstastny commented Mar 16, 2018

fw_att_control: schedule trims for airspeed and flap deployment #8937

fw_att_control: schedule trims for airspeed and flap deployment #8937

Conversation

tstastny commented Feb 22, 2018 • edited by AuterionWrikeBot Loading

tstastny commented Feb 22, 2018

Choose a reason for hiding this comment

dagar commented Feb 22, 2018

acfloria commented Feb 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bresch commented Feb 26, 2018

tstastny commented Feb 26, 2018

bresch left a comment

Choose a reason for hiding this comment

tstastny commented Mar 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dagar commented Mar 4, 2018 • edited Loading

tstastny commented Mar 5, 2018

dagar commented Mar 5, 2018

tstastny commented Mar 5, 2018

dagar commented Mar 5, 2018

dagar commented Mar 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dagar commented Mar 16, 2018

tstastny commented Mar 16, 2018

tstastny commented Feb 22, 2018 •

edited by AuterionWrikeBot

Loading

dagar commented Mar 4, 2018 •

edited

Loading