Skip to content

Add 6328's implementation of PNP distance for Trig Solving to PhotonPoseEstimator#1767

Merged
mcm001 merged 13 commits intoPhotonVision:mainfrom
JuliusZhou124:6328TrigPoseEstimation
Feb 13, 2025
Merged

Add 6328's implementation of PNP distance for Trig Solving to PhotonPoseEstimator#1767
mcm001 merged 13 commits intoPhotonVision:mainfrom
JuliusZhou124:6328TrigPoseEstimation

Conversation

@JuliusZhou124
Copy link
Copy Markdown
Contributor

@JuliusZhou124 JuliusZhou124 requested a review from a team as a code owner February 9, 2025 19:43
Why did this delete 1 line
@mcm001
Copy link
Copy Markdown
Contributor

mcm001 commented Feb 9, 2025

Sweet. I'd like to see some tests as well if we can.

@jwbonner
Copy link
Copy Markdown

jwbonner commented Feb 9, 2025

I would recommend taking a look at this post and the reply. The original version of this logic that we released (which seems to be implemented here) doesn't work properly when ty is nonzero.

@JuliusZhou124
Copy link
Copy Markdown
Contributor Author

I would recommend taking a look at this post and the reply. The original version of this logic that we released (which seems to be implemented here) doesn't work properly when ty is nonzero.

Thanks for the catch! Missed that in the thread. 😅

@mcm001
Copy link
Copy Markdown
Contributor

mcm001 commented Feb 10, 2025

Fya, we will still need to see tests for all the edge cases (camera straight, pitched/yawed/rolled, compound angles) before merging. I think our simulation code should be able to get you the fake targets you need, and you can just copy paste some of our existing pose estimator tests.

@JuliusZhou124
Copy link
Copy Markdown
Contributor Author

I think our simulation code should be able to get you the fake targets you need, and you can just copy paste some of our existing pose estimator tests.

Is there any fast way to get fake targets from simulation? Brute force copying seems rather tedious.

@mcm001
Copy link
Copy Markdown
Contributor

mcm001 commented Feb 10, 2025

Yeah. I think you should be able to use our vision system simulation to generate the target data?

@JuliusZhou124
Copy link
Copy Markdown
Contributor Author

Looks like the poses begin to deviate as the targets approach the corners of the camera feed.
image

Might be an implementation bug? Either way it's 10+ cm off in both x and y when simulating with perfect cameras.

@JuliusZhou124
Copy link
Copy Markdown
Contributor Author

Struggles with rolled cameras as well:
image

@mcm001
Copy link
Copy Markdown
Contributor

mcm001 commented Feb 13, 2025

Broadly Lgtm. Delegating to @spacey-sooty :)

@mcm001
Copy link
Copy Markdown
Contributor

mcm001 commented Feb 13, 2025

Looks like two new flakes:

 Starting process 'command '/usr/bin/clang++''. Working directory: /Users/runner/work/photonvision/photonvision/photon-targeting/build/objs/photontargetingTest/osxuniversal/release/photontargetingCpp Command: /usr/bin/clang++ @/Users/runner/work/photonvision/photonvision/photon-targeting/build/tmp/compilePhotontargetingTestOsxuniversalReleaseGoogleTestExePhotontargetingCpp/options.txt /Users/runner/work/photonvision/photonvision/photon-targeting/src/main/native/cpp/photon/targeting/MultiTargetPNPResult.cpp -o /Users/runner/work/photonvision/photonvision/photon-targeting/build/objs/photontargetingTest/osxuniversal/release/photontargetingCpp/at79tvvs4oj7e6ruemd6xlbm5/MultiTargetPNPResult.o
Successfully started process 'command '/usr/bin/clang++''
libc++abi: terminating due to uncaught exception of type std::__1::system_error: mutex lock failed: Invalid argument
 [ RUN      ] VisionSystemSimTest.TestMultipleTargets
D:\a\photonvision\photonvision\photon-lib\src\test\native\cpp\VisionSystemSimTest.cpp(401): error: Expected equality of these values:
  static_cast<size_t>(11)
    Which is: 11
  tgtList.size()
    Which is: 10

[  FAILED  ] VisionSystemSimTest.TestMultipleTargets (12 ms)

@mcm001 mcm001 merged commit 01a3d31 into PhotonVision:main Feb 13, 2025
36 checks passed
@mcm001
Copy link
Copy Markdown
Contributor

mcm001 commented Feb 13, 2025

@JuliusZhou124 as follow on work, we should update our docs page and stuff. And you should go brag about this in chief delphi!

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.

4 participants