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

Photonlibpy - Best Target Function #1223

Closed
wants to merge 1 commit into from

Conversation

drvladb
Copy link

@drvladb drvladb commented Feb 5, 2024

A rather simple getBestTarget() function for a pipeline result.

The idea to use .get(0) comes from the Java version of the library. This implies the assumption that target packets are sent in order.

I do not have access to a robot at the moment (thus it's a draft), but this matches the behavior of the Java client, so I'm confident it should work.

Vlad & the 5113 software team.

@drvladb drvladb changed the title Best Target Function Photonlibpy - Best Target Function Feb 6, 2024
@drvladb
Copy link
Author

drvladb commented Feb 6, 2024

After testing, we have found that Photonvision does not have a particular sort for AprilTags. As such, it isn't clear why getBestTarget in Java uses .get(0) as it isn't be best AprilTag.

I would appreciate some input on the matter.

Vlad

@drvladb
Copy link
Author

drvladb commented Feb 6, 2024

Additionally, it isn't clear to us whether retroreflective tape is supported in this version of the program.

If it isn't, is it possible to use sort by ambiguity to determine the best AprilTag.

@drvladb drvladb marked this pull request as ready for review February 6, 2024 21:06
@drvladb drvladb requested a review from a team as a code owner February 6, 2024 21:06
@gerth2
Copy link
Contributor

gerth2 commented Feb 21, 2024

Yes, the concept of best target is largely not a thing for apriltags, as false-positives are eliminated by othe means, and multiple valid targets are often in sight at once.

Even though true retro reflective is dead for FRC, there is still some relevance for colored shape or Neural Net pipelines ...

Probably worth test and review this summer, as examples also need updated to be apriltag-first.

@gerth2
Copy link
Contributor

gerth2 commented Aug 1, 2024

Hey @drvladb FYI - Was doing some cleanup, realized this was still hanging. I'll take a look hopefully later this week again after the serde stuff settles out in python, and will propose a path forward.

It's looking more and more like we won't be robotpy wrappering the C++ vendordep like we'd hoped, which means this PR is very relevant to keep API parity between the languages.

@drvladb
Copy link
Author

drvladb commented Aug 1, 2024

Awesome!
I'm a bit out of date in terms of dev, but I'm happy this PR might be useful. It's certainly out of date, but maybe we can make it work.

Vlad

@@ -39,5 +39,10 @@ def getTimestamp(self) -> float:
def getTargets(self) -> list[PhotonTrackedTarget]:
return self.targets

def getBestTarget(self) -> PhotonTrackedTarget:
Copy link
Contributor

Choose a reason for hiding this comment

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

I might just add a doc string explaining how "best" is calculated, and then we should be good to ship it

@mcm001 mcm001 closed this Aug 31, 2024
mcm001 added a commit that referenced this pull request Aug 31, 2024
Supercedes #1223

---------

Co-authored-by: vladb <vlad.bondar@frc5113.com>
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.

3 participants