From f56afb9e1249861ae96c8013d926888c8dc6b6e4 Mon Sep 17 00:00:00 2001
From: Ryan Blue <ryanzblue@gmail.com>
Date: Mon, 28 Aug 2023 12:27:10 -0400
Subject: [PATCH] Improve docs for PhotonPoseEstimator

In particular, document that update() ensures that new data is only used once.
---
 .../org/photonvision/PhotonPoseEstimator.java | 55 ++++++++++++-------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/photon-lib/src/main/java/org/photonvision/PhotonPoseEstimator.java b/photon-lib/src/main/java/org/photonvision/PhotonPoseEstimator.java
index 8bbdaf5744..0eecc02880 100644
--- a/photon-lib/src/main/java/org/photonvision/PhotonPoseEstimator.java
+++ b/photon-lib/src/main/java/org/photonvision/PhotonPoseEstimator.java
@@ -250,10 +250,16 @@ public void setRobotToCameraTransform(Transform3d robotToCamera) {
 
     /**
      * Poll data from the configured cameras and update the estimated position of the robot. Returns
-     * empty if there are no cameras set or no targets were found from the cameras.
+     * empty if:
      *
-     * @return an EstimatedRobotPose with an estimated pose, the timestamp, and targets used to create
-     *     the estimate
+     * <ul>
+     *   <li>New data has not been received since the last call to {@code update()}.
+     *   <li>No targets were found from the camera
+     *   <li>There is no camera set
+     * </ul>
+     *
+     * @return an {@link EstimatedRobotPose} with an estimated pose, timestamp, and targets used to
+     *     create the estimate.
      */
     public Optional<EstimatedRobotPose> update() {
         if (camera == null) {
@@ -267,12 +273,17 @@ public Optional<EstimatedRobotPose> update() {
     }
 
     /**
-     * Updates the estimated position of the robot. Returns empty if there are no cameras set or no
-     * targets were found from the cameras.
+     * Updates the estimated position of the robot. Returns empty if:
+     *
+     * <ul>
+     *   <li>The timestamp of the provided pipeline result is the same as in the previous call to
+     *       {@code update()}.
+     *   <li>No targets were found in the pipeline results.
+     * </ul>
      *
      * @param cameraResult The latest pipeline result from the camera
-     * @return an EstimatedRobotPose with an estimated pose, and information about the camera(s) and
-     *     pipeline results used to create the estimate
+     * @return an {@link EstimatedRobotPose} with an estimated pose, timestamp, and targets used to
+     *     create the estimate.
      */
     public Optional<EstimatedRobotPose> update(PhotonPipelineResult cameraResult) {
         if (camera == null) return update(cameraResult, Optional.empty(), Optional.empty());
@@ -280,21 +291,25 @@ public Optional<EstimatedRobotPose> update(PhotonPipelineResult cameraResult) {
     }
 
     /**
-     * Updates the estimated position of the robot. Returns empty if there are no cameras set or no
-     * targets were found from the cameras.
+     * Updates the estimated position of the robot. Returns empty if:
      *
-     * @param cameraResult The latest pipeline result from the camera
-     * @param cameraMatrixData Camera calibration data that can be used in the case of no assigned
+     * <ul>
+     *   <li>The timestamp of the provided pipeline result is the same as in the previous call to
+     *       {@code update()}.
+     *   <li>No targets were found in the pipeline results.
+     * </ul>
+     *
+     * @param cameraMatrix Camera calibration data that can be used in the case of no assigned
      *     PhotonCamera.
-     * @param coeffsData Camera calibration data that can be used in the case of no assigned
+     * @param distCoeffs Camera calibration data that can be used in the case of no assigned
      *     PhotonCamera
-     * @return an EstimatedRobotPose with an estimated pose, and information about the camera(s) and
-     *     pipeline results used to create the estimate
+     * @return an {@link EstimatedRobotPose} with an estimated pose, timestamp, and targets used to
+     *     create the estimate.
      */
     public Optional<EstimatedRobotPose> update(
             PhotonPipelineResult cameraResult,
-            Optional<Matrix<N3, N3>> cameraMatrixData,
-            Optional<Matrix<N5, N1>> coeffsData) {
+            Optional<Matrix<N3, N3>> cameraMatrix,
+            Optional<Matrix<N5, N1>> distCoeffs) {
         // Time in the past -- give up, since the following if expects times > 0
         if (cameraResult.getTimestampSeconds() < 0) {
             return Optional.empty();
@@ -316,13 +331,13 @@ public Optional<EstimatedRobotPose> update(
             return Optional.empty();
         }
 
-        return update(cameraResult, cameraMatrixData, coeffsData, this.primaryStrategy);
+        return update(cameraResult, cameraMatrix, distCoeffs, this.primaryStrategy);
     }
 
     private Optional<EstimatedRobotPose> update(
             PhotonPipelineResult cameraResult,
-            Optional<Matrix<N3, N3>> cameraMatrixData,
-            Optional<Matrix<N5, N1>> coeffsData,
+            Optional<Matrix<N3, N3>> cameraMatrix,
+            Optional<Matrix<N5, N1>> distCoeffs,
             PoseStrategy strat) {
         Optional<EstimatedRobotPose> estimatedPose;
         switch (strat) {
@@ -343,7 +358,7 @@ private Optional<EstimatedRobotPose> update(
                 estimatedPose = averageBestTargetsStrategy(cameraResult);
                 break;
             case MULTI_TAG_PNP:
-                estimatedPose = multiTagPNPStrategy(cameraResult, cameraMatrixData, coeffsData);
+                estimatedPose = multiTagPNPStrategy(cameraResult, cameraMatrix, distCoeffs);
                 break;
             default:
                 DriverStation.reportError(