Skip to content
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

Another config matching bug #1518

Merged
merged 4 commits into from
Nov 5, 2024
Merged

Conversation

Juniormunk
Copy link
Contributor

This is quite an odd issue/fix.

So this is what happened... Photonvision booted with the camera connected and the camera was working...
After a short time the camera stopped working (for some reason maybe static, maybe temp, maybe wiring, idk).
During this time pv showed

Jul 04 06:25:18 BackLeft java[643]: [2024-07-04 06:25:18] [CSCore - PvCSCoreLogger] [ERROR] CS: ERROR 40: ioctl VIDIOC_QBUF failed at UsbCameraImpl.cpp:723: Invalid argument (UsbUtil.cpp:156)
Jul 04 06:25:18 BackLeft java[643]: [2024-07-04 06:25:18] [CSCore - PvCSCoreLogger] [WARN] CS: WARNING 30: BackLeft: could not queue buffer 0 (UsbCameraImpl.cpp:724)

I went over and played with the wire. The camera fully disconnected but it ended up "reconnecting"
When the camera was "reconnected" photonvision detected a "new camera" except this time with no otherpaths (aka no usb path, or by id path).
That resulted in pv creating a new camera configuration for a camera with no otherpaths
Cscore then started to report errors that look like it attempted to connect to the same camera twice

This fixes it by filtering out USB cameras that have no otherpath on linux.

@Juniormunk Juniormunk requested a review from a team as a code owner November 4, 2024 01:59
@mcm001
Copy link
Contributor

mcm001 commented Nov 4, 2024

Of dmekkfkskskksjfjd course. Thanks V4L. Will dig in tonight

Comment on lines 539 to +540
} else if (device.getIsV4lCsiCamera()) {
} else if (device.otherPaths.length == 0
Copy link
Contributor

Choose a reason for hiding this comment

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

double else-if - syntax error?

Copy link
Contributor

Choose a reason for hiding this comment

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

skill issue, this is a complete statement, just a confusing one with an empty body. add a comment

Comment on lines +545 to +546
// If cscore hasnt passed this other paths aka a path by id or a path as in usb port then we
// cant guarantee it is a valid camera.
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to make any assertions to check that otherPaths does not contain "by-id" or "by-path", or will the otherPaths length always be empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. This specific problem is hard to recreate. I believe if we get either a by-id or by-path we should be good to assume we can read from the camera.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to merge this as-is, or explicitly check only that there are no by-ids or by-paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

im fine doing it as is

Copy link
Contributor

@mcm001 mcm001 left a comment

Choose a reason for hiding this comment

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

send it

@mcm001 mcm001 merged commit 5d55d21 into PhotonVision:master Nov 5, 2024
31 checks passed
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.

2 participants