Skip to content

[photon-lib] Invalidate pose cache when setting referencePose - #2040

Merged
samfreund merged 4 commits into
PhotonVision:mainfrom
kcooney:python-invalidate-pose-cache
Aug 8, 2025
Merged

[photon-lib] Invalidate pose cache when setting referencePose#2040
samfreund merged 4 commits into
PhotonVision:mainfrom
kcooney:python-invalidate-pose-cache

Conversation

@kcooney

@kcooney kcooney commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

Setting PhotonPoseEstimator.referencePose now resets the pose cache, even
if there was not a previous reference pose value.

Previously, the pose cache would only be cleared if the estimator had a
reference pose and was updated to a different reference pose value, which
is likely not what users may expect.

Fixes #2039

Java changes:

  • Fix (and simplify) PhotonPoseEstimator.checkUpdate()
  • Update PhotonPoseEstimatorTest.cacheIsInvalidated() to verify the fix

Python changes:

  • Fix (and simplify) PhotonPoseEstimator._checkUpdate()

  • Update photonPoseEstimator_test.test_cacheIsInvalidated() to verify the
    fix

  • Add PipelineTimestamps in the test package (to make it easier to
    ensure that PhotonCameraResult.getTimestampSeconds() returns non-
    negative values)

  • Fix pyright errors in photonPoseEstimator_test.py

C++ changes:

  • Update PhotonPoseEstimatorTest_PoseCache_Test to confirm this bug did
    not exist in the C++ code

Other changes:

  • Fix photon-lib/py/buildAndTest.sh to specify the shell, to not assume
    that the current directory is the one with the script, and to not assume
    that bdist_wheel was installed

@kcooney
kcooney requested a review from a team as a code owner August 6, 2025 02:48
@github-actions github-actions Bot added photonlib Things related to the PhotonVision library backend Things relating to photon-core and photon-server labels Aug 6, 2025
@kcooney
kcooney force-pushed the python-invalidate-pose-cache branch from b9067c6 to 2041e2b Compare August 6, 2025 02:57
@samfreund
samfreund requested a review from Gold856 August 6, 2025 13:45
@samfreund

Copy link
Copy Markdown
Member

I'd prefer that we not close #2039 after this PR because it will still apply for Java.

@kcooney

kcooney commented Aug 6, 2025

Copy link
Copy Markdown
Contributor Author

I'd prefer that we not close #2039 after this PR because it will still apply for Java.

Agreed. Wasn't sure on the proper way to associate this PR with that bug without auto-closing the bug at merge time.

@samfreund

Copy link
Copy Markdown
Member

I'd prefer that we not close #2039 after this PR because it will still apply for Java.

Agreed. Wasn't sure on the proper way to associate this PR with that bug without auto-closing the bug at merge time.

If u mention it that links it. I just edited it, let me know if that looks good to you.

@kcooney
kcooney force-pushed the python-invalidate-pose-cache branch from 2041e2b to b847b8a Compare August 6, 2025 18:53
@kcooney

kcooney commented Aug 6, 2025

Copy link
Copy Markdown
Contributor Author

I'd prefer that we not close #2039 after this PR because it will still apply for Java.

The Java code was trivial to fix and verify, so pushed a commit to fix that as well.

Note I have not confirmed whether this exists in the C++ code.

It does not happen in the C++ code; updated the tests to confirm.

@kcooney
kcooney force-pushed the python-invalidate-pose-cache branch from 380ec56 to f9df7cc Compare August 6, 2025 19:31
@samfreund

Copy link
Copy Markdown
Member

@kcooney looks like you got a failing test.

@Gold856 Gold856 removed the backend Things relating to photon-core and photon-server label Aug 6, 2025
@kcooney
kcooney force-pushed the python-invalidate-pose-cache branch from f9df7cc to e5e9db4 Compare August 7, 2025 02:08
@kcooney

kcooney commented Aug 7, 2025

Copy link
Copy Markdown
Contributor Author

looks like you got a failing test.

@samfreund when I looked, all checks were reported as passed (excluding 3 skipped tasks). Rebased from main and pushed again, and GitHub is reporting "All checks have passed" 🤷‍♂️

@samfreund

Copy link
Copy Markdown
Member

looks like you got a failing test.

@samfreund when I looked, all checks were reported as passed (excluding 3 skipped tasks). Rebased from main and pushed again, and GitHub is reporting "All checks have passed" 🤷‍♂️

Lemme rephrase, looks like we have a flaky test 💀. Sorry about that!

@Gold856

Gold856 commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

Whoops, that’s my bad. I reran the failing job and forgot to mention that. Sorry!

@mcm001 mcm001 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On mobile but seems reasonable to me

@Gold856 Gold856 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The time units are different in Python between PipelineResult and PipelineMetadata? Weird, but the new test utils seem pretty nice for handling that. LGTM

@mcm001

mcm001 commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

We should probably make that consistent in follow on work

@samfreund
samfreund merged commit ab854e9 into PhotonVision:main Aug 8, 2025
43 checks passed
@kcooney
kcooney deleted the python-invalidate-pose-cache branch August 8, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

photonlib Things related to the PhotonVision library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PhotonPoseEstimator should invalidate the pose cache when setting referencePose

4 participants