Skip to content

Fix OpenCV version checking error #1963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

spacey-sooty
Copy link
Member

Description

Fixed the error in the OpenCV version checking crash.

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.

Signed-off-by: Jade Turner <[email protected]>
@@ -231,11 +231,12 @@ public static void verifyDependencies() {
+ "\n"
+ ">>> but you are using OpenCV "
+ Core.VERSION
+ "\n"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C++ already has this

@Gold856
Copy link
Contributor

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants