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

Cleanup rotations to match MAV_SENSOR_ORIENTATION #14797

Merged
merged 5 commits into from
May 2, 2020

Conversation

baumanta
Copy link
Contributor

clean up rotation enum and fill in the remaining orientations from https://mavlink.io/en/messages/common.html#MAV_SENSOR_ORIENTATION, such that the enums correspond.

Add test to make sure the matrix rotation function yields the same result as the rotate_3f function.

@baumanta baumanta requested a review from MaEtUgR April 30, 2020 11:43
@baumanta baumanta changed the title Cleanup rotations upstream Cleanup rotations to match MAV_SENSOR_ORIENTATION Apr 30, 2020
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Thanks @baumanta !
Short summary:

  • enum numbering and supported rotations were synced with MAVLink implementation https://mavlink.io/en/messages/common.html#MAV_SENSOR_ORIENTATION
  • some rotate_3f implementations were simplified
  • all rotate_3f implementations get unit tested against the general euler angle lookup table implementation
  • rotate_3f implementation of ROTATION_PITCH_90_YAW_180 was wrong
  • Some rotations were named in the wrong euler angle order (not roll, pitch, yaw) e.g. ROTATION_YAW_293_PITCH_68_ROLL_90, ROTATION_PITCH_90_ROLL_90 and hence replaced by the correct order name equivalent already defined in MAVLink

It all looks correct to me now also the odd Solo external mag rotation from 94bfad5 doesn't break according to my check.

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.

3 participants