Merge vtol mc motor handling methods #9028

dagar · 2018-03-05T21:10:11Z

Currently vtol standard disables MC motors by setting the max to PWM 950us, then re-enables them by setting it to 2000 us. PWM min is also overrides the min set in the interface. This PR changes that behaviour to use the configured PWM disarm value to disable the motors, and the set PWM max when re-enabling.

This was tested in a private branch a long time ago, and I tried to get it in with #8040, but it was never merged. As a result this needs to be carefully reviewed and tested from scratch.

With this in place we should be able to get rid of VT_IDLE_PWM_MC.

dagar · 2018-03-05T21:24:21Z

Tiltrotor::set_rear_motor_state() needs review/rethinking.

dagar · 2018-03-05T21:27:52Z

After code review this needs bench testing checking pwm info for each vtol type at startup, after transition to FW, after transition back.

RomanBapst · 2018-03-06T07:09:55Z

@dagar I'll have a look at this today.

RomanBapst · 2018-03-06T16:41:18Z

@dagar Looks good so far. One thing that you could add is to move the set_rear_motor_state() method from the tiltrotor class to the vtol_type class and give it a different name like, e.g. set_motor_state(). Actually now that I think of it we can probably merge all those methods which fiddle with the pwm state of the outputs and use a bitmask (we also already have that for the tiltrotor) to differentiate between the motors.
In the past I was working with a tailsitter which required switching off one motor during fixed wing flight and so I regretted not having the methods to do that in vtol_type.

RomanBapst

Looks good, see comment in conservation about merging with tiltrotor specific motor disable methods.

RomanBapst · 2018-03-08T07:45:55Z

@dagar If you rebase I can test it on the tiltrotor and we can get it in.

RomanBapst · 2018-03-14T20:36:13Z

@dagar I'll take care of this tomorrow.

dagar · 2018-03-15T01:01:05Z

Rebased, but still need to think about how tiltrotor is even supposed to function. Might need to keep VT_IDLE_PWM_MC after all.

RomanBapst · 2018-03-15T15:08:57Z

@dagar I went through this and noticed that there was a fundamental problem. The motors for tiltrotors and tailsitters where shut off in fixed wing flight which is ok for standard vtol but not for the former ones.

I've reworked this according to the following requirements:

We need to disable a selection of mc motors during the transition or in fixed wing mode. An example of this is the convergence vtol where the rear motor needs to be shut off in fixed wing flight.

Proposed solution:
I've added the function set_motor_state(const motor_state current_state, const motor_state next_state, const int value)

which allows to modify the state of a selection of motors (via the parameter VT_FW_MOT_OFFID).
This method was before only used for tiltrotors. However, the method is now implemented in VtolType and can be used to e.g. shut off the mc motors for a quad plane.
As suggested by your PR, this function preserves the maximum pwm value given by the system.

Idle speed for mc vs fw
While in mc mode you want the motors to spin you don't necessarily want this behavior in fw mode.

Solution:
As before we use set_idle_mc()/ set_idle_fw() to adjust the idle speed depending on which mode are in.
The following parameter consideration is important for the system to work:
PWM_MIN: defines the idle speed for fixed wing flight, so should be set such that motors stop when throttle is zero
VT_IDLE_PWM_MC: defnes the idle speed when in mc mode, so should be set such that motors spin when throttle is zero.

Important consideration:
Because the parameter VT_FW_MOT_OFFID was previously not used for standard vtol I added a check to the vtol module which hardcodes the parameter such that all motors will be disabled in fw mode. Otherwise we are going to break all existing setups which have this parameter set to a default value of zero.

RomanBapst · 2018-03-15T15:11:35Z

src/modules/vtol_att_control/standard.cpp

