Remove Euler angle rate names

[CarlOlsson]

Describe problem solved by the proposed feature
Right now in vehicle_attitude we have rollspeed, pitchspeed and yawspeed. This is confusing since it is not the derivative of the Euler angles.

Describe your preferred solution
Rename

rollspeed -> angular_rate_p
pitchspeed -> angular_rate_q
yawspeed -> angular_rate_r

Describe possible alternatives

rollspeed -> angular_rate_body_x
pitchspeed -> angular_rate_body_y
yawspeed -> angular_rate_body_z

Additional context
I started to make a quick PR for this but then I realized that right now in vehicle_attitude "yawspeed" means angular rate around body z, but in vehicle_local_positoin_setpoint there is also "yawspeed" which I guess is rate of change of heading. Do you want to keep this name or rename that to something else?

[julianoes]

@bkueng how bad is a change like this from a log tooling perspective?

[julianoes]

@CarlOlsson angualar -> angular 😄

[LorenzMeier]

@CarlOlsson is your argument that we have an axis mix (which is btw typical usage in this industry) or are you referring to using sensor measurements of the derivative instead of the numerical derivative?

[CarlOlsson]

@CarlOlsson is your argument that we have an axis mix (which is btw typical usage in this industry) or are you referring to using sensor measurements of the derivative instead of the numerical derivative?

I am only talking about changing the naming convention, nothing else.
Not sure what you mean with axis mix but the issue is that for newcomers who are familiar with Euler angles it is confusing that the "rollspeed" is actually not the rate of change of the roll angle. For vehicles that don't experience large attitude changes during flight the difference is small, but for e.g. tailsitters it is completely different.

[bkueng]

@bkueng how bad is a change like this from a log tooling perspective?

It's doable.

[bresch]

in vehicle_attitude "yawspeed" means angular rate around body z, but in vehicle_local_positoin_setpoint there is also "yawspeed" which I guess is rate of change of heading

This is correct and we should double check that we are not mixing body yaw rate and Euler yaw rate in the controllers.

For the rest, I like the angular_rate_p/q/r naming

[jlecoeur]

In vehicle_local_position_setpoint, yawspeed is fine (heading_rate would work too). In vehicle_attitude however, I agree that what is now called rollspeed, pitchspeed and yawspeed are not the time derivatives of roll, pitch and yaw.

This has to be addressed for sake of correctness, but is not critical in my opinion, especially so as it involves a lot of name churn throughout the firmware and tools. Maybe this can be done synchronously with other big changes. For example, I know that @dagar was considering to split into separate topics the attitude, attitude setpoint, angular rate, angular rate setpoint, and thrust setpoints.

For what it's worth, I am ok with both x/y/z and p/q/r, with a slight preference for angular_rate_body_x/y/z because it is more explicit.

[CarlOlsson]

From @dagar on slack

an array is slightly easier for going back and forth to Vector3f

[dagar]

Architecturally I've been working towards cleaning up and splitting mc_att_control (#12225) so that things like the sensor_gyro selection + corrections don't need to live in the controller.

For that I'm trying to create a new standalone message for rates. I was thinking something like vehicle_angular_velocity or vehicle_angular_rate, but I'm open to suggestions.

With that in mind I'm not sure that vehicle_attitude should even contain rates. This rate data comes from integrated gyro mostly unfiltered (soon will be), passed around as delta angle, then back to angular rate for publication in the estimator with bias. I'm pretty sure what nearly all existing users of vehicle_attitude rates actually want is the filtered low latency stream I mentioned above.

[dagar]

From the existing PR (#12225) (WIP) that predates this issue.
https://github.com/PX4/Firmware/blob/cf50d8419ad80d62772c8541b828fb71168adad5/msg/vehicle_angular_velocity.msg#L2-L8

TODO:

• decide on message naming
• decide on individual fields on array + naming

[dagar]

Fixed by #12596.