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

[WIP] VTOL coordinate FW and MC rate controllers only #9190

Closed
wants to merge 24 commits into from

Conversation

dagar
Copy link
Member

@dagar dagar commented Mar 29, 2018

In current VTOL during transitions the VTOL "attitude controller" determines the attitude setpoint and both full FW & MC attitude controllers run respecting that setpoint. There are various issues with this especially for tailsitters (#9189).

In this PR only the rate controllers are allowed to run simultaneously. During transition that means the MC attitude controller is running, but the FW attitude controller is now in a state where it's only running the rate controller and respecting published rate setpoints (coming from the MC controller).

@RomanBapst I believe this was working with VTOL standard, but the rebase was nasty so proceed like it's completely untested.

@dagar
Copy link
Member Author

dagar commented Mar 29, 2018

If we go forward with this I'd also like to consider removing the thrust setpoint from the attitude setpoint and rates setpoint messages.

vehicle_thrust_setpoint

  • float thrust[3]

Then I'd like to use it to initialize the position controllers correctly.

  • TECS after transition to FW
  • TECS after FW manual, acro, rattitude, or stabilize mode
  • MC thrust after acro or rattitude

@RomanBapst
Copy link
Contributor

@dagar I have now the fw rate controller running in hover for the tailsitter. In SITL we don't model any slipstream so I got the following funny behavior.
https://www.youtube.com/watch?v=9fkTu6M0Dzc&feature=youtu.be

_roll_ctrl.set_bodyrate_setpoint(_rates_sp.roll);
vehicle_rates_setpoint_poll();

if (_parameters.vtol_type == vtol_type::TAILSITTER) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Can we handle all of these adjustments right after the orb_copy? Then the actual controller code can be generic.

@RomanBapst
Copy link
Contributor

@dagar Successfully tested in SITL for standard and tiltrotor, tailsitter works in manual but still got to find a bug in mission. Once we get the desired behavior we should discuss some specifics we might want to solve cleaner.

@RomanBapst
Copy link
Contributor

@dagar This now contains changes required to use the duorotor tailsitter in stabilized mode (including transitions and fw flight).
There are a few things I needed to hack and which probably break the other vtol types- we should have a look at them and discuss how to solve the properly.

@@ -574,6 +602,10 @@ void FixedwingAttitudeControl::run()
perf_count(_nonfinite_input_perf);
}

if (_vehicle_status.in_transition_mode || !_vehicle_status.is_rotary_wing) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@dagar This is kind of a gain scheduling. In transition and fw mode the fw controller gains get scaled down as if the vehicle was flying at max airspeed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should be tailsitter only though right? I think we should get all the vtol (and tailsitter) special cases centralized in a function outside of the main loop. Then a single check for tailsitter that corrects everything (attitude, control mode, airspeed).

_att_sp.timestamp = hrt_absolute_time();

/* reset velocity limit */
_vel_max_xy = _params.vel_max_xy;
}