-			_flag_enable_mc_motors = false;
+		// in back transition we need to start the MC motors again
+		if (_motor_state != ENABLED) {
+			_motor_state = set_motor_state(_motor_state, ENABLED);


Thinking about passing the VT_FW_MOT_OFFID parameter as another argument here so that it become more clear that this methods sets the state of a motors selection.

dagar · 2018-03-15T16:56:52Z

I need to go through in detail, but this makes sense for now. We should have some more interesting options to consider once thrust is split x,y,z.

RomanBapst · 2018-03-23T10:48:46Z

@dagar Could you give this a review please? I tested the behavior with my tiltrotor and it seems to work fine.
It's important to test if the correct motors switch off during transition from mc to fw and back. Also the idle speed needs to work for both modes.
I tested the quadplane and the tailsitter in SITL which revealed some minor bugs I fixed.
As soon as you reviewed we should get real test flights in on all three platforms.

dagar · 2018-03-24T20:11:17Z

CI failure is real, but otherwise looks good so far.

This should work for now, but fundamentally it seems like at a lower level the system should already know which motors belong where.

For the shared FW/MC motors in tiltrotor and tailsitter do we need different PWM limits in each state?

dagar · 2018-03-24T20:17:46Z

src/modules/vtol_att_control/tiltrotor.cpp

@@ -337,18 +288,21 @@ void Tiltrotor::update_transition_state()
 		int rear_value = (1.0f - time_since_trans_start / _params_tiltrotor.front_trans_dur_p2) *
 				 (PWM_DEFAULT_MAX - PWM_DEFAULT_MIN) + PWM_DEFAULT_MIN;

-		set_rear_motor_state(VALUE, rear_value);
+
+		_motor_state = set_motor_state(_motor_state, VALUE, rear_value);


This is kind of unfortunate, but I guess we should preserve the existing behaviour for now.

dagar · 2018-03-24T20:20:01Z

src/modules/vtol_att_control/vtol_type.cpp

+	return ret == PX4_OK;
+}
+
+bool VtolType::set_idle_fw()


Why can't we do this with set_motor_state()?

@dagar The problem is that these are two different states which need to be tracked separately.

I mainly wanted to share as much of the code that calls that IOCTLS as we could. At a glance it looks like you've done that now.

@dagar Yep, my last commit does that -only one method is used to do ioctl to the pwm output device.

dagar · 2018-03-24T20:20:11Z

src/modules/vtol_att_control/vtol_type.cpp

@@ -263,5 +252,153 @@ void VtolType::check_quadchute_condition()
 			}
 		}
 	}
+}
+
+bool VtolType::set_idle_mc()


Why can't we do this with set_motor_state()?

RomanBapst · 2018-03-26T13:05:36Z

@dagar I totally agree with you that it's not the best solution when the vtol attitude controller directly alters the pwm limits in order to achieve the desired behavior.
The intent of this PR is not to fix this but rather make what we have usable for all three vtol types.

RomanBapst · 2018-03-26T13:17:00Z

@PX4/testflights Could you please test this on a standard VTOL?

IMPORTANT Please first do a bench test as follows:

switch to stabilized mode and arm system in hover mode
increase throttle to > 30 %
flip transition switch
verify that during the transition the mc motors are still working
verify that after the transition the mc motors stop and do not respond to throttle
(if you use an airspeed sensor then pinch the ram pressure tube to trigger the transition to fw mode)
flip the switch back to hover
verify that the mc motors come alive and control attitude

There should not be any difference in behavior compared to master. If you succeed with the bench tests I'd appreciate a test flight in which you transition from mc to fw a couple of times successively.

Avysuarez · 2018-03-27T19:43:54Z

Flights with Falcon Vertigo Vtol, pixhawk 1 (V2) Good flights, no issues.
https://review.px4.io/plot_app?log=2ad877f6-ba25-44a1-9d2c-5f85a1695390
https://review.px4.io/plot_app?log=da6a3d5c-3426-4df0-aa1f-6c8490ca1023
https://review.px4.io/plot_app?log=d5ff69d3-8035-4e31-a49f-c0fcd37b01d5

RomanBapst · 2018-03-28T08:11:23Z

@dagar I tested this again on the tricopter including bailing out of transitions from both mc and fw.
I fixed up a missing px4_close on the file descriptor.
Could you please review, it would like to bring it in then?

RomanBapst · 2018-04-03T07:54:19Z

@dagar Could you please give this a review if you have some time? I'd like to get it in and it avoids lots of code duplication.

dagar · 2018-04-04T01:36:10Z

Probably needs one more pass, but looks good.

