Add solidity filter #378

Merged
merged 1 commit into from Jan 20, 2016

Projects

None yet

4 participants

@ThomasJClark
Contributor

This lets you filter contours by their "solidity", which is the ratio of their area to the area of their convex hull.

You can technically filter out false positives by just having a minimum area, but that means that actual targets will be filtered out if you're too far away from them. In addition, larger false positives will still show up.

The FIRST Stronghold targets have a solidity of about 1/3 regardless of how far away they are, so using a solidity filter gets them very reliably.

demo

@codecov-io

Current coverage is 47.65%

Merging #378 into master will increase coverage by +0.08% as of 23571c6

@@            master    #378   diff @@
======================================
  Files          127     127       
  Stmts         3817    3832    +15
  Branches       416     418     +2
  Methods          0       0       
======================================
+ Hit           1816    1826    +10
- Partial        115     118     +3
- Missed        1886    1888     +2

Review entire Coverage Diff as of 23571c6


Uncovered Suggestions

  1. +0.70% via ...loyerController.java#102...128
  2. +0.57% via ...InstanceManager.java#182...203
  3. +0.54% via .../ExceptionAlert.java#83...103
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@AustinShalit
Member

Is there a way we can make the text below the slider display a double instead of an int?

untitled

@ThomasJClark
Contributor

Good question. I might just make it a percentage from 0 to 100, so we don't have to deal with figuring how how many digits to round the label to

@AustinShalit
Member

That sounds like a better idea.

@ThomasJClark ThomasJClark Add solidity filter
d33d84d
@AustinShalit
Member

Do we want this code to handle NaN or should teams handle that?

@ThomasJClark
Contributor

I actually didn't realize that convex hulls could have 0 area, but I guess that's possible if it's a single pixel. I think that teams will usually have a minimum area filter of at least one, so it should be fine.

@JLLeitschuh
Member

Won't this break save files?
This socket won't exist in older save files which will cause an exception to be thrown.

@ThomasJClark
Contributor

Nope, it will just use the default value

@JLLeitschuh
Member

You tested this?
I'd really like to see #316 finished and merged before we go messing with existing operations.
@PaulaRudy What is your timeline on getting that finished?

@ThomasJClark
Contributor

You tested this?

Yes

I'd really like to see #316 finished and merged before we go messing with existing operations.

Ideally I guess we would have something like that, but I don't think we need to stop adding new features until then

@JLLeitschuh
Member

I aggree. Feel free to add features, we shouldn't merge them until #316 is finished.
#316 Should be done before we do a new release anyways so this can wait for that.

@ThomasJClark
Contributor

I aggree. Feel free to add features, we shouldn't merge them until #316 is finished.
#316 Should be done before we do a new release anyways so this can wait for that.

I disagree with waiting on #316 to do a release.

@JLLeitschuh
Member

Okay, sure. You can disagree. I'm not merging this yet though.
We can bring this up until the next meeting. Until then, I don't want to risk breaking save files.
Sorry.

@ThomasJClark
Contributor

Okay, sure. You can disagree. I'm not merging this yet though.
We can bring this up until the next meeting. Until then, I don't want to risk breaking save files.
Sorry.

Jonathan, we can decide as a group what to prioritize for the next release. It's not your decision alone. Stop pretending like it is.

@JLLeitschuh
Member

I agree. Let's wait and chat about this at the next meeting.

@JLLeitschuh JLLeitschuh merged commit 018e5c6 into WPIRoboticsProjects:master Jan 20, 2016

4 of 5 checks passed

codecov/patch 28.00% of diff hit (target 70.00%)
Details
codacy/pr Good work! The project quality is stable.
Details
codecov/project 47.65% (+0.08%) compared to abec11c at 47.57%
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment