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

Update quaternion ekf #955

Merged
merged 17 commits into from
Feb 24, 2016
Merged

Conversation

eisoku9618
Copy link
Contributor

  • クォータニオンを正規化する部分でバグがあったので直しました
  • ジャイロセンサの信頼度を表現できるようにしました
  • (リファクタリングしました)

デフォルトでは何の影響もありませんので,よろしくお願いします.

@k-okada
Copy link
Contributor

k-okada commented Feb 23, 2016

テストコードできるとうれしいんだけどな。
% 企業はテストコードからつくるようですよ

◉ Kei Okada

On Tue, Feb 23, 2016 at 4:37 PM, Eisoku Kuroiwa [email protected]
wrote:

  • クォータニオンを正規化する部分でバグがあったので直しました
  • ジャイロセンサの信頼度を表現できるようにしました
  • (リファクタリングしました)

デフォルトでは何の影響もありませんので,よろしくお願いします.

You can view, comment on, or merge this pull request online at:

#955
Commit Summary

  • [KalmanFilter/EKFilter.h] fix bug : we should normalize only
    rotation quaternion
  • [KalmanFilter/EKFilter.h] acceleration reference is to handle in
    KalmanFilter.cpp
  • [KalmanFilter/EKFilter.h] normalize rotation quaternion as soon as
    possible
  • [KalmanFilter/EKFilter.h] Q should be gyro noise covariance in order
    to make it easy to tune parameters
  • [KalmanFilter/EKFilter.h] use initializer list at EKFilter
  • [KalmanFilter/EKFilter.h] add a magic comment to use a 2 space
    indentation
  • [KalmanFilter/EKFilter.h] remove unused old comments
  • [KalmanFilter/EKFilter.h] use const member functions
  • [KalmanFilter/EKFilter.h] update calcF for readability
  • [KalmanFilter/EKFilter.h] do not pass a member variable to member
    functions
  • [KalmanFilter/EKFilter.h] use const reference parameters
  • [KalmanFilter/EKFilter.h] use hrpUtil to get Euler Angles from
    Rotation Matrix
  • [KalmanFilter/EKFilter.h] use rotation quaternion to rotate
    coordinate instead of rotation matrix
  • [KalmanFilter/EKFilter.h] clean up redundant codes
  • [KalmanFilter/EKFilter.h] use reference instead of returning value
  • [KalmanFilter/EKFilter.h] refectering

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#955.

@k-okada
Copy link
Contributor

k-okada commented Feb 23, 2016

Refer to this link for build results (access rights to CI server needed):
http://jenkins.jsk.imi.i.u-tokyo.ac.jp:8080/job/hrpsys-qnx/2565/
Test PASSed.

@@ -7,134 +8,90 @@

class EKFilter {
public:
EKFilter() {
EKFilter()
: P(Eigen::Matrix<double, 7, 7>::Identity() * 0.1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please define a matrix7d for clarity

@haudren
Copy link
Contributor

haudren commented Feb 23, 2016

Hello,
I made a few remarks above, I hope they will not come out as too blunt. Could you fix these style issues? They really hurt readability.

Otherwise there is maybe room for improvement and refactoring in the computation of the covariance matrix.

Thanks

@haudren
Copy link
Contributor

haudren commented Feb 23, 2016

Oh and also, It would be best to squash some commits before merging, so that the commits that are obviously just correcting some mistakes do not appear in the final tree (e.g. 182f8c9, 1d24bd7...) and only the ones adding functionality are preserved.

ご迷惑にかけてしますが、よろしくお願いします。

@eisoku9618
Copy link
Contributor Author

@haudren Thank you for reviewing !
I've fixed all the issues you mentioned.

@k-okada
Copy link
Contributor

k-okada commented Feb 23, 2016

Refer to this link for build results (access rights to CI server needed):
http://jenkins.jsk.imi.i.u-tokyo.ac.jp:8080/job/hrpsys-qnx/2566/
Test PASSed.

fkanehiro added a commit that referenced this pull request Feb 24, 2016
@fkanehiro fkanehiro merged commit f4e6bd2 into fkanehiro:master Feb 24, 2016
@eisoku9618 eisoku9618 deleted the update-quaternion-ekf branch February 24, 2016 06:31
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.

4 participants