RomanBapst · 2018-04-11T07:33:11Z

Tested on the tiltrotor once more, no issues found
https://logs.px4.io/plot_app?log=ae2b19fb-a22d-47e6-96ad-1a0abf91b348

RomanBapst · 2018-04-11T07:41:44Z

@dagar Could you have a look why CI fails, I don't understand it.

- moved methods used to modify channel maximum pwm value to VtolType class so that the can be shared by the differnt vtol types Signed-off-by: Roman <[email protected]>

to VtolType class Signed-off-by: Roman <[email protected]>

Signed-off-by: Roman <[email protected]>

- board config can define PX4_PWM_ALTERNATE_RANGES which needs to be known by the compiler prior to processing this file Signed-off-by: Roman <[email protected]>

- created one method which has access to the pwm output device Signed-off-by: Roman <[email protected]>

Signed-off-by: Roman <[email protected]>

dagar · 2018-04-12T05:45:41Z

Rebased, please check CI again.

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

dagar requested review from RomanBapst and bresch March 5, 2018 21:10

dagar force-pushed the pr-vtol_pwm_preserve branch from afc8174 to 84b39f6 Compare March 5, 2018 21:21

dagar mentioned this pull request Mar 5, 2018

convergence config: increase idle speed in mc mode #9024

Merged

dagar force-pushed the pr-vtol_pwm_preserve branch from 84b39f6 to 7f20813 Compare March 6, 2018 00:41

RomanBapst reviewed Mar 6, 2018

View reviewed changes

dagar force-pushed the pr-vtol_pwm_preserve branch from 7f20813 to 04c5ce7 Compare March 15, 2018 01:00

dagar added this to the Release v1.8.0 milestone Mar 15, 2018

RomanBapst force-pushed the pr-vtol_pwm_preserve branch from 04c5ce7 to a1bdaed Compare March 15, 2018 14:52

RomanBapst changed the title ~~VTOL preserve PWM min and max when enabling/disabling actuators~~ WIP: Merge vtol mc motor handling methods Mar 15, 2018

RomanBapst reviewed Mar 15, 2018

View reviewed changes

RomanBapst force-pushed the pr-vtol_pwm_preserve branch from 2920dcd to 5507d88 Compare March 23, 2018 08:45

dagar commented Mar 24, 2018

View reviewed changes

RomanBapst force-pushed the pr-vtol_pwm_preserve branch 2 times, most recently from 7d10266 to 936503e Compare March 26, 2018 12:50

RomanBapst changed the title ~~WIP: Merge vtol mc motor handling methods~~ Merge vtol mc motor handling methods Mar 26, 2018

RomanBapst force-pushed the pr-vtol_pwm_preserve branch from 936503e to 2613db5 Compare March 26, 2018 13:03

RomanBapst force-pushed the pr-vtol_pwm_preserve branch from 2613db5 to 8022708 Compare March 27, 2018 15:38

RomanBapst force-pushed the pr-vtol_pwm_preserve branch from 8022708 to 8b46978 Compare March 28, 2018 07:47

dagar mentioned this pull request Mar 30, 2018

VTOL standard preserve PWM_MIN and PWM_MAX #8040

Closed

RomanBapst force-pushed the pr-vtol_pwm_preserve branch from 8b46978 to cb49650 Compare April 3, 2018 07:52

RomanBapst force-pushed the pr-vtol_pwm_preserve branch from cb49650 to 939744c Compare April 10, 2018 12:44

RomanBapst force-pushed the pr-vtol_pwm_preserve branch from 939744c to f2da086 Compare April 11, 2018 07:34

dagar and others added 9 commits April 12, 2018 01:45

VTOL preserve PWM min and max when enabling/disabling actuators

cc846b3

vtol_att_control: merge methods to disable a selection of motors

bfb39b6

- moved methods used to modify channel maximum pwm value to VtolType class so that the can be shared by the differnt vtol types Signed-off-by: Roman <[email protected]>

vtol_att_control: move handling of motor state and idle speed selection

48302f7

to VtolType class Signed-off-by: Roman <[email protected]>

vtol_type: improve comments

f1b0f18

Signed-off-by: Roman <[email protected]>

