-
Notifications
You must be signed in to change notification settings - Fork 509
EKF: Add Emergency yaw recovery using EKF-GSF estimator #766
EKF: Add Emergency yaw recovery using EKF-GSF estimator #766
Conversation
This reverts commit 9300530.
Also remove unnecessary operations
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.
There was a problem hiding this 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
EKF/EKFGSF_yaw.cpp
Outdated
|
||
// normalise the weighting function | ||
if (_ekf_gsf_vel_fuse_started && total_w > 1e-6f) { | ||
float total_w_inv = 1.0f / total_w; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
float total_w_inv = 1.0f / total_w; | |
const float total_w_inv = 1.0f / total_w; |
EKF/EKFGSF_yaw.cpp
Outdated
} | ||
} else { | ||
// assign even weights | ||
float default_weight = 1.0f / (float)N_MODELS_EKFGSF; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vector2f yaw_vector = {}; | |
Vector2f yaw_vector; |
There was a problem hiding this comment.
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
return false; | ||
} | ||
|
||
void EKFGSF_yaw::setVelocity(Vector2f velocity, float accuracy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void EKFGSF_yaw::setVelocity(Vector2f velocity, float accuracy) | |
void EKFGSF_yaw::setVelocity(const Vector2f &velocity, float accuracy) |
#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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
Vector2f vel_NE = Vector2f(_gps_sample_delayed.vel(0),_gps_sample_delayed.vel(1)); | ||
yawEstimator.setVelocity(vel_NE, _gps_sample_delayed.vacc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
0ade564
to
b31a21e
Compare
There was a problem hiding this 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?
Here are before and after replay output for the last two commits 37572b3 and 5270fde Replay of multirotor flight with manoeuvring in stabilise mode: Before: After: 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: After: |
@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? |
@ndepal had a flight that looks like he needs this feature: https://logs.px4.io/plot_app?log=c231b7b3-ab35-4f48-be8c-0cb0a836cae4 |
Replaces #764.
Has been rebased.
See #764. for discussion and test evidence