-
Notifications
You must be signed in to change notification settings - Fork 0
camera module #1
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
base: main
Are you sure you want to change the base?
Conversation
src/humanoid_vision/camera.py
Outdated
raise NotImplementedError("Camera module not yet implemented") | ||
|
||
MAX_IDX = 10 | ||
#cv2.utils.logging.setLogLevel(cv2.utils.logging.LOG_LEVEL_ERROR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the commented out lines if possible
src/humanoid_vision/camera.py
Outdated
import torch | ||
import torch.nn as nn | ||
|
||
def placeholder_camera() -> Any: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can get rid of the placeholder now
src/humanoid_vision/camera.py
Outdated
print(available) | ||
|
||
class Camera(): | ||
def __init__(self, idx: int): #could also be a string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference if it is an int vs a string?
src/humanoid_vision/camera.py
Outdated
def set_brightness(self, val:float): | ||
self.cam.set(cv2.CAP_PROP_BRIGHTNESS, val) | ||
|
||
def test(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you refactor the tests to use pytest, and move it to a tests folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a basic camera module for humanoid vision functionality, replacing a placeholder implementation with OpenCV-based camera access. The module provides camera device discovery, initialization, and basic property control.
- Adds camera device enumeration and availability checking
- Implements a Camera class with initialization and brightness control
- Includes a test function to demonstrate camera functionality
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/humanoid_vision/camera.py
Outdated
from typing import Any | ||
|
||
import cv2 | ||
from cv2_enumerate_cameras import enumerate_cameras |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cv2_enumerate_cameras
import is unused in the code. Consider removing it or using it to replace the manual camera enumeration loop.
Copilot uses AI. Check for mistakes.
src/humanoid_vision/camera.py
Outdated
import torch | ||
import torch.nn as nn |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The torch imports are unused in this module. Remove these unused imports to keep the code clean.
import torch | |
import torch.nn as nn |
Copilot uses AI. Check for mistakes.
available = [] | ||
for i in range(MAX_IDX): | ||
cap = cv2.VideoCapture(i) | ||
if cap.isOpened(): | ||
available.append(i) | ||
cap.release() | ||
|
||
print(available) |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Global camera enumeration code should be moved inside a function or class method. Module-level execution can cause issues during imports and testing.
Copilot uses AI. Check for mistakes.
available.append(i) | ||
cap.release() | ||
|
||
print(available) |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug print statement from production code. Consider using logging instead if this information is needed.
Copilot uses AI. Check for mistakes.
src/humanoid_vision/camera.py
Outdated
raise ValueError(f"id isn't an available device, select from {available}") | ||
|
||
self.cam = cv2.VideoCapture(idx) | ||
# Show the frame in a window |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove orphaned comment that doesn't match the actual code. This comment appears to be leftover from development.
# Show the frame in a window |
Copilot uses AI. Check for mistakes.
src/humanoid_vision/camera.py
Outdated
|
||
print(available) | ||
|
||
class Camera(): |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove empty parentheses after class name. In Python, class Camera:
is preferred over class Camera():
.
class Camera(): | |
class Camera: |
Copilot uses AI. Check for mistakes.
src/humanoid_vision/camera.py
Outdated
class Camera(): | ||
def __init__(self, idx: int): #could also be a string | ||
""" | ||
initialize instance to a cv2 index |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalize the first word of the docstring: 'Initialize instance to a cv2 index'.
initialize instance to a cv2 index | |
Initialize instance to a cv2 index |
Copilot uses AI. Check for mistakes.
approve me twin