-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
msg/vehicle_odometry.msg: simplify covariance handling and update all usage #19966
Conversation
fa31f4b
to
3f423dc
Compare
9591bb1
to
4a9c846
Compare
msg/vehicle_odometry.msg
Outdated
uint8 POSITION_COVARIANCE_Y_VAR = 3 | ||
uint8 POSITION_COVARIANCE_YZ_COV = 4 | ||
uint8 POSITION_COVARIANCE_Z_VAR = 5 | ||
float32[6] position_covariance |
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 propose dropping all crossterms and only keeping the variances for each direct measurement (pos, orientation, vel). I'm yet to come across any real system where the crossterms are used/useful.
For real systems, generating actually correct (co)variance estimates is also really challenging in the first place
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.
@james-modalai any opinion?
Covariances dropped as suggested, but it's in a separate commit we can revert if someone disagrees. |
float32 rollspeed # Angular velocity about X body axis | ||
float32 pitchspeed # Angular velocity about Y body axis | ||
float32 yawspeed # Angular velocity about Z body axis | ||
float32[3] angular_velocity # Angular velocity in body-fixed frame (rad/s). NaN if invalid/unknown |
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.
This is probably unnecessary?
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.
Perhaps mildly useful for comparison, but otherwise we're not even using it.
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.
Most systems don't even estimate that in my experience, hence suggestion to drop
c45c681
to
305d1d1
Compare
… usage - replace float32[21] URT covariances with smaller dedicated position/velocity/orientation covariances - these are easier to casually inspect and more representative of what's actually being used currently and reduces the size of vehicle_odometry_s quite a bit - ekf2: add new helper to get roll/pitch/yaw covariances - mavlink: delete unused ATT_POS_MOCAP stream (this is just a passthrough)
dfe38af
to
4e80750
Compare
3b27f8a
to
2110da5
Compare
why NaN is better than 0 ? @dagar for (auto &pc : msg.pose_covariance) {
pc = NAN;
} |
PR is stacked on top of #19965.