Skip to content
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

Enable CI for robot devices with mocked versions #398

Open
wants to merge 55 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
96cc243
Mock OpenCVCamera
Cadene Sep 1, 2024
2469c99
fix unit tests
Cadene Sep 9, 2024
44b8394
add dynamic import for cv2 and pyrealsense2
Cadene Sep 9, 2024
3bd5ea4
WIP
Cadene Sep 10, 2024
4151630
Mock dynamixel_sdk
Cadene Sep 11, 2024
53ebf9c
Mock robots (WIP segmentation fault)
Cadene Sep 11, 2024
cd4d225
Fix unit test
Cadene Sep 12, 2024
3f993d5
fix typo
Cadene Sep 12, 2024
ccc0586
Apply suggestions from code review
Cadene Sep 16, 2024
6636db5
Address comments
Cadene Sep 16, 2024
624551b
Address comments
Cadene Sep 16, 2024
adc8dc9
Address comments
Cadene Sep 16, 2024
886923a
Fix opencv segmentation fault (#442)
aliberts Sep 25, 2024
1bf2845
pre-commit run --all-files
Cadene Sep 25, 2024
f0452c2
Merge branch 'main' into user/rcadene/2024_09_01_mock_robot_devices
Cadene Sep 25, 2024
bcf27b8
Skip mocking tests with minimal pytest
Cadene Sep 25, 2024
5584201
mock=False
Cadene Sep 25, 2024
6377d2a
mock)
Cadene Sep 25, 2024
bded8cb
Fix unit tests
Cadene Sep 25, 2024
2c01716
fix aloha mock
Cadene Sep 25, 2024
500d505
Add support for video=False in record (no tested yet)
Cadene Sep 26, 2024
f2b1842
fix unit test
Cadene Sep 26, 2024
3cb85bc
Fix unit test
Cadene Sep 26, 2024
a236382
fix unit tests
Cadene Sep 26, 2024
8b36223
fix unit tests
Cadene Sep 26, 2024
b6b7fda
custom pytest speedup (TOREMOVE)
Cadene Sep 26, 2024
8a7b5c4
Remove @require_x
Cadene Sep 26, 2024
395720a
Revert "Remove @require_x"
Cadene Sep 26, 2024
48be576
fix unit tests
Cadene Sep 26, 2024
89b2b73
fix unit tests
Cadene Sep 26, 2024
e66900e
mock_motor instead of require_mock_motor
Cadene Sep 26, 2024
7450adc
no more require_mock_motor
Cadene Sep 26, 2024
8da0893
move mock_motor in test_motors.py
Cadene Sep 26, 2024
a7350d9
add mock=False
Cadene Sep 27, 2024
bf7e906
add +COLOR_RGB2BGR
Cadene Sep 27, 2024
81f17d5
if not '~cameras' in overrides
Cadene Sep 27, 2024
e499d60
fix unit test
Cadene Sep 27, 2024
0352c61
Add more exception except
Cadene Sep 27, 2024
c704eb9
improve except
Cadene Sep 27, 2024
3f9f3dd
Add pyserial
Cadene Sep 27, 2024
da1888a
revert to all tests
Cadene Sep 27, 2024
675d428
add
Cadene Sep 27, 2024
76cc479
add
Cadene Sep 27, 2024
50a979d
Check if file exists
Cadene Sep 27, 2024
9dea00e
retest
Cadene Sep 27, 2024
2e694fc
test
Cadene Sep 27, 2024
88c2ed4
fix unit tests
Cadene Sep 27, 2024
cc5c623
test
Cadene Sep 27, 2024
2c9defa
test
Cadene Sep 27, 2024
bc479cb
test
Cadene Sep 27, 2024
0e63f7c
test
Cadene Sep 27, 2024
83cfe60
tests
Cadene Sep 27, 2024
1de04e4
Merge branch 'main' into user/rcadene/2024_09_01_mock_robot_devices
Cadene Sep 27, 2024
5c73bec
Address Jess comments
Cadene Sep 28, 2024
95a708c
Fix slow fps
Cadene Sep 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ on:
- ".github/**"
- "poetry.lock"
- "Makefile"
- ".cache/**"
push:
branches:
- main
Expand All @@ -21,6 +22,7 @@ on:
- ".github/**"
- "poetry.lock"
- "Makefile"
- ".cache/**"

