BUG:The operator function for quaternion division in the quaternion library seems to be incorrect

[SuperLei917]

I am currently studying EKF3 and have discovered a function in the magnetometer fusion section controlMagYawReset()
https://github.com/ArduPilot/ardupilot/blob/aabbe500aa4b682305b3062116edebc3fd0528c6/libraries/AP_NavEKF3/AP_NavEKF3_MagFusion.cpp#L42
Assuming two quaternions are defined as follows:

The product of the two is calculated as follows:

This is consistent with the multiplication operator function in the code, and there is no problem with it. But the subsequent division operator function seems incorrect. Before dividing quaternions, the code has already specified the normalization of quaternions. Combined with the inverse calculation of quaternions, the following can be obtained: In fact, it is the conjugate of y. So x/y is actually using the conjugate of y (y ^ -1) to replace y in the product result, and the calculation result is as follows: But this calculation result is not consistent with the operator function:

https://github.com/ArduPilot/ardupilot/blob/aabbe500aa4b682305b3062116edebc3fd0528c6/libraries/AP_Math/quaternion.cpp#L776C1-L799C2

ret.q1 = (rquat0*quat0 + rquat1*quat1 + rquat2*quat2 + rquat3*quat3);
ret.q2 = (rquat0*quat1 - rquat1*quat0 - rquat2*quat3 + rquat3*quat2);
ret.q3 = (rquat0*quat2 + rquat1*quat3 - rquat2*quat0 - rquat3*quat1);
ret.q4 = (rquat0*quat3 - rquat1*quat2 + rquat2*quat1 - `rquat3*quat0);

The quantities of rquat1... rquat3 in ret.q2, ret.q3, and ret.q4 do not seem to be correctly multiplied by a negative sign.

[tpwrules]

It appears this result is consistent with MATLAB which is where a lot of this EKF code came from: https://stackoverflow.com/a/68712650

MATLAB for whatever bizarre reason calculates division as $y^{-1}x$ instead of your $xy^{-1}$.

Searching for the symbol name _ZNK11QuaternionTIdEdvERKS0_ in a CubeOrange binary, the only other uses are

• ardupilot/libraries/AP_NavEKF3/AP_NavEKF3_MagFusion.cpp

  Line 83 in aabbe50

  deltaQuatTemp = quatAtLastMagReset / stateStruct.quat;

• ardupilot/libraries/AP_NavEKF3/AP_NavEKF3_MagFusion.cpp

  Line 1658 in aabbe50

  QuaternionF quat_delta = stateStruct.quat / quatBeforeReset;

• ardupilot/libraries/AP_NavEKF3/AP_NavEKF3_core.cpp

  Line 917 in 126fbfb

  QuaternionF quatErr = stateStruct.quat / outputDataDelayed.quat;

[tpwrules]

I would support documentation on the function but I don't think this can be classified as a bug.

[tpwrules]

I flipped the signs on the latter three lines in that function and the EKF goes crazy (likely due to how this is used in the last case in the core, as this board has no mag), so yeah we are just using a different convention.

[tpwrules]

@SuperLei917 would you like to file a PR to clarify the documentation?

[SuperLei917]

I flipped the signs on the latter three lines in that function and the EKF goes crazy (likely due to how this is used in the last case in the core, as this board has no mag), so yeah we are just using a different convention.

Yes, I have seen your reply. I think you are right. The calculation in the code does not use the same set of criteria as my calculation method. Thank you for your reply.

[SuperLei917]

@SuperLei917 would you like to file a PR to clarify the documentation?

Thank you for the invitation! I would be happy to submit a PR to help clarify this.I'm not entirely sure about the best way to do it, though. Would it be appropriate to add some explanatory comments within the code block to prevent the same misunderstanding I had?

[tpwrules]

I think it should just be comments on the division operator itself.

Please be sure to see our contributing guidelines in particular the info on commit message formatting.