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

Fix natnet2ivy rotation order #3231

Merged
merged 4 commits into from
Mar 1, 2024

Conversation

tblaha
Copy link
Contributor

@tblaha tblaha commented Jan 26, 2024

@tmldeponti recently found that the pitch and roll components of the natnet2ivy.py are sometimes wrong. Further investigation showed that this happens due to a wrong rotation order that shows up when not using options --x_side right and --ac_nose far, and when the craft is yawed away from pointing to the "north". Also, the yaw angle component seemed unaffected, which is why this bug hasn't shown up before (optitrack pitch/roll components are not typically used by estimators AFAIK?).

The diff looks huge, but actually the only functional change is post-multiplying q_nose_correction instead of pre-multiplying here:

quat = Quat(real=quat[3], imaginary=[quat[0], quat[1], quat[2]])
quat = q_nose_correction * (q_total * quat * q_total.inverse)
quat = quat.elements[[1, 2, 3, 0]].tolist()

To really make sure this time that it works for all options, I generated test cases, and encapsulated some more script code in functions so they can be tested if --test is given as argument (in which case all other arguments are ignored and only the tests are performed)

Todo:

  • test flight with --x_angle set to some numerical value

@fvantienen
Copy link
Member

fvantienen commented Jan 31, 2024

What about

? This seems incorrect I think?
In my opinion the message going to the drone should always be in 1 orientation such that this can be added to the messages.xml file.

Maybe we should add some description of the conversions also in the modules.

@tblaha
Copy link
Contributor Author

tblaha commented Feb 1, 2024

Thanks! Why is this different from ins_ekf2.cpp? And what is the purpose of gps_datalink.c if the INS also read that message directly?

sample.quat(0) = DL_EXTERNAL_POSE_body_qi(buf);
sample.quat(1) = DL_EXTERNAL_POSE_body_qy(buf);
sample.quat(2) = DL_EXTERNAL_POSE_body_qx(buf);
sample.quat(3) = -DL_EXTERNAL_POSE_body_qz(buf);

The way I see it, natnet2ivy.py has always been sending attitude in ENU, meaning the rotation of a Right-Forward-Up local frame with respect to the East-North-Up frame. This can be converted to Forward-Right-Down-wrt-North-East-Down by simple exchanging x and y, and negating z. (the same way as you would convert a position).
This choice is not configurable in natnet2ivy.py, but what is configurable is how that ENU frame is oriented with respect to the observer. But position and attitude should always be consistent in that frame.

@tblaha
Copy link
Contributor Author

tblaha commented Feb 1, 2024

Ah, I didnt see the theta=-theta line a few lines down:

orient.qi = quat_i;
orient.qx = quat_y; //north
orient.qy = -quat_x; //east
orient.qz = -quat_z; //down
float_eulers_of_quat(&orient_eulers, &orient);
orient_eulers.theta = -orient_eulers.theta;

Then it all makes sense, I think and is the same ins_ekf2.cpp and also the same as @tmldeponti 's change in #3233

@gautierhattenberger
Copy link
Member

Thanks for pointing this rotation error. After a recent update of Natnet, we have also a new version of NatNetClient.py, so we need to merge them correctly to check if this is still working for us. I'll let you know.

@tblaha
Copy link
Contributor Author

tblaha commented Feb 5, 2024

Hey Gautier! Perfect, I'm available for testing new stuff as well. On a related note, we have been developing a C++ Natnet client with to goal of keeping the CLI as similar as possible between different target robot/drone operating systems in operation at TU Delft. It's also not completely finished, and I'm not quite sure if it's possible/desirable to include it with paparazzi as well, but I'd like to hear your thoughts.

https://github.com/tudelft/UnifiedOptitrackClients

@Fabien-B Fabien-B merged commit 12da43e into paparazzi:master Mar 1, 2024
1 check passed
@Fabien-B Fabien-B mentioned this pull request Mar 1, 2024
@fvantienen fvantienen deleted the fix_natnet2ivy_rotations branch March 4, 2024 16:24
Dennis-Wijngaarden pushed a commit to tudelft/paparazzi that referenced this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants