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

AP_Baro: Align processing procedures #25199

Closed

Conversation

muramura
Copy link
Contributor

@muramura muramura commented Oct 7, 2023

Same pre-processing.
I think a delay time is needed.

Copy link
Contributor

@magicrub magicrub left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Contributor

@khancyr khancyr left a comment

Choose a reason for hiding this comment

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

That isn't the same.
On first one, we are just discarding the value over the 1 second. So we don't care at what rate we get them
On second case, we want an average of 5 values. Best woould be if they can be continus, in worst case senario the change would lead the everaging to be slightly off. The behavior is already present but limited by the fact we don't add 10ms delay between reading and just try to get the data as they come.
Using a median can help to have better sampling and discard extrem values

@peterbarker
Copy link
Contributor

As @khancyr notes, this is a significant change to the way we sample the baro for the calibration, and the two steps you're trying to make uniform are fulfilling different functions.

We'd need a pretty good reason to change this up, as the existing code has worked well for a very, very long time.

It's also currently objectively worse as it takes longer to calibrate the baro!

I'm going to close this one now - but if you can show that this is a signifcant advantage for some reason, please reopen and explain.

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.

4 participants