EKF: do not fuse multiple times the same height #767
Conversation
The _fuse_height flag was never set to zero, hence the fusion was called at each iteration, even if no new data is available. The effects were: high CPU usage and virtually less measurement noise due to multiple fusion of the same sample Also remve unused variables
|
Is there some way we can add tests to make sure this kind of thing can't happen in the future? |
|
@jkflying Yes, I was thinking about that as well. I'll add that |
| @@ -1331,7 +1328,7 @@ void Ekf::controlFakePosFusion() | |||
| _using_synthetic_position = true; | |||
|
|
|||
| // Fuse synthetic position observations every 200msec | |||
| if (isTimedOut(_time_last_fake_pos, (uint64_t)2e5) || _fuse_height) { | |||
| if (isTimedOut(_time_last_fake_pos, (uint64_t)2e5)) { | |||
There was a problem hiding this comment.
My interpretation of the intent here was that we wanted to fuse the horizontal fake position the same amount as we fuse the baro data which we assume is available. Not sure if this is necessary.
There was a problem hiding this comment.
That's what I also thought, but then I think we have to do it differently because with that, we're fusing every 200ms + every baro update. We can discuss about that in the next call but for now, it seems fine not to do it.
There was a problem hiding this comment.
And the temptation to remove a global variable was too big....
There was a problem hiding this comment.
Yes, I completely agree. If you feel comfortable to do so, please go ahead.
The _fuse_height flag was never set to zero, hence the fusion was called at each iteration, even if no new data is available.
The effects were: high CPU usage and virtually less measurement noise due to multiple fusion of the same sample: see PX4/PX4-Autopilot#14204
Before this PR:
With this PR:
Note that I had to increase the margins in the unit test because we're not fusing height at full speed anymore.
CPU load on a Pixhawk4 (FMUv5, F7), disarmed:
Before this PR:
With this PR:
