Scale FW cruise throttle based on baro pressure #8304

sanderux · 2017-11-17T11:45:09Z

This provides a method to scale cruise throttle based on altitude AMSL.
Mostly useful for flying without airspeed sensor on varying altitudes.

SITL tested and disabled by default.
Would be great if this could still make 1.7

dagar · 2017-11-17T15:48:56Z

This seems fine, although shouldn't it be done in the position controller? The position controller depends on position (obviously), deals with altitude, and actively controls throttle. The attitude controller really shouldn't have any dependency on these things.

sanderux · 2017-11-17T17:01:06Z

I was trying to keep all throttle scaling together. It had global pos anyway for groundspeed

LorenzMeier · 2017-11-17T17:06:52Z

@priseborough any feedback?

dagar · 2017-11-17T17:10:48Z

Manual mode and stabilized should maintain manual throttle passthrough. For example think about something going wrong with the global position (altitude) estimate.

I actually don't think any throttle scaling should happen at this level, but that's another discussion.

LorenzMeier · 2017-11-17T17:20:02Z

I agree with the safety argument. And I think the throttle scaling should definitely come out of TECS, as this is an alternate mode of controlling altitude.

sanderux · 2017-11-17T18:00:19Z

Ok, i'll move it to pos ctl. I dont think tecs has absolute altitude.

priseborough · 2017-11-17T20:13:15Z

A complication is that the TECS total energy controller for throttle contains an integrator which needs to know if throttle is saturated.

Additionally TECS relies on throttle_max passed through being the throttle required to climb at FW_T_CLMB_MAX and throttle_min being the throttle required to descend at FW_T_SINK_MIN when flying at the current speed demand.

This means corrections for the change in performance with altitude should be achieved by modifying the throttle_min and throttle_max value passed through to TECS, rather than scaling the TECS throttle output.

sanderux · 2017-11-17T21:33:18Z

Moved to pos ctl

sanderux · 2017-11-17T21:38:55Z

@priseborough For now its only extending an already existing scaler on throttle (voltage).
Your point on min and max values i understand, however, we (and with me i think many people) have these set 'safe' so altitude is accounted for.

priseborough · 2017-11-20T22:30:37Z

Can you at least limit the scale adjustment so that physical throttle won't clip before TECS throttle has reached its own upper limit.

priseborough · 2017-11-21T06:20:22Z

src/modules/fw_pos_control_l1/FixedwingPositionControl.cpp

