From dca8ff8faaa559389c76c7fbbef7ca6c52d33a57 Mon Sep 17 00:00:00 2001 From: Joe Lockwood Date: Sat, 16 May 2026 20:29:01 -0400 Subject: [PATCH 1/7] fix(photon-lib): align C++/Python getTimestampSeconds on Java's capture-time semantic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C++ and Python compute getTimestampSeconds() as (receive - latency) while Java reads metadata.captureTimestampMicros directly. The C++ path additionally pre-subtracts latency in PhotonCamera before SetReceiveTimestamp, so GetTimestamp() subtracts latency a second time — making every C++ vision timestamp off by ~one full processing latency. This change has all three ports read metadata.captureTimestampMicros directly. Two properties of that field make this safe: 1. It is in the Time Sync Server's nt::Now timebase (FPGA-relative on a real robot). At photon-core/src/main/java/org/photonvision/common/dataflow/networktables/NTDataPublisher.java:200-211 the coprocessor reads getImageCaptureTimestampNanos(), adds the TimeSyncClient offset, and writes the result to the wire — with the in-line comment "Transform the metadata timestamps from the local wpi::nt::Now timebase to the Time Sync Server's timebase". 2. Latency compensation is already baked in by the coprocessor. photon-core/src/main/java/org/photonvision/vision/pipeline/CVPipeline.java:82 stamps frame.timestampNanos onto the result AFTER process() returns, so the value is the moment of capture, not publish. frame.timestampNanos itself is shutter-open time, set by the driver at photon-core/src/main/java/org/photonvision/vision/frame/provider/LibcameraGpuFrameProvider.java:95-101 before any processing happens. The direction is confirmed by photon-core/src/main/java/org/photonvision/vision/pipeline/result/CVPipelineResult.java:112-114 where getLatencyMillis() returns (wpiNanoTime() - capture) — a positive value, meaning capture precedes the latency reading. ntReceiveTimestamp keeps its documented meaning (FPGA time bytes arrived at robot code) for diagnostics. PhotonCamera no longer pre-subtracts latency before SetReceiveTimestamp. C++ tests previously relied on the (receive - 0_latency) projection through GetTimestamp; updated to set captureTimestampMicros in metadata directly, matching the Java test idiom. --- .../targeting/photonPipelineResult.py | 12 ++---- .../py/test/photonPoseEstimator_test.py | 8 ++-- .../main/native/cpp/photon/PhotonCamera.cpp | 8 ++-- .../native/cpp/PhotonPoseEstimatorTest.cpp | 38 ++++++++++++------- .../photon/targeting/PhotonPipelineResult.h | 13 +++---- 5 files changed, 43 insertions(+), 36 deletions(-) diff --git a/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py b/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py index 24d5d709c0..b2d9192b55 100644 --- a/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py +++ b/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py @@ -42,15 +42,11 @@ def getLatencyMillis(self) -> float: def getTimestampSeconds(self) -> float: """ - Returns the estimated time the frame was taken, in the Received system's time base. This is - calculated as (NT Receive time (robot base) - (publish timestamp, coproc timebase - capture - timestamp, coproc timebase)) + Returns the estimated time the frame was taken, in the Time Sync Server's time base + (wpi::nt::Now). Reads metadata.captureTimestampMicros directly — latency compensation + is already baked in by the coprocessor. """ - # TODO - we don't trust NT4 to correctly latency-compensate ntReceiveTimestampMicros - latency = ( - self.metadata.publishTimestampMicros - self.metadata.captureTimestampMicros - ) - return (self.ntReceiveTimestampMicros - latency) / 1e6 + return self.metadata.captureTimestampMicros / 1e6 def getTargets(self) -> list[PhotonTrackedTarget]: return self.targets diff --git a/photon-lib/py/test/photonPoseEstimator_test.py b/photon-lib/py/test/photonPoseEstimator_test.py index 430258dbe9..8ae37f5d11 100644 --- a/photon-lib/py/test/photonPoseEstimator_test.py +++ b/photon-lib/py/test/photonPoseEstimator_test.py @@ -192,9 +192,9 @@ def test_pnpDistanceTrigSolve(): bestTarget = result.getBestTarget() assert bestTarget is not None assert bestTarget.fiducialId == 0 - assert result.ntReceiveTimestampMicros > 0 + assert result.metadata.captureTimestampMicros > 0 # Make test independent of the FPGA time. - result.ntReceiveTimestampMicros = int(fakeTimestampSecs * 1e6) + result.metadata.captureTimestampMicros = int(fakeTimestampSecs * 1e6) estimator.addHeadingData( result.getTimestampSeconds(), realPose.rotation().toRotation2d() @@ -221,9 +221,9 @@ def test_pnpDistanceTrigSolve(): bestTarget = result.getBestTarget() assert bestTarget is not None assert bestTarget.fiducialId == 0 - assert result.ntReceiveTimestampMicros > 0 + assert result.metadata.captureTimestampMicros > 0 # Make test independent of the FPGA time. - result.ntReceiveTimestampMicros = int(fakeTimestampSecs * 1e6) + result.metadata.captureTimestampMicros = int(fakeTimestampSecs * 1e6) estimator.addHeadingData( result.getTimestampSeconds(), realPose.rotation().toRotation2d() diff --git a/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp b/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp index a63fe1aafb..e34da62a31 100644 --- a/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp +++ b/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp @@ -239,10 +239,10 @@ std::vector PhotonCamera::GetAllUnreadResults() { CheckTimeSyncOrWarn(result); - // TODO: NT4 timestamps are still not to be trusted. But it's the best we - // can do until we can make time sync more reliable. - result.SetReceiveTimestamp(wpi::units::microsecond_t(value.time) - - result.GetLatency()); + // ntReceiveTimestamp records when the bytes arrived at robot code, + // independent of capture time. GetTimestamp() reads capture time + // directly from metadata. + result.SetReceiveTimestamp(wpi::units::microsecond_t(value.time)); ret.push_back(result); } diff --git a/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp b/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp index 1cf1ae714a..f96ea1c7d3 100644 --- a/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp +++ b/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp @@ -89,7 +89,8 @@ TEST(PhotonPoseEstimatorTest, LowestAmbiguityStrategy) { cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{11'000'000, 11'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(11)); photon::PhotonPoseEstimator estimator(aprilTags, wpi::math::Transform3d{}); @@ -132,7 +133,8 @@ TEST(PhotonPoseEstimatorTest, LowestAmbiguityIgnoresNonFiducialTargets) { cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{11'000'000, 11'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(11)); photon::PhotonPoseEstimator estimator(aprilTags, wpi::math::Transform3d{}); @@ -191,7 +193,8 @@ TEST(PhotonPoseEstimatorTest, ClosestToCameraHeightStrategy) { cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{17'000'000, 17'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(17_s); photon::PhotonPoseEstimator estimator(aprilTags, {{0_m, 0_m, 4_m}, {}}); @@ -242,7 +245,8 @@ TEST(PhotonPoseEstimatorTest, ClosestToReferencePoseStrategy) { cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{17'000'000, 17'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(17)); photon::PhotonPoseEstimator estimator(aprilTags, {}); @@ -295,7 +299,8 @@ TEST(PhotonPoseEstimatorTest, ClosestToLastPose) { cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{17'000'000, 17'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(17)); photon::PhotonPoseEstimator estimator(aprilTags, {}); @@ -334,8 +339,8 @@ TEST(PhotonPoseEstimatorTest, ClosestToLastPose) { 0.4, corners, detectedCorners}}; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targetsThree, - std::nullopt}}; + photon::PhotonPipelineMetadata{21'000'000, 21'000'000, 2000, 1000}, + targetsThree, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(21)); for (const auto& result : cameraOne.GetAllUnreadResults()) { @@ -383,6 +388,7 @@ TEST(PhotonPoseEstimatorTest, PnpDistanceTrigSolve) { 1_ms, realPose.TransformBy(estimator.GetRobotToCameraTransform()), targets); cameraOne.testResult = {result}; + cameraOne.testResult[0].metadata.captureTimestampMicros = 17'000'000LL; cameraOne.testResult[0].SetReceiveTimestamp(17_s); estimator.AddHeadingData(result.GetTimestamp(), realPose.Rotation()); @@ -415,6 +421,7 @@ TEST(PhotonPoseEstimatorTest, PnpDistanceTrigSolve) { 1_ms, realPose.TransformBy(estimator.GetRobotToCameraTransform()), targets); cameraOne.testResult = {result}; + cameraOne.testResult[0].metadata.captureTimestampMicros = 18'000'000LL; cameraOne.testResult[0].SetReceiveTimestamp(18_s); estimator.AddHeadingData(result.GetTimestamp(), realPose.Rotation()); @@ -471,7 +478,8 @@ TEST(PhotonPoseEstimatorTest, AverageBestPoses) { cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{15'000'000, 15'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(15)); photon::PhotonPoseEstimator estimator(aprilTags, {}); @@ -509,7 +517,8 @@ TEST(PhotonPoseEstimatorTest, cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{4'000'000, 4'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(4)); photon::PhotonPoseEstimator estimator(aprilTags, {{0_m, 0_m, 4_m}, {}}); @@ -531,7 +540,8 @@ TEST(PhotonPoseEstimatorTest, cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{17'000'000, 17'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(17)); photon::PhotonPoseEstimator estimator(aprilTags, {}); @@ -565,7 +575,8 @@ TEST(PhotonPoseEstimatorTest, MultiTagOnCoprocFallback) { cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}}; + photon::PhotonPipelineMetadata{11'000'000, 11'000'000, 2000, 1000}, + targets, std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(11)); photon::PhotonPoseEstimator estimator(aprilTags, wpi::math::Transform3d{}); @@ -596,7 +607,8 @@ TEST(PhotonPoseEstimatorTest, CopyResult) { std::vector targets{}; auto testResult = photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{0, 0, 2000, 1000}, targets, std::nullopt}; + photon::PhotonPipelineMetadata{11'000'000, 11'000'000, 2000, 1000}, + targets, std::nullopt}; testResult.SetReceiveTimestamp(wpi::units::second_t(11)); auto test2 = testResult; @@ -651,7 +663,7 @@ TEST(PhotonPoseEstimatorTest, ConstrainedPnpOneTag) { std::vector{8}); photon::PhotonPipelineResult result{ - photon::PhotonPipelineMetadata{1, 10000, 2000, 100}, targets, + photon::PhotonPipelineMetadata{15'000'000, 15'009'999, 2000, 100}, targets, multiTagResult}; cameraOne.test = true; diff --git a/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h b/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h index 4cd9447e33..acd87aaa09 100644 --- a/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h +++ b/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h @@ -92,15 +92,14 @@ class PhotonPipelineResult : public PhotonPipelineResult_PhotonStruct { /** * Returns the estimated time the frame was taken, in the Time Sync Server's - * time base (nt::Now). This is calculated using the estimated offset between - * Time Sync Server time and local time. The robot shall run a server, so the - * offset shall be 0. - * This is much more accurate than using GetLatency() - * @return The timestamp in seconds or -1 if this result was not initiated - * with a timestamp. + * time base (nt::Now). The robot shall run a server, so this is FPGA-relative + * on a real robot. Reads metadata.captureTimestampMicros directly — latency + * compensation is already baked in by the coprocessor. + * @return The timestamp in seconds. */ wpi::units::second_t GetTimestamp() const { - return ntReceiveTimestamp - GetLatency(); + return wpi::units::microsecond_t{ + static_cast(metadata.captureTimestampMicros)}; } /** From 60678137bb0c8bc08431f41df8400c081dd68ec9 Mon Sep 17 00:00:00 2001 From: Joe Lockwood Date: Sat, 16 May 2026 22:39:09 -0400 Subject: [PATCH 2/7] style(photon-lib): apply wpiformat to PhotonPoseEstimatorTest brace-init wrap wpiformat (clang-format with the WPILib config) reflowed two PhotonPipelineResult brace-initializers in this file. The CI Lint and Format job rejected the prior wrap in both ClosestToCameraHeightReturnsEmptyForNoFiducialTargets and ConstrainedPnpOneTag; this commit applies the formatter's chosen split. Pure whitespace; no semantic change. --- .../src/test/native/cpp/PhotonPoseEstimatorTest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp b/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp index f96ea1c7d3..950a185269 100644 --- a/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp +++ b/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp @@ -517,8 +517,8 @@ TEST(PhotonPoseEstimatorTest, cameraOne.test = true; cameraOne.testResult = {photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{4'000'000, 4'000'000, 2000, 1000}, - targets, std::nullopt}}; + photon::PhotonPipelineMetadata{4'000'000, 4'000'000, 2000, 1000}, targets, + std::nullopt}}; cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(4)); photon::PhotonPoseEstimator estimator(aprilTags, {{0_m, 0_m, 4_m}, {}}); @@ -663,8 +663,8 @@ TEST(PhotonPoseEstimatorTest, ConstrainedPnpOneTag) { std::vector{8}); photon::PhotonPipelineResult result{ - photon::PhotonPipelineMetadata{15'000'000, 15'009'999, 2000, 100}, targets, - multiTagResult}; + photon::PhotonPipelineMetadata{15'000'000, 15'009'999, 2000, 100}, + targets, multiTagResult}; cameraOne.test = true; cameraOne.testResult = {result}; From d323b97b0f98f1036a1483fa6f2d725e6b2243de Mon Sep 17 00:00:00 2001 From: Joe Lockwood Date: Mon, 18 May 2026 18:05:10 -0400 Subject: [PATCH 3/7] refactor(photon-lib): drop dead ntReceiveTimestamp path across C++/Python MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Responds to mcm001's review on PR #2496: with getTimestampSeconds() now reading metadata.captureTimestampMicros directly, the ntReceiveTimestamp fallback has no remaining consumers — across the entire repo, the field is written but never read. Java has never had the analog at all. Deleting it brings the three ports into alignment with Java as the reference. C++ (photon-targeting + photon-lib): - Remove ntReceiveTimestamp field, SetReceiveTimestamp setter, and the five bespoke copy/move/operator= members on PhotonPipelineResult whose sole purpose was propagating that field across the proto- generated Base. Defaulted special members are equivalent now that the only "extra" subclass field is gone. - Drop both SetReceiveTimestamp call sites in PhotonCamera.cpp, the now-unused 'now' local in GetLatestResult, and the include. - Strip 14 SetReceiveTimestamp lines from PhotonPoseEstimatorTest.cpp. Every test still sets metadata.captureTimestampMicros directly, so GetTimestamp() returns the expected value via the metadata path. - Drop the CopyResult test that existed solely to verify the bespoke copy ctor propagated ntReceiveTimestamp; defaulted base copy is exercised by every other test in the suite. Python (photon-lib/py): - Remove ntReceiveTimestampMicros field from the PhotonPipelineResult dataclass and the three assignments (two in PhotonCamera, one in the sim). The sim's "+10" microsecond offset was decorative; the value was never observed. - Drop the now-unused RobotController import in photonCamera.py and the _receiveLatencyMicros / receiveTimestampMicros() helper on PipelineTimestamps in test/testUtil.py. --- photon-lib/py/photonlibpy/photonCamera.py | 12 ++------ .../photonlibpy/simulation/photonCameraSim.py | 1 - .../targeting/photonPipelineResult.py | 3 -- photon-lib/py/test/testUtil.py | 7 ----- .../main/native/cpp/photon/PhotonCamera.cpp | 11 -------- .../native/cpp/PhotonPoseEstimatorTest.cpp | 27 ------------------ .../photon/targeting/PhotonPipelineResult.h | 28 ------------------- 7 files changed, 2 insertions(+), 87 deletions(-) diff --git a/photon-lib/py/photonlibpy/photonCamera.py b/photon-lib/py/photonlibpy/photonCamera.py index cd792d7637..5e387f16ab 100644 --- a/photon-lib/py/photonlibpy/photonCamera.py +++ b/photon-lib/py/photonlibpy/photonCamera.py @@ -24,7 +24,7 @@ # magical import to make serde stuff work import photonlibpy.generated # noqa import wpilib -from wpilib import RobotController, Timer +from wpilib import Timer from .packet import Packet from .targeting.photonPipelineResult import PhotonPipelineResult @@ -144,7 +144,6 @@ def getAllUnreadResults(self) -> List[PhotonPipelineResult]: for change in changes: byteList = change.value - timestamp = change.time if len(byteList) < 1: pass @@ -152,8 +151,6 @@ def getAllUnreadResults(self) -> List[PhotonPipelineResult]: newResult = PhotonPipelineResult() pkt = Packet(byteList) newResult = PhotonPipelineResult.photonStruct.unpack(pkt) - # NT4 allows us to correct the timestamp based on when the message was sent - newResult.ntReceiveTimestampMicros = timestamp ret.append(newResult) return ret @@ -169,19 +166,14 @@ def getLatestResult(self) -> PhotonPipelineResult: self._versionCheck() - now = RobotController.getFPGATime() packetWithTimestamp = self._rawBytesEntry.getAtomic() byteList = packetWithTimestamp.value - packetWithTimestamp.time if len(byteList) < 1: return PhotonPipelineResult() else: pkt = Packet(byteList) - retVal = PhotonPipelineResult.photonStruct.unpack(pkt) - # We don't trust NT4 time, hack around - retVal.ntReceiveTimestampMicros = now - return retVal + return PhotonPipelineResult.photonStruct.unpack(pkt) def getDriverMode(self) -> bool: """Returns whether the camera is in driver mode. diff --git a/photon-lib/py/photonlibpy/simulation/photonCameraSim.py b/photon-lib/py/photonlibpy/simulation/photonCameraSim.py index b804d3e0fa..3273a0baf0 100644 --- a/photon-lib/py/photonlibpy/simulation/photonCameraSim.py +++ b/photon-lib/py/photonlibpy/simulation/photonCameraSim.py @@ -436,7 +436,6 @@ def distance(target: VisionTargetSim): self.heartbeatCounter += 1 publishTimestampMicros = wpilib.Timer.getFPGATimestamp() * 1e6 return PhotonPipelineResult( - ntReceiveTimestampMicros=int(publishTimestampMicros + 10), metadata=PhotonPipelineMetadata( captureTimestampMicros=int(publishTimestampMicros - latency * 1e6), publishTimestampMicros=int(publishTimestampMicros), diff --git a/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py b/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py index b2d9192b55..86a8c5034c 100644 --- a/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py +++ b/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py @@ -26,9 +26,6 @@ class PhotonPipelineMetadata: @dataclass class PhotonPipelineResult: - # Since we don't trust NT time sync, keep track of when we got this packet into robot code - ntReceiveTimestampMicros: int = -1 - targets: list[PhotonTrackedTarget] = field(default_factory=list) # Python users beware! We don't currently run a Time Sync Server, so these timestamps are in # an arbitrary timebase. This is not true in C++ or Java. diff --git a/photon-lib/py/test/testUtil.py b/photon-lib/py/test/testUtil.py index d2be20ebee..16c5c9c762 100644 --- a/photon-lib/py/test/testUtil.py +++ b/photon-lib/py/test/testUtil.py @@ -15,17 +15,13 @@ def __init__( *, captureTimestampMicros: int, pipelineLatencyMicros=2_000, - receiveLatencyMicros=1_000, ): if captureTimestampMicros < 0: raise InvalidTestDataException("captureTimestampMicros cannot be negative") if pipelineLatencyMicros <= 0: raise InvalidTestDataException("pipelineLatencyMicros must be positive") - if receiveLatencyMicros < 0: - raise InvalidTestDataException("receiveLatencyMicros cannot be negative") self._captureTimestampMicros = captureTimestampMicros self._pipelineLatencyMicros = pipelineLatencyMicros - self._receiveLatencyMicros = receiveLatencyMicros self._sequenceID = 0 @property @@ -54,9 +50,6 @@ def incrementTimeMicros(self, micros: int) -> None: def publishTimestampMicros(self) -> int: return self._captureTimestampMicros + self.pipelineLatencyMicros - def receiveTimestampMicros(self) -> int: - return self.publishTimestampMicros() + self._receiveLatencyMicros - def toPhotonPipelineMetadata(self) -> PhotonPipelineMetadata: return PhotonPipelineMetadata( captureTimestampMicros=self.captureTimestampMicros, diff --git a/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp b/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp index e34da62a31..657a1a65c5 100644 --- a/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp +++ b/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp @@ -34,7 +34,6 @@ #include #include #include -#include #include #include #include @@ -193,9 +192,6 @@ PhotonPipelineResult PhotonCamera::GetLatestResult() { // Prints warning if not connected VerifyVersion(); - // Fill the packet with latest data and populate result. - wpi::units::microsecond_t now = - wpi::units::microsecond_t(wpi::RobotController::GetFPGATime()); const auto value = rawBytesEntry.Get(); if (!value.size()) return PhotonPipelineResult{}; @@ -206,8 +202,6 @@ PhotonPipelineResult PhotonCamera::GetLatestResult() { CheckTimeSyncOrWarn(result); - result.SetReceiveTimestamp(now); - return result; } @@ -239,11 +233,6 @@ std::vector PhotonCamera::GetAllUnreadResults() { CheckTimeSyncOrWarn(result); - // ntReceiveTimestamp records when the bytes arrived at robot code, - // independent of capture time. GetTimestamp() reads capture time - // directly from metadata. - result.SetReceiveTimestamp(wpi::units::microsecond_t(value.time)); - ret.push_back(result); } diff --git a/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp b/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp index 950a185269..c9ae8e6100 100644 --- a/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp +++ b/photon-lib/src/test/native/cpp/PhotonPoseEstimatorTest.cpp @@ -91,7 +91,6 @@ TEST(PhotonPoseEstimatorTest, LowestAmbiguityStrategy) { cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{11'000'000, 11'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(11)); photon::PhotonPoseEstimator estimator(aprilTags, wpi::math::Transform3d{}); @@ -135,7 +134,6 @@ TEST(PhotonPoseEstimatorTest, LowestAmbiguityIgnoresNonFiducialTargets) { cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{11'000'000, 11'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(11)); photon::PhotonPoseEstimator estimator(aprilTags, wpi::math::Transform3d{}); @@ -195,7 +193,6 @@ TEST(PhotonPoseEstimatorTest, ClosestToCameraHeightStrategy) { cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{17'000'000, 17'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(17_s); photon::PhotonPoseEstimator estimator(aprilTags, {{0_m, 0_m, 4_m}, {}}); @@ -247,7 +244,6 @@ TEST(PhotonPoseEstimatorTest, ClosestToReferencePoseStrategy) { cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{17'000'000, 17'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(17)); photon::PhotonPoseEstimator estimator(aprilTags, {}); @@ -301,7 +297,6 @@ TEST(PhotonPoseEstimatorTest, ClosestToLastPose) { cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{17'000'000, 17'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(17)); photon::PhotonPoseEstimator estimator(aprilTags, {}); @@ -341,7 +336,6 @@ TEST(PhotonPoseEstimatorTest, ClosestToLastPose) { cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{21'000'000, 21'000'000, 2000, 1000}, targetsThree, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(21)); for (const auto& result : cameraOne.GetAllUnreadResults()) { estimatedPose = estimator.EstimateClosestToReferencePose(result, pose); @@ -389,7 +383,6 @@ TEST(PhotonPoseEstimatorTest, PnpDistanceTrigSolve) { targets); cameraOne.testResult = {result}; cameraOne.testResult[0].metadata.captureTimestampMicros = 17'000'000LL; - cameraOne.testResult[0].SetReceiveTimestamp(17_s); estimator.AddHeadingData(result.GetTimestamp(), realPose.Rotation()); @@ -422,7 +415,6 @@ TEST(PhotonPoseEstimatorTest, PnpDistanceTrigSolve) { targets); cameraOne.testResult = {result}; cameraOne.testResult[0].metadata.captureTimestampMicros = 18'000'000LL; - cameraOne.testResult[0].SetReceiveTimestamp(18_s); estimator.AddHeadingData(result.GetTimestamp(), realPose.Rotation()); @@ -480,7 +472,6 @@ TEST(PhotonPoseEstimatorTest, AverageBestPoses) { cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{15'000'000, 15'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(15)); photon::PhotonPoseEstimator estimator(aprilTags, {}); @@ -519,7 +510,6 @@ TEST(PhotonPoseEstimatorTest, cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{4'000'000, 4'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(4)); photon::PhotonPoseEstimator estimator(aprilTags, {{0_m, 0_m, 4_m}, {}}); @@ -542,7 +532,6 @@ TEST(PhotonPoseEstimatorTest, cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{17'000'000, 17'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(17)); photon::PhotonPoseEstimator estimator(aprilTags, {}); @@ -577,7 +566,6 @@ TEST(PhotonPoseEstimatorTest, MultiTagOnCoprocFallback) { cameraOne.testResult = {photon::PhotonPipelineResult{ photon::PhotonPipelineMetadata{11'000'000, 11'000'000, 2000, 1000}, targets, std::nullopt}}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(11)); photon::PhotonPoseEstimator estimator(aprilTags, wpi::math::Transform3d{}); @@ -603,20 +591,6 @@ TEST(PhotonPoseEstimatorTest, MultiTagOnCoprocFallback) { EXPECT_EQ(1, estimatedPose.value().targetsUsed[0].GetFiducialId()); } -TEST(PhotonPoseEstimatorTest, CopyResult) { - std::vector targets{}; - - auto testResult = photon::PhotonPipelineResult{ - photon::PhotonPipelineMetadata{11'000'000, 11'000'000, 2000, 1000}, - targets, std::nullopt}; - testResult.SetReceiveTimestamp(wpi::units::second_t(11)); - - auto test2 = testResult; - - EXPECT_NEAR(testResult.GetTimestamp().to(), - test2.GetTimestamp().to(), 0.001); -} - TEST(PhotonPoseEstimatorTest, ConstrainedPnpEmptyCase) { photon::PhotonPoseEstimator estimator( wpi::apriltag::AprilTagFieldLayout::LoadField( @@ -668,7 +642,6 @@ TEST(PhotonPoseEstimatorTest, ConstrainedPnpOneTag) { cameraOne.test = true; cameraOne.testResult = {result}; - cameraOne.testResult[0].SetReceiveTimestamp(wpi::units::second_t(15)); const wpi::units::radian_t camPitch = 30_deg; const wpi::math::Transform3d kRobotToCam{ diff --git a/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h b/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h index acd87aaa09..846e2857d1 100644 --- a/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h +++ b/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h @@ -38,25 +38,6 @@ class PhotonPipelineResult : public PhotonPipelineResult_PhotonStruct { PhotonPipelineResult() : Base() {} explicit PhotonPipelineResult(Base&& data) : Base(data) {} - // Don't forget to deal with our ntReceiveTimestamp - PhotonPipelineResult(const PhotonPipelineResult& other) - : Base(other), ntReceiveTimestamp(other.ntReceiveTimestamp) {} - PhotonPipelineResult(PhotonPipelineResult& other) - : Base(other), ntReceiveTimestamp(other.ntReceiveTimestamp) {} - PhotonPipelineResult(PhotonPipelineResult&& other) - : Base(std::move(other)), - ntReceiveTimestamp(std::move(other.ntReceiveTimestamp)) {} - auto& operator=(const PhotonPipelineResult& other) { - Base::operator=(other); - ntReceiveTimestamp = other.ntReceiveTimestamp; - return *this; - } - auto& operator=(PhotonPipelineResult&& other) { - ntReceiveTimestamp = other.ntReceiveTimestamp; - Base::operator=(std::move(other)); - return *this; - } - template explicit PhotonPipelineResult(Args&&... args) : Base{std::forward(args)...} {} @@ -120,11 +101,6 @@ class PhotonPipelineResult : public PhotonPipelineResult_PhotonStruct { */ int64_t SequenceID() const { return metadata.sequenceID; } - /** Sets the FPGA timestamp this result was Received by robot code */ - void SetReceiveTimestamp(const wpi::units::second_t timestamp) { - this->ntReceiveTimestamp = timestamp; - } - /** * Returns whether the pipeline has targets. * @return Whether the pipeline has targets. @@ -143,10 +119,6 @@ class PhotonPipelineResult : public PhotonPipelineResult_PhotonStruct { friend bool operator==(PhotonPipelineResult const&, PhotonPipelineResult const&) = default; - // Since we don't trust NT time sync, keep track of when we got this packet - // into robot code - wpi::units::microsecond_t ntReceiveTimestamp = -1_s; - inline static bool HAS_WARNED = false; }; } // namespace photon From 038295642ef4013505f76f4db8565160b2c313f1 Mon Sep 17 00:00:00 2001 From: Joe Lockwood Date: Mon, 18 May 2026 18:10:14 -0400 Subject: [PATCH 4/7] fix(photon-lib): correct inverted CheckTimeSyncOrWarn debounce in C++ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The C++ port of CheckTimeSyncOrWarn writes the debounce comparison backwards: it warns when LESS than WARN_DEBOUNCE_SEC has elapsed since the last warning rather than MORE. Java has the same function with the correct sense at PhotonCamera.java:319. Practical effect: at 50 Hz NT packet rate with timeSinceLastPong > 5s, every call sets prevTimeSyncWarnTime to "now", so the next call ~20 ms later checks `now+0.02 < now+5` — always true — and fires another warning. Result is a 50 Hz warning storm on the rio console rather than the once-per-five-seconds debounce the surrounding code is clearly trying to implement. In a less common path (packets pause for >5 s, then resume with timesync still broken), the inverted check becomes false on resume and the warning goes permanently silent until timesync recovers and resets prevTimeSyncWarnTime to 0. Fix is the one character that brings C++ in line with Java: flip `<` to `>`. No semantic change to the alert (timesyncAlert.Set(true)) or the reset branch. --- photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp b/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp index 657a1a65c5..fed82effc2 100644 --- a/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp +++ b/photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp @@ -254,7 +254,7 @@ void PhotonCamera::CheckTimeSyncOrWarn(photon::PhotonPipelineResult& result) { timesyncAlert.SetText(warningText); timesyncAlert.Set(true); - if (wpi::Timer::GetFPGATimestamp() < + if (wpi::Timer::GetFPGATimestamp() > (prevTimeSyncWarnTime + WARN_DEBOUNCE_SEC)) { prevTimeSyncWarnTime = wpi::Timer::GetFPGATimestamp(); From c85a5b9e73605b0f5ea5126c0c557ecddb9eb05b Mon Sep 17 00:00:00 2001 From: Joe Lockwood Date: Mon, 18 May 2026 22:51:49 -0400 Subject: [PATCH 5/7] docs(photon-lib): simplify getTimestampSeconds doc across C++/Java/Python --- .../py/photonlibpy/targeting/photonPipelineResult.py | 7 ++++--- .../org/photonvision/targeting/PhotonPipelineResult.java | 7 +++---- .../native/include/photon/targeting/PhotonPipelineResult.h | 7 +++---- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py b/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py index 86a8c5034c..8ca3169fa7 100644 --- a/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py +++ b/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py @@ -39,9 +39,10 @@ def getLatencyMillis(self) -> float: def getTimestampSeconds(self) -> float: """ - Returns the estimated time the frame was taken, in the Time Sync Server's time base - (wpi::nt::Now). Reads metadata.captureTimestampMicros directly — latency compensation - is already baked in by the coprocessor. + Returns the estimated time the frame was captured, in the same time base as + ``wpilib.Timer.getFPGATimestamp()``. + + :returns: The timestamp in seconds. """ return self.metadata.captureTimestampMicros / 1e6 diff --git a/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineResult.java b/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineResult.java index 7945276d99..10b1729917 100644 --- a/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineResult.java +++ b/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineResult.java @@ -168,11 +168,10 @@ public Optional getMultiTagResult() { } /** - * Returns the estimated time the frame was taken, in the Time Sync Server's time base - * (wpi::nt::Now). This is calculated using the estimated offset between Time Sync Server time and - * local time. The robot shall run a server, so the offset shall be 0. + * Returns the estimated time the frame was captured, in the same time base as {@link + * edu.wpi.first.wpilibj.Timer#getFPGATimestamp()}. * - * @return The timestamp in seconds + * @return The timestamp in seconds. */ public double getTimestampSeconds() { return metadata.captureTimestampMicros / 1e6; diff --git a/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h b/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h index 846e2857d1..7ae44cf74d 100644 --- a/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h +++ b/photon-targeting/src/main/native/include/photon/targeting/PhotonPipelineResult.h @@ -72,10 +72,9 @@ class PhotonPipelineResult : public PhotonPipelineResult_PhotonStruct { } /** - * Returns the estimated time the frame was taken, in the Time Sync Server's - * time base (nt::Now). The robot shall run a server, so this is FPGA-relative - * on a real robot. Reads metadata.captureTimestampMicros directly — latency - * compensation is already baked in by the coprocessor. + * Returns the estimated time the frame was captured, in the same time base as + * wpi::Timer::GetMonotonicTimestamp + * * @return The timestamp in seconds. */ wpi::units::second_t GetTimestamp() const { From 22027879fb1d72d42d231763e326765db025ade8 Mon Sep 17 00:00:00 2001 From: Joe Lockwood Date: Sat, 16 May 2026 21:18:44 -0400 Subject: [PATCH 6/7] docs(photon-targeting): correct PhotonPipelineMetadata timestamp Javadocs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The getCaptureTimestampMicros / getPublishTimestampMicros Javadocs claim "coprocessor's time base", but NTDataPublisher applies the TimeSyncClient offset to both values before publishing — the on-wire value is in the Time Sync Server's nt::Now timebase, not the coprocessor's local clock. The field-level comment at PhotonPipelineMetadata.java:25-26 already states the correct timebase ("wpi::nt::Now on the time sync server"). This commit aligns the getter Javadocs (which users see in IDE autocomplete and online API docs) with that truth. The Python per-field comment carried the same misleading prose; updated to match. No code or behavior change. --- .../py/photonlibpy/targeting/photonPipelineResult.py | 5 +++-- .../photonvision/targeting/PhotonPipelineMetadata.java | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py b/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py index 8ca3169fa7..e410c1c8e2 100644 --- a/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py +++ b/photon-lib/py/photonlibpy/targeting/photonPipelineResult.py @@ -11,8 +11,9 @@ @dataclass class PhotonPipelineMetadata: - # Image capture and NT publish timestamp, in microseconds and in the coprocessor timebase. As - # reported by WPIUtilJNI::now. + # Image capture and NT publish timestamp, in microseconds and in the Time Sync Server's + # timebase (wpi::nt::Now). The robot shall run a server, so this is FPGA-relative on a real + # robot. NTDataPublisher applies the time-sync offset before publishing. captureTimestampMicros: int = -1 publishTimestampMicros: int = -1 diff --git a/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineMetadata.java b/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineMetadata.java index e5687ca427..e6b4290aa5 100644 --- a/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineMetadata.java +++ b/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineMetadata.java @@ -58,7 +58,9 @@ public double getLatencyMillis() { } /** - * The time that this image was captured, in the coprocessor's time base. + * The time that this image was captured, in microseconds and in the Time Sync Server's time + * base ({@code wpi::nt::Now}). The robot shall run a server, so this is FPGA-relative on a real + * robot. NTDataPublisher applies the time-sync offset before publishing. * * @return The time in microseconds */ @@ -67,7 +69,9 @@ public long getCaptureTimestampMicros() { } /** - * The time that this result was published to NT, in the coprocessor's time base. + * The time that this result was published to NT, in microseconds and in the Time Sync Server's + * time base ({@code wpi::nt::Now}). The robot shall run a server, so this is FPGA-relative on a + * real robot. NTDataPublisher applies the time-sync offset before publishing. * * @return The time in microseconds */ From 8c54b008dc3f866a7a57f660900e7bea7311c870 Mon Sep 17 00:00:00 2001 From: Joe Lockwood Date: Sat, 16 May 2026 22:35:44 -0400 Subject: [PATCH 7/7] style(photon-targeting): rewrap PhotonPipelineMetadata getCaptureTimestampMicros Javadoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spotless (google-java-format) rejected the prior wrap because the first line fell under the 100-col target — its preferred split lands "time base" on line 1 and "real robot." on line 2. CI Java Formatting job failed on this file only. Pure whitespace reflow; identical wording. --- .../org/photonvision/targeting/PhotonPipelineMetadata.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineMetadata.java b/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineMetadata.java index e6b4290aa5..be4e73a31b 100644 --- a/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineMetadata.java +++ b/photon-targeting/src/main/java/org/photonvision/targeting/PhotonPipelineMetadata.java @@ -58,9 +58,9 @@ public double getLatencyMillis() { } /** - * The time that this image was captured, in microseconds and in the Time Sync Server's time - * base ({@code wpi::nt::Now}). The robot shall run a server, so this is FPGA-relative on a real - * robot. NTDataPublisher applies the time-sync offset before publishing. + * The time that this image was captured, in microseconds and in the Time Sync Server's time base + * ({@code wpi::nt::Now}). The robot shall run a server, so this is FPGA-relative on a real robot. + * NTDataPublisher applies the time-sync offset before publishing. * * @return The time in microseconds */