Workaround for cheap misreporting webcam#339
Conversation
It looks like the camera advertises that it has a menu but then that component is just empty.
Adding this additional constraint fixed my webcam, but it's not obvious to the reader why this is the case.
The traceback I was chasing down is as follows:
[06/05/26 23:54:35] INFO: Detect available Devices
Traceback (most recent call last):
File "<frozen runpy>", line 198, in _run_module_as_main
File "<frozen runpy>", line 88, in _run_code
File "/home/klipper/crowsnest/crowsnest/__main__.py", line 8, in <module>
asyncio.run(main())
File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
return runner.run(main)
^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
return self._loop.run_until_complete(task)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/asyncio/base_events.py", line 653, in run_until_complete
return future.result()
^^^^^^^^^^^^^^^
File "/home/klipper/crowsnest/crowsnest/crowsnest.py", line 195, in main
logging_helper.log_cams()
File "/home/klipper/crowsnest/crowsnest/logging_helper.py", line 114, in log_cams
uvc = camera.camera_manager.init_camera_type(camera.UVC)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/klipper/crowsnest/crowsnest/camera/camera_manager.py", line 22, in init_camera_type
cams = obj.init_camera_type()
^^^^^^^^^^^^^^^^^^^^^^
File "/home/klipper/crowsnest/crowsnest/camera/types/uvc.py", line 108, in init_camera_type
return [
^
File "/home/klipper/crowsnest/crowsnest/camera/types/uvc.py", line 109, in <listcomp>
UVC(dev_path, other=other_paths)
File "/home/klipper/crowsnest/crowsnest/camera/types/uvc.py", line 33, in __init__
parsed_qc = v4l2.ctl.parse_qc_of_path(self.path, qc)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/klipper/crowsnest/crowsnest/v4l2/ctl.py", line 69, in parse_qc_of_path
controls = parse_qc(fd, qc)
^^^^^^^^^^^^^^^^
File "/home/klipper/crowsnest/crowsnest/v4l2/ctl.py", line 46, in parse_qc
for menu in utils.ioctl_iter(
File "/home/klipper/crowsnest/crowsnest/v4l2/utils.py", line 35, in ioctl_iter
for i in range(start, stop, step):
^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: range() arg 3 must not be zero
📝 WalkthroughWalkthroughThe PR refines menu control detection in ChangesMenu Control Condition Guard
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crowsnest/v4l2/ctl.py (1)
41-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe
qc.minimum > 0guard silently drops valid menu controls and doesn't fix the root cause.The crash reported in the commit message (
ValueError: range() arg 3 must not be zero) occurs becauseqc.stepcan be zero for menu-type controls. Whenutils.ioctl_itercallsrange(start, stop, step)with a zero step, it fails. The current guard at line 44 masks this by skipping controls withminimum <= 0, but V4L2 allows menu controls to have any minimum value (including 0, which is common). This approach silently loses menu data on affected devices instead of handling the root cause.A more robust fix checks that the bounds are valid and forces a non-zero iteration step:
Suggested patch
- if qc.type in ( - constants.V4L2_CTRL_TYPE_MENU, - constants.V4L2_CTRL_TYPE_INTEGER_MENU, - ) and qc.minimum > 0: + if qc.type in ( + constants.V4L2_CTRL_TYPE_MENU, + constants.V4L2_CTRL_TYPE_INTEGER_MENU, + ) and qc.maximum >= qc.minimum: controls["menu"] = {} + menu_step = qc.step if qc.step and qc.step > 0 else 1 for menu in utils.ioctl_iter( fd, raw.VIDIOC_QUERYMENU, raw.v4l2_querymenu(id=qc.id), qc.minimum, qc.maximum + 1, - qc.step, + menu_step, True, ):This allows menu controls with
minimum == 0to be processed while preventing the range step-zero crash.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crowsnest/v4l2/ctl.py` around lines 41 - 53, The current guard "qc.minimum > 0" silently skips valid menu controls; instead remove that guard and fix the root cause by ensuring ioctl_iter is invoked with a non-zero, valid step and sane bounds: in the controls["menu"] block where utils.ioctl_iter(...) is called with arguments (fd, raw.VIDIOC_QUERYMENU, raw.v4l2_querymenu(id=qc.id), qc.minimum, qc.maximum + 1, qc.step, True), compute a safe_step = qc.step if qc.step and qc.step != 0 else 1 and use safe_step, and ensure the start/stop passed (qc.minimum and qc.maximum + 1) form a valid range (e.g., only call ioctl_iter when qc.maximum + 1 > qc.minimum); keep references to qc, utils.ioctl_iter, raw.v4l2_querymenu and controls["menu"] so the change is easy to find.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crowsnest/v4l2/ctl.py`:
- Around line 41-53: The current guard "qc.minimum > 0" silently skips valid
menu controls; instead remove that guard and fix the root cause by ensuring
ioctl_iter is invoked with a non-zero, valid step and sane bounds: in the
controls["menu"] block where utils.ioctl_iter(...) is called with arguments (fd,
raw.VIDIOC_QUERYMENU, raw.v4l2_querymenu(id=qc.id), qc.minimum, qc.maximum + 1,
qc.step, True), compute a safe_step = qc.step if qc.step and qc.step != 0 else 1
and use safe_step, and ensure the start/stop passed (qc.minimum and qc.maximum +
1) form a valid range (e.g., only call ioctl_iter when qc.maximum + 1 >
qc.minimum); keep references to qc, utils.ioctl_iter, raw.v4l2_querymenu and
controls["menu"] so the change is easy to find.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5fe3cee-a42b-4f43-84a0-617185354e66
📒 Files selected for processing (1)
crowsnest/v4l2/ctl.py
|
Hi, first of all thank you for the PR. Can you revert your change locally and change it like this? constants.V4L2_CTRL_TYPE_MENU,
constants.V4L2_CTRL_TYPE_INTEGER_MENU,
):
- controls["menu"] = {}
+ controls["menu"] = {"min": qc.minimum, "max": qc.maximum + 1, "step": qc.step}
+ return controls
for menu in utils.ioctl_iter(
fd,
raw.VIDIOC_QUERYMENU,Then we should see the problematic values inside the menu in your log and can check if the Code Rabbit solution actually makes sense. |
|
Hey, any updates on this? |
2 participants
It looks like the camera advertises that it has a menu but then that component is just empty.
Adding this additional constraint fixed my webcam, but it's not obvious to the reader why this is the case.
The traceback I was chasing down is as follows:
I'm open to a more structural change, but I'm new to Klipper and this was preventing me from getting going; I don't know what would be more appropriate than this quick patch.
Thank you for crowsnest, it's wonderful to see video showing up in Mainsail for the first time 🙂