Open up pose estimator strategy methods#2252
Merged
mcm001 merged 10 commits intoPhotonVision:mainfrom Jan 11, 2026
Merged
Conversation
github-actions
Bot
added
documentation
Gold856
force-pushed
the
pose-est
branch
9 times, most recently
from
4249f76 to
df63de7
Compare
mcm001
reviewed
Dec 20, 2025
Contributor
mcm001
left a comment
mcm001
left a comment
There was a problem hiding this comment.
broadly lgtm. big fan of this new api.
Gold856
force-pushed
the
pose-est
branch
2 times, most recently
from
adba869 to
d548a29
Compare
amquake
reviewed
Dec 20, 2025
Gold856
force-pushed
the
pose-est
branch
2 times, most recently
from
7abc1e8 to
722719a
Compare
Contributor
|
@amquake @spacey-sooty since you left comments, can you review and approve if you're happy? |
spacey-sooty
previously approved these changes
Jan 4, 2026
mcm001
reviewed
Jan 7, 2026
mcm001
reviewed
Jan 7, 2026
Gold856
force-pushed
the
pose-est
branch
2 times, most recently
from
aeb31ad to
590b721
Compare
mcm001
approved these changes
Jan 11, 2026
…java Co-authored-by: Nolan Brown <noleetarrr@gmail.com>
amquake
approved these changes
Jan 11, 2026
Member
amquake
left a comment
amquake
left a comment
There was a problem hiding this comment.
Only really looking at java and not validating this in sim but tests are green so LGTM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PhotonPoseEstimator has some rather leaky abstractions surrounding the processing of pipeline results. Pose caching makes attempts at comparing the same result with different strategies nearly impossible, and the singular
updatemethod with several optional arguments is error prone when you need to provide data for strategies like Constrained SolvePnP, as you can completely miss passing those arguments. Constrained SolvePnP itself has some abstraction leaks due to needing a seed pose, and it currently uses either the multi-tag result, or the multi-tag fallback, which is leaky and inflexible.Following the design laid out in #1835, all the strategies have now been opened up into their own methods, giving the user complete control over how they want to use a pipeline result. All formerly optional arguments have either been removed for strategies that don't need them, or required for the strategies that do need them. All the old PoseStrategy and update methods have been deprecated for backwards compatibility, except for Python, which is very behind in terms of API parity, which this PR intends to help fix.
Closes #1805, closes #1835, closes #2239.
Meta
Merge checklist: