[DRAFT] 6 modify cameras config update #2638
Conversation
Update LeRobot
Separate parts in config
…-fix-reachy2-tests fix(tests) fixes for reachy2 tests; removing reachy2 references from …
There was a problem hiding this comment.
Pull request overview
This PR refactors the Reachy 2 camera and robot integration to improve usability and resolve import issues. The main changes include removing threaded image reading from the camera implementation, making FPS non-configurable (fixed at 30 FPS), and implementing lazy imports to avoid dependency errors when reachy2_sdk is not installed.
Key changes:
- Removed background threading from Reachy2Camera, simplifying async_read to directly call read()
- Changed default camera configuration to disable cameras by default and made FPS non-configurable
- Added lazy import pattern for reachy2_sdk dependencies to prevent import errors
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lerobot/cameras/reachy2_camera/reachy2_camera.py | Removed threading logic, simplified async_read, added lazy import for reachy2_sdk |
| src/lerobot/cameras/reachy2_camera/configuration_reachy2_camera.py | Made FPS fixed at 30, updated documentation, added default values for name/image_type |
| src/lerobot/robots/reachy2/robot_reachy2.py | Added lazy import for reachy2_sdk, reformatted long lines |
| src/lerobot/robots/reachy2/configuration_reachy2.py | Changed default camera settings to disabled, removed FPS parameters |
| src/lerobot/teleoperators/reachy2_teleoperator/reachy2_teleoperator.py | Added lazy import for reachy2_sdk, reformatted long lines |
| src/lerobot/scripts/lerobot_record.py | Added reachy2 and reachy2_teleoperator imports |
| src/lerobot/scripts/lerobot_replay.py | Added reachy2 import |
| docs/source/reachy2.mdx | Updated command examples from python -m to lerobot-* commands, clarified camera configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Raises: | ||
| DeviceNotConnectedError: If the camera is not connected. | ||
| """ | ||
| self.CameraView, CameraManager = _require_reachy2_sdk_cameras() |
There was a problem hiding this comment.
The variable name self.CameraView uses PascalCase which is typically reserved for class names, not instance variables. Consider renaming to self.camera_view_class or storing it as a local variable since it appears to only be used within the read method.
| self.CameraView, CameraManager = _require_reachy2_sdk_cameras() | |
| CameraView, CameraManager = _require_reachy2_sdk_cameras() |
|
|
||
| @staticmethod | ||
| def find_cameras(ip_address: str = "localhost", port: int = 50065) -> list[dict[str, Any]]: | ||
| def find_cameras() -> list[dict[str, Any]]: |
There was a problem hiding this comment.
The find_cameras method signature has been changed to remove parameters (ip_address and port), but it's a static method that may be called externally. This is a breaking API change that could affect existing code that passes these parameters.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Gaëlle Lannuzel <[email protected]>
3 participants
What this does
Modify a few things on reachy2_camera:
Modifications have also be made to avoid draccus problem that made complicated to use
lerobot-recordandlerobot-replaywith Reachy 2 (was not a accepted robot), and avoid not imported module error for reachy2_sdk if users did not installed it.How it was tested
Still to be tested