+		if (orb_copy(ORB_ID(sensor_baro), _sensor_baro_sub, &baro) == PX4_OK) {
+			// scale throttle as a function of sqrt(p0/p) (~ EAS -> TAS at low speeds and altitudes ignoring temperature)
+			const float scale = constrain(sqrtf(MSL_PRESSURE_MILLIBAR / baro.pressure), 0.9f, 1.1f);
+			throttle_cruise = throttle_cruise * scale;


throttle_max should also be scaled up to a max value of 1.0 to prevent possible loss of speed an stalling during climbs at altitude.

Can you elaborate on 1.0? shouldnt the same scaling apply? or do you mean as a lower limit?

Or do you mean an absolute value of 1.0f ?

priseborough · 2017-11-21T06:22:35Z

src/modules/fw_pos_control_l1/fw_pos_control_l1_params.c

+ * @boolean
+ * @group FW L1 Control
+ */
+PARAM_DEFINE_INT32(FW_THR_ALT_SC_EN, 0);


Thus should be a float that multiplies by the pressure ratio to set the scale correction. A fixed ratio of 1 is not going to work for all propulsion types.

priseborough · 2017-11-21T06:24:41Z

At the moment It's only adjusting throttle_cruise up and down. It should also adjust throttle_max up to a maximum of 1.0, otherwise there may be insufficient throttle to maintain safe airspeed during climbs without an airspeed sensor.

Also, why make the gain from pressure ratio to throttle either 1.0 or nothing? A unity scaling from pressure ratio to throttle scaler seems arbitrary and will not work for all propulsion types.

sanderux · 2017-11-21T08:32:10Z

@priseborough i'll address your comments, they make sense.
Should we also scale thr_min?

bresch · 2017-11-21T09:10:50Z

@sanderux Good idea, we should also add this for the multicopter part (but in the mixer since the input of the multicopter mixer are forces and moments). It would be nice to scale with the air temperature too (sqrt(T/T_ref)) if a reliable measurement is available.

priseborough · 2017-11-21T09:39:16Z

I don't think we have a basis for specifying how to scale throttle_min. It would depend on whether you were using the propeller as a brake or a thruster during that flight condition.

Also have you thought about using the eas2tas instead of pressure ratio? With voltage or RPM control of the motor and a fixed pitch prop this does have a physical basis and will compensate for loss of air density and the higher TAS required at higher air temperatures.

sanderux · 2017-11-22T08:59:46Z

@priseborough this method was mostly meant for flying without airspeed sensor. I will address your scaling issue and apply to throttle_max too

priseborough · 2017-11-22T10:00:53Z

Sorry, I forgot that eas2tas wasn't available without an airspeed sensor.

sanderux · 2017-11-23T15:38:23Z

@dagar @priseborough changes made, please do another pass

dagar · 2017-11-23T16:39:13Z

src/modules/fw_pos_control_l1/fw_pos_control_l1_params.c

+ * When flying without airspeed sensor this will help to keep a constant performance over large altitude ranges.
+ *
+ * A value of 0 will disable scaling.
+ * A value of 1 will add 1% throttle for every 1% of pressure descrease relative to standard pressure (1013.25)


Param description out of date.

Whats out of date about it? i just rewrote it

dagar · 2017-11-23T20:33:44Z

I fixed the trivial CI failure (formatting), moved the scaling to within the constrain, and added finite checks for the baro and input parameter. I also increased the scaling maximum incase you want to go flying on everest or something.

I still think the param description could be improved, but I'm fine with merging this.

@priseborough @bresch any suggestions for describing the scaling parameter?

Scale throttle by pressure

This parameter enables throttle scaling based on barometric pressure
When flying without airspeed sensor this will help to keep a constant performance over large altitude ranges.

A value of 0 will disable scaling.
A value of 1 will add 1% throttle for every 1% of pressure descrease relative to standard pressure (1013.25)

It seems to me we should be saying something about propulsion types.

priseborough · 2017-11-23T22:11:57Z

src/modules/fw_pos_control_l1/FixedwingPositionControl.cpp

+				const float scale = constrain(eas2tas * _parameters.throttle_alt_scale, 0.9f, 1.8f);
+
+				throttle_cruise = throttle_cruise * scale;
+				throttle_max = throttle_max * scale;


limit throttle_max upper value to 1.0 and limit upper value of throttle_cruise to be lower than throttle_max.

priseborough · 2017-11-23T22:12:42Z

I'm OK with describing what it does and leaving it to the user to tune. It is an advanced user parameter and set to 0 by default. We can update the description with suggested values when we have some flight test data of performance modelling for different propulsion system setups.

sanderux · 2017-11-23T22:53:29Z

Looks good to me now

priseborough · 2017-11-24T02:23:44Z

DO NOT PUSH this PR

priseborough · 2017-11-24T02:28:35Z

My previous comment has not been addressed. It still allows throttle_cruise = throttle_max which will cause a div0 condition here: https://github.com/PX4/ecl/blob/master/tecs/tecs.cpp#L351

LorenzMeier

Please address the concerns from @priseborough.

LorenzMeier · 2017-11-24T08:08:02Z

@sanderux Please handle this with priority - we are now ready to release otherwise and it would be bad if your users would have a suboptimal experience without this patch.

sanderux · 2017-11-24T08:12:53Z

I'm on it

sanderux · 2017-11-24T08:43:21Z

@LorenzMeier @dagar @priseborough
I changed the constrain to always be at least 1% above throttle_min and 1% below throttle_max

sanderux · 2017-11-24T08:44:04Z

SITL tested, all still works as expected

see other approval

dagar · 2017-11-24T19:36:44Z

Looks good, but we never check it otherwise (param updates, or DO_CHANGE_SPEED). I'm going to do the constrain in general in another PR>

sanderux added Admin: Enhancement (improvement) 💡 Fixed Wing 🛩️ Hybrid VTOL 🛩️🚁 Multirotor + Fixedwing! labels Nov 17, 2017

sanderux added this to the Release v1.7.0 milestone Nov 17, 2017

sanderux requested review from dagar and LorenzMeier November 17, 2017 11:45

sanderux changed the title ~~Added param to allow throttle scaling based on altitude ASML~~ Added param to allow throttle scaling based on altitude AMSL Nov 17, 2017

sanderux force-pushed the pr-fw_thr_alt_scl branch from af61fef to b1474b0 Compare November 18, 2017 22:48

sanderux changed the title ~~Added param to allow throttle scaling based on altitude AMSL~~ Scale FW cruise throttle based on baro pressure Nov 18, 2017

sanderux force-pushed the pr-fw_thr_alt_scl branch 2 times, most recently from 573c319 to 52f8fbc Compare November 21, 2017 01:28

priseborough reviewed Nov 21, 2017

View reviewed changes

sanderux force-pushed the pr-fw_thr_alt_scl branch from b49e377 to f72f4d7 Compare November 23, 2017 15:40

dagar reviewed Nov 23, 2017

View reviewed changes

dagar force-pushed the pr-fw_thr_alt_scl branch 2 times, most recently from ac632c3 to 2b18cda Compare November 23, 2017 20:32

priseborough reviewed Nov 23, 2017

View reviewed changes

dagar force-pushed the pr-fw_thr_alt_scl branch from 2b18cda to ef6a134 Compare November 23, 2017 22:27

dagar previously approved these changes Nov 24, 2017

View reviewed changes

LorenzMeier previously requested changes Nov 24, 2017

View reviewed changes

sanderux dismissed dagar’s stale review via 7f5c0e3 November 24, 2017 08:42

sanderux force-pushed the pr-fw_thr_alt_scl branch from ef6a134 to 7f5c0e3 Compare November 24, 2017 08:42

Scale FW cruise throttle based on baro presure

c2a2033

sanderux force-pushed the pr-fw_thr_alt_scl branch from 7f5c0e3 to c2a2033 Compare November 24, 2017 08:45

priseborough approved these changes Nov 24, 2017

View reviewed changes

sanderux merged commit b305900 into master Nov 24, 2017

sanderux deleted the pr-fw_thr_alt_scl branch November 24, 2017 15:34

Scale FW cruise throttle based on baro pressure #8304

Scale FW cruise throttle based on baro pressure #8304

Conversation

sanderux commented Nov 17, 2017 • edited Loading

dagar commented Nov 17, 2017

sanderux commented Nov 17, 2017

LorenzMeier commented Nov 17, 2017

dagar commented Nov 17, 2017

LorenzMeier commented Nov 17, 2017

sanderux commented Nov 17, 2017

priseborough commented Nov 17, 2017

sanderux commented Nov 17, 2017

sanderux commented Nov 17, 2017

priseborough commented Nov 20, 2017

priseborough Nov 21, 2017

Choose a reason for hiding this comment

sanderux Nov 21, 2017

Choose a reason for hiding this comment

sanderux Nov 21, 2017

Choose a reason for hiding this comment

priseborough Nov 21, 2017

Choose a reason for hiding this comment

priseborough commented Nov 21, 2017

sanderux commented Nov 21, 2017

bresch commented Nov 21, 2017

priseborough commented Nov 21, 2017

sanderux commented Nov 22, 2017

priseborough commented Nov 22, 2017

sanderux commented Nov 23, 2017

dagar Nov 23, 2017

Choose a reason for hiding this comment

sanderux Nov 23, 2017

Choose a reason for hiding this comment

dagar commented Nov 23, 2017

priseborough Nov 23, 2017

Choose a reason for hiding this comment

priseborough commented Nov 23, 2017 • edited Loading

sanderux commented Nov 23, 2017

priseborough commented Nov 24, 2017

priseborough commented Nov 24, 2017

LorenzMeier left a comment

Choose a reason for hiding this comment

LorenzMeier commented Nov 24, 2017

sanderux commented Nov 24, 2017

sanderux commented Nov 24, 2017

sanderux commented Nov 24, 2017

dagar commented Nov 24, 2017

sanderux commented Nov 17, 2017 •

edited

Loading

priseborough commented Nov 23, 2017 •

edited

Loading