Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

EKF: Add Emergency yaw recovery using EKF-GSF estimator #766

Merged
merged 37 commits into from
Mar 5, 2020

Conversation

priseborough
Copy link
Collaborator

Replaces #764.

Has been rebased.

See #764. for discussion and test evidence

Use conversion from rotation matrix to Euler angles instead of quaternion to euler angles.
memcpy bypasses compiler sanity checks and is unnecessary in this instance.
Adds a common function to support yaw reset that can be used elsewhere.
Copy link
Contributor

@kamilritz kamilritz left a comment

Choose a reason for hiding this comment

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

Thanks a lot for all your work.

_gsf_yaw = 0.0f;
}

void EKFGSF_yaw::update(const Vector3f del_ang, // IMU delta angle rotation vector meassured in body frame (rad)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would pass these vectors as a const-reference. Or even better pass an imuSample as const-reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then you have less input arguments, which makes it more readable.


// normalise the weighting function
if (_ekf_gsf_vel_fuse_started && total_w > 1e-6f) {
float total_w_inv = 1.0f / total_w;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
float total_w_inv = 1.0f / total_w;
const float total_w_inv = 1.0f / total_w;

}
} else {
// assign even weights
float default_weight = 1.0f / (float)N_MODELS_EKFGSF;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
float default_weight = 1.0f / (float)N_MODELS_EKFGSF;
const float default_weight = 1.0f / (float)N_MODELS_EKFGSF;

// Calculate a composite yaw vector as a weighted average of the states for each model.
// To avoid issues with angle wrapping, the yaw state is converted to a vector with legnth
// equal to the weighting value before it is summed.
Vector2f yaw_vector = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Vector2f yaw_vector = {};
Vector2f yaw_vector;

Copy link
Contributor

Choose a reason for hiding this comment

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

Is filled with zero automatically

EKF/EKFGSF_yaw.h Show resolved Hide resolved
return false;
}

void EKFGSF_yaw::setVelocity(Vector2f velocity, float accuracy)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
void EKFGSF_yaw::setVelocity(Vector2f velocity, float accuracy)
void EKFGSF_yaw::setVelocity(const Vector2f &velocity, float accuracy)

Comment on lines +16 to +28
#define N_MODELS_EKFGSF 5

#ifndef M_PI_F
#define M_PI_F 3.14159265f
#endif

#ifndef M_PI_2_F
#define M_PI_2_F 1.57079632f
#endif

#ifndef M_TWOPI_INV
#define M_TWOPI_INV 0.159154943f
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

If some of them are not define else where, I would define them as const float/int

matrix::Vector3f X; // Vel North (m/s), Vel East (m/s), yaw (rad)s
matrix::SquareMatrix<float, 3> P; // covariance matrix
float W = 0.0f; // weighting
matrix::SquareMatrix<float, 2> S; // innovation covariance matrix
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could get away without storing S, but only its inverse. Just an idea.


}

void Ekf::requestEmergencyNavReset(uint8_t counter)
Copy link
Contributor

Choose a reason for hiding this comment

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

If the client can decide, if he wants to increase the counter or not. He is also able to decide if he want to call this function or not. Therefore the counter argument is not necessary. You can have an internal counter.

Comment on lines +1837 to +1838
Vector2f vel_NE = Vector2f(_gps_sample_delayed.vel(0),_gps_sample_delayed.vel(1));
yawEstimator.setVelocity(vel_NE, _gps_sample_delayed.vacc);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Vector2f vel_NE = Vector2f(_gps_sample_delayed.vel(0),_gps_sample_delayed.vel(1));
yawEstimator.setVelocity(vel_NE, _gps_sample_delayed.vacc);
yawEstimator.setVelocity(_gps_sample_delayed.vel.xy(), _gps_sample_delayed.vacc);

jkflying
jkflying previously approved these changes Mar 4, 2020
Copy link
Contributor

@jkflying jkflying left a comment

Choose a reason for hiding this comment

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

I've taken read over and I don't see anything which looks like a big problem. There's a lot of value here, and it would be great to get this into the firmware immediately after the 1.11 branch-off (next week) to give it maximum testing for 1.12.

@kamilritz I think a lot of these issues you could fix quicker yourself, and don't affect safety/performance/functionality, they are more aesthetic or tiny performance gain. Why don't we get this PR in and you can make a follow-up PR with the remaining cleanup things. Is that OK with you?

@priseborough
Copy link
Collaborator Author

priseborough commented Mar 5, 2020

Here are before and after replay output for the last two commits 37572b3 and 5270fde
showing the individual innovation magnitudes and the yaw estimates. The innovation levels during manoeuvres are lower overall and the EKF solution index 3 that initialises near the reciprocal heading converges faster:

Replay of multirotor flight with manoeuvring in stabilise mode:

Before:

before - innovations

before - yaw

After:

after - innovations

after - yaw

This is the effect of the changes applied to the stable release version and used to replay a VTOL log with higher levels of vibration and a dynamic transition manoeuvre.

Before:

innovations - before

After:

innovations - after

@kamilritz
Copy link
Contributor

@jkflying Your idea is fine with me. I can open a follow-up PR with some style changes. But I would rely on @priseborough for the functional testing part. Is that okay?
I also have not spotted any critical issue. Lets get this squashed and merged!

@priseborough priseborough merged commit 4669aa6 into PX4:master Mar 5, 2020
@LorenzMeier
Copy link
Member

@ndepal had a flight that looks like he needs this feature: https://logs.px4.io/plot_app?log=c231b7b3-ab35-4f48-be8c-0cb0a836cae4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants