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

Ekf3: sensors provide body offset #18242

Closed

Conversation

rmackay9
Copy link
Contributor

@rmackay9 rmackay9 commented Aug 7, 2021

This PR is a result of this 4.1.0-beta discussion with Yuri_Rage.

We seem to have a discrepancy between our sensor position offset wiki and the EKF3 (and EKF2) implementation.

The relevant part of the wiki description is here

In practice the distance to the sensor can be measured from the center of the autopilot unless the autopilot itself is placed a significant distance from the vehicle’s center of gravity in which case the IMU position offsets can be specified and then the other sensor’s position offsets can be specified from the vehicle’s center of gravity.

..but the "bodyPosOffset" calculations for the various sensors always also include the IMU offsets which, according to the description above, they should not. If we move the IMU on the vehicle we shouldn't then also have to update every sensor's position as well should we?

I suspect either way works but either the docs or the code should probably be fixed.

@priseborough
Copy link
Contributor

It is required that the EKF does all fo its internal calculations at the IMU, then relocates the position and velocity solution to the body frame centroid at the output. This change breaks that requirement.

If the IMU is moved, then only the IMU position param needs to be changed.

@rmackay9
Copy link
Contributor Author

rmackay9 commented Aug 8, 2021

@priseborough, I'm pretty sure there's a mismatch between the code and the wiki instructions. Can you peek at what's on the wiki especially the bit I've copied above?

I think the core question is, are the sensor offsets relative to the IMU or the COG?

@rmackay9 rmackay9 deleted the ekf3-sensors-provide-body-offset branch August 9, 2021 08:08
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.

None yet

2 participants