-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Feat/add zmq camera #2563
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?
Feat/add zmq camera #2563
Conversation
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 adds ZMQ (ZeroMQ) camera support to the lerobot framework, enabling remote camera streaming over network connections. The implementation includes a new ZMQCamera class with support for multiple message formats (msgpack, JSON, and raw JPEG), automatic camera discovery via network scanning, and both synchronous and asynchronous frame reading capabilities. However, the PR contains several critical bugs that will prevent the code from running:
Critical Issues:
- All script files attempt to import and use a non-existent
busy_waitfunction instead of the existingprecise_sleepfunction, causing ImportError - The
customteleoperator import doesn't exist and will cause ImportError - ZMQ socket thread-safety issue where the same socket is used from multiple threads
Key Changes:
- Added complete ZMQ camera implementation with multi-format support and network scanning
- Updated recording, replay, and teleoperation scripts to support ZMQ cameras
- Modified recording behavior to support neutral/idle actions when no teleoperator or policy is provided
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 25 comments.
Show a summary per file
| File | Description |
|---|---|
src/lerobot/cameras/zmq/__init__.py |
Module initialization for ZMQ camera package |
src/lerobot/cameras/zmq/configuration_zmq.py |
Configuration class for ZMQ cameras with validation |
src/lerobot/cameras/zmq/camera_zmq.py |
Core ZMQ camera implementation with network discovery and async reading |
src/lerobot/scripts/lerobot_teleoperate.py |
Added ZMQ camera support and unitree_g1 robot; changed to non-existent busy_wait function (critical bug) |
src/lerobot/scripts/lerobot_replay.py |
Added unitree_g1 robot support; changed to non-existent busy_wait function (critical bug) |
src/lerobot/scripts/lerobot_record.py |
Added ZMQ camera, unitree_g1 robot, neutral action mode; changed to non-existent busy_wait function (critical bug) |
src/lerobot/scripts/lerobot_find_cameras.py |
Extended camera discovery to include ZMQ cameras with network scanning |
Comments suppressed due to low confidence (9)
src/lerobot/cameras/zmq/camera_zmq.py:321
- Multiple bare except clauses (lines 309-310, 320-321). Consider catching specific exception types or at minimum use
except Exceptionto avoid masking system-exiting exceptions.
except:
pass
except:
pass
# Raw JPEG
if frame is None:
try:
frame = cv2.imdecode(np.frombuffer(msg, np.uint8), cv2.IMREAD_COLOR)
if frame is not None:
fmt = "raw_jpeg"
except:
pass
src/lerobot/cameras/zmq/camera_zmq.py:294
- Except block directly handles BaseException.
except:
src/lerobot/cameras/zmq/camera_zmq.py:309
- Except block directly handles BaseException.
except:
src/lerobot/cameras/zmq/camera_zmq.py:311
- Except block directly handles BaseException.
except:
src/lerobot/cameras/zmq/camera_zmq.py:320
- Except block directly handles BaseException.
except:
src/lerobot/cameras/zmq/camera_zmq.py:294
- 'except' clause does nothing but pass and there is no explanatory comment.
except:
src/lerobot/cameras/zmq/camera_zmq.py:309
- 'except' clause does nothing but pass and there is no explanatory comment.
except:
src/lerobot/cameras/zmq/camera_zmq.py:311
- 'except' clause does nothing but pass and there is no explanatory comment.
except:
src/lerobot/cameras/zmq/camera_zmq.py:320
- 'except' clause does nothing but pass and there is no explanatory comment.
except:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
|
||
| dt_s = time.perf_counter() - start_episode_t | ||
| precise_sleep(1 / dataset.fps - dt_s) | ||
| busy_wait(1 / dataset.fps - dt_s) |
Copilot
AI
Dec 1, 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 call to busy_wait(1 / dataset.fps - dt_s) will fail because the busy_wait function doesn't exist in lerobot.utils.robot_utils. The correct function name is precise_sleep.
|
|
||
| dt_s = time.perf_counter() - start_loop_t | ||
| precise_sleep(1 / fps - dt_s) | ||
| busy_wait(1 / fps - dt_s) |
Copilot
AI
Dec 1, 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 call to busy_wait(1 / fps - dt_s) will fail because the busy_wait function doesn't exist in lerobot.utils.robot_utils. The correct function name is precise_sleep.
| except: | ||
| pass |
Copilot
AI
Dec 1, 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.
Bare except clause. Consider catching specific exception types or at minimum use except Exception to avoid masking system-exiting exceptions.
| def test_target(host_ip: str, port: int) -> dict | None: | ||
| """Test one host:port for ZMQ camera.""" | ||
| ctx = zmq.Context() | ||
| sock = ctx.socket(zmq.SUB) | ||
| sock.connect(f"tcp://{host_ip}:{port}") | ||
| sock.setsockopt_string(zmq.SUBSCRIBE, "") | ||
| sock.setsockopt(zmq.RCVTIMEO, timeout_ms) | ||
|
|
||
| # Wait for subscription to establish (ZMQ "slow joiner" problem) | ||
| time.sleep(0.1) | ||
|
|
||
| # Try receiving a few times | ||
| msg = None | ||
| for _ in range(3): | ||
| try: | ||
| msg = sock.recv() | ||
| break | ||
| except zmq.Again: | ||
| time.sleep(0.05) | ||
|
|
||
| if msg is None: | ||
| sock.close() | ||
| ctx.term() | ||
| return None |
Copilot
AI
Dec 1, 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.
[nitpick] The test_target function uses hard-coded small delays and retry logic (lines 264, 273) without clear timeout bounds. While the individual socket has timeout_ms, the overall function could hang longer than expected when dealing with slow or partially-responsive servers. Consider adding an overall timeout mechanism or making the retry logic more explicit.
|
|
||
| if frame is None: | ||
| raise RuntimeError(f"Internal error: Event set but no frame available for {self}.") | ||
|
|
Copilot
AI
Dec 1, 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.
[nitpick] Potential race condition: The latest_frame is accessed without holding the lock on line 592 (in the check if frame is None), but it was read with the lock on lines 588-590. While this is likely harmless in practice (since frame is a local copy), the check on line 592 should logically be inside the with self.frame_lock: block for consistency and to avoid any potential issues with Python's memory model.
| if frame is None: | |
| raise RuntimeError(f"Internal error: Event set but no frame available for {self}.") | |
| if frame is None: | |
| raise RuntimeError(f"Internal error: Event set but no frame available for {self}.") |
| make_teleoperator_from_config, | ||
| so100_leader, | ||
| so101_leader, | ||
| custom |
Copilot
AI
Dec 1, 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.
[nitpick] Missing comma at the end of the import. This line should end with a comma for consistency with Python import formatting conventions when imports span multiple lines.
| custom | |
| custom, |
| make_teleoperator_from_config, | ||
| so100_leader, | ||
| so101_leader, | ||
| custom |
Copilot
AI
Dec 1, 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.
[nitpick] Missing comma at the end of the import. This line should end with a comma for consistency with Python import formatting conventions when imports span multiple lines.
| custom | |
| custom, |
|
|
||
| import json | ||
| import logging | ||
| import os |
Copilot
AI
Dec 1, 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.
Import of 'os' is not used.
| import os |
| import json | ||
| import logging | ||
| import os | ||
| import threading |
Copilot
AI
Dec 1, 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.
Import of 'threading' is not used.
| import threading |
| import os | ||
| import threading | ||
| import time | ||
| from pathlib import Path |
Copilot
AI
Dec 1, 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.
Import of 'Path' is not used.
| from pathlib import Path |
add zmq camera, should allow for lerobot teleoperate / record / replay.