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

Kalman: only initialise param vars one time #1037

Merged
merged 2 commits into from
May 9, 2022

Conversation

krichardsson
Copy link
Contributor

The parameters used in kalman core were extracted into a struct in #869.
The variables used for the parameters are reset to default values when the kalman estimator is reset, and this causes two problems:

  1. Parameter values that were set earlier are lost, for instance the starting point used in the Position commander (see Position estimate can no longer be initialized to non-zero value #1014)
  2. The contract for parameters is that the underlying variable should not be changed directly by the firmware code, as the client can not detect the change. Re-initializing the parameters from time to time breaks this contract.

In this PR the initialization of the parameter data is moved from estimatorKalmanInit() to estimatorKalmanTaskInit() to make sure it is only done one time.

Fixes #1014

@krichardsson krichardsson requested a review from whoenig May 3, 2022 11:53
@krichardsson krichardsson changed the title Only initialise param vars one time Kalman: only initialise param vars one time May 3, 2022
@krichardsson krichardsson merged commit 52a9bf9 into master May 9, 2022
Copy link
Member

@tobbeanton tobbeanton left a comment

Choose a reason for hiding this comment

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

Good change.

@krichardsson krichardsson deleted the krichardsson/issue-1014 branch May 9, 2022 08:44
@krichardsson
Copy link
Contributor Author

@whoenig We decided to merge this even though you did not review it, please let us know if this causes any issues for the python bindings

@knmcguire knmcguire added this to the next release milestone May 10, 2022
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.

Position estimate can no longer be initialized to non-zero value
3 participants