fix(photon-lib): do not crash constrained solvepnp when heading-free with empty buffer#2529
Open
JosephTLockwood wants to merge 2 commits into
Open
Conversation
…with empty buffer estimateConstrainedSolvepnpPose only checked for a heading sample when headingFree was false, but then unconditionally sampled the heading buffer again and unwrapped it when building the solver call. A user running CONSTRAINED_SOLVEPNP with headingFree=true who never called addHeadingData therefore got a NoSuchElementException (Java) or std::bad_optional_access (C++) instead of a pose estimate, even though the heading-free solver never reads that heading: the generated casadi cost functions for the heading-free problems never dereference the gyro_theta argument (no arg[6] access in any constrained_solvepnp_*_tags_free.c), only the heading-fixed variants do. Sample the buffer once per invocation, keep returning empty when the heading is fixed and no sample exists, and fall back to a zero Rotation2d placeholder in heading-free mode. This also removes the redundant double/triple buffer sampling per estimate. Add a Java regression test that runs the heading-free strategy with no heading data and asserts it does not throw.
mcm001
reviewed
Jul 7, 2026
| if (!headingFree && headingSampleOpt.isEmpty()) { | ||
| return Optional.empty(); | ||
| } | ||
| // The solver's heading-free cost function never reads the provided heading, so any |
Contributor
There was a problem hiding this comment.
Comment feels unhelpful and specific to this bug
Contributor
Author
There was a problem hiding this comment.
Dropped it on both Java and C++.
Address review feedback on PhotonVision#2529: remove the comment describing why an empty heading buffer is safe in heading-free mode. It described this specific fix rather than documenting the code; the guard above and the .orElse/.value_or placeholder are self-explanatory. Removed on both Java and C++.
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
2 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
Constrained solvePnP unconditionally unwrapped the heading-buffer sample (
Optional.get()in Java,std::optional::value()in C++), so running heading-free with an empty heading buffer crashed (NoSuchElementException/bad_optional_access).Both implementations now early-return only when a heading is actually required, and pass a safe placeholder otherwise — the solver's heading-free cost function never reads the provided heading. A regression test (
PhotonPoseEstimatorTest) exercises the heading-free + empty-buffer path.Meta
Merge checklist: