Skip to content

Clarify and fix OpenCV/WPILib version checking error#1963

Merged
Gold856 merged 3 commits intoPhotonVision:mainfrom
spacey-sooty:fix-opencv-error
Jul 12, 2025
Merged

Clarify and fix OpenCV/WPILib version checking error#1963
Gold856 merged 3 commits intoPhotonVision:mainfrom
spacey-sooty:fix-opencv-error

Conversation

@spacey-sooty
Copy link
Member

@spacey-sooty spacey-sooty commented May 15, 2025

Description

Fixed the error in the OpenCV/WPILib version checking crash and clarified it, since it's not PhotonVision that needs updating, but rather WPILib.

Reported on chief https://www.chiefdelphi.com/t/opencv-is-version-4-6-0-and-needs-to-be-4-10-0/501751/7

Meta

Merge checklist:

  • Pull Request title is short, imperative summary of proposed changes
  • The description documents the what and why
  • If this PR changes behavior or adds a feature, user documentation is updated
  • If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly
  • If this PR touches configuration, this is backwards compatible with settings back to v2024.3.1
  • If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated
  • If this PR addresses a bug, a regression test for it is added

@spacey-sooty spacey-sooty requested a review from a team as a code owner May 15, 2025 01:22
@mcm001
Copy link
Contributor

mcm001 commented May 15, 2025

The comment on chief Delphi also had a missing return character. Could you add that here?

@spacey-sooty spacey-sooty changed the title Fix opencv version checking error Fix OpenCV version checking error May 15, 2025
@spacey-sooty spacey-sooty requested a review from mcm001 May 15, 2025 01:23
@mcm001
Copy link
Contributor

mcm001 commented May 15, 2025

The other failure mode is a vendor dependency, pulling in something stupid. Should we mention that?

@samfreund
Copy link
Member

Perhaps something about how the versions need to match, not just about they need to be upgraded?

@samfreund
Copy link
Member

The other failure mode is a vendor dependency, pulling in something stupid. Should we mention that?

Something like
upgrade photonlib, wpilib, and other vendordeps?

@spacey-sooty
Copy link
Member Author

spacey-sooty commented May 15, 2025

The other failure mode is a vendor dependency, pulling in something stupid. Should we mention that?

I wouldn't bother unless you can think of a vendor which does, it would only lead people down the wrong path right now.

Perhaps something about how the versions need to match, not just about they need to be upgraded?

Versions needing to match is difficult, we often have mismatching versions. If they are both latest it should work.

@Gold856
Copy link
Member

Gold856 commented May 15, 2025

There should be an explanation on the dependencies task. I don’t think it’s obvious what you’re supposed to do with it. It should be reworded to say that you need to look through the output and see if there’s an old version of OpenCV being pulled in (same goes for WPILib).

Signed-off-by: Jade Turner <spacey-sooty@proton.me>
Signed-off-by: Jade Turner <spacey-sooty@proton.me>
@spacey-sooty
Copy link
Member Author

WPILib version message also needs to be updated 💀

@Gold856 Gold856 changed the title Fix OpenCV version checking error Clarify and fix OpenCV/WPILib version checking error Jul 12, 2025
@Gold856 Gold856 force-pushed the fix-opencv-error branch from 213bab0 to a4b6d67 Compare July 12, 2025 07:18
@Gold856 Gold856 enabled auto-merge (squash) July 12, 2025 07:42
@Gold856 Gold856 merged commit ef5e646 into PhotonVision:main Jul 12, 2025
39 checks passed
@Gold856 Gold856 added the photonlib Things related to the PhotonVision library label Jul 12, 2025
@spacey-sooty spacey-sooty deleted the fix-opencv-error branch July 12, 2025 13:31
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