Multicopter mixer saturation handling strategy -- continue #9062

bresch · 2018-03-12T13:56:17Z

Follows #7920
Rebased onto master

Works on:

When MC_AIRMODE is disabled, it is possible to tune the attitude/rate controllers really safely since no positive boosting will be applied by the mixer. This has the consequence to be able to cut completely the throttle of all the motors (idle) if the RC throttle is at zero.

bkueng · 2018-03-16T19:24:16Z

src/lib/mixer/mixer_multirotor.cpp

-		roll_pitch_scale = (thrust + boost) / (thrust - min_out);
+	if (!_airmode) {
+		// disable positive boosting if not in air-mode
+		// boosting is positive when min_out < 0.0


I suggest a minor wording update to: boosting can only be positive when min_out < 0.0, because boosting can also be negative when min_out < 0 (in case of delta_out_max > 1.0f).

MaEtUgR

Thanks for starting a new pr which is clear again.
I hope my questions are useful.

I have to add that if you need to change 6 drivers to introduce one bool available to the mixer that's according to my understanding an architecture fault. Maybe we can improve this in the future.

MaEtUgR · 2018-03-17T08:59:22Z

src/lib/mixer/mixer.h

+	/**
+	 * @brief Set airmode. Airmode allows the mixer to increase the total thrust in order to unsaturate the motors.
+	 *
+	 * @param[in]  airmode   Dis/-activate airmode by setting it to false/true


I'd suggest De/-activate or Diasable/activate. I've never seen disactivate before.

MaEtUgR · 2018-03-17T09:41:51Z

src/lib/mixer/mixer_multirotor.cpp

-		boost = math::constrain(-(max_out - 1.0f - min_out) / 2.0f, -max_thrust_diff, 0.0f);
-		roll_pitch_scale = (1 - (thrust + boost)) / (max_out - thrust);
+	} else {
+		roll_pitch_scale = 1 / (delta_out_max);


Please make it a float literal 1.f for consistency.

MaEtUgR · 2018-03-17T12:28:10Z

src/lib/mixer/mixer_multirotor.cpp

-		roll_pitch_scale = (1 - (thrust + boost)) / (max_out - thrust);
+	} else {
+		roll_pitch_scale = 1 / (delta_out_max);
+		boost = 1.0f - ((max_out - thrust) * roll_pitch_scale + thrust);


I'm not sure what this formula is for...
When min and max are symetric with respect to the middle (thrust) then it's correct but can be simplified to boost = 0.5f - thrust;

In an asymetric case, which I suppose the math is for, the result is incorrect according to my analysis. When I find any mistake I might have made or a better solution I'll suggest it.

I found that the calculation here: https://github.com/PX4/Firmware/pull/9062/files#diff-40032af52ac957eae697d7d0a1856ef2R252 only scales roll and pitch (like the name suggests). This makes things unnecessary complicated when the mixer is not symetric.

If it's symetric boosting the thrust to be 0.5 is the right solution because all the authority gets used.

If it's not symetric it would be a lot easier to directly generate the correct output instead of rerunning and rescaling the whole mixer... Similar to what I did here: https://github.com/PX4/Firmware/pull/8485/files#diff-40032af52ac957eae697d7d0a1856ef2R215

Hopefully I'll get around to do a suggestion in a pr.

This formula is for the asymmetric case and is from the constraint
(max_out - thrust*thrust_scale)*roll_pitch_scale + (thrust + boost)*thrust_scale = 1.0.
Assuming the thrust_scale is 1.0, the constraint is
(max_out - thrust)*roll_pitch_scale + thrust + boost = 1.0
then
boost = 1 - (max_out - thrust)*roll_pitch_scale - thrust.

It could also be simplified by using the min output:
(min_out - thrust)*roll_pitch_scale + thrust + boost = 0
which gives
boost = -(min_out - thrust)*roll_pitch_scale - thrust

@bresch Yeah I found all these variants also with the min:

boost1 = 1-thrust-roll_pitch_scale*(max_out - thrust); boost2 = -thrust -roll_pitch_scale*(min_out - thrust);

but my script still gave the wrong output and just now when I wrote with you on slack I realized my script had a mistake in it. Sorry for the churn, it's correct.

MaEtUgR · 2018-03-17T12:32:35Z

src/drivers/snapdragon_pwm_out/snapdragon_pwm_out.cpp

@@ -355,6 +373,10 @@ void task_main(int argc, char *argv[])
 	// Main loop
 	while (!_task_should_exit) {

+		if (_mixer) {
+			_mixer->set_airmode(airmode);


Why does it need to be set at every loop iteration and not only on parameter change?

This is currently needed, because a mixer can dynamically be loaded with an ioctl. In that case the parameter would not be applied to the new mixer. I agree it's not nice, but it's ok for now.

MaEtUgR · 2018-03-17T12:35:21Z

src/modules/mc_att_control/mc_att_control_params.c

+/**
+ * Multicopter air-mode
+ *
+ * The air-mode enables the mixer to increase or decrease the total thrust of the multirotor


Maybe the comment should mention that it's only disabling "upwards boost".
I think it's the right way to do it but the comment right now says increase or decrease.

bkueng · 2018-03-23T08:37:46Z

Test flights:

MC_AIRMODE=0 in Manual: https://logs.px4.io/plot_app?log=57456ec3-b4b3-4a2b-b05b-f3f43fe04b08
MC_AIRMODE=1 in Acro/Manual: https://logs.px4.io/plot_app?log=6811dca1-18ba-42a4-8755-52b754e76fec

It works as expected.

…ible

… saturation handling

- fmu: in case of _mot_t_max==0 _airmode was not set in the mixer - px4io: param_val is a float

Added in 262d9c7.

this avoids calling param_get() on every loop iteration.

If not in air-mode the mixer is not able to apply positive boosting and roll_pitch_scale is recomputed to apply symmetric - reduced - thrust. This has the consequence to cut completely the outputs when the thrust is set to zero.

bresch · 2018-03-23T08:58:59Z

Thanks @bkueng and @MaEtUgR for the review, I made the changes and rebased onto master.
@bkueng thanks for the test flight!

mhkabir · 2018-03-24T04:44:34Z

Can we get some docs for this soon? :) Not super clear for an end user yet.

With the updated mixer (#9062) it's safe to set maximum thrust to 1 (in both cases, if MC_AIRMODE is set, or not set). So I see no reason to limit the maximum thrust.

bresch requested review from bkueng and MaEtUgR March 12, 2018 13:56

bresch added Admin: Enhancement (improvement) 💡 Multirotor 🛸 Hybrid VTOL 🛩️🚁 Multirotor + Fixedwing! labels Mar 12, 2018

bresch added this to the Release v1.8.0 milestone Mar 12, 2018

bresch force-pushed the pr-mc-mixer-sat branch 2 times, most recently from 8940046 to c5406a7 Compare March 12, 2018 14:08

bkueng reviewed Mar 16, 2018

View reviewed changes

MaEtUgR reviewed Mar 17, 2018

View reviewed changes

bresch and others added 17 commits March 23, 2018 09:48

Multicopter mixer - Always unsaturate low-saturated motors when possible

b59cd4c

Multicopter mixer - Rewrite unnecessarily complicated conditions

5e02cbf

Multicopter mixer - Always unsaturate high-saturated motors when poss…

50d521e

…ible

Multicopter mixer - Remove arbitraty boost gain during saturation

1d1f375

Multicopter mixer - Simplify and correct mistakes of roll-pitch motor…

0728034

… saturation handling

Multicopter mixer - Use already computed value instead of recomputing it

077ca2f

Airmode - Add airmode parameter for multicopter mixer

9332881

Airmode - Add support for UAVCAN

57edc89

fmu + px4io: fixes for MC_AIRMODE parameter

7552f4d

- fmu: in case of _mot_t_max==0 _airmode was not set in the mixer - px4io: param_val is a float

mixer_multirotor: fix comment thrust_gain -> thrust_scale

d716637

Added in 262d9c7.

cleanup uavcan_main: replace warnx with PX4_{INFO,ERR,DEBUG}

c988ba2

uavcan_main: use parameter_update to check for param updates

57c5bdd

this avoids calling param_get() on every loop iteration.

snapdragon_pwm_out: add MC_AIRMODE support

abc9502

linux_pwm_out: add MC_AIRMODE support

e78872e

PWMSim : add MC_AIRMODE support

67ea636

Airmode - Minor rewording

6821696

bresch force-pushed the pr-mc-mixer-sat branch from ec35e10 to 6821696 Compare March 23, 2018 08:57

RomanBapst merged commit 81a80e0 into PX4:master Mar 23, 2018

dagar mentioned this pull request Mar 24, 2018

MultiRotor: Unstable manual flight #9150

Closed

MaEtUgR mentioned this pull request Mar 26, 2018

mc_att_control: revert to tested cutoff frequency #9162

Merged

bkueng added a commit that referenced this pull request Apr 23, 2018

MC: set default MPC_THR_MAX & MPC_MANTHR_MAX to 1

e69f420

With the updated mixer (#9062) it's safe to set maximum thrust to 1 (in both cases, if MC_AIRMODE is set, or not set). So I see no reason to limit the maximum thrust.

bkueng mentioned this pull request Apr 23, 2018

MC: set default MPC_THR_MAX & MPC_MANTHR_MAX to 1 #9359

Merged

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

Multicopter mixer saturation handling strategy -- continue #9062

Multicopter mixer saturation handling strategy -- continue #9062

bresch commented Mar 12, 2018 •

edited by AuterionWrikeBot

Loading

bkueng Mar 16, 2018

MaEtUgR left a comment •

edited

Loading

MaEtUgR Mar 17, 2018

MaEtUgR Mar 17, 2018

MaEtUgR Mar 17, 2018 •

edited

Loading

MaEtUgR Mar 19, 2018 •

edited

Loading

bresch Mar 19, 2018

MaEtUgR Mar 19, 2018 •

edited

Loading

MaEtUgR Mar 17, 2018

bkueng Mar 23, 2018

MaEtUgR Mar 17, 2018 •

edited

Loading

bkueng commented Mar 23, 2018

bresch commented Mar 23, 2018

mhkabir commented Mar 24, 2018

Multicopter mixer saturation handling strategy -- continue #9062

Multicopter mixer saturation handling strategy -- continue #9062

Conversation

bresch commented Mar 12, 2018 • edited by AuterionWrikeBot Loading

bkueng Mar 16, 2018

Choose a reason for hiding this comment

MaEtUgR left a comment • edited Loading

Choose a reason for hiding this comment

MaEtUgR Mar 17, 2018

Choose a reason for hiding this comment

MaEtUgR Mar 17, 2018

Choose a reason for hiding this comment

MaEtUgR Mar 17, 2018 • edited Loading

Choose a reason for hiding this comment

MaEtUgR Mar 19, 2018 • edited Loading

Choose a reason for hiding this comment

bresch Mar 19, 2018

Choose a reason for hiding this comment

MaEtUgR Mar 19, 2018 • edited Loading

Choose a reason for hiding this comment

MaEtUgR Mar 17, 2018

Choose a reason for hiding this comment

bkueng Mar 23, 2018

Choose a reason for hiding this comment

MaEtUgR Mar 17, 2018 • edited Loading

Choose a reason for hiding this comment

bkueng commented Mar 23, 2018

bresch commented Mar 23, 2018

mhkabir commented Mar 24, 2018

bresch commented Mar 12, 2018 •

edited by AuterionWrikeBot

Loading

MaEtUgR left a comment •

edited

Loading

MaEtUgR Mar 17, 2018 •

edited

Loading

MaEtUgR Mar 19, 2018 •

edited

Loading

MaEtUgR Mar 19, 2018 •

edited

Loading

MaEtUgR Mar 17, 2018 •

edited

Loading