jobs:
pytest:
Expand Down Expand Up @@ -60,7 +62,6 @@ jobs:
-W ignore::UserWarning:gymnasium.utils.env_checker:247 \
&& rm -rf tests/outputs outputs
pytest-minimal:
name: Pytest (minimal install)
runs-on: ubuntu-latest
Expand Down
13 changes: 13 additions & 0 deletions lerobot/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
print(lerobot.available_policies)
print(lerobot.available_policies_per_env)
print(lerobot.available_robots)
print(lerobot.available_cameras)
print(lerobot.available_motors)
```
When implementing a new dataset loadable with LeRobotDataset follow these steps:
Expand Down Expand Up @@ -198,6 +200,17 @@
"aloha",
]

# lists all available cameras from `lerobot/common/robot_devices/cameras`
available_cameras = [
"opencv",
"intelrealsense",
]

# lists all available motors from `lerobot/common/robot_devices/motors`
available_motors = [
"dynamixel",
]

# keys and values refer to yaml files
available_policies_per_env = {
"aloha": ["act"],
Expand Down
99 changes: 81 additions & 18 deletions lerobot/common/robot_devices/cameras/intelrealsense.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import argparse
import concurrent.futures
import logging
import math
import shutil
import threading
import time
Expand All @@ -13,9 +14,7 @@
from pathlib import Path
from threading import Thread

import cv2
import numpy as np
import pyrealsense2 as rs
from PIL import Image

from lerobot.common.robot_devices.utils import (
Expand All @@ -28,14 +27,23 @@
SERIAL_NUMBER_INDEX = 1


def find_camera_indices(raise_when_empty=True) -> list[int]:
def find_camera_indices(raise_when_empty=True, mock=False) -> list[int]:
"""
Find the serial numbers of the Intel RealSense cameras
connected to the computer.
"""
if mock:
from tests.mock_pyrealsense2 import (
RSCameraInfo,
RSContext,
)
else:
from pyrealsense2 import camera_info as RSCameraInfo # noqa: N812
from pyrealsense2 import context as RSContext # noqa: N812

camera_ids = []
for device in rs.context().query_devices():
serial_number = int(device.get_info(rs.camera_info(SERIAL_NUMBER_INDEX)))
for device in RSContext().query_devices():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: In the if...else statement above you say from pyrealsense2 import context as RSContext # noqa: N812 I presume you do this so this line can become for device in context.query_devices():. To do that you would need to import the mock values as follows:

        from tests.mock_intelrealsenseimport camera_info as RSCameraInfo  # noqa: N812
        from tests.mock_intelrealsenseimport context as RSContext  # noqa: N812

However, if you don't want to do that and keep this line as is then I would change the named imports above since they aren't being used anyways, i.e.

        from pyrealsense2 import (
            RSCameraInfo,
            RSContext,
        )

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry I didnt understand.

This works:

from tests.mock_intelrealsense import RSCameraInfo
RSCameraInfo(SERIAL_NUMBER_INDEX)

This works:

from pyrealsense2 import camera_info as RSCameraInfo
RSCameraInfo(SERIAL_NUMBER_INDEX)

This works:

from pyrealsense2 import camera_info
camera_info(SERIAL_NUMBER_INDEX)

But this doesnt work:

from pyrealsense2 import camera_info as RSCameraInfo
camera_info(SERIAL_NUMBER_INDEX)

serial_number = int(device.get_info(RSCameraInfo(SERIAL_NUMBER_INDEX)))
camera_ids.append(serial_number)

if raise_when_empty and len(camera_ids) == 0:
Expand Down Expand Up @@ -64,18 +72,24 @@ def save_images_from_cameras(
width=None,
height=None,
record_time_s=2,
mock=False,
):
"""
Initializes all the cameras and saves images to the directory. Useful to visually identify the camera
associated to a given camera index.
"""
if camera_ids is None:
camera_ids = find_camera_indices()
camera_ids = find_camera_indices(mock=mock)

if mock:
from tests.mock_cv2 import COLOR_RGB2BGR, cvtColor
else:
from cv2 import COLOR_RGB2BGR, cvtColor

print("Connecting cameras")
cameras = []
for cam_idx in camera_ids:
camera = IntelRealSenseCamera(cam_idx, fps=fps, width=width, height=height)
camera = IntelRealSenseCamera(cam_idx, fps=fps, width=width, height=height, mock=mock)
camera.connect()
print(
f"IntelRealSenseCamera({camera.camera_index}, fps={camera.fps}, width={camera.width}, height={camera.height}, color_mode={camera.color_mode})"
Expand Down Expand Up @@ -103,7 +117,8 @@ def save_images_from_cameras(
image = camera.read() if fps is None else camera.async_read()
if image is None:
print("No Frame")
bgr_converted_image = cv2.cvtColor(image, cv2.COLOR_RGB2BGR)

bgr_converted_image = cvtColor(image, COLOR_RGB2BGR)

executor.submit(
save_image,
Expand Down Expand Up @@ -149,14 +164,17 @@ class IntelRealSenseCameraConfig:
color_mode: str = "rgb"
use_depth: bool = False
force_hardware_reset: bool = True
mock: bool = False

def __post_init__(self):
if self.color_mode not in ["rgb", "bgr"]:
raise ValueError(
f"`color_mode` is expected to be 'rgb' or 'bgr', but {self.color_mode} is provided."
)

if (self.fps or self.width or self.height) and not (self.fps and self.width and self.height):
at_least_one_is_not_none = self.fps is not None or self.width is not None or self.height is not None
at_least_one_is_none = self.fps is None or self.width is None or self.height is None
if at_least_one_is_not_none and at_least_one_is_none:
raise ValueError(
"For `fps`, `width` and `height`, either all of them need to be set, or none of them, "
f"but {self.fps=}, {self.width=}, {self.height=} were provided."
Expand Down Expand Up @@ -228,6 +246,7 @@ def __init__(
self.color_mode = config.color_mode
self.use_depth = config.use_depth
self.force_hardware_reset = config.force_hardware_reset
self.mock = config.mock

self.camera = None
self.is_connected = False
Expand All @@ -243,24 +262,37 @@ def connect(self):
f"IntelRealSenseCamera({self.camera_index}) is already connected."
)

config = rs.config()
if self.mock:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Same comment as above, I would either import these values as named variables like you do when it is not mock, i.e., config, format, pipeline, stream, or leave them all unnamed like you do when it is mock.

Copy link
Collaborator Author

@Cadene Cadene Sep 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry I didnt understand

The original element name is config but I rename it to RSConfig to fit python CamelCase format for naming classes:

from pyrealsense2 import config as RSConfig

from tests.mock_pyrealsense2 import (
RSConfig,
RSFormat,
RSPipeline,
RSStream,
)
else:
from pyrealsense2 import config as RSConfig # noqa: N812
from pyrealsense2 import format as RSFormat # noqa: N812
from pyrealsense2 import pipeline as RSPipeline # noqa: N812
from pyrealsense2 import stream as RSStream # noqa: N812

config = RSConfig()
config.enable_device(str(self.camera_index))

if self.fps and self.width and self.height:
# TODO(rcadene): can we set rgb8 directly?
config.enable_stream(rs.stream.color, self.width, self.height, rs.format.rgb8, self.fps)
config.enable_stream(RSStream.color, self.width, self.height, RSFormat.rgb8, self.fps)
else:
config.enable_stream(rs.stream.color)
config.enable_stream(RSStream.color)

if self.use_depth:
if self.fps and self.width and self.height:
config.enable_stream(rs.stream.depth, self.width, self.height, rs.format.z16, self.fps)
config.enable_stream(RSStream.depth, self.width, self.height, RSFormat.z16, self.fps)
else:
config.enable_stream(rs.stream.depth)
config.enable_stream(RSStream.depth)

self.camera = rs.pipeline()
self.camera = RSPipeline()
try:
self.camera.start(config)
profile = self.camera.start(config)
is_camera_open = True
except RuntimeError:
is_camera_open = False
Expand All @@ -279,6 +311,31 @@ def connect(self):

raise OSError(f"Can't access IntelRealSenseCamera({self.camera_index}).")

color_stream = profile.get_stream(RSStream.color)
color_profile = color_stream.as_video_stream_profile()
actual_fps = color_profile.fps()
actual_width = color_profile.width()
actual_height = color_profile.height()

# Using `math.isclose` since actual fps can be a float (e.g. 29.9 instead of 30)
if self.fps is not None and not math.isclose(self.fps, actual_fps, rel_tol=1e-3):
jess-moss marked this conversation as resolved.
Show resolved Hide resolved
# Using `OSError` since it's a broad that encompasses issues related to device communication
raise OSError(
aliberts marked this conversation as resolved.
Show resolved Hide resolved
f"Can't set {self.fps=} for IntelRealSenseCamera({self.camera_index}). Actual value is {actual_fps}."
)
if self.width is not None and self.width != actual_width:
raise OSError(
aliberts marked this conversation as resolved.
Show resolved Hide resolved
f"Can't set {self.width=} for IntelRealSenseCamera({self.camera_index}). Actual value is {actual_width}."
)
if self.height is not None and self.height != actual_height:
raise OSError(
aliberts marked this conversation as resolved.
Show resolved Hide resolved
f"Can't set {self.height=} for IntelRealSenseCamera({self.camera_index}). Actual value is {actual_height}."
)

self.fps = actual_fps
self.width = actual_width
self.height = actual_height

self.is_connected = True

def read(self, temporary_color: str | None = None) -> np.ndarray | tuple[np.ndarray, np.ndarray]:
Expand Down Expand Up @@ -315,7 +372,12 @@ def read(self, temporary_color: str | None = None) -> np.ndarray | tuple[np.ndar

# IntelRealSense uses RGB format as default (red, green, blue).
if requested_color_mode == "bgr":
color_image = cv2.cvtColor(color_image, cv2.COLOR_RGB2BGR)
if self.mock:
from tests.mock_cv2 import COLOR_RGB2BGR, cvtColor
else:
from cv2 import COLOR_RGB2BGR, cvtColor

color_image = cvtColor(color_image, COLOR_RGB2BGR)

h, w, _ = color_image.shape
if h != self.height or w != self.width:
Expand Down Expand Up @@ -347,7 +409,7 @@ def read(self, temporary_color: str | None = None) -> np.ndarray | tuple[np.ndar
return color_image

def read_loop(self):
while self.stop_event is None or not self.stop_event.is_set():
while not self.stop_event.is_set():
Comment on lines -350 to +412
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that self.stop_event is None was not required.

if self.use_depth:
self.color_image, self.depth_map = self.read()
else:
Expand All @@ -368,6 +430,7 @@ def async_read(self):

num_tries = 0
while self.color_image is None:
# TODO(rcadene, aliberts): intelrealsense has diverged compared to opencv over here
num_tries += 1
time.sleep(1 / self.fps)
if num_tries > self.fps and (self.thread.ident is None or not self.thread.is_alive()):
Expand Down
Loading
Loading