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

autotest: large outliers in sub terrain test have low sq #27684

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

clydemcqueen
Copy link
Contributor

@clydemcqueen clydemcqueen commented Jul 28, 2024

The current test.Sub.SimTerrainSurftrak noise configuration is causing more test failures than we'd like.

This PR adds outlier_good_sq_limit to the configuration. If the error exceeds outlier_good_sq_limit then the signal quality drops from 100 (best) to 50 (bad). Sub::read_rangefinder() tests for this, so large outliers are rejected. This should minimize eliminate test failures, and it also exercises the sq test in Sub::read_rangefinder().

Edit: I also fixed some test names and a few typos

@clydemcqueen
Copy link
Contributor Author

@ptrmu Please take a look

@peterbarker
Copy link
Contributor

Just a note. No test failures are acceptable - the test must be 100% (as-designed ;-) ), even in the face of adding random noise.

People seem to have this thing against flapping tests :-)

@clydemcqueen
Copy link
Contributor Author

@peterbarker got it!

I pushed another commit (I'll squash later), this should eliminate all test failures:

  • all outliers > 1m have low signal quality, so they will be dropped
  • the rf target is explicitly set

@ptrmu
Copy link
Contributor

ptrmu commented Aug 1, 2024

Looks great!

@clydemcqueen clydemcqueen marked this pull request as ready for review August 1, 2024 15:27
Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Anything which reduces false positives is good in my book :-)

Copy link
Contributor

@Williangalvani Williangalvani left a comment

Choose a reason for hiding this comment

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

Looks good!

@peterbarker peterbarker merged commit 8b37100 into ArduPilot:master Sep 23, 2024
96 checks passed
@peterbarker
Copy link
Contributor

Merged, thanks!

@clydemcqueen clydemcqueen deleted the fix_sub_terrain_test branch September 23, 2024 16: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.

5 participants