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.

Sign up for GitHub

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

Jump to bottom

[math] fix quat_of_axis functions #2367

Merged
merged 1 commit into from
Dec 18, 2018
Merged

[math] fix quat_of_axis functions #2367

merged 1 commit into from
Dec 18, 2018

Conversation

gautierhattenberger
Copy link
Member

Improve doc and choose a more reasonable output resolution of the integer version.

The only possible issue might be in the hybrid guidance:

  • quat setpoint should now have the proper value, but since the "wrong" value was currently compensated later in the code it shouldn't change anything
  • euler setpoint should be correct now, but it is unclear if it was even used with hybrid guidance (it shouldn't)
  • hybrid guidance from master is currently not much used apparently and might need some extra cleaning and better integration anyway, better have this part correct now at least

I suggest to merge this now for next release even if hybrid guidance can't be fully checked before that.
fix #2331

@gautierhattenberger
[math] fix quat_of_axis functions
ee7a0eb 
improve doc and choose a more reasonable output resolution of the
integer version

fix #2331
@gautierhattenberger gautierhattenberger added this to the v5.14 milestone Dec 11, 2018
kirkscheper
kirkscheper approved these changes Dec 18, 2018
View reviewed changes
Copy link
Member

@kirkscheper kirkscheper left a comment

Choose a reason for hiding this comment

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

In principle it looks fine although its surprising this wasn't noticed sooner.

sw/airborne/math/pprz_algebra_int.c
q->qx = san2 * uv->x;
q->qy = san2 * uv->y;
q->qz = san2 * uv->z;
q->qi = (can2 << (INT32_QUAT_FRAC-INT32_TRIG_FRAC));
Copy link
Member

Choose a reason for hiding this comment

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

Else where in this file, INT_MULT_RSHIFT is used to complete multiplications which multiplies then scales, any specific reason that isn't done here?

Copy link
Member Author

Choose a reason for hiding this comment

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

it should be INT_MULT_LSHIFT eventually, but this one doesn't exist...

@gautierhattenberger
Copy link
Member Author

it just have no side effect on the rest of the code and it's barely not used anyway

@gautierhattenberger gautierhattenberger merged commit bb35c16 into master Dec 18, 2018
@gautierhattenberger gautierhattenberger deleted the quat_of_axis-fix branch December 18, 2018 16:13
shuoli-robotics pushed a commit to shuoli-robotics/paparazzi that referenced this pull request Jan 23, 2019
@gautierhattenberger
[math] fix quat_of_axis functions (paparazzi#2367)
9be6be1 
improve doc and choose a more reasonable output resolution of the
integer version

fix paparazzi#2331
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kirkscheper kirkscheper kirkscheper approved these changes

Assignees
No one assigned
Labels
Airborne Documentation
Projects
None yet
Milestone
v5.14
Development

Successfully merging this pull request may close these issues.

quat_of_axis_angle does not yield a normalized quaternion
2 participants
@gautierhattenberger @kirkscheper