Copter: Yaw-rate behavior in SET_ATTITUDE_TARGET changed in Guided

[ajshank]

Bug report

The new implementation of Mavlink's SET_ATTITUDE_TARGET (82) in Copter 4.2 has introduced a bug (or 'feature') -- it is no longer possible to set target angles for roll/pitch and only angular rate for yaw for the attitude controller.

In 4.1, the BODY_YAW_RATE_IGNORE field was used to select yaw vs yaw-rate. If ATTITUDE_IGNORE is false, and BODY_YAW_RATE_IGNORE is also false, 4.1's behavior was to choose yaw-rate over the yaw from attitude -- i.e., rate-ignores got priority.

In 4.2, the implementation seems to use both. In this commit, the behavior is different (and somewhat strange). It seems like the rate controller is called for all axes if input quaternion is invalid/zero, and not called if there's any valid attitude. This makes it impossible to run yaw-rate control without yaw. Furthermore, the documentation here may be outdated too.

We do a lot of high-level control which generates target roll/pitch angles, and target yaw-rates (sounds strange, but isn't unusual). It would be nice to retain support for independent yaw-rate control somehow.

Feature

I think there's a broader question here -- what should be done if both attitude and rates are provided (and neither are set to 'ignore'). I would not interpret rate to be the rate at which the provided attitude should be attained; but I suspect others might read it differently. @lthall, thoughts?

Version
4.2 // Copter 4.2.

Platform
[ ] All
[ ] AntennaTracker
[ x ] Copter
[ ] Plane
[ ] Rover
[ ] Submarine

[rmackay9]

Txs for the report, I've added it to the 4.2 issues list.

Just as a procedural matter normally we ask that issues are raised in the 4.2 forum but it's ok this time.

[lthall]

This is not a bug.

SET_ATTITUDE_TARGET (82) does not support roll or pitch inputs at all. There is no way to specify euler angles, much less partially.

Attitude is completly specified by a single quaternion. Attitude is either used or ignored.

Rate inputs can be ignored but the only way to do that in reality is to assume they are set to zero when ignored.

The commit you mentioned fixes the very old, very incomplete and very broken interpretation of SET_ATTITUDE_TARGET (82).

We do a lot of high-level control which generates target roll/pitch angles, and target yaw-rates (sounds strange, but isn't unusual). It would be nice to retain support for independent yaw-rate control somehow
This is not supported by SET_ATTITUDE_TARGET (82). You should be using Quaternion and angular vectors as is clearly specified in SET_ATTITUDE_TARGET (82). You will find you get much more precise and responsive control.

If you want to suggest some other mavlink message that does support roll/pitch yaw rate inputs you are welcome to open a feature request.

This is clearly not a bug and I am closing this issue.

[rmackay9]

I've re-opened this because I think we may need to provide an alternative that allows control of the vehicle using roll and pitch angles and yaw rate. So we can discuss what that is but we can't just consider this closed.

[ajshank]

Thanks very much, @lthall and @rmackay9.
I can't agree that there's no way to specify Euler angles -- clearly, the specified quaternion can be converted to roll, pitch and yaw angles. The problem is with the rate targets (particularly in yaw), and how they should work with a quaternion target. I agree that Mavlink's poor definition of (82) leaves this quite open to interpretation.

This is not supported by SET_ATTITUDE_TARGET (82). You should be using Quaternion and angular vectors as is clearly specified in SET_ATTITUDE_TARGET (82). You will find you get much more precise and responsive control.

It is "supported" in the sense that the message contains the field we need. We convert target angles to Quaternion, and have Copter ignore the yaw in it (because we are also sending target yaw-rate). I have used this from 3.5 to until 4.1, even before thrust-setpoint was supported. The implementation may have been hasty, but it provided a robust means to control the vehicle as a pilot would in STABILIZE mode.

Ideally, the Mavlink message should contain a mask for ignoring individual attitude axes (just like it does for individual rate axes). But, with the current SET_ATTITUDE_TARGET (82), Copter can, say, define three supported combinations:

• STABILIZE/ALTHOLD-style, where only roll/pitch are from quaternion, and yaw-rate is used.
• ACRO/SPORT-style, where only rates are used (require ATTITUDE_IGNORE).
• STABILIZE-style, but with full attitude from Quaternion (rates, if provided, used for feedforward).

I do understand the difficulties in making this explicit and unambiguous to all users though. I am happy to work towards this.

[lthall]

I can't agree that there's no way to specify Euler angles -- clearly, the specified quaternion can be converted to roll, pitch and yaw angles.

Yes but it can't be converted to just roll and pitch angles, by definition. The Quaternion defines the complete attitude without any ambiguity.

I agree that Mavlink's poor definition of (82) leaves this quite open to interpretation.

This is actually a very good and VERY clear specification. The message clearly defines both the instantaneous attitude and/or angular velocity. This is exactly what is needed for fast precise attitude control.

It is "supported" in the sense that the message contains the field we need.

No, it isn't supported in even the most imaginative interpretation of the message. It is ridiculous to suggest that it is acceptable to butcher this specification as you describe here.

I have used this from 3.5 to until 4.1, even before thrust-setpoint was supported. The implementation may have been hasty, but it provided a robust means to control the vehicle as a pilot would in STABILIZE mode.

And now the message is implemented properly and you can implement attitude control much more effectively and significantly more responsively. If you have been using this for that long it will not be a problem for you. In fact, if you have been using it for this long I am surprised you aren't dancing with joy that we finally fixed it properly.

This was discussed on the dev call again today and like each time before the conclusion was the same. This message has been implemented correctly and will not be changed. Using this message as it is right now you are able to implement any type of attitude control system you want with simple transformations at your end.

[ajshank]

@lthall Thanks for offering your thoughts on this.
I posit that Mavlink only offers message definitions, and as such, cannot be clear on what to do with it. More generally, though, the fact that it doesn't offer masking for attitude axes makes it incomplete for several use-cases. In the same vein, ATTITUDE_IGNORE should make no sense for a message that is called SET_ATTITUDE_TARGET, but we happily do rate control when attitude is ignored. And therefore, even without a "most imaginative" interpretation, the message supported yaw-rate control, as Copter had originally implemented. And it surely wasn't ridiculous to have the use_yaw_rate bool this way.

In fact, if you have been using it for this long I am surprised you aren't dancing with joy that we finally fixed it properly.

I haven't had the time, given that the "fix" actually breaks all of our current setup.
Adapting our outer-loops isn't a dramatic problem, but extensive retesting & re-verification across many platforms is a quite a bit. There's nothing to rejoice about, because the previous implementation wasn't even broken for a STABILIZE-like use-case.

I'm not calling the current implementation incorrect - something must've been lost in communication here if it has come across so (I've referred to this as a new 'feature' above), and I apologize if that's the case. My point, in fact, is that it departs from previous behavior without retaining any means to obtain that same behavior, and without a full documentation of the change. I am also not against the current implementation -- I am merely stating the fact that it leaves no option available to mimic STABILIZE or ALTHOLD-like control, for whatever reasons one might need it. Perhaps there's yet another DEV_OPTIONS field that can be used for this.

This message has been implemented correctly and will not be changed.

Thanks for such decisive clarification. Unfortunately, it isn't what one would expect from a massive open-source project.

[lthall]

I haven't had the time, given that the "fix" actually breaks all of our current setup.

You are welcome to not upgrade to 4.2. That would minimise any need for "extensive retesting & re-verification" to check all the other changes I made in 4.2.

Or you could maintain your own patch on 4.2 that maintains the previous behaviour. Open source is great that way.

I am sure you will enjoy the additional flexibility and control the complete support of this message brings when you get around to updating your systems to 4.2.

[ajshank]

Noting this for general reference (and for mine as well):
The "additional flexibility" you state is quite welcome, but if I'm not mistaken, this change will cause Guided_NoGPS to no longer work the same, and may render it almost useless. Since SET_ATTITUDE_TARGET is the only message this mode works with, an external computer can no longer regulate the vehicle's heading -- yaw will always be controlled by the autopilot using the target quaternion. This would mean that (1) yaw control is now split from an outer-loop, and (2) the autopilot will now need yaw information sent to it from whatever positioning source was being used.

Here, (1) is definitely problematic for those who've worked with 4D control so far [3D pos + yaw]. And, with (2), might as well send positions too and move to purely Guided then.
Furthermore, this doc on Guided_NoGPS will need to be updated.

[ajshank]

Also, yes, I will need to implement some fix for this, because 4.2 is pretty great; all the work the devs and testers have put into it is phenomenal. Having previously maintained patches for this message (to enable ThrustAsThrust and lift_max), I would've preferred to avoid doing so yet again. I'll wait around a bit to see if there's an alternative resolution.
Thanks.