-
Notifications
You must be signed in to change notification settings - Fork 40
fix: 3D joint view drifting out of sync during long teleoperation ses… #73
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,6 +164,11 @@ class ConnectionManager: | |
| def __init__(self): | ||
| self.active_connections: list[WebSocket] = [] | ||
| self.broadcast_queue = queue.Queue() | ||
| # Latest-wins slot for live joint frames. Joint updates supersede one | ||
| # another, so a slow consumer must never make them pile up in the | ||
| # queue — otherwise the 3D view drifts further and further behind the | ||
| # real robot as the backlog grows. | ||
| self._latest_joint_frame: dict[str, Any] | None = None | ||
| self.broadcast_thread = None | ||
| self.is_running = False | ||
| # Guards `active_connections` since the broadcast worker thread also | ||
|
|
@@ -220,8 +225,18 @@ def _broadcast_worker(self): | |
| try: | ||
| while self.is_running: | ||
| try: | ||
| # Get data from queue with timeout | ||
| data = self.broadcast_queue.get(timeout=0.1) | ||
| # Send the freshest joint frame first (latest-wins slot). | ||
| # A frame published between the read and the reset below is | ||
| # simply picked up on the next iteration. | ||
| frame = self._latest_joint_frame | ||
| if frame is not None: | ||
| self._latest_joint_frame = None | ||
| if self.active_connections: | ||
| loop.run_until_complete(self._send_to_all_connections(frame)) | ||
|
Comment on lines
+231
to
+235
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A frame sitting in this slot when the last client disconnects is never drained, and the next client that connects gets it as its first payload.
Reproduced against your head commit with a mocked WebSocket: In practice: stop teleoperation, close the tab, reopen it, and the 3D arm snaps to a pose the real arm is no longer in, with nothing to correct it until teleoperation starts again. Clearing the slot in self._latest_joint_frame = None
self.is_running = True
self.broadcast_thread = threading.Thread(target=self._broadcast_worker, daemon=True) |
||
|
|
||
| # Get data from queue with a short timeout so joint frames | ||
| # are polled frequently enough to stay real-time. | ||
| data = self.broadcast_queue.get(timeout=0.02) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional. Now that joint frames bypass the queue entirely, this timeout is what caps the joint frame rate: the worker can only ship one frame per poll, so 0.02 makes the ceiling 50Hz and the thread wakes 50 times a second even when nothing is happening. A |
||
| if data is None: # Poison pill to stop | ||
| break | ||
|
|
||
|
|
@@ -258,6 +273,11 @@ async def _send_to_all_connections(self, data: dict[str, Any]): | |
| def broadcast_joint_data_sync(self, data: dict[str, Any]): | ||
| """Thread-safe method to queue data for broadcasting""" | ||
| if self.is_running and self.active_connections: | ||
| if isinstance(data, dict) and data.get("type") == "joint_update": | ||
| # Replace any unsent frame instead of queueing: only the most | ||
| # recent robot pose is worth sending. | ||
| self._latest_joint_frame = data | ||
| return | ||
| try: | ||
| self.broadcast_queue.put_nowait(data) | ||
| except queue.Full: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,27 @@ | |
| "elbow_flex": (+1, 1029), | ||
| } | ||
|
|
||
|
|
||
| def _load_urdf_view_zero() -> dict[str, dict[str, float]] | None: | ||
| """Per-arm 3D-view zero captured by the user at the URDF sleep pose. | ||
|
|
||
| The hardcoded tick corrections above assume a canonical calibration; real | ||
| calibrations vary enough that the 3D view can be wildly offset. If | ||
| ``calibration/urdf_view_zero.json`` exists (mapping motor name to | ||
| ``{"zero_deg": float, "sign": ±1}``), it takes priority: | ||
| URDF angle = sign * (raw_normalized_deg - zero_deg). | ||
|
Comment on lines
+52
to
+58
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two things here. The docstring says "Per-arm", but the path is a single file at the calibration root, shared by every arm and every saved config. Two followers with different calibrations get the same zero pose, and the file silently applies to whichever one is connected. Either key it by config name (next to the calibration JSON it belongs to) or drop "per-arm" from the wording. Second, the justification. The comment above says the tick constant holds as long as the user pressed ENTER at the middle of range pose during |
||
| """ | ||
| import json | ||
| from pathlib import Path | ||
|
|
||
| path = Path.home() / ".cache" / "huggingface" / "lerobot" / "calibration" / "urdf_view_zero.json" | ||
|
Comment on lines
+60
to
+63
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
# lelab/utils/config.py
URDF_VIEW_ZERO_FILE = os.path.expanduser("~/.cache/huggingface/lerobot/calibration/urdf_view_zero.json")
|
||
| try: | ||
| with open(path, encoding="utf-8") as f: | ||
| data = json.load(f) | ||
| return {k: v for k, v in data.items() if isinstance(v, dict) and "zero_deg" in v} | ||
| except (OSError, ValueError): | ||
| return None | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| # Global variables for teleoperation state | ||
| teleoperation_active = False | ||
| teleoperation_thread: threading.Thread | None = None | ||
|
|
@@ -85,6 +106,12 @@ def get_joint_positions_from_robot(robot) -> dict[str, float]: | |
| try: | ||
| observation = robot.get_observation() | ||
| calibration = robot.calibration or {} | ||
| view_zero = getattr(get_joint_positions_from_robot, "_view_zero_cache", "unset") | ||
| if view_zero == "unset": | ||
| view_zero = _load_urdf_view_zero() | ||
| get_joint_positions_from_robot._view_zero_cache = view_zero | ||
| if view_zero: | ||
| logger.info("Using user-captured urdf_view_zero.json for the 3D view") | ||
|
Comment on lines
+109
to
+114
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This caches a Loading it once per teleoperation session, in |
||
|
|
||
| joint_positions: dict[str, float] = {} | ||
| debug_rows = [] | ||
|
|
@@ -97,13 +124,17 @@ def get_joint_positions_from_robot(robot) -> dict[str, float]: | |
|
|
||
| raw_deg = observation[motor_key] | ||
| angle_degrees = raw_deg | ||
| correction = _SO101_URDF_CORRECTIONS.get(motor_name) | ||
| if correction is not None and motor_name in calibration: | ||
| sign, urdf_zero_ticks = correction | ||
| cal = calibration[motor_name] | ||
| mid = (cal.range_min + cal.range_max) / 2 | ||
| motor_at_urdf_zero = (urdf_zero_ticks - mid) * 360 / _STS3215_MAX_RES | ||
| angle_degrees = sign * (raw_deg - motor_at_urdf_zero) | ||
| if view_zero and motor_name in view_zero: | ||
| entry = view_zero[motor_name] | ||
| angle_degrees = float(entry.get("sign", 1)) * (raw_deg - float(entry["zero_deg"])) | ||
| else: | ||
| correction = _SO101_URDF_CORRECTIONS.get(motor_name) | ||
| if correction is not None and motor_name in calibration: | ||
| sign, urdf_zero_ticks = correction | ||
| cal = calibration[motor_name] | ||
| mid = (cal.range_min + cal.range_max) / 2 | ||
| motor_at_urdf_zero = (urdf_zero_ticks - mid) * 360 / _STS3215_MAX_RES | ||
| angle_degrees = sign * (raw_deg - motor_at_urdf_zero) | ||
|
|
||
| joint_positions[urdf_joint_name] = angle_degrees * math.pi / 180.0 | ||
| debug_rows.append( | ||
|
|
@@ -227,7 +258,7 @@ def teleoperation_worker(): | |
| logger.info("Starting teleoperation loop...") | ||
| try: | ||
| last_broadcast_time = 0 | ||
| broadcast_interval = 0.05 # 20 FPS | ||
| broadcast_interval = 1 / 30 # 30 FPS | ||
|
|
||
| while teleoperation_active: | ||
| action = teleop_device.get_action() | ||
|
|
||
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.
This comment says the opposite of what the code does. A frame published between line 231 and line 233 is not picked up on the next iteration, it is destroyed: the read already returned the previous frame and the unconditional
= Noneoverwrites the new one.The consequence is harmless for a latest-wins slot (the next producer frame is 33ms away and supersedes it anyway), so this is only a matter of the comment being wrong, but the window is not narrow: it spans the whole
run_until_completesend. A read and clear in one step under a lock removes it, otherwise please just correct the comment to say the frame is dropped.