-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Improved TDoA outlier filter #1237
Conversation
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.
Just a couple of comments on these!
@@ -71,6 +76,7 @@ void kalmanCoreUpdateWithTDOA(kalmanCoreData_t* this, tdoaMeasurement_t *tdoa) | |||
h[KC_STATE_Y] = (dy1 / d1 - dy0 / d0); | |||
h[KC_STATE_Z] = (dz1 / d1 - dz0 / d0); | |||
|
|||
#if USE_STEP_OUTLIER_FILTER |
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.
Shouldn't this be part of the kbuild config?
@@ -29,6 +29,7 @@ | |||
|
|||
bool outlierFilterValidateTdoaSimple(const tdoaMeasurement_t* tdoa); | |||
bool outlierFilterValidateTdoaSteps(const tdoaMeasurement_t* tdoa, const float error, const vector_t* jacobian, const point_t* estPos); | |||
bool outlierFilterValidateTdoaIntegrator(const tdoaMeasurement_t* tdoa, const float error, const uint32_t nowMs); |
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.
Just a comment, with kalmanCoreUpdateWithTDOA we use all caps for TDOA, but here it is only Tdoa. Shouldn't all be TDOA for concistency?
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.
We could make a separate PR for this, it might be too much of a rabbit hole for now.
src/modules/src/outlierFilter.c
Outdated
} | ||
|
||
|
||
// Integrator TDoA outlier filter ------------------------------------- |
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.
Great comments! But perhaps it should be more according to doxygen convention and put it closer to the function. If we ever want to make doxygen comments for these, it might get lost
src/modules/src/outlierFilter.c
Outdated
|
||
|
||
// The maximum size of the integrator. This size determines the time [in ms] needed to open/close the filter. | ||
static const float INTEGRATOR_SIZE = 300.0f; |
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'm wondering if we should make a struct for these values and initialize the avalues when the kalman filter is initialize? What do you think? It will make it more suitable for wrapping and the bindings?
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.
Also it would make it possible to use this filter for something else
|
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'll also give it a quick flight test
* You should have received a copy of the GNU General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
* outlierFilter.c: Outlier rejection filter for the LPS system |
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.
for the lighthouse system
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.
flight test is okay! If the copy paste error I mentioned before is fixed go ahead and merge it
Fixed the comment, and some other I found as well |
This PR contains an updated outlier filter for TDoA samples in the kalman filter. It has generally been simplified and now better supports various packet rates and nose levels in the system. This should make it work with TDoA long range and in systems with many anchors.
Closes #1158