Upgrade to wpilib alpha-5#2434
Conversation
5c982c1 to
b6376cc
Compare
There was a problem hiding this comment.
Pull request overview
Updates PhotonVision’s build + source to match API changes in a newer WPILib (alpha-5 per PR title), including timestamp/pixel format enum renames and an added JSON-B dependency.
Changes:
- Bump WPILib/NativeUtils versions and update year labeling in Gradle configuration.
- Replace deprecated WPILib APIs (FPGA timestamp/time, PixelFormat, NT event enums, MatchType enum values) across Java/C++/Python and docs/tests.
- Add
io.avaje:avaje-jsonbas a shared dependency.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/javacommon.gradle | Adds avaje-jsonb dependency to shared Java common deps. |
| shared/common.gradle | Adds avaje-jsonb dependency to shared deps used across projects. |
| photon-targeting/src/test/java/jni/CscoreExtrasTest.java | Updates cscore PixelFormat enum names in tests. |
| photon-targeting/src/main/native/cpp/photon/constrained_solvepnp/wrap/casadi_wrapper.cpp | Adjusts Sleipnir ExitStatus value to match updated API. |
| photon-lib/src/test/java/org/photonvision/PhotonCameraTest.java | Updates Timer API usage in tests. |
| photon-lib/src/main/native/include/photon/simulation/VisionSystemSim.h | Updates Timer API usage for sim buffers. |
| photon-lib/src/main/native/cpp/photon/simulation/PhotonCameraSim.cpp | Updates PixelFormat enum name in sim output. |
| photon-lib/src/main/native/cpp/photon/PhotonCamera.cpp | Updates monotonic time APIs for timestamps, heartbeat, and version checks. |
| photon-lib/src/main/java/org/photonvision/simulation/VisionSystemSim.java | Updates Timer API usage for sim buffers. |
| photon-lib/src/main/java/org/photonvision/simulation/PhotonCameraSim.java | Updates PixelFormat + monotonic time APIs in simulation. |
| photon-lib/src/main/java/org/photonvision/PhotonCamera.java | Updates Timer API usage for warnings, heartbeat, and version checks. |
| photon-lib/py/photonlibpy/timesync/timeSyncServer.py | Switches default time provider to monotonic timestamp API. |
| photon-lib/py/photonlibpy/simulation/visionSystemSim.py | Switches sim timestamps to monotonic timestamp API. |
| photon-lib/py/photonlibpy/simulation/photonCameraSim.py | Updates sim timestamps and PixelFormat enum names. |
| photon-lib/py/photonlibpy/photonCamera.py | Updates timestamp sources to monotonic time APIs. |
| photon-core/src/main/java/org/photonvision/vision/pipe/impl/CalculateFPSPipe.java | Uses monotonic timestamp for FPS calculations. |
| photon-core/src/main/java/org/photonvision/vision/frame/provider/USBFrameProvider.java | Updates PixelFormat enum name. |
| photon-core/src/main/java/org/photonvision/vision/frame/consumer/MJPGFrameConsumer.java | Updates PixelFormat enum name. |
| photon-core/src/main/java/org/photonvision/vision/frame/consumer/FileSaveFrameConsumer.java | Updates MatchType enum value name. |
| photon-core/src/main/java/org/photonvision/vision/camera/csi/LibcameraGpuSettables.java | Updates PixelFormat enum name. |
| photon-core/src/main/java/org/photonvision/vision/camera/USBCameras/GenericUSBCameraSettables.java | Updates PixelFormat enum name in filtering logic. |
| photon-core/src/main/java/org/photonvision/vision/camera/FileVisionSource.java | Updates PixelFormat enum name in file source mode. |
| photon-core/src/main/java/org/photonvision/common/dataflow/networktables/NetworkTablesManager.java | Updates NT logging + event Kind enum names. |
| photon-core/src/main/java/org/photonvision/common/dataflow/networktables/NTDriverStation.java | Updates NT event Kind enum names. |
| photon-core/src/main/java/org/photonvision/common/dataflow/networktables/NTDataChangeListener.java | Updates NT event Kind enum names. |
| docs/source/docs/contributing/design-descriptions/e2e-latency.md | Updates Timer API in documentation snippet. |
| build.gradle | Updates NativeUtils version, wpilibVersion/frcYear, and adds avajeJsonbVersion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def getRobotToCamera( | ||
| self, | ||
| cameraSim: PhotonCameraSim, | ||
| time: seconds = wpilib.Timer.getFPGATimestamp(), | ||
| time: seconds = wpilib.Timer.getMonotonicTimestamp(), | ||
| ) -> Transform3d | None: |
There was a problem hiding this comment.
These methods use a call to wpilib.Timer.getMonotonicTimestamp() as a default argument. In Python, default argument expressions are evaluated once at function definition time, so calls that omit time will use a stale timestamp. Use time: seconds | None = None (or similar) and assign time = wpilib.Timer.getMonotonicTimestamp() inside the method when it is None.
| def getCameraPose( | ||
| self, | ||
| cameraSim: PhotonCameraSim, | ||
| time: seconds = wpilib.Timer.getFPGATimestamp(), | ||
| time: seconds = wpilib.Timer.getMonotonicTimestamp(), | ||
| ) -> Pose3d | None: |
There was a problem hiding this comment.
getCameraPose() also uses wpilib.Timer.getMonotonicTimestamp() as a default argument, which is evaluated only once when the function is defined. This will return incorrect results when callers omit time. Prefer a None default and compute the timestamp at call time.
| def getRobotPose( | ||
| self, timestamp: seconds = wpilib.Timer.getFPGATimestamp() | ||
| self, timestamp: seconds = wpilib.Timer.getMonotonicTimestamp() | ||
| ) -> Pose3d | None: |
There was a problem hiding this comment.
getRobotPose() uses wpilib.Timer.getMonotonicTimestamp() as a default argument, which is evaluated at import/definition time rather than call time. This makes getRobotPose() return a pose for an effectively fixed timestamp unless the caller explicitly passes one. Use a None default and compute the current time inside the function when omitted.
fed9faf to
40e5d2e
Compare
114d043 to
5ada78b
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
0c53539 to
84b269a
Compare
|
I think the errors we're currently seeing stem from avaje being funky and doing stuff, more investigation to come. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (field2points.cols() != (nTags * 4) || | ||
| point_observations.cols() != (nTags * 4)) { | ||
| if constexpr (VERBOSE) fmt::println("Got unexpected num cols!"); | ||
| // TODO find a new error code | ||
| return wpi::util::unexpected{ | ||
| slp::ExitStatus::NONFINITE_INITIAL_COST_OR_CONSTRAINTS}; | ||
| return wpi::util::unexpected{slp::ExitStatus::NONFINITE_INITIAL_GUESS}; | ||
| } |
There was a problem hiding this comment.
This branch is returning slp::ExitStatus::NONFINITE_INITIAL_GUESS when the observation matrices have the wrong dimensions. That status doesn't match the actual failure mode (invalid input dimensions) and can mislead downstream diagnostics/telemetry. Prefer a dedicated/accurate error code (or propagate an explicit input-validation error) rather than reusing a numerical failure status.
| auto problemOpt = createProblem(nTags, heading_free); | ||
| if (!problemOpt) { | ||
| return wpi::util::unexpected{ | ||
| slp::ExitStatus::NONFINITE_INITIAL_COST_OR_CONSTRAINTS}; | ||
| return wpi::util::unexpected{slp::ExitStatus::NONFINITE_INITIAL_GUESS}; | ||
| } |
There was a problem hiding this comment.
createProblem() failing maps to NONFINITE_INITIAL_GUESS, which again doesn't reflect the real error (problem construction failed, likely unsupported tag count / config). Returning a more representative status (or logging the specific reason) will make failures much easier to debug.
| .configure(SerializationFeature.FAIL_ON_SELF_REFERENCES, false) | ||
| .configure(SerializationFeature.WRITE_SELF_REFERENCES_AS_NULL, true) |
There was a problem hiding this comment.
Disabling FAIL_ON_SELF_REFERENCES and enabling WRITE_SELF_REFERENCES_AS_NULL in the shared ObjectMapper can silently drop data when a config object accidentally becomes cyclic (it will serialize to null instead of failing fast). Since this mapper is used for writing persistent configuration, consider fixing the underlying self-reference (or applying a targeted Jackson annotation/serializer for the specific type) rather than globally masking it.
| .configure(SerializationFeature.FAIL_ON_SELF_REFERENCES, false) | |
| .configure(SerializationFeature.WRITE_SELF_REFERENCES_AS_NULL, true) |
No description provided.