/* reset setpoints and integrators VTOL in FW mode */
if (_vehicle_status.is_vtol && !_vehicle_status.is_rotary_wing) {
if (_vehicle_status.is_vtol && (!_vehicle_status.is_rotary_wing || _vehicle_status.in_transition_mode)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@dagar This was required to reset the yaw_sp after a transition, otherwise the vehicle would mostly do a 180 in yaw.
Need to be done properly, other vtol types need the controller to run during transition.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh okay, let's get it back in.

vehicle_control_mode_poll();
vehicle_manual_poll();
vehicle_status_poll();
vehicle_land_detected_poll();

// set initial maximum body rate setpoints
_roll_ctrl.set_max_rate(_parameters.acro_max_x_rate_rad);
Copy link
Member Author

Choose a reason for hiding this comment

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

The existing set_max_rate() update code below doesn't handle this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@dagar Nope, it does not. The problem is that for VTOL the condition
_vcontrol_mode.flag_control_attitude_enabled != _flag_control_attitude_enabled_last
does not hold because since the mc attitude controller is in charge _vcontrol_mode.flag_control_attitude_enabled is false in the beginning.
So you either have to switch to fw mode first of you have to change a parameter.
Without this change all of the max rates of the rate controller were set to zero, so it essentially did nothing.

@dagar
Copy link
Member Author

dagar commented Apr 5, 2018

@thomasgubler thomasgubler added this to the Release v1.7.4 milestone Apr 11, 2018
@dagar
Copy link
Member Author

dagar commented Apr 12, 2018

Take a look at the MC offboard attitude control CI failure?
http://ci.px4.io:8080/blue/organizations/jenkins/Firmware/detail/PR-9190/11/pipeline

image

@jlecoeur
Copy link
Contributor

Good to see that you are pushing towards separated XYZ thrust setpoints. I am sure that will make the transition phase easier to handle. For quadplane, it may simplify things like X thrust aid to fight headwind during hover, or Z thrust aid to prevent falling after wing stall.
I have few comments:

  1. Thrust frame
    The X Y and Z thrust components should be in front-right-down body frame. That means you will need to multiply thrust_z by -1.

  2. Actuators[3]
    Should actuator_controls also contain X Y and Z thrust? As of now it contains THROTTLE. I think this is fine to update it in a later pass, but then we have to agree on the sign. I would say that throttle is always positive, even if thrust_z is negative. (i.e. actuator_controls.controls[3] = + thrust_x and actuator_controls.controls[3] = - thrust_z). The advantage is that the mixers do not have to be changed. Let me know if you want to add XYZ thrust in the actuator_controls because I did it in another branch.

  3. Tailsitter body frame
    This more a question than a comment. Why do tailsitters use different body frames during hover and forward flight? Hovering with body X axis pointed up should not be a problem if the mc attitude controller uses quaternions all the way.

  4. Simultaneous rate controllers
    Is it planned to also prevent fw rate controller and mc rate controller from running simultaneously? In the tailsitter, both control the same set of actuators, so it seems that the original issue VTOL: Control architecture limitation for tailsitters #9189 is unsolved?

@RomanBapst
Copy link
Contributor

@jlecoeur Good to see you also participating in this!

Thrust frame
The X Y and Z thrust components should be in front-right-down body frame. That means you will need to multiply thrust_z by -1.

Yes, I totally agree. This is still on my TODO list for this PR.

Actuators[3]
Should actuator_controls also contain X Y and Z thrust? As of now it contains THROTTLE. I think this is fine to update it in a later pass, but then we have to agree on the sign. I would say that throttle is always positive, even if thrust_z is negative. (i.e. actuator_controls.controls[3] = + thrust_x and actuator_controls.controls[3] = - thrust_z). The advantage is that the mixers do not have to be changed. Let me know if you want to add XYZ thrust in the actuator_controls because I did it in another branch.

I think it should. So what if we want to support reverse thrust as well? In that case throttle would need to cover negative ranges as well, right? I'm also thinking about vehicles with variable pitch props.
Can you give me the link to your branch so that I can have a look?

Tailsitter body frame
This more a question than a comment. Why do tailsitters use different body frames during hover and forward flight? Hovering with body X axis pointed up should not be a problem if the mc attitude controller uses quaternions all the way.

The reason why it was done like this was because it was the easiest solution at that time I guess.
But I would also prefer if we don't have to transform attitudes between attitude controllers.
Given that the fixed wing attitude controller is euler angle based we should definitely choose the 'fixed wing' frame as reference (neutral attitude is when fixed wing is level).
Should we discuss this together? I'm available for a skype call or we can discuss it on the dev call.
We should also sync with @dagar

Simultaneous rate controllers
Is it planned to also prevent fw rate controller and mc rate controller from running simultaneously? In the tailsitter, both control the same set of actuators, so it seems that the original issue #9189 is unsolved?

For the tailsitter, why do you think they control the same sets of actuators? In that case the multirotor rate controller controls motors and the fw rate controller controls elevons.
The idea was to run them simultaneously either because it's needed (e.g. a duorotor tailsitter needs elevons in hover to control pitch and yaw) or because it might be beneficial (quadplane elevons can help stabilize during transition).

@jlecoeur
Copy link
Contributor

  1. all right!
  2. Yes, I would rather have XYZ thrust added to actuators_controls, and throttle removed. Reverse thrust would be done with negative thrust_x, and inverted multirotor flight with positive thrust_z.
    Here is the branch where I added 3d thrust: Multirotor mixers 6dof jlecoeur/Firmware#1.
  3. let's discuss during the dev call
  4. ok my bad I was under the impression that both rate controllers controlled the motors. So in forward flight is this the mc rate controller that provides thrust command?

@dagar
Copy link
Member Author

dagar commented Apr 12, 2018

  1. Yes, thanks for clarifying this. My first pass was lazy find and replace.

  2. I'm interested in reverse thrust for planes in the near future.

Should we implement a vehicle_thrust_setpoint message in this PR and get the thrust out of the attitude and rate setpoints entirely?

I was putting off the actuator control changes for now as that will have ramifications for the mixers (depending on how we do it).

RomanBapst and others added 17 commits September 16, 2018 12:40
tailsitters

Signed-off-by: Roman <bapstroman@gmail.com>
- TODO: we should define a frame, e.g. NED.

Signed-off-by: Roman <bapstroman@gmail.com>
… frame

Signed-off-by: Roman <bapstroman@gmail.com>
Signed-off-by: Roman <bapstroman@gmail.com>
…tter

- enable yaw control via mc roll actuator to coordinate turns in fw mode

Signed-off-by: Roman <bapstroman@gmail.com>
Signed-off-by: Roman <bapstroman@gmail.com>
Signed-off-by: Roman <bapstroman@gmail.com>
Signed-off-by: Roman <bapstroman@gmail.com>
Signed-off-by: Roman <bapstroman@gmail.com>
- this means hover and transition modes for vtol

Signed-off-by: Roman <bapstroman@gmail.com>
- we cannot have position control running during transitions because it's
not aware of the forward flight capabilities. Therefore, tell the controller
to just control altitude.

Signed-off-by: Roman <bapstroman@gmail.com>
…ules

for vtol

Signed-off-by: Roman <bapstroman@gmail.com>
- thrust_z should be negative for positive throttle

Signed-off-by: Roman <bapstroman@gmail.com>
@dagar
Copy link
Member Author

dagar commented Sep 16, 2018

@RomanBapst I rebased on master and fixed a few minor issues. The remaining SITL test failures are real.

Copy link
Contributor

@jlecoeur jlecoeur left a comment

Choose a reason for hiding this comment

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

LGTM except for the minor comments.

@dagar I guess that the next logical step relative to 3D thrust will be to pass the 3 thrust components through actuator_controls and let the mixer allocate it to lifting/pushing actuators?

float32 thrust # Thrust in Newton the power system should generate
float32 thrust_x # Throttle command in body x direction [-1,1]
float32 thrust_y # Throttle command in body y direction [-1,1]
float32 thrust_z # Throttle command in body z direction [-1,1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding ", thrust_z is negative for upwards thrust."

# TOPICS vehicle_rates_setpoint mc_virtual_rates_setpoint fw_virtual_rates_setpoint
float32 thrust_x # Throttle command in body x direction [-1,1]
float32 thrust_y # Throttle command in body y direction [-1,1]
float32 thrust_z # Throttle command in body z direction [-1,1]
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as in vehicle_attitude_setpoint

@@ -749,7 +749,7 @@ MulticopterPositionControl::task_main()
matrix::Quatf q_sp = matrix::Eulerf(_att_sp.roll_body, _att_sp.pitch_body, _att_sp.yaw_body);
q_sp.copyTo(_att_sp.q_d);
_att_sp.q_d_valid = true;
_att_sp.thrust = 0.0f;
_att_sp.thrust_z = 0.0f;
Copy link
Contributor

Choose a reason for hiding this comment

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

General comment: should we ensure that thrust components X and Y are equal to 0 in MC (and that components Y and Z are null in FW)?

// MC
_att_sp.thrust_x = 0.0f;
_att_sp.thrust_y = 0.0f;
_att_sp.thrust_z = value;

Copy link
Contributor

Choose a reason for hiding this comment

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

@jlecoeur good point! Will do.

@dagar
Copy link
Member Author

dagar commented Sep 19, 2018

@RomanBapst do you have an example failure log review (possible stability regression mentioned on the dev call)?

@stale
Copy link

stale bot commented Jan 20, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Feb 4, 2019

Closing as stale.

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