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

Fixing and simplifying mavlink odometry handling #12793

Merged
merged 2 commits into from
Aug 30, 2019

Conversation

kamilritz
Copy link
Contributor

Fixes wrong usage of attitude quaternion in transforming odometry information from body to local frame and vice versa . The quaternion of the EKF state rotates from body frame to local frame and not the other way.
The number of supported frames in mavlink_receiver are reduced to one, to make it clearer what is expected from the FCU. A few of the droped cases where also not making sense.

Until now most external velocity information reaching the EKF was not correct. Luckily, velocity estimates from external sensors were not used in the EKF.

The name of some frames is also slightly adapted to follow the changes in mavlink/mavlink#1207 .

Copy link
Contributor

@jkflying jkflying left a comment

Choose a reason for hiding this comment

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

I'm a little concerned about dropping all of these frames. However it's difficult to know if unlikely they were even working, so I understand the reasoning.

@TSC21 any input?

src/modules/mavlink/mavlink_receiver.cpp Show resolved Hide resolved
jkflying
jkflying previously approved these changes Aug 30, 2019
Copy link
Contributor

@jkflying jkflying left a comment

Choose a reason for hiding this comment

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

This looks correct. As @TSC21 said in Slack VIO channel, one working implementation is better than a bunch of broken ones 😉

mhkabir
mhkabir previously approved these changes Aug 30, 2019
@kamilritz kamilritz dismissed stale reviews from mhkabir and jkflying via 7d9756e August 30, 2019 12:37
Copy link
Member

@TSC21 TSC21 left a comment

Choose a reason for hiding this comment

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

Looks good.

@TSC21 TSC21 requested review from jkflying and mhkabir August 30, 2019 12:55
@LorenzMeier LorenzMeier merged commit 9ed2dae into PX4:master Aug 30, 2019
@@ -2,7 +2,7 @@

uint64 timestamp # time since system start (microseconds)

float32[4] q # Quaternion rotation from NED earth frame to XYZ body frame
float32[4] q # Quaternion rotation from XYZ body frame to NED earth frame.
Copy link
Member

@MaEtUgR MaEtUgR Aug 31, 2019

Choose a reason for hiding this comment

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

@kamilritz Good catch, we're consistently using hamilton quaternion convention since PX4/PX4-Matrix#34 which has the 4 properties:

  • the first element is real [w, x, y, z]
  • right-handedness i * j = k
  • passive operation, vectors stay we just change frame
  • q rotates from local to global q * [0; v_local] * q* = v_global

I always overlooked the comment that was introduced at the same time.

bozkurthan pushed a commit to bozkurthan/Firmware that referenced this pull request Sep 4, 2019
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.

6 participants