Skip to content

Conversation

@mdurrani808
Copy link
Contributor

if someone could test, that would be great. mainly 3d stuff, worked fine on my machine.

@mdurrani808 mdurrani808 requested a review from a team as a code owner December 25, 2022 19:46
@amquake
Copy link
Member

amquake commented Dec 26, 2022

Works for me, but I would like to keep the decision margin. I've had camera settings before where the margin may even get as low as 15 to reliably detect the tag.

@mcm001
Copy link
Contributor

mcm001 commented Dec 27, 2022

What's the goal of removing these, versus just setting sane defaults?

@mdurrani808
Copy link
Contributor Author

Test broken because changing the default to 16h5 broke things.

@amquake will add that back.

@mcm001 Makes things simpler for the end user. If they don't need to touch it, they probably don't need to see it. Everyone should be detecting 16h5 tags and the selector wasn't really intuitive anyways.

@gerth2
Copy link
Contributor

gerth2 commented Dec 27, 2022

Matt and I just talked.... request - leave the front end the same (selectors still available) but change the default so it's correct for 2023

@gerth2
Copy link
Contributor

gerth2 commented Dec 27, 2022

Definitely still want decision marker and error bits exposed - I'm ok with whatever amquake wants

@mdurrani808
Copy link
Contributor Author

Matt and I just talked.... request - leave the front end the same (selectors still available) but change the default so it's correct for 2023

Thoughts on condensing it down to just one selector for the family then? I'll add decision margin back but if anything above 0 error bits causes issues, it shouldn't need to be edited.

@amquake
Copy link
Member

amquake commented Dec 28, 2022

It seemed to me any value of error bits above 0 for 16h5 is detrimental-- if the family selector is kept in, error bits should still have a slider or be automatically set (0 for 16h5, previous default for 36h11). Otherwise I think it would be better to remove it.

@gerth2
Copy link
Contributor

gerth2 commented Dec 28, 2022

It seemed to me any value of error bits above 0 for 16h5 is detrimental

I assume false positives? I hadn't seen these but then again have done all testing in my basement. Locked at zero is fine by me (that's what the apriltag papers talk about most folks doing anyway)

Thoughts on condensing it down to just one selector for the family then?

I do like this.

# Conflicts:
#	photon-core/src/main/java/org/photonvision/vision/apriltag/AprilTagDetector.java
#	photon-core/src/main/java/org/photonvision/vision/apriltag/AprilTagDetectorParams.java
#	photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipelineSettings.java
@gerth2 gerth2 requested a review from mcm001 December 31, 2022 20:16
@mcm001 mcm001 added this to the 2023 Kickoff milestone Dec 31, 2022
@mcm001 mcm001 merged commit 1ab5b66 into PhotonVision:master Dec 31, 2022
FarkasJoseph pushed a commit to FarkasJoseph/photonvision that referenced this pull request Feb 8, 2023
…get family selector (PhotonVision#652)

* clean up front end ui

* address changes

* Further tweaks to camera default gains to help make sure users get a good first impression

* even more saner defaults

* Even even more camera sane defaults

* lint

* lint pt 2

* unit test fixup

Co-authored-by: Chris <chrisgerth010592@gmail.com>
@mdurrani808 mdurrani808 deleted the frontendCleanup branch February 15, 2025 00:14
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