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

Sub/Super-script mismatch in pose_measurement.h? #132

Closed
versatran01 opened this issue Dec 4, 2015 · 6 comments
Closed

Sub/Super-script mismatch in pose_measurement.h? #132

versatran01 opened this issue Dec 4, 2015 · 6 comments

Comments

@versatran01
Copy link

In
https://github.com/ethz-asl/ethzasl_msf/blob/master/msf_updates/include/msf_updates/pose_sensor_handler/pose_measurement.h

line 313 when you calculate q_err:

q_err = (state.Get<StateQwvIdx>()
          * state.Get<StateDefinition_T::q>()
          * state.Get<StateQicIdx>()).conjugate() * z_q_;

This looks like you are doing (q_wv * q_wi * q_ic)^-1 * z_q_vc, but should it be (q_vw * q_wi * q_ic)^-1 * z_q_vc?

@versatran01 versatran01 changed the title Sub/Super script mismatch in pose_measurement.h? Sub/Super-script mismatch in pose_measurement.h? Dec 4, 2015
@v0n0
Copy link

v0n0 commented Jan 19, 2016

I looked multiple times at your two formulas, but they look exactly the same?

@versatran01
Copy link
Author

@v0n0 the first quaternion is questionable.
In their code it is q_wv that represents a rotation from vision to world, the one I put is q_vw, which represents the opposite rotation.

@v0n0
Copy link

v0n0 commented Jan 20, 2016

Oh yeah, just saw that. These indexes are quite confusing, c = camera is used, then v = visual is used for another, aren't they the same thing? If the variable names were written fully like quaternion_world_to_vision, it would be much simpler and more readable and there would be less errors like this.

@versatran01
Copy link
Author

Yes I agree that they are confusing, but more importantly, this code might be wrong.
q_wv * q_wi is meaningless, while q_vw * q_wi gives you q_vi.
Just curious how they get it to work with a wrong expression.

@simonlynen
Copy link
Contributor

Please look at #116

@versatran01
Copy link
Author

@simonlynen Thanks.

So this is fixed in that branch but not the master one?

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

No branches or pull requests

3 participants