vtol_type.h: initialise flag_idle_mc correctly

76c063a

Signed-off-by: Roman <[email protected]>

vtol_att_control: initialise VT_FW_MOT_OFFID correcly for standard vtol

29e4585

Signed-off-by: Roman <[email protected]>

drv_pwm_output: include board config header

97d7e57

- board config can define PX4_PWM_ALTERNATE_RANGES which needs to be known by the compiler prior to processing this file Signed-off-by: Roman <[email protected]>

vtol_type: avoid code duplication

5cfb07e

- created one method which has access to the pwm output device Signed-off-by: Roman <[email protected]>

vtol_type.cpp: close pwm output device file descriptor

cfa4d57

Signed-off-by: Roman <[email protected]>

dagar force-pushed the pr-vtol_pwm_preserve branch from f2da086 to cfa4d57 Compare April 12, 2018 05:45

RomanBapst merged commit c206e0f into master Apr 12, 2018

RomanBapst deleted the pr-vtol_pwm_preserve branch April 12, 2018 07:19

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

Merge vtol mc motor handling methods #9028

Merge vtol mc motor handling methods #9028

dagar commented Mar 5, 2018 •

edited by AuterionWrikeBot

Loading

dagar commented Mar 5, 2018

dagar commented Mar 5, 2018

RomanBapst commented Mar 6, 2018

RomanBapst commented Mar 6, 2018

RomanBapst left a comment

RomanBapst commented Mar 8, 2018

RomanBapst commented Mar 14, 2018

dagar commented Mar 15, 2018

RomanBapst commented Mar 15, 2018

RomanBapst Mar 15, 2018

dagar commented Mar 15, 2018

RomanBapst commented Mar 23, 2018 •

edited

Loading

dagar commented Mar 24, 2018

dagar Mar 24, 2018

dagar Mar 24, 2018

RomanBapst Mar 26, 2018

dagar Mar 26, 2018

RomanBapst Mar 26, 2018

dagar Mar 24, 2018

RomanBapst commented Mar 26, 2018

RomanBapst commented Mar 26, 2018

Avysuarez commented Mar 27, 2018

RomanBapst commented Mar 28, 2018

RomanBapst commented Apr 3, 2018

dagar commented Apr 4, 2018

RomanBapst commented Apr 11, 2018

RomanBapst commented Apr 11, 2018

dagar commented Apr 12, 2018

Merge vtol mc motor handling methods #9028

Merge vtol mc motor handling methods #9028

Conversation

dagar commented Mar 5, 2018 • edited by AuterionWrikeBot Loading

dagar commented Mar 5, 2018

dagar commented Mar 5, 2018

RomanBapst commented Mar 6, 2018

RomanBapst commented Mar 6, 2018

RomanBapst left a comment

Choose a reason for hiding this comment

RomanBapst commented Mar 8, 2018

RomanBapst commented Mar 14, 2018

dagar commented Mar 15, 2018

RomanBapst commented Mar 15, 2018

RomanBapst Mar 15, 2018

Choose a reason for hiding this comment

dagar commented Mar 15, 2018

RomanBapst commented Mar 23, 2018 • edited Loading

dagar commented Mar 24, 2018

dagar Mar 24, 2018

Choose a reason for hiding this comment

dagar Mar 24, 2018

Choose a reason for hiding this comment

RomanBapst Mar 26, 2018

Choose a reason for hiding this comment

dagar Mar 26, 2018

Choose a reason for hiding this comment

RomanBapst Mar 26, 2018

Choose a reason for hiding this comment

dagar Mar 24, 2018

Choose a reason for hiding this comment

RomanBapst commented Mar 26, 2018

RomanBapst commented Mar 26, 2018

Avysuarez commented Mar 27, 2018

RomanBapst commented Mar 28, 2018

RomanBapst commented Apr 3, 2018

dagar commented Apr 4, 2018

RomanBapst commented Apr 11, 2018

RomanBapst commented Apr 11, 2018

dagar commented Apr 12, 2018

dagar commented Mar 5, 2018 •

edited by AuterionWrikeBot

Loading

RomanBapst commented Mar 23, 2018 •

edited

Loading