From 96cc2433d64fdf9ec63480d0ece5b9825ab4419f Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Sun, 1 Sep 2024 17:36:58 +0200 Subject: [PATCH 01/53] Mock OpenCVCamera --- .../common/robot_devices/cameras/opencv.py | 2 + tests/test_cameras.py | 17 ++---- tests/utils.py | 60 +++++++++++++++++-- 3 files changed, 63 insertions(+), 16 deletions(-) diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index b066a451a..eb7242847 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -337,6 +337,8 @@ def read(self, temporary_color_mode: str | None = None) -> np.ndarray: return color_image def read_loop(self): + # TODO(rcadene): implement safe exit for the threads, + # to avoid Segfault when main process finishes while self.stop_event is None or not self.stop_event.is_set(): self.color_image = self.read() diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 0d5d94425..090eb472c 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -10,10 +10,9 @@ import numpy as np import pytest -from lerobot import available_robots from lerobot.common.robot_devices.cameras.opencv import OpenCVCamera, save_images_from_cameras from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import require_robot +from tests.utils import TEST_ROBOT_TYPES, require_robot CAMERA_INDEX = 2 # Maximum absolute difference between two consecutive images recored by a camera. @@ -25,7 +24,7 @@ def compute_max_pixel_difference(first_image, second_image): return np.abs(first_image.astype(float) - second_image.astype(float)).max() -@pytest.mark.parametrize("robot_type", available_robots) +@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) @require_robot def test_camera(request, robot_type): """Test assumes that `camera.read()` returns the same image when called multiple times in a row. @@ -78,11 +77,11 @@ def test_camera(request, robot_type): camera.read() color_image = camera.read() async_color_image = camera.async_read() - print( + error_msg = ( "max_pixel_difference between read() and async_read()", compute_max_pixel_difference(color_image, async_color_image), ) - assert np.allclose(color_image, async_color_image, rtol=1e-5, atol=MAX_PIXEL_DIFFERENCE) + assert np.allclose(color_image, async_color_image, rtol=1e-5, atol=MAX_PIXEL_DIFFERENCE), error_msg # Test disconnecting camera.disconnect() @@ -105,12 +104,6 @@ def test_camera(request, robot_type): # TODO(rcadene): Add a test for a camera that doesnt support fps=60 and raises an OSError # TODO(rcadene): Add a test for a camera that supports fps=60 - # Test fps=10 raises an OSError - camera = OpenCVCamera(CAMERA_INDEX, fps=10) - with pytest.raises(OSError): - camera.connect() - del camera - # Test width and height can be set camera = OpenCVCamera(CAMERA_INDEX, fps=30, width=1280, height=720) camera.connect() @@ -131,7 +124,7 @@ def test_camera(request, robot_type): del camera -@pytest.mark.parametrize("robot_type", available_robots) +@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) @require_robot def test_save_images_from_cameras(tmpdir, request, robot_type): save_images_from_cameras(tmpdir, record_time_s=1) diff --git a/tests/utils.py b/tests/utils.py index db214aeac..1feb69cb5 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -16,9 +16,12 @@ import platform from functools import wraps +import cv2 +import numpy as np import pytest import torch +from lerobot import available_robots from lerobot.common.utils.import_utils import is_package_available DEVICE = "cuda" if torch.cuda.is_available() else "cpu" @@ -28,6 +31,8 @@ ROBOT_CONFIG_PATH_TEMPLATE = "lerobot/configs/robot/{robot}.yaml" +TEST_ROBOT_TYPES = available_robots + [f"mocked_{robot_type}" for robot_type in available_robots] + def require_x86_64_kernel(func): """ @@ -175,11 +180,58 @@ def wrapper(*args, **kwargs): robot_type = kwargs.get("robot_type") if request is None: - raise ValueError("The 'request' fixture must be passed to the test function as a parameter.") + raise ValueError("The 'request' fixture must be an argument of the test function.") + + # Run test with a monkeypatched version of the robot devices. + if robot_type.startswith("mocked_"): + kwargs["robot_type"] = robot_type.replace("mocked_", "") + + monkeypatch = request.getfixturevalue("monkeypatch") + monkeypatch.setattr(cv2, "VideoCapture", MockVideoCapture) + + # Run test with a real robot. Skip test if robot connection fails. + else: + # `is_robot_available` is defined in `tests/conftest.py` + if not request.getfixturevalue("is_robot_available"): + pytest.skip(f"A {robot_type} robot is not available.") - # The function `is_robot_available` is defined in `tests/conftest.py` - if not request.getfixturevalue("is_robot_available"): - pytest.skip(f"A {robot_type} robot is not available.") return func(*args, **kwargs) return wrapper + + +class MockVideoCapture(cv2.VideoCapture): + image = { + "480x640": np.random.randint(0, 256, size=(480, 640, 3), dtype=np.uint8), + "720x1280": np.random.randint(0, 256, size=(720, 1280, 3), dtype=np.uint8), + } + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._mock_dict = { + cv2.CAP_PROP_FPS: 30, + cv2.CAP_PROP_FRAME_WIDTH: 640, + cv2.CAP_PROP_FRAME_HEIGHT: 480, + } + + def isOpened(self): # noqa: N802 + return True + + def set(self, propId: int, value: float) -> bool: # noqa: N803 + self._mock_dict[propId] = value + return True + + def get(self, propId: int) -> float: # noqa: N803 + value = self._mock_dict[propId] + if value == 0: + if propId == cv2.CAP_PROP_FRAME_HEIGHT: + value = 480 + elif propId == cv2.CAP_PROP_FRAME_WIDTH: + value = 640 + return value + + def read(self): + h = self.get(cv2.CAP_PROP_FRAME_HEIGHT) + w = self.get(cv2.CAP_PROP_FRAME_WIDTH) + ret = True + return ret, self.image[f"{h}x{w}"] From 2469c99053d243c511189b486b6e2ae7697103b2 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Mon, 9 Sep 2024 19:19:05 +0200 Subject: [PATCH 02/53] fix unit tests --- lerobot/common/robot_devices/cameras/opencv.py | 4 +--- tests/utils.py | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index eb7242847..6652b5388 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -337,9 +337,7 @@ def read(self, temporary_color_mode: str | None = None) -> np.ndarray: return color_image def read_loop(self): - # TODO(rcadene): implement safe exit for the threads, - # to avoid Segfault when main process finishes - while self.stop_event is None or not self.stop_event.is_set(): + while not self.stop_event.is_set(): self.color_image = self.read() def async_read(self): diff --git a/tests/utils.py b/tests/utils.py index 1feb69cb5..9e07219fb 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -207,7 +207,6 @@ class MockVideoCapture(cv2.VideoCapture): } def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) self._mock_dict = { cv2.CAP_PROP_FPS: 30, cv2.CAP_PROP_FRAME_WIDTH: 640, @@ -235,3 +234,6 @@ def read(self): w = self.get(cv2.CAP_PROP_FRAME_WIDTH) ret = True return ret, self.image[f"{h}x{w}"] + + def release(self): + pass From 44b83943654cd0b74bf7385bb1772b0e90bd7aaf Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Mon, 9 Sep 2024 19:32:35 +0200 Subject: [PATCH 03/53] add dynamic import for cv2 and pyrealsense2 --- .../robot_devices/cameras/intelrealsense.py | 2 +- tests/mock_opencv.py | 41 +++++++++++++ tests/test_cameras.py | 5 ++ tests/utils.py | 61 ++++++------------- 4 files changed, 66 insertions(+), 43 deletions(-) create mode 100644 tests/mock_opencv.py diff --git a/lerobot/common/robot_devices/cameras/intelrealsense.py b/lerobot/common/robot_devices/cameras/intelrealsense.py index 4806bf785..21ac0b458 100644 --- a/lerobot/common/robot_devices/cameras/intelrealsense.py +++ b/lerobot/common/robot_devices/cameras/intelrealsense.py @@ -347,7 +347,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(): if self.use_depth: self.color_image, self.depth_map = self.read() else: diff --git a/tests/mock_opencv.py b/tests/mock_opencv.py new file mode 100644 index 000000000..c54ae285d --- /dev/null +++ b/tests/mock_opencv.py @@ -0,0 +1,41 @@ +import cv2 +import numpy as np + + +class MockVideoCapture(cv2.VideoCapture): + image = { + "480x640": np.random.randint(0, 256, size=(480, 640, 3), dtype=np.uint8), + "720x1280": np.random.randint(0, 256, size=(720, 1280, 3), dtype=np.uint8), + } + + def __init__(self, *args, **kwargs): + self._mock_dict = { + cv2.CAP_PROP_FPS: 30, + cv2.CAP_PROP_FRAME_WIDTH: 640, + cv2.CAP_PROP_FRAME_HEIGHT: 480, + } + + def isOpened(self): # noqa: N802 + return True + + def set(self, propId: int, value: float) -> bool: # noqa: N803 + self._mock_dict[propId] = value + return True + + def get(self, propId: int) -> float: # noqa: N803 + value = self._mock_dict[propId] + if value == 0: + if propId == cv2.CAP_PROP_FRAME_HEIGHT: + value = 480 + elif propId == cv2.CAP_PROP_FRAME_WIDTH: + value = 640 + return value + + def read(self): + h = self.get(cv2.CAP_PROP_FRAME_HEIGHT) + w = self.get(cv2.CAP_PROP_FRAME_WIDTH) + ret = True + return ret, self.image[f"{h}x{w}"] + + def release(self): + pass diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 090eb472c..7c9d9e38a 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -5,6 +5,11 @@ ```bash pytest -sx tests/test_cameras.py::test_camera ``` + +Example of running test on mocked_koch: +```bash +python -m pytest -sx -k "mocked_koch" tests/test_cameras.py::test_camera +``` """ import numpy as np diff --git a/tests/utils.py b/tests/utils.py index 9e07219fb..98b5a82a0 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -14,10 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. import platform +import traceback from functools import wraps -import cv2 -import numpy as np import pytest import torch @@ -187,7 +186,24 @@ def wrapper(*args, **kwargs): kwargs["robot_type"] = robot_type.replace("mocked_", "") monkeypatch = request.getfixturevalue("monkeypatch") - monkeypatch.setattr(cv2, "VideoCapture", MockVideoCapture) + + try: + import cv2 + + from tests.mock_opencv import MockVideoCapture + + monkeypatch.setattr(cv2, "VideoCapture", MockVideoCapture) + except ImportError: + traceback.print_exc() + + try: + import pyrealsense2 as rs + + # TODO(rcadene): + mock_pipeline = None + monkeypatch.setattr(rs, "pipeline", mock_pipeline) + except ImportError: + traceback.print_exc() # Run test with a real robot. Skip test if robot connection fails. else: @@ -198,42 +214,3 @@ def wrapper(*args, **kwargs): return func(*args, **kwargs) return wrapper - - -class MockVideoCapture(cv2.VideoCapture): - image = { - "480x640": np.random.randint(0, 256, size=(480, 640, 3), dtype=np.uint8), - "720x1280": np.random.randint(0, 256, size=(720, 1280, 3), dtype=np.uint8), - } - - def __init__(self, *args, **kwargs): - self._mock_dict = { - cv2.CAP_PROP_FPS: 30, - cv2.CAP_PROP_FRAME_WIDTH: 640, - cv2.CAP_PROP_FRAME_HEIGHT: 480, - } - - def isOpened(self): # noqa: N802 - return True - - def set(self, propId: int, value: float) -> bool: # noqa: N803 - self._mock_dict[propId] = value - return True - - def get(self, propId: int) -> float: # noqa: N803 - value = self._mock_dict[propId] - if value == 0: - if propId == cv2.CAP_PROP_FRAME_HEIGHT: - value = 480 - elif propId == cv2.CAP_PROP_FRAME_WIDTH: - value = 640 - return value - - def read(self): - h = self.get(cv2.CAP_PROP_FRAME_HEIGHT) - w = self.get(cv2.CAP_PROP_FRAME_WIDTH) - ret = True - return ret, self.image[f"{h}x{w}"] - - def release(self): - pass From 3bd5ea4d7a2a33e601dd84cc6e25012cc90ccfe3 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Tue, 10 Sep 2024 18:30:39 +0200 Subject: [PATCH 04/53] WIP --- lerobot/__init__.py | 13 ++ .../robot_devices/cameras/intelrealsense.py | 30 +++- tests/conftest.py | 31 +++- tests/mock_dynamixel.py | 48 ++++++ tests/mock_intelrealsense.py | 110 ++++++++++++++ tests/test_cameras.py | 71 ++++++--- tests/test_motors.py | 65 +++++--- tests/utils.py | 142 +++++++++++++++--- 8 files changed, 448 insertions(+), 62 deletions(-) create mode 100644 tests/mock_dynamixel.py create mode 100644 tests/mock_intelrealsense.py diff --git a/lerobot/__init__.py b/lerobot/__init__.py index aeae31008..2fedb1222 100644 --- a/lerobot/__init__.py +++ b/lerobot/__init__.py @@ -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: @@ -198,6 +200,17 @@ "aloha", ] +# lists all available robots from `lerobot/common/robot_devices/cameras` +available_cameras = [ + "opencv", + "intelrealsense", +] + +# lists all available robots from `lerobot/common/robot_devices/motors` +available_motors = [ + "dynamixel", +] + # keys and values refer to yaml files available_policies_per_env = { "aloha": ["act"], diff --git a/lerobot/common/robot_devices/cameras/intelrealsense.py b/lerobot/common/robot_devices/cameras/intelrealsense.py index 21ac0b458..77a015cf1 100644 --- a/lerobot/common/robot_devices/cameras/intelrealsense.py +++ b/lerobot/common/robot_devices/cameras/intelrealsense.py @@ -5,6 +5,7 @@ import argparse import concurrent.futures import logging +import math import shutil import threading import time @@ -156,7 +157,9 @@ def __post_init__(self): 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." @@ -260,7 +263,7 @@ def connect(self): self.camera = rs.pipeline() try: - self.camera.start(config) + profile = self.camera.start(config) is_camera_open = True except RuntimeError: is_camera_open = False @@ -279,6 +282,29 @@ def connect(self): raise OSError(f"Can't access IntelRealSenseCamera({self.camera_index}).") + color_stream = profile.get_stream(rs.stream.color) + color_profile = color_stream.as_video_stream_profile() + actual_fps = color_profile.fps() + actual_width = color_profile.width() + actual_height = color_profile.height() + + if self.fps is not None and not math.isclose(self.fps, actual_fps, rel_tol=1e-3): + raise OSError( + 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( + 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( + 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]: diff --git a/tests/conftest.py b/tests/conftest.py index 52006f331..8c754cb3f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,8 +18,9 @@ import pytest from lerobot.common.utils.utils import init_hydra_config - -from .utils import DEVICE, ROBOT_CONFIG_PATH_TEMPLATE +from tests.test_cameras import make_camera +from tests.test_motors import make_motors_bus +from tests.utils import DEVICE, ROBOT_CONFIG_PATH_TEMPLATE def pytest_collection_finish(): @@ -41,3 +42,29 @@ def is_robot_available(robot_type): traceback.print_exc() print(f"\nA {robot_type} robot is not available.") return False + + +@pytest.fixture +def is_camera_available(camera_type): + try: + camera = make_camera(camera_type) + camera.connect() + del camera + return True + except Exception: + traceback.print_exc() + print(f"\nA {camera_type} camera is not available.") + return False + + +@pytest.fixture +def is_motor_available(motor_type): + try: + motors_bus = make_motors_bus(motor_type) + motors_bus.connect() + del motors_bus + return True + except Exception: + traceback.print_exc() + print(f"\nA {motor_type} motor is not available.") + return False diff --git a/tests/mock_dynamixel.py b/tests/mock_dynamixel.py new file mode 100644 index 000000000..16831a726 --- /dev/null +++ b/tests/mock_dynamixel.py @@ -0,0 +1,48 @@ +from dynamixel_sdk import COMM_SUCCESS + + +class PortHandler: + def __init__(self, port): + self.port = port + + def openPort(self): # noqa: N802 + return True + + def closePort(self): # noqa: N802 + pass + + def setPacketTimeoutMillis(self, timeout_ms): # noqa: N802 + del timeout_ms # unused + + +class PacketHandler: + def __init__(self, protocol_version): + del protocol_version # unused + + +class GroupSyncRead: + def __init__(self, port_handler, packet_handler, address, bytes): + pass + + def addParam(self, motor_index): # noqa: N802 + pass + + def txRxPacket(self): # noqa: N802 + return COMM_SUCCESS + + def getData(self, index, address, bytes): # noqa: N802 + return value # noqa: F821 + + +class GroupSyncWrite: + def __init__(self, port_handler, packet_handler, address, bytes): + pass + + def addParam(self, index, data): # noqa: N802 + pass + + def txPacket(self): # noqa: N802 + return COMM_SUCCESS + + def changeParam(self, index, data): # noqa: N802 + pass diff --git a/tests/mock_intelrealsense.py b/tests/mock_intelrealsense.py new file mode 100644 index 000000000..1e6c3f5f5 --- /dev/null +++ b/tests/mock_intelrealsense.py @@ -0,0 +1,110 @@ +import enum + +import numpy as np + + +class MockStream(enum.Enum): + color = 0 + depth = 1 + + +class MockFormat(enum.Enum): + rgb8 = 0 + z16 = 1 + + +class MockConfig: + def enable_device(self, device_id: str): + self.device_enabled = device_id + + def enable_stream( + self, stream_type: MockStream, width=None, height=None, color_format: MockFormat = None, fps=None + ): + self.stream_type = stream_type + # Overwrite default values when possible + self.width = 848 if width is None else width + self.height = 480 if height is None else height + self.color_format = MockFormat.rgb8 if color_format is None else color_format + self.fps = 30 if fps is None else fps + + +class MockColorProfile: + def __init__(self, config: MockConfig): + self.config = config + + def fps(self): + return self.config.fps + + def width(self): + return self.config.width + + def height(self): + return self.config.height + + +class MockColorStream: + def __init__(self, config: MockConfig): + self.config = config + + def as_video_stream_profile(self): + return MockColorProfile(self.config) + + +class MockProfile: + def __init__(self, config: MockConfig): + self.config = config + + def get_stream(self, color_format: MockFormat): + del color_format # unused + return MockColorStream(self.config) + + +class MockPipeline: + def __init__(self): + self.started = False + self.config = None + + def start(self, config: MockConfig): + self.started = True + self.config = config + return MockProfile(self.config) + + def stop(self): + if not self.started: + raise RuntimeError("You need to start the camera before stop.") + self.started = False + self.config = None + + def wait_for_frames(self, timeout_ms=50000): + del timeout_ms # unused + return MockFrames(self.config) + + +class MockFrames: + def __init__(self, config: MockConfig): + self.config = config + + def get_color_frame(self): + return MockColorFrame(self.config) + + def get_depth_frame(self): + return MockDepthFrame(self.config) + + +class MockColorFrame: + def __init__(self, config: MockConfig): + self.config = config + + def get_data(self): + data = np.ones((self.config.height, self.config.width, 3), dtype=np.uint8) + # Create a difference between rgb and bgr + data[:, :, 0] = 2 + return data + + +class MockDepthFrame: + def __init__(self, config: MockConfig): + self.config = config + + def get_data(self): + return np.ones((self.config.height, self.config.width), dtype=np.uint16) diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 7c9d9e38a..161d9a190 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -1,25 +1,34 @@ """ -Tests meant to be used locally and launched manually. +Tests for physical cameras and their mocked versions. +If the physical camera is not connected to the computer, or not working, +the test will be skipped. -Example usage: +Example of running a specific test: ```bash pytest -sx tests/test_cameras.py::test_camera ``` -Example of running test on mocked_koch: +Example of running test on a real OpenCV camera connected to the computer: ```bash -python -m pytest -sx -k "mocked_koch" tests/test_cameras.py::test_camera +pytest -sx tests/test_cameras.py::test_camera[opencv] +``` + +Example of running test on a mocked version of an OpenCV camera: +```bash +pytest -sx -k "mocked_opencv" tests/test_cameras.py::test_camera ``` """ import numpy as np import pytest -from lerobot.common.robot_devices.cameras.opencv import OpenCVCamera, save_images_from_cameras from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_ROBOT_TYPES, require_robot +from tests.utils import TEST_CAMERA_TYPES, require_camera + +# Camera indices used for connecting physical cameras +OPENCV_CAMERA_INDEX = 0 +INTELREALSENSE_CAMERA_INDEX = 128422271614 -CAMERA_INDEX = 2 # Maximum absolute difference between two consecutive images recored by a camera. # This value differs with respect to the camera. MAX_PIXEL_DIFFERENCE = 25 @@ -29,9 +38,26 @@ def compute_max_pixel_difference(first_image, second_image): return np.abs(first_image.astype(float) - second_image.astype(float)).max() -@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) -@require_robot -def test_camera(request, robot_type): +def make_camera(camera_type, **kwargs): + if camera_type == "opencv": + from lerobot.common.robot_devices.cameras.opencv import OpenCVCamera + + camera_index = kwargs.pop("camera_index", OPENCV_CAMERA_INDEX) + return OpenCVCamera(camera_index, **kwargs) + + elif camera_type == "intelrealsense": + from lerobot.common.robot_devices.cameras.intelrealsense import IntelRealSenseCamera + + camera_index = kwargs.pop("camera_index", INTELREALSENSE_CAMERA_INDEX) + return IntelRealSenseCamera(camera_index, **kwargs) + + else: + raise ValueError(f"The camera type '{camera_type}' is not valid.") + + +@pytest.mark.parametrize("camera_type", TEST_CAMERA_TYPES) +@require_camera +def test_camera(request, camera_type): """Test assumes that `camera.read()` returns the same image when called multiple times in a row. So the environment should not change (you shouldnt be in front of the camera) and the camera should not be moving. @@ -40,10 +66,9 @@ def test_camera(request, robot_type): """ # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs - # TODO(rcadene): add compatibility with other camera APIs # Test instantiating - camera = OpenCVCamera(CAMERA_INDEX) + camera = make_camera(camera_type) # Test reading, async reading, disconnecting before connecting raises an error with pytest.raises(RobotDeviceNotConnectedError): @@ -57,7 +82,7 @@ def test_camera(request, robot_type): del camera # Test connecting - camera = OpenCVCamera(CAMERA_INDEX) + camera = make_camera(camera_type) camera.connect() assert camera.is_connected assert camera.fps is not None @@ -94,12 +119,12 @@ def test_camera(request, robot_type): assert camera.thread is None # Test disconnecting with `__del__` - camera = OpenCVCamera(CAMERA_INDEX) + camera = make_camera(camera_type) camera.connect() del camera # Test acquiring a bgr image - camera = OpenCVCamera(CAMERA_INDEX, color_mode="bgr") + camera = make_camera(camera_type, color_mode="bgr") camera.connect() assert camera.color_mode == "bgr" bgr_color_image = camera.read() @@ -110,7 +135,7 @@ def test_camera(request, robot_type): # TODO(rcadene): Add a test for a camera that supports fps=60 # Test width and height can be set - camera = OpenCVCamera(CAMERA_INDEX, fps=30, width=1280, height=720) + camera = make_camera(camera_type, fps=30, width=1280, height=720) camera.connect() assert camera.fps == 30 assert camera.width == 1280 @@ -123,13 +148,19 @@ def test_camera(request, robot_type): del camera # Test not supported width and height raise an error - camera = OpenCVCamera(CAMERA_INDEX, fps=30, width=0, height=0) + camera = make_camera(camera_type, fps=30, width=0, height=0) with pytest.raises(OSError): camera.connect() del camera -@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) -@require_robot -def test_save_images_from_cameras(tmpdir, request, robot_type): +@pytest.mark.parametrize("camera_type", TEST_CAMERA_TYPES) +@require_camera +def test_save_images_from_cameras(tmpdir, request, camera_type): + # TODO(rcadene): refactor + if camera_type == "opencv": + from lerobot.common.robot_devices.cameras.opencv import save_images_from_cameras + elif camera_type == "intelrealsense": + from lerobot.common.robot_devices.cameras.intelrealsense import save_images_from_cameras + save_images_from_cameras(tmpdir, record_time_s=1) diff --git a/tests/test_motors.py b/tests/test_motors.py index 48c2e8d8d..63ee3f1db 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -1,11 +1,24 @@ """ -Tests meant to be used locally and launched manually. +Tests for physical motors and their mocked versions. +If the physical motors are not connected to the computer, or not working, +the test will be skipped. -Example usage: +Example of running a specific test: ```bash pytest -sx tests/test_motors.py::test_find_port pytest -sx tests/test_motors.py::test_motors_bus ``` + + +Example of running test on real dynamixel motors connected to the computer: +```bash +pytest -sx tests/test_motors.py::test_motors_bus[dynamixel] +``` + +Example of running test on a mocked version of dynamixel motors: +```bash +pytest -sx -k "mocked_dynamixel" tests/test_motors.py::test_motors_bus +``` """ # TODO(rcadene): measure fps in nightly? @@ -18,34 +31,46 @@ import numpy as np import pytest -from lerobot import available_robots +from lerobot import available_motors from lerobot.common.robot_devices.motors.utils import MotorsBus -from lerobot.common.robot_devices.robots.factory import make_robot from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from lerobot.common.utils.utils import init_hydra_config -from tests.utils import ROBOT_CONFIG_PATH_TEMPLATE, require_robot +from tests.utils import TEST_MOTOR_TYPES, require_motor + +DYNAMIXEL_PORT = "/dev/tty.usbmodem575E0032081" +DYNAMIXEL_MOTORS = { + "shoulder_pan": [1, "xl430-w250"], + "shoulder_lift": [2, "xl430-w250"], + "elbow_flex": [3, "xl330-m288"], + "wrist_flex": [4, "xl330-m288"], + "wrist_roll": [5, "xl330-m288"], + "gripper": [6, "xl330-m288"], +} + + +def make_motors_bus(motor_type: str, **kwargs) -> MotorsBus: + if motor_type == "dynamixel": + from lerobot.common.robot_devices.motors.dynamixel import DynamixelMotorsBus + port = kwargs.pop("port", DYNAMIXEL_PORT) + motors = kwargs.pop("motors", DYNAMIXEL_MOTORS) + return DynamixelMotorsBus(port, motors, **kwargs) -def make_motors_bus(robot_type: str) -> MotorsBus: - # Instantiate a robot and return one of its leader arms - config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) - robot_cfg = init_hydra_config(config_path) - robot = make_robot(robot_cfg) - first_bus_name = list(robot.leader_arms.keys())[0] - motors_bus = robot.leader_arms[first_bus_name] - return motors_bus + else: + raise ValueError(f"The motor type '{motor_type}' is not valid.") -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot +# TODO(rcadene): implement mocked version of this test +@pytest.mark.parametrize("motor_type", available_motors) +@require_motor def test_find_port(request, robot_type): from lerobot.common.robot_devices.motors.dynamixel import find_port find_port() -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot +# TODO(rcadene): implement mocked version of this test +@pytest.mark.parametrize("motor_type", available_motors) +@require_motor def test_configure_motors_all_ids_1(request, robot_type): input("Are you sure you want to re-configure the motors? Press enter to continue...") # This test expect the configuration was already correct. @@ -63,8 +88,8 @@ def test_configure_motors_all_ids_1(request, robot_type): del motors_bus -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot +@pytest.mark.parametrize("motor_type", TEST_MOTOR_TYPES) +@require_motor def test_motors_bus(request, robot_type): motors_bus = make_motors_bus(robot_type) diff --git a/tests/utils.py b/tests/utils.py index 98b5a82a0..af8d7faf7 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -20,7 +20,7 @@ import pytest import torch -from lerobot import available_robots +from lerobot import available_cameras, available_motors, available_robots from lerobot.common.utils.import_utils import is_package_available DEVICE = "cuda" if torch.cuda.is_available() else "cpu" @@ -31,6 +31,8 @@ ROBOT_CONFIG_PATH_TEMPLATE = "lerobot/configs/robot/{robot}.yaml" TEST_ROBOT_TYPES = available_robots + [f"mocked_{robot_type}" for robot_type in available_robots] +TEST_CAMERA_TYPES = available_cameras + [f"mocked_{camera_type}" for camera_type in available_cameras] +TEST_MOTOR_TYPES = available_motors + [f"mocked_{motor_type}" for motor_type in available_motors] def require_x86_64_kernel(func): @@ -178,39 +180,143 @@ def wrapper(*args, **kwargs): request = kwargs.get("request") robot_type = kwargs.get("robot_type") + if robot_type is None: + raise ValueError("The 'robot_type' must be an argument of the test function.") + + if robot_type not in TEST_ROBOT_TYPES: + raise ValueError( + f"The camera type '{robot_type}' is not valid. Expected one of these '{TEST_ROBOT_TYPES}" + ) + if request is None: raise ValueError("The 'request' fixture must be an argument of the test function.") # Run test with a monkeypatched version of the robot devices. if robot_type.startswith("mocked_"): kwargs["robot_type"] = robot_type.replace("mocked_", "") + mock_cameras(request) + mock_motors(request) + + # Run test with a real robot. Skip test if robot connection fails. + else: + # `is_robot_available` is defined in `tests/conftest.py` + if not request.getfixturevalue("is_robot_available"): + pytest.skip(f"A {robot_type} robot is not available.") - monkeypatch = request.getfixturevalue("monkeypatch") + return func(*args, **kwargs) - try: - import cv2 + return wrapper - from tests.mock_opencv import MockVideoCapture - monkeypatch.setattr(cv2, "VideoCapture", MockVideoCapture) - except ImportError: - traceback.print_exc() +def require_camera(func): + @wraps(func) + def wrapper(*args, **kwargs): + # Access the pytest request context to get the is_camera_available fixture + request = kwargs.get("request") + camera_type = kwargs.get("camera_type") - try: - import pyrealsense2 as rs + if camera_type is None: + raise ValueError("The 'camera_type' must be an argument of the test function.") - # TODO(rcadene): - mock_pipeline = None - monkeypatch.setattr(rs, "pipeline", mock_pipeline) - except ImportError: - traceback.print_exc() + if camera_type not in TEST_CAMERA_TYPES: + raise ValueError( + f"The camera type '{camera_type}' is not valid. Expected one of these '{TEST_CAMERA_TYPES}" + ) + + if request is None: + raise ValueError("The 'request' fixture must be an argument of the test function.") + + # Run test with a monkeypatched version of the robot devices. + if camera_type.startswith("mocked_"): + kwargs["camera_type"] = camera_type.replace("mocked_", "") + mock_cameras(request) # Run test with a real robot. Skip test if robot connection fails. else: - # `is_robot_available` is defined in `tests/conftest.py` - if not request.getfixturevalue("is_robot_available"): - pytest.skip(f"A {robot_type} robot is not available.") + # `is_camera_available` is defined in `tests/conftest.py` + if not request.getfixturevalue("is_camera_available"): + pytest.skip(f"A {camera_type} camera is not available.") return func(*args, **kwargs) return wrapper + + +def require_motor(func): + @wraps(func) + def wrapper(*args, **kwargs): + # Access the pytest request context to get the is_motor_available fixture + request = kwargs.get("request") + motor_type = kwargs.get("motor_type") + + if motor_type is None: + raise ValueError("The 'motor_type' must be an argument of the test function.") + + if motor_type not in TEST_MOTOR_TYPES: + raise ValueError( + f"The motor type '{motor_type}' is not valid. Expected one of these '{TEST_MOTOR_TYPES}" + ) + + if request is None: + raise ValueError("The 'request' fixture must be an argument of the test function.") + + # Run test with a monkeypatched version of the robot devices. + if motor_type.startswith("mocked_"): + kwargs["motor_type"] = motor_type.replace("mocked_", "") + mock_motors(request) + + # Run test with a real robot. Skip test if robot connection fails. + else: + # `is_motor_available` is defined in `tests/conftest.py` + if not request.getfixturevalue("is_motor_available"): + pytest.skip(f"A {motor_type} motor is not available.") + + return func(*args, **kwargs) + + return wrapper + + +def mock_cameras(request): + monkeypatch = request.getfixturevalue("monkeypatch") + + try: + import cv2 + + from tests.mock_opencv import MockVideoCapture + + monkeypatch.setattr(cv2, "VideoCapture", MockVideoCapture) + except ImportError: + traceback.print_exc() + + try: + import pyrealsense2 as rs + + from tests.mock_intelrealsense import MockConfig, MockFormat, MockPipeline, MockStream + + monkeypatch.setattr(rs, "config", MockConfig) + monkeypatch.setattr(rs, "pipeline", MockPipeline) + monkeypatch.setattr(rs, "stream", MockStream) + monkeypatch.setattr(rs, "format", MockFormat) + except ImportError: + traceback.print_exc() + + +def mock_motors(request): + monkeypatch = request.getfixturevalue("monkeypatch") + + try: + import dynamixel_sdk + + from tests.mock_dynamixel import ( + MockGroupSyncRead, + MockGroupSyncWrite, + MockPacketHandler, + MockPortHandler, + ) + + monkeypatch.setattr(dynamixel_sdk, "GroupSyncRead", MockGroupSyncRead) + monkeypatch.setattr(dynamixel_sdk, "GroupSyncWrite", MockGroupSyncWrite) + monkeypatch.setattr(dynamixel_sdk, "PacketHandler", MockPacketHandler) + monkeypatch.setattr(dynamixel_sdk, "PortHandler", MockPortHandler) + except ImportError: + traceback.print_exc() From 4151630c2448aced5c8332aed2f9737b33f4b90d Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 12 Sep 2024 01:08:44 +0200 Subject: [PATCH 05/53] Mock dynamixel_sdk --- tests/mock_dynamixel.py | 47 ++++++++++++++++++++++++++++++++--------- tests/test_motors.py | 14 ++++++------ tests/utils.py | 8 +++++++ 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/tests/mock_dynamixel.py b/tests/mock_dynamixel.py index 16831a726..9a2e62612 100644 --- a/tests/mock_dynamixel.py +++ b/tests/mock_dynamixel.py @@ -1,9 +1,18 @@ from dynamixel_sdk import COMM_SUCCESS +DEFAULT_BAUDRATE = 9_600 -class PortHandler: + +def mock_convert_to_bytes(value, bytes): + del bytes # unused + return value + + +class MockPortHandler: def __init__(self, port): self.port = port + # default fresh from factory baudrate + self.baudrate = DEFAULT_BAUDRATE def openPort(self): # noqa: N802 return True @@ -14,35 +23,53 @@ def closePort(self): # noqa: N802 def setPacketTimeoutMillis(self, timeout_ms): # noqa: N802 del timeout_ms # unused + def getBaudRate(self): # noqa: N802 + return self.baudrate + + def setBaudRate(self, baudrate): # noqa: N802 + self.baudrate = baudrate + -class PacketHandler: +class MockPacketHandler: def __init__(self, protocol_version): del protocol_version # unused + # Use packet_handler.data to communicate across Read and Write + self.data = {} -class GroupSyncRead: +class MockGroupSyncRead: def __init__(self, port_handler, packet_handler, address, bytes): - pass + self.packet_handler = packet_handler def addParam(self, motor_index): # noqa: N802 - pass + if motor_index not in self.packet_handler.data: + # Initialize motor default values + self.packet_handler.data[motor_index] = { + # Key (int) are from X_SERIES_CONTROL_TABLE + 7: motor_index, # ID + 8: DEFAULT_BAUDRATE, # Baud_rate + 10: 0, # Drive_Mode + 64: 0, # Torque_Enable + 132: 0, # Present_Position + } def txRxPacket(self): # noqa: N802 return COMM_SUCCESS def getData(self, index, address, bytes): # noqa: N802 - return value # noqa: F821 + return self.packet_handler.data[index][address] -class GroupSyncWrite: +class MockGroupSyncWrite: def __init__(self, port_handler, packet_handler, address, bytes): - pass + self.packet_handler = packet_handler + self.address = address def addParam(self, index, data): # noqa: N802 - pass + self.changeParam(index, data) def txPacket(self): # noqa: N802 return COMM_SUCCESS def changeParam(self, index, data): # noqa: N802 - pass + self.packet_handler.data[index][self.address] = data diff --git a/tests/test_motors.py b/tests/test_motors.py index 63ee3f1db..dfdb33183 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -62,7 +62,7 @@ def make_motors_bus(motor_type: str, **kwargs) -> MotorsBus: # TODO(rcadene): implement mocked version of this test @pytest.mark.parametrize("motor_type", available_motors) @require_motor -def test_find_port(request, robot_type): +def test_find_port(request, motor_type): from lerobot.common.robot_devices.motors.dynamixel import find_port find_port() @@ -71,10 +71,10 @@ def test_find_port(request, robot_type): # TODO(rcadene): implement mocked version of this test @pytest.mark.parametrize("motor_type", available_motors) @require_motor -def test_configure_motors_all_ids_1(request, robot_type): +def test_configure_motors_all_ids_1(request, motor_type): input("Are you sure you want to re-configure the motors? Press enter to continue...") # This test expect the configuration was already correct. - motors_bus = make_motors_bus(robot_type) + motors_bus = make_motors_bus(motor_type) motors_bus.connect() motors_bus.write("Baud_Rate", [0] * len(motors_bus.motors)) motors_bus.set_bus_baudrate(9_600) @@ -82,7 +82,7 @@ def test_configure_motors_all_ids_1(request, robot_type): del motors_bus # Test configure - motors_bus = make_motors_bus(robot_type) + motors_bus = make_motors_bus(motor_type) motors_bus.connect() assert motors_bus.are_motors_configured() del motors_bus @@ -90,8 +90,8 @@ def test_configure_motors_all_ids_1(request, robot_type): @pytest.mark.parametrize("motor_type", TEST_MOTOR_TYPES) @require_motor -def test_motors_bus(request, robot_type): - motors_bus = make_motors_bus(robot_type) +def test_motors_bus(request, motor_type): + motors_bus = make_motors_bus(motor_type) # Test reading and writting before connecting raises an error with pytest.raises(RobotDeviceNotConnectedError): @@ -105,7 +105,7 @@ def test_motors_bus(request, robot_type): del motors_bus # Test connecting - motors_bus = make_motors_bus(robot_type) + motors_bus = make_motors_bus(motor_type) motors_bus.connect() # Test connecting twice raises an error diff --git a/tests/utils.py b/tests/utils.py index af8d7faf7..433be47da 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -312,11 +312,19 @@ def mock_motors(request): MockGroupSyncWrite, MockPacketHandler, MockPortHandler, + mock_convert_to_bytes, ) monkeypatch.setattr(dynamixel_sdk, "GroupSyncRead", MockGroupSyncRead) monkeypatch.setattr(dynamixel_sdk, "GroupSyncWrite", MockGroupSyncWrite) monkeypatch.setattr(dynamixel_sdk, "PacketHandler", MockPacketHandler) monkeypatch.setattr(dynamixel_sdk, "PortHandler", MockPortHandler) + + # Import dynamixel AFTER mocking dynamixel_sdk to use mocked classes + from lerobot.common.robot_devices.motors import dynamixel + + # TODO(rcadene): remove need to mock `convert_to_bytes` by implemented the inverse transform + # `convert_bytes_to_value` + monkeypatch.setattr(dynamixel, "convert_to_bytes", mock_convert_to_bytes) except ImportError: traceback.print_exc() From 53ebf9cf9ffb7d1ffc86fc5aff2808c5d4092cbb Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 12 Sep 2024 01:43:32 +0200 Subject: [PATCH 06/53] Mock robots (WIP segmentation fault) --- tests/test_motors.py | 1 - tests/test_robots.py | 26 +++++++++++++++++++++----- tests/utils.py | 7 +++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/tests/test_motors.py b/tests/test_motors.py index dfdb33183..372016e3e 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -9,7 +9,6 @@ pytest -sx tests/test_motors.py::test_motors_bus ``` - Example of running test on real dynamixel motors connected to the computer: ```bash pytest -sx tests/test_motors.py::test_motors_bus[dynamixel] diff --git a/tests/test_robots.py b/tests/test_robots.py index 4ce3805ee..9dbcc540b 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -1,10 +1,27 @@ """ -Tests meant to be used locally and launched manually. +Tests for physical robots and their mocked versions. +If the physical robots are not connected to the computer, or not working, +the test will be skipped. -Example usage: +Example of running a specific test: ```bash pytest -sx tests/test_robots.py::test_robot ``` + +Example of running test on real robots connected to the computer: +```bash +pytest -sx tests/test_robots.py::test_robot[koch] +pytest -sx tests/test_robots.py::test_robot[koch_bimanual] +pytest -sx tests/test_robots.py::test_robot[aloha] +``` + +Example of running test on a mocked version of robots: +```bash +pytest -sx -k "mocked_koch" tests/test_robots.py::test_robot +pytest -sx -k "mocked_koch_bimanual" tests/test_robots.py::test_robot +pytest -sx -k "mocked_aloha" tests/test_robots.py::test_robot +``` + """ from pathlib import Path @@ -12,12 +29,11 @@ import pytest import torch -from lerobot import available_robots from lerobot.common.robot_devices.robots.factory import make_robot as make_robot_from_cfg from lerobot.common.robot_devices.robots.utils import Robot from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError from lerobot.common.utils.utils import init_hydra_config -from tests.utils import ROBOT_CONFIG_PATH_TEMPLATE, require_robot +from tests.utils import ROBOT_CONFIG_PATH_TEMPLATE, TEST_ROBOT_TYPES, require_robot def make_robot(robot_type: str, overrides: list[str] | None = None) -> Robot: @@ -27,7 +43,7 @@ def make_robot(robot_type: str, overrides: list[str] | None = None) -> Robot: return robot -@pytest.mark.parametrize("robot_type", available_robots) +@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) @require_robot def test_robot(tmpdir, request, robot_type): # TODO(rcadene): measure fps in nightly? diff --git a/tests/utils.py b/tests/utils.py index 433be47da..c7915ea2e 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -197,6 +197,13 @@ def wrapper(*args, **kwargs): mock_cameras(request) mock_motors(request) + def mock_input(text): + print(text) + + monkeypatch = request.getfixturevalue("monkeypatch") + # To run calibration without user input + monkeypatch.setattr("builtins.input", mock_input) + # Run test with a real robot. Skip test if robot connection fails. else: # `is_robot_available` is defined in `tests/conftest.py` From cd4d2257d31d4302e62de442e72fda28d0ac024e Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 12 Sep 2024 02:43:21 +0200 Subject: [PATCH 07/53] Fix unit test --- tests/mock_intelrealsense.py | 18 ++++++++++++++++++ tests/test_cameras.py | 6 ++++-- tests/test_control_robot.py | 11 +++++------ tests/utils.py | 3 ++- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/tests/mock_intelrealsense.py b/tests/mock_intelrealsense.py index 1e6c3f5f5..c1d6add21 100644 --- a/tests/mock_intelrealsense.py +++ b/tests/mock_intelrealsense.py @@ -108,3 +108,21 @@ def __init__(self, config: MockConfig): def get_data(self): return np.ones((self.config.height, self.config.width), dtype=np.uint16) + + +class MockDevice: + def __init__(self): + pass + + def get_info(self, camera_info) -> str: + del camera_info # unused + # return fake serial number + return "123456789" + + +class MockContext: + def __init__(self): + pass + + def query_devices(self): + return [MockDevice()] diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 161d9a190..0c6ad08a3 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -8,14 +8,16 @@ pytest -sx tests/test_cameras.py::test_camera ``` -Example of running test on a real OpenCV camera connected to the computer: +Example of running test on a real camera connected to the computer: ```bash pytest -sx tests/test_cameras.py::test_camera[opencv] +pytest -sx tests/test_cameras.py::test_camera[intelrealsense] ``` -Example of running test on a mocked version of an OpenCV camera: +Example of running test on a mocked version of the camera: ```bash pytest -sx -k "mocked_opencv" tests/test_cameras.py::test_camera +pytest -sx -k "mocked_intelrealsense" tests/test_cameras.py::test_camera ``` """ diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index 406edeb4f..cd70263bd 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -2,15 +2,14 @@ import pytest -from lerobot import available_robots from lerobot.common.policies.factory import make_policy from lerobot.common.utils.utils import init_hydra_config from lerobot.scripts.control_robot import calibrate, record, replay, teleoperate from tests.test_robots import make_robot -from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, require_robot +from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_MOTOR_TYPES, require_robot -@pytest.mark.parametrize("robot_type", available_robots) +@pytest.mark.parametrize("robot_type", TEST_MOTOR_TYPES) @require_robot def test_teleoperate(request, robot_type): robot = make_robot(robot_type) @@ -20,7 +19,7 @@ def test_teleoperate(request, robot_type): del robot -@pytest.mark.parametrize("robot_type", available_robots) +@pytest.mark.parametrize("robot_type", TEST_MOTOR_TYPES) @require_robot def test_calibrate(request, robot_type): robot = make_robot(robot_type) @@ -28,7 +27,7 @@ def test_calibrate(request, robot_type): del robot -@pytest.mark.parametrize("robot_type", available_robots) +@pytest.mark.parametrize("robot_type", TEST_MOTOR_TYPES) @require_robot def test_record_without_cameras(tmpdir, request, robot_type): root = Path(tmpdir) @@ -38,7 +37,7 @@ def test_record_without_cameras(tmpdir, request, robot_type): record(robot, fps=30, root=root, repo_id=repo_id, warmup_time_s=1, episode_time_s=1, num_episodes=2) -@pytest.mark.parametrize("robot_type", available_robots) +@pytest.mark.parametrize("robot_type", TEST_MOTOR_TYPES) @require_robot def test_record_and_replay_and_policy(tmpdir, request, robot_type): env_name = "koch_real" diff --git a/tests/utils.py b/tests/utils.py index c7915ea2e..b1eb052ed 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -298,12 +298,13 @@ def mock_cameras(request): try: import pyrealsense2 as rs - from tests.mock_intelrealsense import MockConfig, MockFormat, MockPipeline, MockStream + from tests.mock_intelrealsense import MockConfig, MockContext, MockFormat, MockPipeline, MockStream monkeypatch.setattr(rs, "config", MockConfig) monkeypatch.setattr(rs, "pipeline", MockPipeline) monkeypatch.setattr(rs, "stream", MockStream) monkeypatch.setattr(rs, "format", MockFormat) + monkeypatch.setattr(rs, "context", MockContext) except ImportError: traceback.print_exc() From 3f993d52500b240d04062537fe26ebd452f12e66 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 12 Sep 2024 02:44:46 +0200 Subject: [PATCH 08/53] fix typo --- tests/test_control_robot.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index cd70263bd..f85e94fb7 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -6,10 +6,10 @@ from lerobot.common.utils.utils import init_hydra_config from lerobot.scripts.control_robot import calibrate, record, replay, teleoperate from tests.test_robots import make_robot -from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_MOTOR_TYPES, require_robot +from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_ROBOT_TYPES, require_robot -@pytest.mark.parametrize("robot_type", TEST_MOTOR_TYPES) +@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) @require_robot def test_teleoperate(request, robot_type): robot = make_robot(robot_type) @@ -19,7 +19,7 @@ def test_teleoperate(request, robot_type): del robot -@pytest.mark.parametrize("robot_type", TEST_MOTOR_TYPES) +@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) @require_robot def test_calibrate(request, robot_type): robot = make_robot(robot_type) @@ -27,7 +27,7 @@ def test_calibrate(request, robot_type): del robot -@pytest.mark.parametrize("robot_type", TEST_MOTOR_TYPES) +@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) @require_robot def test_record_without_cameras(tmpdir, request, robot_type): root = Path(tmpdir) @@ -37,7 +37,7 @@ def test_record_without_cameras(tmpdir, request, robot_type): record(robot, fps=30, root=root, repo_id=repo_id, warmup_time_s=1, episode_time_s=1, num_episodes=2) -@pytest.mark.parametrize("robot_type", TEST_MOTOR_TYPES) +@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) @require_robot def test_record_and_replay_and_policy(tmpdir, request, robot_type): env_name = "koch_real" From ccc0586d4517ea86ef1b1d32c5eff307c51e3171 Mon Sep 17 00:00:00 2001 From: Remi Date: Mon, 16 Sep 2024 14:49:19 +0200 Subject: [PATCH 09/53] Apply suggestions from code review Co-authored-by: Simon Alibert <75076266+aliberts@users.noreply.github.com> --- tests/mock_dynamixel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mock_dynamixel.py b/tests/mock_dynamixel.py index 9a2e62612..3206a5d12 100644 --- a/tests/mock_dynamixel.py +++ b/tests/mock_dynamixel.py @@ -11,7 +11,7 @@ def mock_convert_to_bytes(value, bytes): class MockPortHandler: def __init__(self, port): self.port = port - # default fresh from factory baudrate + # factory default baudrate self.baudrate = DEFAULT_BAUDRATE def openPort(self): # noqa: N802 From 6636db5b512788557e04e9a7189a17f225c9baaa Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Mon, 16 Sep 2024 14:50:53 +0200 Subject: [PATCH 10/53] Address comments --- .../robot_devices/cameras/intelrealsense.py | 2 + .../common/robot_devices/cameras/opencv.py | 2 + tests/mock_dynamixel.py | 9 +++ tests/test_cameras.py | 22 +++---- tests/test_control_robot.py | 41 ++++++++++--- tests/test_motors.py | 8 +-- tests/test_robots.py | 17 +++--- tests/utils.py | 59 +++++++++++-------- 8 files changed, 105 insertions(+), 55 deletions(-) diff --git a/lerobot/common/robot_devices/cameras/intelrealsense.py b/lerobot/common/robot_devices/cameras/intelrealsense.py index 77a015cf1..4a9310acb 100644 --- a/lerobot/common/robot_devices/cameras/intelrealsense.py +++ b/lerobot/common/robot_devices/cameras/intelrealsense.py @@ -288,7 +288,9 @@ def connect(self): 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): + # Using `OSError` since it's a broad that encompasses issues related to device communication raise OSError( f"Can't set {self.fps=} for IntelRealSenseCamera({self.camera_index}). Actual value is {actual_fps}." ) diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index 6652b5388..a27fec18c 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -272,7 +272,9 @@ def connect(self): actual_width = self.camera.get(cv2.CAP_PROP_FRAME_WIDTH) actual_height = self.camera.get(cv2.CAP_PROP_FRAME_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): + # Using `OSError` since it's a broad that encompasses issues related to device communication raise OSError( f"Can't set {self.fps=} for OpenCVCamera({self.camera_index}). Actual value is {actual_fps}." ) diff --git a/tests/mock_dynamixel.py b/tests/mock_dynamixel.py index 3206a5d12..bf0a09076 100644 --- a/tests/mock_dynamixel.py +++ b/tests/mock_dynamixel.py @@ -1,9 +1,18 @@ +"""Mocked classes and functions from dynamixel_sdk to allow for continuous integration +and testing code logic that requires hardware and devices (e.g. robot arms, cameras) + +Warning: These mocked versions are minimalist. They do not exactly mock every behaviors +from the original classes and functions (e.g. return types might be None instead of boolean). +""" + from dynamixel_sdk import COMM_SUCCESS DEFAULT_BAUDRATE = 9_600 def mock_convert_to_bytes(value, bytes): + # TODO(rcadene): remove need to mock `convert_to_bytes` by implemented the inverse transform + # `convert_bytes_to_value` del bytes # unused return value diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 0c6ad08a3..c9ca5a8e5 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -10,17 +10,19 @@ Example of running test on a real camera connected to the computer: ```bash -pytest -sx tests/test_cameras.py::test_camera[opencv] -pytest -sx tests/test_cameras.py::test_camera[intelrealsense] +pytest -sx 'tests/test_cameras.py::test_camera[opencv-False]' +pytest -sx 'tests/test_cameras.py::test_camera[intelrealsense-False]' ``` Example of running test on a mocked version of the camera: ```bash -pytest -sx -k "mocked_opencv" tests/test_cameras.py::test_camera -pytest -sx -k "mocked_intelrealsense" tests/test_cameras.py::test_camera +pytest -sx 'tests/test_cameras.py::test_camera[opencv-True]' +pytest -sx 'tests/test_cameras.py::test_camera[intelrealsense-True]' ``` """ +import os + import numpy as np import pytest @@ -28,8 +30,8 @@ from tests.utils import TEST_CAMERA_TYPES, require_camera # Camera indices used for connecting physical cameras -OPENCV_CAMERA_INDEX = 0 -INTELREALSENSE_CAMERA_INDEX = 128422271614 +OPENCV_CAMERA_INDEX = int(os.environ.get("LEROBOT_TEST_OPENCV_CAMERA_INDEX", 0)) +INTELREALSENSE_CAMERA_INDEX = int(os.environ.get("LEROBOT_TEST_INTELREALSENSE_CAMERA_INDEX", 128422271614)) # Maximum absolute difference between two consecutive images recored by a camera. # This value differs with respect to the camera. @@ -57,9 +59,9 @@ def make_camera(camera_type, **kwargs): raise ValueError(f"The camera type '{camera_type}' is not valid.") -@pytest.mark.parametrize("camera_type", TEST_CAMERA_TYPES) +@pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) @require_camera -def test_camera(request, camera_type): +def test_camera(request, camera_type, mock): """Test assumes that `camera.read()` returns the same image when called multiple times in a row. So the environment should not change (you shouldnt be in front of the camera) and the camera should not be moving. @@ -156,9 +158,9 @@ def test_camera(request, camera_type): del camera -@pytest.mark.parametrize("camera_type", TEST_CAMERA_TYPES) +@pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) @require_camera -def test_save_images_from_cameras(tmpdir, request, camera_type): +def test_save_images_from_cameras(tmpdir, request, camera_type, mock): # TODO(rcadene): refactor if camera_type == "opencv": from lerobot.common.robot_devices.cameras.opencv import save_images_from_cameras diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index f85e94fb7..fe73d9ffa 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -1,3 +1,28 @@ +""" +Tests for physical robots and their mocked versions. +If the physical robots are not connected to the computer, or not working, +the test will be skipped. + +Example of running a specific test: +```bash +pytest -sx tests/test_control_robot.py::test_teleoperate +``` + +Example of running test on real robots connected to the computer: +```bash +pytest -sx 'tests/test_control_robot.py::test_teleoperate[koch-False]' +pytest -sx 'tests/test_control_robot.py::test_teleoperate[koch_bimanual-False]' +pytest -sx 'tests/test_control_robot.py::test_teleoperate[aloha-False]' +``` + +Example of running test on a mocked version of robots: +```bash +pytest -sx 'tests/test_control_robot.py::test_teleoperate[koch-True]' +pytest -sx 'tests/test_control_robot.py::test_teleoperate[koch_bimanual-True]' +pytest -sx 'tests/test_control_robot.py::test_teleoperate[aloha-True]' +``` +""" + from pathlib import Path import pytest @@ -9,9 +34,9 @@ from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_ROBOT_TYPES, require_robot -@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) @require_robot -def test_teleoperate(request, robot_type): +def test_teleoperate(request, robot_type, mock): robot = make_robot(robot_type) teleoperate(robot, teleop_time_s=1) teleoperate(robot, fps=30, teleop_time_s=1) @@ -19,17 +44,17 @@ def test_teleoperate(request, robot_type): del robot -@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) @require_robot -def test_calibrate(request, robot_type): +def test_calibrate(request, robot_type, mock): robot = make_robot(robot_type) calibrate(robot) del robot -@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) @require_robot -def test_record_without_cameras(tmpdir, request, robot_type): +def test_record_without_cameras(tmpdir, request, robot_type, mock): root = Path(tmpdir) repo_id = "lerobot/debug" @@ -37,9 +62,9 @@ def test_record_without_cameras(tmpdir, request, robot_type): record(robot, fps=30, root=root, repo_id=repo_id, warmup_time_s=1, episode_time_s=1, num_episodes=2) -@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) @require_robot -def test_record_and_replay_and_policy(tmpdir, request, robot_type): +def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): env_name = "koch_real" policy_name = "act_koch_real" diff --git a/tests/test_motors.py b/tests/test_motors.py index 372016e3e..ba1f9ba64 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -11,12 +11,12 @@ Example of running test on real dynamixel motors connected to the computer: ```bash -pytest -sx tests/test_motors.py::test_motors_bus[dynamixel] +pytest -sx 'tests/test_motors.py::test_motors_bus[dynamixel-False]' ``` Example of running test on a mocked version of dynamixel motors: ```bash -pytest -sx -k "mocked_dynamixel" tests/test_motors.py::test_motors_bus +pytest -sx 'tests/test_motors.py::test_motors_bus[dynamixel-True]' ``` """ @@ -87,9 +87,9 @@ def test_configure_motors_all_ids_1(request, motor_type): del motors_bus -@pytest.mark.parametrize("motor_type", TEST_MOTOR_TYPES) +@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) @require_motor -def test_motors_bus(request, motor_type): +def test_motors_bus(request, motor_type, mock): motors_bus = make_motors_bus(motor_type) # Test reading and writting before connecting raises an error diff --git a/tests/test_robots.py b/tests/test_robots.py index 9dbcc540b..8bc8205df 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -10,18 +10,17 @@ Example of running test on real robots connected to the computer: ```bash -pytest -sx tests/test_robots.py::test_robot[koch] -pytest -sx tests/test_robots.py::test_robot[koch_bimanual] -pytest -sx tests/test_robots.py::test_robot[aloha] +pytest -sx 'tests/test_robots.py::test_robot[koch-False]' +pytest -sx 'tests/test_robots.py::test_robot[koch_bimanual-False]' +pytest -sx 'tests/test_robots.py::test_robot[aloha-False]' ``` Example of running test on a mocked version of robots: ```bash -pytest -sx -k "mocked_koch" tests/test_robots.py::test_robot -pytest -sx -k "mocked_koch_bimanual" tests/test_robots.py::test_robot -pytest -sx -k "mocked_aloha" tests/test_robots.py::test_robot +pytest -sx 'tests/test_robots.py::test_robot[koch-True]' +pytest -sx 'tests/test_robots.py::test_robot[koch_bimanual-True]' +pytest -sx 'tests/test_robots.py::test_robot[aloha-True]' ``` - """ from pathlib import Path @@ -43,9 +42,9 @@ def make_robot(robot_type: str, overrides: list[str] | None = None) -> Robot: return robot -@pytest.mark.parametrize("robot_type", TEST_ROBOT_TYPES) +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) @require_robot -def test_robot(tmpdir, request, robot_type): +def test_robot(tmpdir, request, robot_type, mock): # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs # TODO(rcadene): add compatibility with other robots diff --git a/tests/utils.py b/tests/utils.py index b1eb052ed..338069704 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -30,9 +30,17 @@ ROBOT_CONFIG_PATH_TEMPLATE = "lerobot/configs/robot/{robot}.yaml" -TEST_ROBOT_TYPES = available_robots + [f"mocked_{robot_type}" for robot_type in available_robots] -TEST_CAMERA_TYPES = available_cameras + [f"mocked_{camera_type}" for camera_type in available_cameras] -TEST_MOTOR_TYPES = available_motors + [f"mocked_{motor_type}" for motor_type in available_motors] +TEST_ROBOT_TYPES = [] +for robot_type in available_robots: + TEST_ROBOT_TYPES += [(robot_type, True), (robot_type, False)] + +TEST_CAMERA_TYPES = [] +for camera_type in available_cameras: + TEST_CAMERA_TYPES += [(camera_type, True), (camera_type, False)] + +TEST_MOTOR_TYPES = [] +for motor_type in available_motors: + TEST_MOTOR_TYPES += [(motor_type, True), (motor_type, False)] def require_x86_64_kernel(func): @@ -179,21 +187,22 @@ def wrapper(*args, **kwargs): # Access the pytest request context to get the is_robot_available fixture request = kwargs.get("request") robot_type = kwargs.get("robot_type") + mock = kwargs.get("mock") if robot_type is None: raise ValueError("The 'robot_type' must be an argument of the test function.") + if request is None: + raise ValueError("The 'request' fixture must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") - if robot_type not in TEST_ROBOT_TYPES: + if robot_type not in available_robots: raise ValueError( - f"The camera type '{robot_type}' is not valid. Expected one of these '{TEST_ROBOT_TYPES}" + f"The camera type '{robot_type}' is not valid. Expected one of these '{available_robots}" ) - if request is None: - raise ValueError("The 'request' fixture must be an argument of the test function.") - # Run test with a monkeypatched version of the robot devices. - if robot_type.startswith("mocked_"): - kwargs["robot_type"] = robot_type.replace("mocked_", "") + if mock: mock_cameras(request) mock_motors(request) @@ -221,21 +230,22 @@ def wrapper(*args, **kwargs): # Access the pytest request context to get the is_camera_available fixture request = kwargs.get("request") camera_type = kwargs.get("camera_type") + mock = kwargs.get("mock") + if request is None: + raise ValueError("The 'request' fixture must be an argument of the test function.") if camera_type is None: raise ValueError("The 'camera_type' must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") - if camera_type not in TEST_CAMERA_TYPES: + if camera_type not in available_cameras: raise ValueError( - f"The camera type '{camera_type}' is not valid. Expected one of these '{TEST_CAMERA_TYPES}" + f"The camera type '{camera_type}' is not valid. Expected one of these '{available_cameras}" ) - if request is None: - raise ValueError("The 'request' fixture must be an argument of the test function.") - # Run test with a monkeypatched version of the robot devices. - if camera_type.startswith("mocked_"): - kwargs["camera_type"] = camera_type.replace("mocked_", "") + if mock: mock_cameras(request) # Run test with a real robot. Skip test if robot connection fails. @@ -255,21 +265,22 @@ def wrapper(*args, **kwargs): # Access the pytest request context to get the is_motor_available fixture request = kwargs.get("request") motor_type = kwargs.get("motor_type") + mock = kwargs.get("mock") + if request is None: + raise ValueError("The 'request' fixture must be an argument of the test function.") if motor_type is None: raise ValueError("The 'motor_type' must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") - if motor_type not in TEST_MOTOR_TYPES: + if motor_type not in available_motors: raise ValueError( - f"The motor type '{motor_type}' is not valid. Expected one of these '{TEST_MOTOR_TYPES}" + f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" ) - if request is None: - raise ValueError("The 'request' fixture must be an argument of the test function.") - # Run test with a monkeypatched version of the robot devices. - if motor_type.startswith("mocked_"): - kwargs["motor_type"] = motor_type.replace("mocked_", "") + if mock: mock_motors(request) # Run test with a real robot. Skip test if robot connection fails. From 624551bea9f848eb4815f01630b456b230fa3084 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Mon, 16 Sep 2024 14:52:27 +0200 Subject: [PATCH 11/53] Address comments --- lerobot/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lerobot/__init__.py b/lerobot/__init__.py index 2fedb1222..851383dd9 100644 --- a/lerobot/__init__.py +++ b/lerobot/__init__.py @@ -200,13 +200,13 @@ "aloha", ] -# lists all available robots from `lerobot/common/robot_devices/cameras` +# lists all available cameras from `lerobot/common/robot_devices/cameras` available_cameras = [ "opencv", "intelrealsense", ] -# lists all available robots from `lerobot/common/robot_devices/motors` +# lists all available motors from `lerobot/common/robot_devices/motors` available_motors = [ "dynamixel", ] From adc8dc9bfbd90de53758e910c60926a6c819bb2f Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Mon, 16 Sep 2024 14:53:45 +0200 Subject: [PATCH 12/53] Address comments --- tests/test_cameras.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_cameras.py b/tests/test_cameras.py index c9ca5a8e5..1ddbbd076 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -115,6 +115,7 @@ def test_camera(request, camera_type, mock): "max_pixel_difference between read() and async_read()", compute_max_pixel_difference(color_image, async_color_image), ) + # TODO(rcadene): properly set `rtol` assert np.allclose(color_image, async_color_image, rtol=1e-5, atol=MAX_PIXEL_DIFFERENCE), error_msg # Test disconnecting From 886923a8905d16a50023087fe6d40a4dccf36539 Mon Sep 17 00:00:00 2001 From: Simon Alibert <75076266+aliberts@users.noreply.github.com> Date: Wed, 25 Sep 2024 11:29:59 +0200 Subject: [PATCH 13/53] Fix opencv segmentation fault (#442) Co-authored-by: Remi --- .../common/robot_devices/cameras/opencv.py | 165 ++++++++++-------- tests/mock_opencv.py | 29 ++- tests/test_cameras.py | 8 +- 3 files changed, 118 insertions(+), 84 deletions(-) diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index a27fec18c..661fae6c0 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -219,6 +219,14 @@ def __init__(self, camera_index: int, config: OpenCVCameraConfig | None = None, self.camera = None self.is_connected = False self.thread = None + # Using Lock to avoid race condition and segfault when multiple threads + # access the same camera. This is not one of our use case, but we add this + # for safety if users want to use this class with threads. As a result, threads + # will go sequentially through the code logic protected by a lock, instead of + # in parallel. Also, we use Recursive Lock to avoid deadlock by allowing each + # thread to acquire the lock multiple times. + # TODO(rcadene, aliberts): Add RLock on every robot devices where it makes sense? + self.lock = threading.RLock() self.stop_event = None self.color_image = None self.logs = {} @@ -227,21 +235,18 @@ def connect(self): if self.is_connected: raise RobotDeviceAlreadyConnectedError(f"OpenCVCamera({self.camera_index}) is already connected.") - # First create a temporary camera trying to access `camera_index`, - # and verify it is a valid camera by calling `isOpened`. - - if platform.system() == "Linux": - # Linux uses ports for connecting to cameras - tmp_camera = cv2.VideoCapture(f"/dev/video{self.camera_index}") - else: - tmp_camera = cv2.VideoCapture(self.camera_index) - - is_camera_open = tmp_camera.isOpened() - # Release camera to make it accessible for `find_camera_indices` - del tmp_camera - - # If the camera doesn't work, display the camera indices corresponding to - # valid cameras. + camera_idx = f"/dev/video{self.camera_index}" if platform.system() == "Linux" else self.camera_index + with self.lock: + # First create a temporary camera trying to access `camera_index`, + # and verify it is a valid camera by calling `isOpened`. + tmp_camera = cv2.VideoCapture(camera_idx) + is_camera_open = tmp_camera.isOpened() + # Release camera to make it accessible for `find_camera_indices` + tmp_camera.release() + del tmp_camera + + # If the camera doesn't work, display the camera indices corresponding to + # valid cameras. if not is_camera_open: # Verify that the provided `camera_index` is valid before printing the traceback available_cam_ids = find_camera_indices() @@ -251,47 +256,45 @@ def connect(self): "To find the camera index you should use, run `python lerobot/common/robot_devices/cameras/opencv.py`." ) - raise OSError(f"Can't access OpenCVCamera({self.camera_index}).") + raise OSError(f"Can't access OpenCVCamera({camera_idx}).") # Secondly, create the camera that will be used downstream. # Note: For some unknown reason, calling `isOpened` blocks the camera which then # needs to be re-created. - if platform.system() == "Linux": - self.camera = cv2.VideoCapture(f"/dev/video{self.camera_index}") - else: - self.camera = cv2.VideoCapture(self.camera_index) - - if self.fps is not None: - self.camera.set(cv2.CAP_PROP_FPS, self.fps) - if self.width is not None: - self.camera.set(cv2.CAP_PROP_FRAME_WIDTH, self.width) - if self.height is not None: - self.camera.set(cv2.CAP_PROP_FRAME_HEIGHT, self.height) - - actual_fps = self.camera.get(cv2.CAP_PROP_FPS) - actual_width = self.camera.get(cv2.CAP_PROP_FRAME_WIDTH) - actual_height = self.camera.get(cv2.CAP_PROP_FRAME_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): - # Using `OSError` since it's a broad that encompasses issues related to device communication - raise OSError( - f"Can't set {self.fps=} for OpenCVCamera({self.camera_index}). Actual value is {actual_fps}." - ) - if self.width is not None and self.width != actual_width: - raise OSError( - f"Can't set {self.width=} for OpenCVCamera({self.camera_index}). Actual value is {actual_width}." - ) - if self.height is not None and self.height != actual_height: - raise OSError( - f"Can't set {self.height=} for OpenCVCamera({self.camera_index}). Actual value is {actual_height}." - ) + with self.lock: + self.camera = cv2.VideoCapture(camera_idx) + + if self.fps is not None: + self.camera.set(cv2.CAP_PROP_FPS, self.fps) + if self.width is not None: + self.camera.set(cv2.CAP_PROP_FRAME_WIDTH, self.width) + if self.height is not None: + self.camera.set(cv2.CAP_PROP_FRAME_HEIGHT, self.height) + + actual_fps = self.camera.get(cv2.CAP_PROP_FPS) + actual_width = self.camera.get(cv2.CAP_PROP_FRAME_WIDTH) + actual_height = self.camera.get(cv2.CAP_PROP_FRAME_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): + # Using `OSError` since it's a broad that encompasses issues related to device communication + raise OSError( + f"Can't set {self.fps=} for OpenCVCamera({self.camera_index}). Actual value is {actual_fps}." + ) + if self.width is not None and self.width != actual_width: + raise OSError( + f"Can't set {self.width=} for OpenCVCamera({self.camera_index}). Actual value is {actual_width}." + ) + if self.height is not None and self.height != actual_height: + raise OSError( + f"Can't set {self.height=} for OpenCVCamera({self.camera_index}). Actual value is {actual_height}." + ) - self.fps = actual_fps - self.width = actual_width - self.height = actual_height + self.fps = int(actual_fps) + self.width = int(actual_width) + self.height = int(actual_height) - self.is_connected = True + self.is_connected = True def read(self, temporary_color_mode: str | None = None) -> np.ndarray: """Read a frame from the camera returned in the format (height, width, channels) @@ -307,7 +310,9 @@ def read(self, temporary_color_mode: str | None = None) -> np.ndarray: start_time = time.perf_counter() - ret, color_image = self.camera.read() + with self.lock: + ret, color_image = self.camera.read() + if not ret: raise OSError(f"Can't capture color image from camera {self.camera_index}.") @@ -336,11 +341,17 @@ def read(self, temporary_color_mode: str | None = None) -> np.ndarray: # log the utc time at which the image was received self.logs["timestamp_utc"] = capture_timestamp_utc() + with self.lock: + self.color_image = color_image + return color_image def read_loop(self): while not self.stop_event.is_set(): - self.color_image = self.read() + try: + self.color_image = self.read() + except Exception as e: + print(f"Error reading in thread: {e}") def async_read(self): if not self.is_connected: @@ -349,21 +360,26 @@ def async_read(self): ) if self.thread is None: - self.stop_event = threading.Event() - self.thread = Thread(target=self.read_loop, args=()) - self.thread.daemon = True - self.thread.start() + with self.lock: + self.stop_event = threading.Event() + self.thread = Thread(target=self.read_loop, args=()) + self.thread.daemon = True + self.thread.start() num_tries = 0 - while self.color_image is None: - 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()): - raise Exception( - "The thread responsible for `self.async_read()` took too much time to start. There might be an issue. Verify that `self.thread.start()` has been called." - ) + while True: + with self.lock: + if self.color_image is not None: + return self.color_image - return self.color_image + time.sleep(1 / self.fps) + num_tries += 1 + if num_tries > self.fps * 2: + raise TimeoutError("Timed out waiting for async_read() to start.") + # if num_tries > self.fps and (self.thread.ident is None or not self.thread.is_alive()): + # raise Exception( + # "The thread responsible for `self.async_read()` took too much time to start. There might be an issue. Verify that `self.thread.start()` has been called." + # ) def disconnect(self): if not self.is_connected: @@ -371,21 +387,22 @@ def disconnect(self): f"OpenCVCamera({self.camera_index}) is not connected. Try running `camera.connect()` first." ) - if self.thread is not None and self.thread.is_alive(): - # wait for the thread to finish - self.stop_event.set() - self.thread.join() + if self.thread is not None: + with self.lock: + self.stop_event.set() + self.thread.join() # wait for the thread to finish self.thread = None self.stop_event = None - self.camera.release() - self.camera = None - - self.is_connected = False + with self.lock: + self.camera.release() + self.camera = None + self.is_connected = False def __del__(self): - if getattr(self, "is_connected", False): - self.disconnect() + with self.lock: + if getattr(self, "is_connected", False): + self.disconnect() if __name__ == "__main__": diff --git a/tests/mock_opencv.py b/tests/mock_opencv.py index c54ae285d..8a89c0b5c 100644 --- a/tests/mock_opencv.py +++ b/tests/mock_opencv.py @@ -1,28 +1,35 @@ +from functools import cache + import cv2 import numpy as np -class MockVideoCapture(cv2.VideoCapture): - image = { - "480x640": np.random.randint(0, 256, size=(480, 640, 3), dtype=np.uint8), - "720x1280": np.random.randint(0, 256, size=(720, 1280, 3), dtype=np.uint8), - } +@cache +def _generate_image(width: int, height: int): + return np.random.randint(0, 256, size=(height, width, 3), dtype=np.uint8) + +class MockVideoCapture: def __init__(self, *args, **kwargs): self._mock_dict = { cv2.CAP_PROP_FPS: 30, cv2.CAP_PROP_FRAME_WIDTH: 640, cv2.CAP_PROP_FRAME_HEIGHT: 480, } + self._is_opened = True def isOpened(self): # noqa: N802 - return True + return self._is_opened def set(self, propId: int, value: float) -> bool: # noqa: N803 + if not self._is_opened: + raise RuntimeError("Camera is not opened") self._mock_dict[propId] = value return True def get(self, propId: int) -> float: # noqa: N803 + if not self._is_opened: + raise RuntimeError("Camera is not opened") value = self._mock_dict[propId] if value == 0: if propId == cv2.CAP_PROP_FRAME_HEIGHT: @@ -32,10 +39,16 @@ def get(self, propId: int) -> float: # noqa: N803 return value def read(self): + if not self._is_opened: + raise RuntimeError("Camera is not opened") h = self.get(cv2.CAP_PROP_FRAME_HEIGHT) w = self.get(cv2.CAP_PROP_FRAME_WIDTH) ret = True - return ret, self.image[f"{h}x{w}"] + return ret, _generate_image(width=w, height=h) def release(self): - pass + self._is_opened = False + + def __del__(self): + if self._is_opened: + self.release() diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 1ddbbd076..a5d30f6ca 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -116,7 +116,9 @@ def test_camera(request, camera_type, mock): compute_max_pixel_difference(color_image, async_color_image), ) # TODO(rcadene): properly set `rtol` - assert np.allclose(color_image, async_color_image, rtol=1e-5, atol=MAX_PIXEL_DIFFERENCE), error_msg + np.testing.assert_allclose( + color_image, async_color_image, rtol=1e-5, atol=MAX_PIXEL_DIFFERENCE, err_msg=error_msg + ) # Test disconnecting camera.disconnect() @@ -133,7 +135,9 @@ def test_camera(request, camera_type, mock): camera.connect() assert camera.color_mode == "bgr" bgr_color_image = camera.read() - assert np.allclose(color_image, bgr_color_image[:, :, [2, 1, 0]], rtol=1e-5, atol=MAX_PIXEL_DIFFERENCE) + np.testing.assert_allclose( + color_image, bgr_color_image[:, :, [2, 1, 0]], rtol=1e-5, atol=MAX_PIXEL_DIFFERENCE, err_msg=error_msg + ) del camera # TODO(rcadene): Add a test for a camera that doesnt support fps=60 and raises an OSError From 1bf284562e956aa0ab96d1e014c828d4af5201bc Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Wed, 25 Sep 2024 11:36:08 +0200 Subject: [PATCH 14/53] pre-commit run --all-files --- lerobot/common/robot_devices/cameras/opencv.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index 661fae6c0..a2a4407e2 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -219,10 +219,10 @@ def __init__(self, camera_index: int, config: OpenCVCameraConfig | None = None, self.camera = None self.is_connected = False self.thread = None - # Using Lock to avoid race condition and segfault when multiple threads - # access the same camera. This is not one of our use case, but we add this - # for safety if users want to use this class with threads. As a result, threads - # will go sequentially through the code logic protected by a lock, instead of + # Using Lock to avoid race condition and segfault when threads access the + # same camera. Though not currently a use case for LeRobot, lock ensures + # safety if users want to use this class with threads. As a result, threads + # will go sequentially through the code logic protected by a lock, instead of # in parallel. Also, we use Recursive Lock to avoid deadlock by allowing each # thread to acquire the lock multiple times. # TODO(rcadene, aliberts): Add RLock on every robot devices where it makes sense? From bcf27b8c01eba8af863394b28220c2edc2751781 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Wed, 25 Sep 2024 12:11:27 +0200 Subject: [PATCH 15/53] Skip mocking tests with minimal pytest --- tests/utils.py | 60 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 338069704..726d04a04 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -203,7 +203,14 @@ def wrapper(*args, **kwargs): # Run test with a monkeypatched version of the robot devices. if mock: - mock_cameras(request) + # TODO(rcadene): redesign mocking to not have this hardcoded logic + if robot_type == "koch": + camera_type = "opencv" + elif robot_type == "aloha": + camera_type = "intelrealsense" + else: + camera_type = "all" + mock_cameras(request, camera_type) mock_motors(request) def mock_input(text): @@ -246,7 +253,7 @@ def wrapper(*args, **kwargs): # Run test with a monkeypatched version of the robot devices. if mock: - mock_cameras(request) + mock_cameras(request, camera_type) # Run test with a real robot. Skip test if robot connection fails. else: @@ -294,33 +301,47 @@ def wrapper(*args, **kwargs): return wrapper -def mock_cameras(request): +def mock_cameras(request, camera_type="all"): + # TODO(rcadene): Redesign the mocking tests monkeypatch = request.getfixturevalue("monkeypatch") - try: - import cv2 + if camera_type in ["opencv", "all"]: + try: + import cv2 - from tests.mock_opencv import MockVideoCapture + from tests.mock_opencv import MockVideoCapture - monkeypatch.setattr(cv2, "VideoCapture", MockVideoCapture) - except ImportError: - traceback.print_exc() + monkeypatch.setattr(cv2, "VideoCapture", MockVideoCapture) + except ImportError: + traceback.print_exc() + pytest.skip("To avoid skipping tests mocking opencv cameras, run `pip install opencv-python`.") - try: - import pyrealsense2 as rs + if camera_type in ["intelrealsense", "all"]: + try: + import pyrealsense2 as rs - from tests.mock_intelrealsense import MockConfig, MockContext, MockFormat, MockPipeline, MockStream + from tests.mock_intelrealsense import ( + MockConfig, + MockContext, + MockFormat, + MockPipeline, + MockStream, + ) - monkeypatch.setattr(rs, "config", MockConfig) - monkeypatch.setattr(rs, "pipeline", MockPipeline) - monkeypatch.setattr(rs, "stream", MockStream) - monkeypatch.setattr(rs, "format", MockFormat) - monkeypatch.setattr(rs, "context", MockContext) - except ImportError: - traceback.print_exc() + monkeypatch.setattr(rs, "config", MockConfig) + monkeypatch.setattr(rs, "pipeline", MockPipeline) + monkeypatch.setattr(rs, "stream", MockStream) + monkeypatch.setattr(rs, "format", MockFormat) + monkeypatch.setattr(rs, "context", MockContext) + except ImportError: + traceback.print_exc() + pytest.skip( + "To avoid skipping tests mocking intelrealsense cameras, run `pip install pyrealsense2`." + ) def mock_motors(request): + # TODO(rcadene): Redesign the mocking tests monkeypatch = request.getfixturevalue("monkeypatch") try: @@ -347,3 +368,4 @@ def mock_motors(request): monkeypatch.setattr(dynamixel, "convert_to_bytes", mock_convert_to_bytes) except ImportError: traceback.print_exc() + pytest.skip("To avoid skipping tests mocking dynamixel motors, run `pip install dynamixel-sdk`.") From 558420115edd701280b9ec3f90fbc68b7b2f74ab Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Wed, 25 Sep 2024 12:22:22 +0200 Subject: [PATCH 16/53] mock=False --- tests/test_motors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_motors.py b/tests/test_motors.py index ba1f9ba64..55f2f113a 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -61,7 +61,7 @@ def make_motors_bus(motor_type: str, **kwargs) -> MotorsBus: # TODO(rcadene): implement mocked version of this test @pytest.mark.parametrize("motor_type", available_motors) @require_motor -def test_find_port(request, motor_type): +def test_find_port(request, motor_type, mock=False): from lerobot.common.robot_devices.motors.dynamixel import find_port find_port() @@ -70,7 +70,7 @@ def test_find_port(request, motor_type): # TODO(rcadene): implement mocked version of this test @pytest.mark.parametrize("motor_type", available_motors) @require_motor -def test_configure_motors_all_ids_1(request, motor_type): +def test_configure_motors_all_ids_1(request, motor_type, mock=False): input("Are you sure you want to re-configure the motors? Press enter to continue...") # This test expect the configuration was already correct. motors_bus = make_motors_bus(motor_type) From 6377d2a96c689a3dd55ca73b4967f9e028bb083d Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Wed, 25 Sep 2024 12:29:53 +0200 Subject: [PATCH 17/53] mock) --- tests/test_motors.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_motors.py b/tests/test_motors.py index 55f2f113a..cb416989f 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -59,18 +59,18 @@ def make_motors_bus(motor_type: str, **kwargs) -> MotorsBus: # TODO(rcadene): implement mocked version of this test -@pytest.mark.parametrize("motor_type", available_motors) +@pytest.mark.parametrize("motor_type, mock", [(m, False) for m in available_motors]) @require_motor -def test_find_port(request, motor_type, mock=False): +def test_find_port(request, motor_type, mock): from lerobot.common.robot_devices.motors.dynamixel import find_port find_port() # TODO(rcadene): implement mocked version of this test -@pytest.mark.parametrize("motor_type", available_motors) +@pytest.mark.parametrize("motor_type, mock", [(m, False) for m in available_motors]) @require_motor -def test_configure_motors_all_ids_1(request, motor_type, mock=False): +def test_configure_motors_all_ids_1(request, motor_type, mock): input("Are you sure you want to re-configure the motors? Press enter to continue...") # This test expect the configuration was already correct. motors_bus = make_motors_bus(motor_type) From bded8cbbe9c6487b00f8060eda5b88ed75be6f4e Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Wed, 25 Sep 2024 14:11:28 +0200 Subject: [PATCH 18/53] Fix unit tests --- lerobot/scripts/control_robot.py | 9 +++++++-- poetry.lock | 3 +-- tests/test_control_robot.py | 27 ++++++++++++++++++++++----- tests/utils.py | 2 +- 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/lerobot/scripts/control_robot.py b/lerobot/scripts/control_robot.py index a6506a3fe..a23c21c6e 100644 --- a/lerobot/scripts/control_robot.py +++ b/lerobot/scripts/control_robot.py @@ -242,7 +242,8 @@ def is_headless(): ######################################################################################## -def calibrate(robot: Robot, arms: list[str] | None): +def get_available_arms(robot): + # TODO(rcadene): moves this function in manipulator class? available_arms = [] for name in robot.follower_arms: arm_id = get_arm_id(name, "follower") @@ -250,9 +251,12 @@ def calibrate(robot: Robot, arms: list[str] | None): for name in robot.leader_arms: arm_id = get_arm_id(name, "leader") available_arms.append(arm_id) + return available_arms - unknown_arms = [arm_id for arm_id in arms if arm_id not in available_arms] +def calibrate(robot: Robot, arms: list[str] | None): + available_arms = get_available_arms(robot) + unknown_arms = [arm_id for arm_id in arms if arm_id not in available_arms] available_arms_str = " ".join(available_arms) unknown_arms_str = " ".join(unknown_arms) @@ -445,6 +449,7 @@ def on_press(key): # Using `with` to exist smoothly if an execption is raised. futures = [] num_image_writers = num_image_writers_per_camera * len(robot.cameras) + num_image_writers = max(num_image_writers, 1) with concurrent.futures.ThreadPoolExecutor(max_workers=num_image_writers) as executor: # Start recording all episodes while episode_index < num_episodes: diff --git a/poetry.lock b/poetry.lock index 40bf29eff..d8165371e 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. [[package]] name = "absl-py" @@ -2406,7 +2406,6 @@ description = "Nvidia JIT LTO Library" optional = false python-versions = ">=3" files = [ - {file = "nvidia_nvjitlink_cu12-12.5.82-py3-none-manylinux2014_aarch64.whl", hash = "sha256:98103729cc5226e13ca319a10bbf9433bbbd44ef64fe72f45f067cacc14b8d27"}, {file = "nvidia_nvjitlink_cu12-12.5.82-py3-none-manylinux2014_x86_64.whl", hash = "sha256:f9b37bc5c8cf7509665cb6ada5aaa0ce65618f2332b7d3e78e9790511f111212"}, {file = "nvidia_nvjitlink_cu12-12.5.82-py3-none-win_amd64.whl", hash = "sha256:e782564d705ff0bf61ac3e1bf730166da66dd2fe9012f111ede5fc49b64ae697"}, ] diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index fe73d9ffa..14fe91585 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -29,7 +29,7 @@ from lerobot.common.policies.factory import make_policy from lerobot.common.utils.utils import init_hydra_config -from lerobot.scripts.control_robot import calibrate, record, replay, teleoperate +from lerobot.scripts.control_robot import calibrate, get_available_arms, record, replay, teleoperate from tests.test_robots import make_robot from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_ROBOT_TYPES, require_robot @@ -48,7 +48,7 @@ def test_teleoperate(request, robot_type, mock): @require_robot def test_calibrate(request, robot_type, mock): robot = make_robot(robot_type) - calibrate(robot) + calibrate(robot, arms=get_available_arms(robot)) del robot @@ -59,7 +59,17 @@ def test_record_without_cameras(tmpdir, request, robot_type, mock): repo_id = "lerobot/debug" robot = make_robot(robot_type, overrides=["~cameras"]) - record(robot, fps=30, root=root, repo_id=repo_id, warmup_time_s=1, episode_time_s=1, num_episodes=2) + record( + robot, + fps=30, + root=root, + repo_id=repo_id, + warmup_time_s=1, + episode_time_s=1, + num_episodes=2, + run_compute_stats=False, + push_to_hub=False, + ) @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) @@ -73,7 +83,14 @@ def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): robot = make_robot(robot_type) dataset = record( - robot, fps=30, root=root, repo_id=repo_id, warmup_time_s=1, episode_time_s=1, num_episodes=2 + robot, + fps=30, + root=root, + repo_id=repo_id, + warmup_time_s=1, + episode_time_s=1, + num_episodes=2, + push_to_hub=False, ) replay(robot, episode=0, fps=30, root=root, repo_id=repo_id) @@ -89,6 +106,6 @@ def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): policy = make_policy(hydra_cfg=cfg, dataset_stats=dataset.stats) - record(robot, policy, cfg, run_time_s=1) + record(robot, policy, cfg, warmup_time_s=1, episode_time_s=1, run_compute_stats=False, push_to_hub=False) del robot diff --git a/tests/utils.py b/tests/utils.py index 726d04a04..aeafd9d19 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -204,7 +204,7 @@ def wrapper(*args, **kwargs): # Run test with a monkeypatched version of the robot devices. if mock: # TODO(rcadene): redesign mocking to not have this hardcoded logic - if robot_type == "koch": + if robot_type in ["koch", "koch_bimanual"]: camera_type = "opencv" elif robot_type == "aloha": camera_type = "intelrealsense" From 2c0171632f57c09e6e3774efa0467377ecb00721 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Wed, 25 Sep 2024 15:18:21 +0200 Subject: [PATCH 19/53] fix aloha mock --- tests/mock_dynamixel.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/mock_dynamixel.py b/tests/mock_dynamixel.py index bf0a09076..a3063e63e 100644 --- a/tests/mock_dynamixel.py +++ b/tests/mock_dynamixel.py @@ -59,7 +59,9 @@ def addParam(self, motor_index): # noqa: N802 8: DEFAULT_BAUDRATE, # Baud_rate 10: 0, # Drive_Mode 64: 0, # Torque_Enable - 132: 0, # Present_Position + # Set 2560 since calibration values for Aloha gripper is between start_pos=2499 and end_pos=3144 + # For other joints, 2560 will be autocorrected to be in calibration range + 132: 2560, # Present_Position } def txRxPacket(self): # noqa: N802 From 500d505bf6f951dcfa5f209f5d4f2a77146395ff Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 11:41:19 +0200 Subject: [PATCH 20/53] Add support for video=False in record (no tested yet) --- lerobot/scripts/control_robot.py | 60 ++++++++++++++++++-------------- tests/test_control_robot.py | 13 ++++++- 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/lerobot/scripts/control_robot.py b/lerobot/scripts/control_robot.py index a23c21c6e..995b19c9e 100644 --- a/lerobot/scripts/control_robot.py +++ b/lerobot/scripts/control_robot.py @@ -337,9 +337,6 @@ def record( f"Your dataset name begins by 'eval_' ({dataset_name}) but no policy is provided ({policy})." ) - if not video: - raise NotImplementedError() - if not robot.is_connected: robot.connect() @@ -550,15 +547,23 @@ def on_press(key): num_frames = frame_index for key in image_keys: - tmp_imgs_dir = videos_dir / f"{key}_episode_{episode_index:06d}" - fname = f"{key}_episode_{episode_index:06d}.mp4" - video_path = local_dir / "videos" / fname - if video_path.exists(): - video_path.unlink() - # Store the reference to the video frame, even tho the videos are not yet encoded - ep_dict[key] = [] - for i in range(num_frames): - ep_dict[key].append({"path": f"videos/{fname}", "timestamp": i / fps}) + if video: + tmp_imgs_dir = videos_dir / f"{key}_episode_{episode_index:06d}" + fname = f"{key}_episode_{episode_index:06d}.mp4" + video_path = local_dir / "videos" / fname + if video_path.exists(): + video_path.unlink() + # Store the reference to the video frame, even tho the videos are not yet encoded + ep_dict[key] = [] + for i in range(num_frames): + ep_dict[key].append({"path": f"videos/{fname}", "timestamp": i / fps}) + + else: + imgs_dir = videos_dir / f"{key}_episode_{episode_index:06d}" + ep_dict[key] = [] + for frame_index in range(num_frames): + img_path = imgs_dir / f"frame_{frame_index:06d}.png" + ep_dict[key].append(img_path) for key in not_image_keys: ep_dict[key] = torch.stack(ep_dict[key]) @@ -622,21 +627,22 @@ def on_press(key): num_episodes = episode_index - logging.info("Encoding videos") - say("Encoding videos") - # Use ffmpeg to convert frames stored as png into mp4 videos - for episode_index in tqdm.tqdm(range(num_episodes)): - for key in image_keys: - tmp_imgs_dir = videos_dir / f"{key}_episode_{episode_index:06d}" - fname = f"{key}_episode_{episode_index:06d}.mp4" - video_path = local_dir / "videos" / fname - if video_path.exists(): - # Skip if video is already encoded. Could be the case when resuming data recording. - continue - # note: `encode_video_frames` is a blocking call. Making it asynchronous shouldn't speedup encoding, - # since video encoding with ffmpeg is already using multithreading. - encode_video_frames(tmp_imgs_dir, video_path, fps, overwrite=True) - shutil.rmtree(tmp_imgs_dir) + if video: + logging.info("Encoding videos") + say("Encoding videos") + # Use ffmpeg to convert frames stored as png into mp4 videos + for episode_index in tqdm.tqdm(range(num_episodes)): + for key in image_keys: + tmp_imgs_dir = videos_dir / f"{key}_episode_{episode_index:06d}" + fname = f"{key}_episode_{episode_index:06d}.mp4" + video_path = local_dir / "videos" / fname + if video_path.exists(): + # Skip if video is already encoded. Could be the case when resuming data recording. + continue + # note: `encode_video_frames` is a blocking call. Making it asynchronous shouldn't speedup encoding, + # since video encoding with ffmpeg is already using multithreading. + encode_video_frames(tmp_imgs_dir, video_path, fps, overwrite=True) + shutil.rmtree(tmp_imgs_dir) logging.info("Concatenating episodes") ep_dicts = [] diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index 14fe91585..857d809bb 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -69,6 +69,7 @@ def test_record_without_cameras(tmpdir, request, robot_type, mock): num_episodes=2, run_compute_stats=False, push_to_hub=False, + video=False, ) @@ -91,6 +92,7 @@ def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): episode_time_s=1, num_episodes=2, push_to_hub=False, + video=False, ) replay(robot, episode=0, fps=30, root=root, repo_id=repo_id) @@ -106,6 +108,15 @@ def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): policy = make_policy(hydra_cfg=cfg, dataset_stats=dataset.stats) - record(robot, policy, cfg, warmup_time_s=1, episode_time_s=1, run_compute_stats=False, push_to_hub=False) + record( + robot, + policy, + cfg, + warmup_time_s=1, + episode_time_s=1, + run_compute_stats=False, + push_to_hub=False, + video=False, + ) del robot From f2b1842d6929fa6ca981a3a0d3a3ad3136011aa3 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 11:48:22 +0200 Subject: [PATCH 21/53] fix unit test --- lerobot/scripts/control_robot.py | 6 +++--- tests/test_control_robot.py | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lerobot/scripts/control_robot.py b/lerobot/scripts/control_robot.py index 995b19c9e..c07602a19 100644 --- a/lerobot/scripts/control_robot.py +++ b/lerobot/scripts/control_robot.py @@ -561,9 +561,9 @@ def on_press(key): else: imgs_dir = videos_dir / f"{key}_episode_{episode_index:06d}" ep_dict[key] = [] - for frame_index in range(num_frames): - img_path = imgs_dir / f"frame_{frame_index:06d}.png" - ep_dict[key].append(img_path) + for i in range(num_frames): + img_path = imgs_dir / f"frame_{i:06d}.png" + ep_dict[key].append({"path": str(img_path)}) for key in not_image_keys: ep_dict[key] = torch.stack(ep_dict[key]) diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index 857d809bb..2b35e7984 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -114,6 +114,7 @@ def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): cfg, warmup_time_s=1, episode_time_s=1, + num_episodes=2, run_compute_stats=False, push_to_hub=False, video=False, From 3cb85bcd4bace7402b7905fb6f3055be1b32b0f7 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 13:09:08 +0200 Subject: [PATCH 22/53] Fix unit test --- tests/test_control_robot.py | 1 + tests/test_motors.py | 24 +++++++++++++++++------- tests/utils.py | 10 ++++++---- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index 2b35e7984..efb324198 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -92,6 +92,7 @@ def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): episode_time_s=1, num_episodes=2, push_to_hub=False, + # TODO(rcadene, aliberts): test video=True video=False, ) diff --git a/tests/test_motors.py b/tests/test_motors.py index cb416989f..103803302 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -30,10 +30,9 @@ import numpy as np import pytest -from lerobot import available_motors from lerobot.common.robot_devices.motors.utils import MotorsBus from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_MOTOR_TYPES, require_motor +from tests.utils import TEST_MOTOR_TYPES, mock_input, require_motor DYNAMIXEL_PORT = "/dev/tty.usbmodem575E0032081" DYNAMIXEL_MOTORS = { @@ -58,19 +57,30 @@ def make_motors_bus(motor_type: str, **kwargs) -> MotorsBus: raise ValueError(f"The motor type '{motor_type}' is not valid.") -# TODO(rcadene): implement mocked version of this test -@pytest.mark.parametrize("motor_type, mock", [(m, False) for m in available_motors]) +@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) @require_motor def test_find_port(request, motor_type, mock): from lerobot.common.robot_devices.motors.dynamixel import find_port - find_port() + if mock: + # To run find_port without user input + monkeypatch = request.getfixturevalue("monkeypatch") + monkeypatch.setattr("builtins.input", mock_input) + + with pytest.raises(OSError): + find_port() + else: + find_port() -# TODO(rcadene): implement mocked version of this test -@pytest.mark.parametrize("motor_type, mock", [(m, False) for m in available_motors]) +@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) @require_motor def test_configure_motors_all_ids_1(request, motor_type, mock): + if mock: + # To run find_port without user input + monkeypatch = request.getfixturevalue("monkeypatch") + monkeypatch.setattr("builtins.input", mock_input) + input("Are you sure you want to re-configure the motors? Press enter to continue...") # This test expect the configuration was already correct. motors_bus = make_motors_bus(motor_type) diff --git a/tests/utils.py b/tests/utils.py index aeafd9d19..dd2c904e1 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -213,11 +213,8 @@ def wrapper(*args, **kwargs): mock_cameras(request, camera_type) mock_motors(request) - def mock_input(text): - print(text) - - monkeypatch = request.getfixturevalue("monkeypatch") # To run calibration without user input + monkeypatch = request.getfixturevalue("monkeypatch") monkeypatch.setattr("builtins.input", mock_input) # Run test with a real robot. Skip test if robot connection fails. @@ -301,6 +298,11 @@ def wrapper(*args, **kwargs): return wrapper +def mock_input(text=None): + if text is not None: + print(text) + + def mock_cameras(request, camera_type="all"): # TODO(rcadene): Redesign the mocking tests monkeypatch = request.getfixturevalue("monkeypatch") From a236382590111c69bde718e3773037c3530de733 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 13:19:29 +0200 Subject: [PATCH 23/53] fix unit tests --- tests/test_robots.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/test_robots.py b/tests/test_robots.py index 8bc8205df..ebba8d386 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -50,9 +50,15 @@ def test_robot(tmpdir, request, robot_type, mock): # TODO(rcadene): add compatibility with other robots from lerobot.common.robot_devices.robots.manipulator import ManipulatorRobot - # Save calibration preset - tmpdir = Path(tmpdir) - calibration_dir = tmpdir / robot_type + if robot_type == "aloha" and mock: + # To simplify unit test, we do not rerun manual calibration for Aloha mock=True. + # Instead, we use the files from '.cache/calibration/aloha_default' + overrides_calibration_dir = None + else: + # Create an empty calibration directory to trigger manual calibration + tmpdir = Path(tmpdir) + calibration_dir = tmpdir / robot_type + overrides_calibration_dir = [f"calibration_dir={calibration_dir}"] # Test connecting without devices raises an error robot = ManipulatorRobot() @@ -76,9 +82,9 @@ def test_robot(tmpdir, request, robot_type, mock): # Test deleting the object without connecting first del robot - # Test connecting - robot = make_robot(robot_type, overrides=[f"calibration_dir={calibration_dir}"]) - robot.connect() # run the manual calibration precedure + # Test connecting (triggers manual calibration) + robot = make_robot(robot_type, overrides=overrides_calibration_dir) + robot.connect() assert robot.is_connected # Test connecting twice raises an error @@ -89,8 +95,9 @@ def test_robot(tmpdir, request, robot_type, mock): del robot # Test teleop can run - robot = make_robot(robot_type, overrides=[f"calibration_dir={calibration_dir}"]) - robot.calibration_dir = calibration_dir + robot = make_robot(robot_type, overrides=overrides_calibration_dir) + if overrides_calibration_dir is not None: + robot.calibration_dir = calibration_dir robot.connect() robot.teleop_step() From 8b362238323dd175a286d460d5740052cd808788 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 13:51:45 +0200 Subject: [PATCH 24/53] fix unit tests --- tests/conftest.py | 50 +------------------ tests/test_cameras.py | 25 +--------- tests/test_motors.py | 25 +--------- tests/test_robots.py | 12 +---- tests/utils.py | 111 ++++++++++++++++++++++++++++++++++++++---- 5 files changed, 106 insertions(+), 117 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8c754cb3f..ca8b75afd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,58 +13,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import traceback -import pytest -from lerobot.common.utils.utils import init_hydra_config -from tests.test_cameras import make_camera -from tests.test_motors import make_motors_bus -from tests.utils import DEVICE, ROBOT_CONFIG_PATH_TEMPLATE +from tests.utils import DEVICE def pytest_collection_finish(): print(f"\nTesting with {DEVICE=}") - - -@pytest.fixture -def is_robot_available(robot_type): - try: - from lerobot.common.robot_devices.robots.factory import make_robot - - config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) - robot_cfg = init_hydra_config(config_path) - robot = make_robot(robot_cfg) - robot.connect() - del robot - return True - except Exception: - traceback.print_exc() - print(f"\nA {robot_type} robot is not available.") - return False - - -@pytest.fixture -def is_camera_available(camera_type): - try: - camera = make_camera(camera_type) - camera.connect() - del camera - return True - except Exception: - traceback.print_exc() - print(f"\nA {camera_type} camera is not available.") - return False - - -@pytest.fixture -def is_motor_available(motor_type): - try: - motors_bus = make_motors_bus(motor_type) - motors_bus.connect() - del motors_bus - return True - except Exception: - traceback.print_exc() - print(f"\nA {motor_type} motor is not available.") - return False diff --git a/tests/test_cameras.py b/tests/test_cameras.py index a5d30f6ca..a2f2a42c6 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -21,17 +21,11 @@ ``` """ -import os - import numpy as np import pytest from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_CAMERA_TYPES, require_camera - -# Camera indices used for connecting physical cameras -OPENCV_CAMERA_INDEX = int(os.environ.get("LEROBOT_TEST_OPENCV_CAMERA_INDEX", 0)) -INTELREALSENSE_CAMERA_INDEX = int(os.environ.get("LEROBOT_TEST_INTELREALSENSE_CAMERA_INDEX", 128422271614)) +from tests.utils import TEST_CAMERA_TYPES, make_camera, require_camera # Maximum absolute difference between two consecutive images recored by a camera. # This value differs with respect to the camera. @@ -42,23 +36,6 @@ def compute_max_pixel_difference(first_image, second_image): return np.abs(first_image.astype(float) - second_image.astype(float)).max() -def make_camera(camera_type, **kwargs): - if camera_type == "opencv": - from lerobot.common.robot_devices.cameras.opencv import OpenCVCamera - - camera_index = kwargs.pop("camera_index", OPENCV_CAMERA_INDEX) - return OpenCVCamera(camera_index, **kwargs) - - elif camera_type == "intelrealsense": - from lerobot.common.robot_devices.cameras.intelrealsense import IntelRealSenseCamera - - camera_index = kwargs.pop("camera_index", INTELREALSENSE_CAMERA_INDEX) - return IntelRealSenseCamera(camera_index, **kwargs) - - else: - raise ValueError(f"The camera type '{camera_type}' is not valid.") - - @pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) @require_camera def test_camera(request, camera_type, mock): diff --git a/tests/test_motors.py b/tests/test_motors.py index 103803302..672ecc6f5 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -30,31 +30,8 @@ import numpy as np import pytest -from lerobot.common.robot_devices.motors.utils import MotorsBus from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_MOTOR_TYPES, mock_input, require_motor - -DYNAMIXEL_PORT = "/dev/tty.usbmodem575E0032081" -DYNAMIXEL_MOTORS = { - "shoulder_pan": [1, "xl430-w250"], - "shoulder_lift": [2, "xl430-w250"], - "elbow_flex": [3, "xl330-m288"], - "wrist_flex": [4, "xl330-m288"], - "wrist_roll": [5, "xl330-m288"], - "gripper": [6, "xl330-m288"], -} - - -def make_motors_bus(motor_type: str, **kwargs) -> MotorsBus: - if motor_type == "dynamixel": - from lerobot.common.robot_devices.motors.dynamixel import DynamixelMotorsBus - - port = kwargs.pop("port", DYNAMIXEL_PORT) - motors = kwargs.pop("motors", DYNAMIXEL_MOTORS) - return DynamixelMotorsBus(port, motors, **kwargs) - - else: - raise ValueError(f"The motor type '{motor_type}' is not valid.") +from tests.utils import TEST_MOTOR_TYPES, make_motors_bus, mock_input, require_motor @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) diff --git a/tests/test_robots.py b/tests/test_robots.py index ebba8d386..cd49049f0 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -28,18 +28,8 @@ import pytest import torch -from lerobot.common.robot_devices.robots.factory import make_robot as make_robot_from_cfg -from lerobot.common.robot_devices.robots.utils import Robot from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from lerobot.common.utils.utils import init_hydra_config -from tests.utils import ROBOT_CONFIG_PATH_TEMPLATE, TEST_ROBOT_TYPES, require_robot - - -def make_robot(robot_type: str, overrides: list[str] | None = None) -> Robot: - config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) - robot_cfg = init_hydra_config(config_path, overrides) - robot = make_robot_from_cfg(robot_cfg) - return robot +from tests.utils import TEST_ROBOT_TYPES, make_robot, require_robot @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) diff --git a/tests/utils.py b/tests/utils.py index dd2c904e1..cff1970e1 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -13,6 +13,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import os import platform import traceback from functools import wraps @@ -21,7 +22,12 @@ import torch from lerobot import available_cameras, available_motors, available_robots +from lerobot.common.robot_devices.cameras.utils import Camera +from lerobot.common.robot_devices.motors.utils import MotorsBus +from lerobot.common.robot_devices.robots.factory import make_robot as make_robot_from_cfg +from lerobot.common.robot_devices.robots.utils import Robot from lerobot.common.utils.import_utils import is_package_available +from lerobot.common.utils.utils import init_hydra_config DEVICE = "cuda" if torch.cuda.is_available() else "cpu" @@ -42,6 +48,20 @@ for motor_type in available_motors: TEST_MOTOR_TYPES += [(motor_type, True), (motor_type, False)] +# Camera indices used for connecting physical cameras +OPENCV_CAMERA_INDEX = int(os.environ.get("LEROBOT_TEST_OPENCV_CAMERA_INDEX", 0)) +INTELREALSENSE_CAMERA_INDEX = int(os.environ.get("LEROBOT_TEST_INTELREALSENSE_CAMERA_INDEX", 128422271614)) + +DYNAMIXEL_PORT = "/dev/tty.usbmodem575E0032081" +DYNAMIXEL_MOTORS = { + "shoulder_pan": [1, "xl430-w250"], + "shoulder_lift": [2, "xl430-w250"], + "elbow_flex": [3, "xl330-m288"], + "wrist_flex": [4, "xl330-m288"], + "wrist_roll": [5, "xl330-m288"], + "gripper": [6, "xl330-m288"], +} + def require_x86_64_kernel(func): """ @@ -184,7 +204,7 @@ def test_require_robot(request, robot_type): @wraps(func) def wrapper(*args, **kwargs): - # Access the pytest request context to get the is_robot_available fixture + # Access the pytest request context to get the mockeypatch fixture request = kwargs.get("request") robot_type = kwargs.get("robot_type") mock = kwargs.get("mock") @@ -219,8 +239,7 @@ def wrapper(*args, **kwargs): # Run test with a real robot. Skip test if robot connection fails. else: - # `is_robot_available` is defined in `tests/conftest.py` - if not request.getfixturevalue("is_robot_available"): + if not is_robot_available(robot_type): pytest.skip(f"A {robot_type} robot is not available.") return func(*args, **kwargs) @@ -231,7 +250,7 @@ def wrapper(*args, **kwargs): def require_camera(func): @wraps(func) def wrapper(*args, **kwargs): - # Access the pytest request context to get the is_camera_available fixture + # Access the pytest request context to get the mockeypatch fixture request = kwargs.get("request") camera_type = kwargs.get("camera_type") mock = kwargs.get("mock") @@ -254,8 +273,7 @@ def wrapper(*args, **kwargs): # Run test with a real robot. Skip test if robot connection fails. else: - # `is_camera_available` is defined in `tests/conftest.py` - if not request.getfixturevalue("is_camera_available"): + if not is_camera_available(camera_type): pytest.skip(f"A {camera_type} camera is not available.") return func(*args, **kwargs) @@ -266,7 +284,7 @@ def wrapper(*args, **kwargs): def require_motor(func): @wraps(func) def wrapper(*args, **kwargs): - # Access the pytest request context to get the is_motor_available fixture + # Access the pytest request context to get the mockeypatch fixture request = kwargs.get("request") motor_type = kwargs.get("motor_type") mock = kwargs.get("mock") @@ -289,8 +307,7 @@ def wrapper(*args, **kwargs): # Run test with a real robot. Skip test if robot connection fails. else: - # `is_motor_available` is defined in `tests/conftest.py` - if not request.getfixturevalue("is_motor_available"): + if not is_motor_available(motor_type): pytest.skip(f"A {motor_type} motor is not available.") return func(*args, **kwargs) @@ -371,3 +388,79 @@ def mock_motors(request): except ImportError: traceback.print_exc() pytest.skip("To avoid skipping tests mocking dynamixel motors, run `pip install dynamixel-sdk`.") + + +def make_robot(robot_type: str, overrides: list[str] | None = None) -> Robot: + config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) + robot_cfg = init_hydra_config(config_path, overrides) + robot = make_robot_from_cfg(robot_cfg) + return robot + + +def make_camera(camera_type, **kwargs) -> Camera: + if camera_type == "opencv": + from lerobot.common.robot_devices.cameras.opencv import OpenCVCamera + + camera_index = kwargs.pop("camera_index", OPENCV_CAMERA_INDEX) + return OpenCVCamera(camera_index, **kwargs) + + elif camera_type == "intelrealsense": + from lerobot.common.robot_devices.cameras.intelrealsense import IntelRealSenseCamera + + camera_index = kwargs.pop("camera_index", INTELREALSENSE_CAMERA_INDEX) + return IntelRealSenseCamera(camera_index, **kwargs) + + else: + raise ValueError(f"The camera type '{camera_type}' is not valid.") + + +def make_motors_bus(motor_type: str, **kwargs) -> MotorsBus: + if motor_type == "dynamixel": + from lerobot.common.robot_devices.motors.dynamixel import DynamixelMotorsBus + + port = kwargs.pop("port", DYNAMIXEL_PORT) + motors = kwargs.pop("motors", DYNAMIXEL_MOTORS) + return DynamixelMotorsBus(port, motors, **kwargs) + + else: + raise ValueError(f"The motor type '{motor_type}' is not valid.") + + +def is_robot_available(robot_type): + try: + from lerobot.common.robot_devices.robots.factory import make_robot + + config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) + robot_cfg = init_hydra_config(config_path) + robot = make_robot(robot_cfg) + robot.connect() + del robot + return True + except Exception: + traceback.print_exc() + print(f"\nA {robot_type} robot is not available.") + return False + + +def is_camera_available(camera_type): + try: + camera = make_camera(camera_type) + camera.connect() + del camera + return True + except Exception: + traceback.print_exc() + print(f"\nA {camera_type} camera is not available.") + return False + + +def is_motor_available(motor_type): + try: + motors_bus = make_motors_bus(motor_type) + motors_bus.connect() + del motors_bus + return True + except Exception: + traceback.print_exc() + print(f"\nA {motor_type} motor is not available.") + return False From b6b7fda5f88fdcc90c02699856b5749fb906e29b Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 13:53:31 +0200 Subject: [PATCH 25/53] custom pytest speedup (TOREMOVE) --- .github/workflows/test.yml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 038b44582..a123eb658 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -54,7 +54,31 @@ jobs: - name: Test with pytest run: | - pytest tests -v --cov=./lerobot --durations=0 \ + pytest tests/test_motors.py -v --cov=./lerobot --durations=0 \ + -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ + -W ignore::UserWarning:torch.utils.data.dataloader:558 \ + -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ + && rm -rf tests/outputs outputs + + - name: Test with pytest + run: | + pytest tests/test_cameras.py -v --cov=./lerobot --durations=0 \ + -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ + -W ignore::UserWarning:torch.utils.data.dataloader:558 \ + -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ + && rm -rf tests/outputs outputs + + - name: Test with pytest + run: | + pytest tests/test_robots.py -v --cov=./lerobot --durations=0 \ + -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ + -W ignore::UserWarning:torch.utils.data.dataloader:558 \ + -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ + && rm -rf tests/outputs outputs + + - name: Test with pytest + run: | + pytest tests/test_control_robot.py -v --cov=./lerobot --durations=0 \ -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ -W ignore::UserWarning:torch.utils.data.dataloader:558 \ -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ From 8a7b5c45c71856366d17ddfab4c0abdd77aa6089 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 14:35:17 +0200 Subject: [PATCH 26/53] Remove @require_x --- tests/test_cameras.py | 16 ++-- tests/test_control_robot.py | 27 ++++-- tests/test_motors.py | 24 ++++-- tests/test_robots.py | 7 +- tests/utils.py | 166 +++++++++--------------------------- 5 files changed, 90 insertions(+), 150 deletions(-) diff --git a/tests/test_cameras.py b/tests/test_cameras.py index a2f2a42c6..7b9d04eb2 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -25,7 +25,11 @@ import pytest from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_CAMERA_TYPES, make_camera, require_camera +from tests.utils import ( + TEST_CAMERA_TYPES, + make_camera, + mock_camera_or_skip_test_when_not_available, +) # Maximum absolute difference between two consecutive images recored by a camera. # This value differs with respect to the camera. @@ -37,8 +41,7 @@ def compute_max_pixel_difference(first_image, second_image): @pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) -@require_camera -def test_camera(request, camera_type, mock): +def test_camera(monkeypatch, camera_type, mock): """Test assumes that `camera.read()` returns the same image when called multiple times in a row. So the environment should not change (you shouldnt be in front of the camera) and the camera should not be moving. @@ -48,6 +51,8 @@ def test_camera(request, camera_type, mock): # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs + mock_camera_or_skip_test_when_not_available(monkeypatch, camera_type, mock) + # Test instantiating camera = make_camera(camera_type) @@ -141,9 +146,10 @@ def test_camera(request, camera_type, mock): @pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) -@require_camera -def test_save_images_from_cameras(tmpdir, request, camera_type, mock): +def test_save_images_from_cameras(tmpdir, monkeypatch, camera_type, mock): # TODO(rcadene): refactor + mock_camera_or_skip_test_when_not_available(monkeypatch, camera_type, mock) + if camera_type == "opencv": from lerobot.common.robot_devices.cameras.opencv import save_images_from_cameras elif camera_type == "intelrealsense": diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index efb324198..6bec8d032 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -31,12 +31,18 @@ from lerobot.common.utils.utils import init_hydra_config from lerobot.scripts.control_robot import calibrate, get_available_arms, record, replay, teleoperate from tests.test_robots import make_robot -from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_ROBOT_TYPES, require_robot +from tests.utils import ( + DEFAULT_CONFIG_PATH, + DEVICE, + TEST_ROBOT_TYPES, + mock_robot_or_skip_test_when_not_available, +) @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_teleoperate(request, robot_type, mock): +def test_teleoperate(monkeypatch, robot_type, mock): + mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) + robot = make_robot(robot_type) teleoperate(robot, teleop_time_s=1) teleoperate(robot, fps=30, teleop_time_s=1) @@ -45,16 +51,18 @@ def test_teleoperate(request, robot_type, mock): @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_calibrate(request, robot_type, mock): +def test_calibrate(monkeypatch, robot_type, mock): + mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) + robot = make_robot(robot_type) calibrate(robot, arms=get_available_arms(robot)) del robot @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_record_without_cameras(tmpdir, request, robot_type, mock): +def test_record_without_cameras(tmpdir, monkeypatch, robot_type, mock): + mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) + root = Path(tmpdir) repo_id = "lerobot/debug" @@ -74,8 +82,9 @@ def test_record_without_cameras(tmpdir, request, robot_type, mock): @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): +def test_record_and_replay_and_policy(tmpdir, monkeypatch, robot_type, mock): + mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) + env_name = "koch_real" policy_name = "act_koch_real" diff --git a/tests/test_motors.py b/tests/test_motors.py index 672ecc6f5..3a034a613 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -31,17 +31,22 @@ import pytest from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_MOTOR_TYPES, make_motors_bus, mock_input, require_motor +from tests.utils import ( + TEST_MOTOR_TYPES, + make_motors_bus, + mock_input, + mock_motor_or_skip_test_when_not_available, +) @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -@require_motor -def test_find_port(request, motor_type, mock): +def test_find_port(monkeypatch, motor_type, mock): + mock_motor_or_skip_test_when_not_available(monkeypatch, motor_type, mock) + from lerobot.common.robot_devices.motors.dynamixel import find_port if mock: # To run find_port without user input - monkeypatch = request.getfixturevalue("monkeypatch") monkeypatch.setattr("builtins.input", mock_input) with pytest.raises(OSError): @@ -51,11 +56,11 @@ def test_find_port(request, motor_type, mock): @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -@require_motor -def test_configure_motors_all_ids_1(request, motor_type, mock): +def test_configure_motors_all_ids_1(monkeypatch, motor_type, mock): + mock_motor_or_skip_test_when_not_available(monkeypatch, motor_type, mock) + if mock: # To run find_port without user input - monkeypatch = request.getfixturevalue("monkeypatch") monkeypatch.setattr("builtins.input", mock_input) input("Are you sure you want to re-configure the motors? Press enter to continue...") @@ -75,8 +80,9 @@ def test_configure_motors_all_ids_1(request, motor_type, mock): @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -@require_motor -def test_motors_bus(request, motor_type, mock): +def test_motors_bus(monkeypatch, motor_type, mock): + mock_motor_or_skip_test_when_not_available(monkeypatch, motor_type, mock) + motors_bus = make_motors_bus(motor_type) # Test reading and writting before connecting raises an error diff --git a/tests/test_robots.py b/tests/test_robots.py index cd49049f0..be650616e 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -29,15 +29,16 @@ import torch from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_ROBOT_TYPES, make_robot, require_robot +from tests.utils import TEST_ROBOT_TYPES, make_robot, mock_robot_or_skip_test_when_not_available @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_robot(tmpdir, request, robot_type, mock): +def test_robot(tmpdir, monkeypatch, robot_type, mock): # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs # TODO(rcadene): add compatibility with other robots + mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) + from lerobot.common.robot_devices.robots.manipulator import ManipulatorRobot if robot_type == "aloha" and mock: diff --git a/tests/utils.py b/tests/utils.py index cff1970e1..ea10edbd0 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -185,134 +185,52 @@ def wrapper(*args, **kwargs): return decorator -def require_robot(func): - """ - Decorator that skips the test if a robot is not available - - The decorated function must have two arguments `request` and `robot_type`. - - Example of usage: - ```python - @pytest.mark.parametrize( - "robot_type", ["koch", "aloha"] - ) - @require_robot - def test_require_robot(request, robot_type): - pass - ``` - """ - - @wraps(func) - def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture - request = kwargs.get("request") - robot_type = kwargs.get("robot_type") - mock = kwargs.get("mock") - - if robot_type is None: - raise ValueError("The 'robot_type' must be an argument of the test function.") - if request is None: - raise ValueError("The 'request' fixture must be an argument of the test function.") - if mock is None: - raise ValueError("The 'mock' variable must be an argument of the test function.") - - if robot_type not in available_robots: - raise ValueError( - f"The camera type '{robot_type}' is not valid. Expected one of these '{available_robots}" - ) - +def mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock): + if mock: # Run test with a monkeypatched version of the robot devices. - if mock: - # TODO(rcadene): redesign mocking to not have this hardcoded logic - if robot_type in ["koch", "koch_bimanual"]: - camera_type = "opencv" - elif robot_type == "aloha": - camera_type = "intelrealsense" - else: - camera_type = "all" - mock_cameras(request, camera_type) - mock_motors(request) - - # To run calibration without user input - monkeypatch = request.getfixturevalue("monkeypatch") - monkeypatch.setattr("builtins.input", mock_input) - - # Run test with a real robot. Skip test if robot connection fails. + # TODO(rcadene): redesign mocking to not have this hardcoded logic + if robot_type in ["koch", "koch_bimanual"]: + camera_type = "opencv" + elif robot_type == "aloha": + camera_type = "intelrealsense" else: - if not is_robot_available(robot_type): - pytest.skip(f"A {robot_type} robot is not available.") + camera_type = "all" - return func(*args, **kwargs) - - return wrapper + mock_cameras(monkeypatch, camera_type) + mock_motors(monkeypatch) + # To run calibration without user input + monkeypatch.setattr("builtins.input", mock_input) -def require_camera(func): - @wraps(func) - def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture - request = kwargs.get("request") - camera_type = kwargs.get("camera_type") - mock = kwargs.get("mock") - - if request is None: - raise ValueError("The 'request' fixture must be an argument of the test function.") - if camera_type is None: - raise ValueError("The 'camera_type' must be an argument of the test function.") - if mock is None: - raise ValueError("The 'mock' variable must be an argument of the test function.") - - if camera_type not in available_cameras: - raise ValueError( - f"The camera type '{camera_type}' is not valid. Expected one of these '{available_cameras}" - ) - - # Run test with a monkeypatched version of the robot devices. - if mock: - mock_cameras(request, camera_type) - + elif not is_robot_available(robot_type): # Run test with a real robot. Skip test if robot connection fails. - else: - if not is_camera_available(camera_type): - pytest.skip(f"A {camera_type} camera is not available.") + pytest.skip(f"A {robot_type} robot is not available.") - return func(*args, **kwargs) - - return wrapper - - -def require_motor(func): - @wraps(func) - def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture - request = kwargs.get("request") - motor_type = kwargs.get("motor_type") - mock = kwargs.get("mock") - - if request is None: - raise ValueError("The 'request' fixture must be an argument of the test function.") - if motor_type is None: - raise ValueError("The 'motor_type' must be an argument of the test function.") - if mock is None: - raise ValueError("The 'mock' variable must be an argument of the test function.") - - if motor_type not in available_motors: - raise ValueError( - f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" - ) - - # Run test with a monkeypatched version of the robot devices. - if mock: - mock_motors(request) - # Run test with a real robot. Skip test if robot connection fails. - else: - if not is_motor_available(motor_type): - pytest.skip(f"A {motor_type} motor is not available.") - - return func(*args, **kwargs) - - return wrapper +def mock_camera_or_skip_test_when_not_available(monkeypatch, camera_type, mock): + if camera_type not in available_cameras: + raise ValueError( + f"The camera type '{camera_type}' is not valid. Expected one of these '{available_cameras}" + ) + if mock: + # Run test with a monkeypatched version of the cameras + mock_cameras(monkeypatch, camera_type) + elif not is_camera_available(camera_type): + # Run test with a real camera. Skip test if camera connection fails + pytest.skip(f"A {camera_type} camera is not available.") + + +def mock_motor_or_skip_test_when_not_available(monkeypatch, motor_type, mock): + if motor_type not in available_motors: + raise ValueError( + f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" + ) + if mock: + # Run test with a monkeypatched version of the motors + mock_motors(monkeypatch) + elif not is_motor_available(motor_type): + # Run test with a real motor. Skip test if motor connection fails + pytest.skip(f"A {motor_type} motor is not available.") def mock_input(text=None): @@ -320,9 +238,8 @@ def mock_input(text=None): print(text) -def mock_cameras(request, camera_type="all"): +def mock_cameras(monkeypatch, camera_type="all"): # TODO(rcadene): Redesign the mocking tests - monkeypatch = request.getfixturevalue("monkeypatch") if camera_type in ["opencv", "all"]: try: @@ -359,9 +276,8 @@ def mock_cameras(request, camera_type="all"): ) -def mock_motors(request): +def mock_motors(monkeypatch): # TODO(rcadene): Redesign the mocking tests - monkeypatch = request.getfixturevalue("monkeypatch") try: import dynamixel_sdk @@ -433,6 +349,8 @@ def is_robot_available(robot_type): config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) robot_cfg = init_hydra_config(config_path) robot = make_robot(robot_cfg) + print("DEBUG", robot.leader_arms) + print("DEBUG", robot.cameras) robot.connect() del robot return True From 395720a5de47fcd8b08e29e5a67bc9d2e0630907 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 14:35:26 +0200 Subject: [PATCH 27/53] Revert "Remove @require_x" This reverts commit 8a7b5c45c71856366d17ddfab4c0abdd77aa6089. --- tests/test_cameras.py | 16 ++-- tests/test_control_robot.py | 27 ++---- tests/test_motors.py | 24 ++---- tests/test_robots.py | 7 +- tests/utils.py | 166 +++++++++++++++++++++++++++--------- 5 files changed, 150 insertions(+), 90 deletions(-) diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 7b9d04eb2..a2f2a42c6 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -25,11 +25,7 @@ import pytest from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import ( - TEST_CAMERA_TYPES, - make_camera, - mock_camera_or_skip_test_when_not_available, -) +from tests.utils import TEST_CAMERA_TYPES, make_camera, require_camera # Maximum absolute difference between two consecutive images recored by a camera. # This value differs with respect to the camera. @@ -41,7 +37,8 @@ def compute_max_pixel_difference(first_image, second_image): @pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) -def test_camera(monkeypatch, camera_type, mock): +@require_camera +def test_camera(request, camera_type, mock): """Test assumes that `camera.read()` returns the same image when called multiple times in a row. So the environment should not change (you shouldnt be in front of the camera) and the camera should not be moving. @@ -51,8 +48,6 @@ def test_camera(monkeypatch, camera_type, mock): # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs - mock_camera_or_skip_test_when_not_available(monkeypatch, camera_type, mock) - # Test instantiating camera = make_camera(camera_type) @@ -146,10 +141,9 @@ def test_camera(monkeypatch, camera_type, mock): @pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) -def test_save_images_from_cameras(tmpdir, monkeypatch, camera_type, mock): +@require_camera +def test_save_images_from_cameras(tmpdir, request, camera_type, mock): # TODO(rcadene): refactor - mock_camera_or_skip_test_when_not_available(monkeypatch, camera_type, mock) - if camera_type == "opencv": from lerobot.common.robot_devices.cameras.opencv import save_images_from_cameras elif camera_type == "intelrealsense": diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index 6bec8d032..efb324198 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -31,18 +31,12 @@ from lerobot.common.utils.utils import init_hydra_config from lerobot.scripts.control_robot import calibrate, get_available_arms, record, replay, teleoperate from tests.test_robots import make_robot -from tests.utils import ( - DEFAULT_CONFIG_PATH, - DEVICE, - TEST_ROBOT_TYPES, - mock_robot_or_skip_test_when_not_available, -) +from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_ROBOT_TYPES, require_robot @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -def test_teleoperate(monkeypatch, robot_type, mock): - mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) - +@require_robot +def test_teleoperate(request, robot_type, mock): robot = make_robot(robot_type) teleoperate(robot, teleop_time_s=1) teleoperate(robot, fps=30, teleop_time_s=1) @@ -51,18 +45,16 @@ def test_teleoperate(monkeypatch, robot_type, mock): @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -def test_calibrate(monkeypatch, robot_type, mock): - mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) - +@require_robot +def test_calibrate(request, robot_type, mock): robot = make_robot(robot_type) calibrate(robot, arms=get_available_arms(robot)) del robot @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -def test_record_without_cameras(tmpdir, monkeypatch, robot_type, mock): - mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) - +@require_robot +def test_record_without_cameras(tmpdir, request, robot_type, mock): root = Path(tmpdir) repo_id = "lerobot/debug" @@ -82,9 +74,8 @@ def test_record_without_cameras(tmpdir, monkeypatch, robot_type, mock): @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -def test_record_and_replay_and_policy(tmpdir, monkeypatch, robot_type, mock): - mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) - +@require_robot +def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): env_name = "koch_real" policy_name = "act_koch_real" diff --git a/tests/test_motors.py b/tests/test_motors.py index 3a034a613..672ecc6f5 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -31,22 +31,17 @@ import pytest from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import ( - TEST_MOTOR_TYPES, - make_motors_bus, - mock_input, - mock_motor_or_skip_test_when_not_available, -) +from tests.utils import TEST_MOTOR_TYPES, make_motors_bus, mock_input, require_motor @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -def test_find_port(monkeypatch, motor_type, mock): - mock_motor_or_skip_test_when_not_available(monkeypatch, motor_type, mock) - +@require_motor +def test_find_port(request, motor_type, mock): from lerobot.common.robot_devices.motors.dynamixel import find_port if mock: # To run find_port without user input + monkeypatch = request.getfixturevalue("monkeypatch") monkeypatch.setattr("builtins.input", mock_input) with pytest.raises(OSError): @@ -56,11 +51,11 @@ def test_find_port(monkeypatch, motor_type, mock): @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -def test_configure_motors_all_ids_1(monkeypatch, motor_type, mock): - mock_motor_or_skip_test_when_not_available(monkeypatch, motor_type, mock) - +@require_motor +def test_configure_motors_all_ids_1(request, motor_type, mock): if mock: # To run find_port without user input + monkeypatch = request.getfixturevalue("monkeypatch") monkeypatch.setattr("builtins.input", mock_input) input("Are you sure you want to re-configure the motors? Press enter to continue...") @@ -80,9 +75,8 @@ def test_configure_motors_all_ids_1(monkeypatch, motor_type, mock): @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -def test_motors_bus(monkeypatch, motor_type, mock): - mock_motor_or_skip_test_when_not_available(monkeypatch, motor_type, mock) - +@require_motor +def test_motors_bus(request, motor_type, mock): motors_bus = make_motors_bus(motor_type) # Test reading and writting before connecting raises an error diff --git a/tests/test_robots.py b/tests/test_robots.py index be650616e..cd49049f0 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -29,16 +29,15 @@ import torch from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_ROBOT_TYPES, make_robot, mock_robot_or_skip_test_when_not_available +from tests.utils import TEST_ROBOT_TYPES, make_robot, require_robot @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -def test_robot(tmpdir, monkeypatch, robot_type, mock): +@require_robot +def test_robot(tmpdir, request, robot_type, mock): # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs # TODO(rcadene): add compatibility with other robots - mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock) - from lerobot.common.robot_devices.robots.manipulator import ManipulatorRobot if robot_type == "aloha" and mock: diff --git a/tests/utils.py b/tests/utils.py index ea10edbd0..cff1970e1 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -185,52 +185,134 @@ def wrapper(*args, **kwargs): return decorator -def mock_robot_or_skip_test_when_not_available(monkeypatch, robot_type, mock): - if mock: +def require_robot(func): + """ + Decorator that skips the test if a robot is not available + + The decorated function must have two arguments `request` and `robot_type`. + + Example of usage: + ```python + @pytest.mark.parametrize( + "robot_type", ["koch", "aloha"] + ) + @require_robot + def test_require_robot(request, robot_type): + pass + ``` + """ + + @wraps(func) + def wrapper(*args, **kwargs): + # Access the pytest request context to get the mockeypatch fixture + request = kwargs.get("request") + robot_type = kwargs.get("robot_type") + mock = kwargs.get("mock") + + if robot_type is None: + raise ValueError("The 'robot_type' must be an argument of the test function.") + if request is None: + raise ValueError("The 'request' fixture must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") + + if robot_type not in available_robots: + raise ValueError( + f"The camera type '{robot_type}' is not valid. Expected one of these '{available_robots}" + ) + # Run test with a monkeypatched version of the robot devices. - # TODO(rcadene): redesign mocking to not have this hardcoded logic - if robot_type in ["koch", "koch_bimanual"]: - camera_type = "opencv" - elif robot_type == "aloha": - camera_type = "intelrealsense" + if mock: + # TODO(rcadene): redesign mocking to not have this hardcoded logic + if robot_type in ["koch", "koch_bimanual"]: + camera_type = "opencv" + elif robot_type == "aloha": + camera_type = "intelrealsense" + else: + camera_type = "all" + mock_cameras(request, camera_type) + mock_motors(request) + + # To run calibration without user input + monkeypatch = request.getfixturevalue("monkeypatch") + monkeypatch.setattr("builtins.input", mock_input) + + # Run test with a real robot. Skip test if robot connection fails. else: - camera_type = "all" + if not is_robot_available(robot_type): + pytest.skip(f"A {robot_type} robot is not available.") - mock_cameras(monkeypatch, camera_type) - mock_motors(monkeypatch) + return func(*args, **kwargs) + + return wrapper - # To run calibration without user input - monkeypatch.setattr("builtins.input", mock_input) - elif not is_robot_available(robot_type): +def require_camera(func): + @wraps(func) + def wrapper(*args, **kwargs): + # Access the pytest request context to get the mockeypatch fixture + request = kwargs.get("request") + camera_type = kwargs.get("camera_type") + mock = kwargs.get("mock") + + if request is None: + raise ValueError("The 'request' fixture must be an argument of the test function.") + if camera_type is None: + raise ValueError("The 'camera_type' must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") + + if camera_type not in available_cameras: + raise ValueError( + f"The camera type '{camera_type}' is not valid. Expected one of these '{available_cameras}" + ) + + # Run test with a monkeypatched version of the robot devices. + if mock: + mock_cameras(request, camera_type) + # Run test with a real robot. Skip test if robot connection fails. - pytest.skip(f"A {robot_type} robot is not available.") + else: + if not is_camera_available(camera_type): + pytest.skip(f"A {camera_type} camera is not available.") + return func(*args, **kwargs) -def mock_camera_or_skip_test_when_not_available(monkeypatch, camera_type, mock): - if camera_type not in available_cameras: - raise ValueError( - f"The camera type '{camera_type}' is not valid. Expected one of these '{available_cameras}" - ) - if mock: - # Run test with a monkeypatched version of the cameras - mock_cameras(monkeypatch, camera_type) - elif not is_camera_available(camera_type): - # Run test with a real camera. Skip test if camera connection fails - pytest.skip(f"A {camera_type} camera is not available.") - - -def mock_motor_or_skip_test_when_not_available(monkeypatch, motor_type, mock): - if motor_type not in available_motors: - raise ValueError( - f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" - ) - if mock: - # Run test with a monkeypatched version of the motors - mock_motors(monkeypatch) - elif not is_motor_available(motor_type): - # Run test with a real motor. Skip test if motor connection fails - pytest.skip(f"A {motor_type} motor is not available.") + return wrapper + + +def require_motor(func): + @wraps(func) + def wrapper(*args, **kwargs): + # Access the pytest request context to get the mockeypatch fixture + request = kwargs.get("request") + motor_type = kwargs.get("motor_type") + mock = kwargs.get("mock") + + if request is None: + raise ValueError("The 'request' fixture must be an argument of the test function.") + if motor_type is None: + raise ValueError("The 'motor_type' must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") + + if motor_type not in available_motors: + raise ValueError( + f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" + ) + + # Run test with a monkeypatched version of the robot devices. + if mock: + mock_motors(request) + + # Run test with a real robot. Skip test if robot connection fails. + else: + if not is_motor_available(motor_type): + pytest.skip(f"A {motor_type} motor is not available.") + + return func(*args, **kwargs) + + return wrapper def mock_input(text=None): @@ -238,8 +320,9 @@ def mock_input(text=None): print(text) -def mock_cameras(monkeypatch, camera_type="all"): +def mock_cameras(request, camera_type="all"): # TODO(rcadene): Redesign the mocking tests + monkeypatch = request.getfixturevalue("monkeypatch") if camera_type in ["opencv", "all"]: try: @@ -276,8 +359,9 @@ def mock_cameras(monkeypatch, camera_type="all"): ) -def mock_motors(monkeypatch): +def mock_motors(request): # TODO(rcadene): Redesign the mocking tests + monkeypatch = request.getfixturevalue("monkeypatch") try: import dynamixel_sdk @@ -349,8 +433,6 @@ def is_robot_available(robot_type): config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) robot_cfg = init_hydra_config(config_path) robot = make_robot(robot_cfg) - print("DEBUG", robot.leader_arms) - print("DEBUG", robot.cameras) robot.connect() del robot return True From 48be576cc6ca524cc3282f0ff009b824b025a709 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 16:28:08 +0200 Subject: [PATCH 28/53] fix unit tests --- tests/conftest.py | 65 ++++++++- tests/test_cameras.py | 35 ++++- tests/test_control_robot.py | 67 +++++++-- tests/test_motors.py | 75 ++++++---- tests/test_robots.py | 19 ++- tests/utils.py | 268 +++++++++++++++++++----------------- 6 files changed, 346 insertions(+), 183 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index ca8b75afd..ac1d0b76a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,9 +14,72 @@ # See the License for the specific language governing permissions and # limitations under the License. +import traceback -from tests.utils import DEVICE +import pytest + +from lerobot import available_cameras, available_motors, available_robots +from lerobot.common.utils.utils import init_hydra_config +from tests.utils import DEVICE, ROBOT_CONFIG_PATH_TEMPLATE, make_camera, make_motors_bus def pytest_collection_finish(): print(f"\nTesting with {DEVICE=}") + + +@pytest.fixture +def is_robot_available(robot_type): + if robot_type not in available_robots: + raise ValueError( + f"The robot type '{robot_type}' is not valid. Expected one of these '{available_robots}" + ) + + try: + from lerobot.common.robot_devices.robots.factory import make_robot + + config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) + robot_cfg = init_hydra_config(config_path) + robot = make_robot(robot_cfg) + robot.connect() + del robot + return True + except Exception: + traceback.print_exc() + print(f"\nA {robot_type} robot is not available.") + return False + + +@pytest.fixture +def is_camera_available(camera_type): + if camera_type not in available_cameras: + raise ValueError( + f"The camera type '{camera_type}' is not valid. Expected one of these '{available_cameras}" + ) + + try: + camera = make_camera(camera_type) + camera.connect() + del camera + return True + except Exception: + traceback.print_exc() + print(f"\nA {camera_type} camera is not available.") + return False + + +@pytest.fixture +def is_motor_available(motor_type): + if motor_type not in available_motors: + raise ValueError( + f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" + ) + + try: + motors_bus = make_motors_bus(motor_type) + motors_bus.connect() + del motors_bus + return True + except Exception: + traceback.print_exc() + print(f"\nA {motor_type} motor is not available.") + return False diff --git a/tests/test_cameras.py b/tests/test_cameras.py index a2f2a42c6..2adeb92e7 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -24,8 +24,9 @@ import numpy as np import pytest +from lerobot import available_cameras from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_CAMERA_TYPES, make_camera, require_camera +from tests.utils import make_camera, require_camera, require_mock_camera # Maximum absolute difference between two consecutive images recored by a camera. # This value differs with respect to the camera. @@ -36,9 +37,7 @@ def compute_max_pixel_difference(first_image, second_image): return np.abs(first_image.astype(float) - second_image.astype(float)).max() -@pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) -@require_camera -def test_camera(request, camera_type, mock): +def _test_camera(camera_type): """Test assumes that `camera.read()` returns the same image when called multiple times in a row. So the environment should not change (you shouldnt be in front of the camera) and the camera should not be moving. @@ -140,9 +139,7 @@ def test_camera(request, camera_type, mock): del camera -@pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) -@require_camera -def test_save_images_from_cameras(tmpdir, request, camera_type, mock): +def _test_save_images_from_cameras(tmpdir, camera_type): # TODO(rcadene): refactor if camera_type == "opencv": from lerobot.common.robot_devices.cameras.opencv import save_images_from_cameras @@ -150,3 +147,27 @@ def test_save_images_from_cameras(tmpdir, request, camera_type, mock): from lerobot.common.robot_devices.cameras.intelrealsense import save_images_from_cameras save_images_from_cameras(tmpdir, record_time_s=1) + + +@pytest.mark.parametrize("camera_type", available_cameras) +@require_mock_camera +def test_camera_mock(monkeypatch, camera_type): + _test_camera(camera_type) + + +@pytest.mark.parametrize("camera_type", available_cameras) +@require_camera +def test_camera(request, camera_type): + _test_camera(camera_type) + + +@pytest.mark.parametrize("camera_type", available_cameras) +@require_mock_camera +def test_save_images_from_cameras_mock(tmpdir, monkeypatch, camera_type): + _test_save_images_from_cameras(tmpdir, camera_type) + + +@pytest.mark.parametrize("camera_type", available_cameras) +@require_camera +def test_save_images_from_cameras(tmpdir, request, camera_type): + _test_save_images_from_cameras(tmpdir, camera_type) diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index efb324198..9940c8885 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -27,16 +27,15 @@ import pytest +from lerobot import available_robots from lerobot.common.policies.factory import make_policy from lerobot.common.utils.utils import init_hydra_config from lerobot.scripts.control_robot import calibrate, get_available_arms, record, replay, teleoperate from tests.test_robots import make_robot -from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_ROBOT_TYPES, require_robot +from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, require_mock_robot, require_robot -@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_teleoperate(request, robot_type, mock): +def _test_teleoperate(robot_type): robot = make_robot(robot_type) teleoperate(robot, teleop_time_s=1) teleoperate(robot, fps=30, teleop_time_s=1) @@ -44,17 +43,13 @@ def test_teleoperate(request, robot_type, mock): del robot -@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_calibrate(request, robot_type, mock): +def _test_calibrate(robot_type): robot = make_robot(robot_type) calibrate(robot, arms=get_available_arms(robot)) del robot -@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_record_without_cameras(tmpdir, request, robot_type, mock): +def _test_record_without_cameras(tmpdir, robot_type): root = Path(tmpdir) repo_id = "lerobot/debug" @@ -73,9 +68,7 @@ def test_record_without_cameras(tmpdir, request, robot_type, mock): ) -@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): +def _test_record_and_replay_and_policy(tmpdir, robot_type): env_name = "koch_real" policy_name = "act_koch_real" @@ -122,3 +115,51 @@ def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): ) del robot + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_mock_robot +def test_teleoperate_mock(monkeypatch, robot_type): + _test_teleoperate(robot_type) + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_robot +def test_teleoperate(request, robot_type): + _test_teleoperate(robot_type) + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_mock_robot +def test_calibrate_mock(monkeypatch, robot_type): + _test_calibrate(robot_type) + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_robot +def test_calibrate(request, robot_type): + _test_calibrate(robot_type) + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_mock_robot +def test_record_without_cameras_mock(tmpdir, monkeypatch, robot_type): + _test_record_without_cameras(tmpdir, robot_type) + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_robot +def test_record_without_cameras(tmpdir, request, robot_type): + _test_record_without_cameras(tmpdir, robot_type) + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_mock_robot +def test_record_and_replay_and_policy_mock(tmpdir, monkeypatch, robot_type): + _test_record_and_replay_and_policy(tmpdir, robot_type) + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_robot +def test_record_and_replay_and_policy(tmpdir, request, robot_type): + _test_record_and_replay_and_policy(tmpdir, robot_type) diff --git a/tests/test_motors.py b/tests/test_motors.py index 672ecc6f5..07cca187d 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -30,34 +30,12 @@ import numpy as np import pytest +from lerobot import available_motors from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_MOTOR_TYPES, make_motors_bus, mock_input, require_motor +from tests.utils import make_motors_bus, mock_builtins_input, require_mock_motor, require_motor -@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -@require_motor -def test_find_port(request, motor_type, mock): - from lerobot.common.robot_devices.motors.dynamixel import find_port - - if mock: - # To run find_port without user input - monkeypatch = request.getfixturevalue("monkeypatch") - monkeypatch.setattr("builtins.input", mock_input) - - with pytest.raises(OSError): - find_port() - else: - find_port() - - -@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -@require_motor -def test_configure_motors_all_ids_1(request, motor_type, mock): - if mock: - # To run find_port without user input - monkeypatch = request.getfixturevalue("monkeypatch") - monkeypatch.setattr("builtins.input", mock_input) - +def _test_configure_motors_all_ids_1(motor_type): input("Are you sure you want to re-configure the motors? Press enter to continue...") # This test expect the configuration was already correct. motors_bus = make_motors_bus(motor_type) @@ -74,9 +52,7 @@ def test_configure_motors_all_ids_1(request, motor_type, mock): del motors_bus -@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) -@require_motor -def test_motors_bus(request, motor_type, mock): +def _test_motors_bus(request, motor_type, mock): motors_bus = make_motors_bus(motor_type) # Test reading and writting before connecting raises an error @@ -133,3 +109,46 @@ def test_motors_bus(request, motor_type, mock): time.sleep(1) new_values = motors_bus.read("Present_Position") assert (new_values == values).all() + + +@pytest.mark.parametrize("motor_type", available_motors) +@require_mock_motor +def test_find_port_mock(monkeypatch, motor_type): + from lerobot.common.robot_devices.motors.dynamixel import find_port + + # To run find_port without user input + mock_builtins_input(monkeypatch) + with pytest.raises(OSError): + find_port() + + +@pytest.mark.parametrize("motor_type", available_motors) +@require_motor +def test_find_port(request, motor_type): + from lerobot.common.robot_devices.motors.dynamixel import find_port + + find_port() + + +@pytest.mark.parametrize("motor_type", available_motors) +@require_mock_motor +def test_configure_motors_all_ids_1_mock(monkeypatch, motor_type): + _test_configure_motors_all_ids_1(motor_type) + + +@pytest.mark.parametrize("motor_type", available_motors) +@require_motor +def test_configure_motors_all_ids_1(request, motor_type): + _test_configure_motors_all_ids_1(motor_type) + + +@pytest.mark.parametrize("motor_type", available_motors) +@require_mock_motor +def test_motors_bus_mock(monkeypatch, motor_type): + _test_motors_bus(motor_type) + + +@pytest.mark.parametrize("motor_type", available_motors) +@require_motor +def test_motors_bus(request, motor_type): + _test_motors_bus(motor_type) diff --git a/tests/test_robots.py b/tests/test_robots.py index cd49049f0..d3160804b 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -28,13 +28,12 @@ import pytest import torch +from lerobot import available_robots from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import TEST_ROBOT_TYPES, make_robot, require_robot +from tests.utils import make_robot, require_mock_robot, require_robot -@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) -@require_robot -def test_robot(tmpdir, request, robot_type, mock): +def _test_robot(tmpdir, robot_type, mock): # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs # TODO(rcadene): add compatibility with other robots @@ -134,3 +133,15 @@ def test_robot(tmpdir, request, robot_type, mock): for name in robot.cameras: assert not robot.cameras[name].is_connected del robot + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_mock_robot +def test_robot_mock(tmpdir, monkeypatch, robot_type): + _test_robot(tmpdir, robot_type, mock=True) + + +@pytest.mark.parametrize("robot_type", available_robots) +@require_robot +def test_robot(tmpdir, request, robot_type): + _test_robot(tmpdir, robot_type, mock=False) diff --git a/tests/utils.py b/tests/utils.py index cff1970e1..a21891bf7 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -207,40 +207,15 @@ def wrapper(*args, **kwargs): # Access the pytest request context to get the mockeypatch fixture request = kwargs.get("request") robot_type = kwargs.get("robot_type") - mock = kwargs.get("mock") if robot_type is None: raise ValueError("The 'robot_type' must be an argument of the test function.") if request is None: raise ValueError("The 'request' fixture must be an argument of the test function.") - if mock is None: - raise ValueError("The 'mock' variable must be an argument of the test function.") - - if robot_type not in available_robots: - raise ValueError( - f"The camera type '{robot_type}' is not valid. Expected one of these '{available_robots}" - ) - - # Run test with a monkeypatched version of the robot devices. - if mock: - # TODO(rcadene): redesign mocking to not have this hardcoded logic - if robot_type in ["koch", "koch_bimanual"]: - camera_type = "opencv" - elif robot_type == "aloha": - camera_type = "intelrealsense" - else: - camera_type = "all" - mock_cameras(request, camera_type) - mock_motors(request) - - # To run calibration without user input - monkeypatch = request.getfixturevalue("monkeypatch") - monkeypatch.setattr("builtins.input", mock_input) # Run test with a real robot. Skip test if robot connection fails. - else: - if not is_robot_available(robot_type): - pytest.skip(f"A {robot_type} robot is not available.") + if not request.getfixturevalue("is_robot_available"): + pytest.skip(f"A {robot_type} robot is not available.") return func(*args, **kwargs) @@ -250,31 +225,16 @@ def wrapper(*args, **kwargs): def require_camera(func): @wraps(func) def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture request = kwargs.get("request") camera_type = kwargs.get("camera_type") - mock = kwargs.get("mock") if request is None: raise ValueError("The 'request' fixture must be an argument of the test function.") if camera_type is None: raise ValueError("The 'camera_type' must be an argument of the test function.") - if mock is None: - raise ValueError("The 'mock' variable must be an argument of the test function.") - if camera_type not in available_cameras: - raise ValueError( - f"The camera type '{camera_type}' is not valid. Expected one of these '{available_cameras}" - ) - - # Run test with a monkeypatched version of the robot devices. - if mock: - mock_cameras(request, camera_type) - - # Run test with a real robot. Skip test if robot connection fails. - else: - if not is_camera_available(camera_type): - pytest.skip(f"A {camera_type} camera is not available.") + if not request.getfixturevalue("is_camera_available"): + pytest.skip(f"A {camera_type} camera is not available.") return func(*args, **kwargs) @@ -287,44 +247,117 @@ def wrapper(*args, **kwargs): # Access the pytest request context to get the mockeypatch fixture request = kwargs.get("request") motor_type = kwargs.get("motor_type") - mock = kwargs.get("mock") if request is None: raise ValueError("The 'request' fixture must be an argument of the test function.") if motor_type is None: raise ValueError("The 'motor_type' must be an argument of the test function.") - if mock is None: - raise ValueError("The 'mock' variable must be an argument of the test function.") - if motor_type not in available_motors: - raise ValueError( - f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" - ) + if not request.getfixturevalue("is_motor_available"): + pytest.skip(f"A {motor_type} motor is not available.") - # Run test with a monkeypatched version of the robot devices. - if mock: - mock_motors(request) + return func(*args, **kwargs) + + return wrapper - # Run test with a real robot. Skip test if robot connection fails. - else: - if not is_motor_available(motor_type): - pytest.skip(f"A {motor_type} motor is not available.") +def require_mock_robot(func): + """ + Decorator over test function to mock the robot + + The decorated function must have two arguments `monkeypatch` and `robot_type`. + + Example of usage: + ```python + @pytest.mark.parametrize( + "robot_type", ["koch", "aloha"] + ) + @require_robot + def test_require_robot(request, robot_type): + pass + ``` + """ + + @wraps(func) + def wrapper(*args, **kwargs): + # Access the pytest request context to get the mockeypatch fixture + monkeypatch = kwargs.get("monkeypatch") + robot_type = kwargs.get("robot_type") + + if monkeypatch is None: + raise ValueError("The 'monkeypatch' fixture must be an argument of the test function.") + + if robot_type is None: + raise ValueError("The 'robot_type' must be an argument of the test function.") + + mock_robot(monkeypatch, robot_type) return func(*args, **kwargs) return wrapper -def mock_input(text=None): - if text is not None: - print(text) +def require_mock_camera(func): + @wraps(func) + def wrapper(*args, **kwargs): + # Access the pytest request context to get the mockeypatch fixture + monkeypatch = kwargs.get("monkeypatch") + camera_type = kwargs.get("camera_type") + if monkeypatch is None: + raise ValueError("The 'monkeypatch' fixture must be an argument of the test function.") + if camera_type is None: + raise ValueError("The 'camera_type' must be an argument of the test function.") + + mock_camera(monkeypatch, camera_type) + return func(*args, **kwargs) + + return wrapper + + +def require_mock_motor(func): + @wraps(func) + def wrapper(*args, **kwargs): + # Access the pytest request context to get the mockeypatch fixture + monkeypatch = kwargs.get("monkeypatch") + motor_type = kwargs.get("motor_type") + + if monkeypatch is None: + raise ValueError("The 'monkeypatch' fixture must be an argument of the test function.") + if motor_type is None: + raise ValueError("The 'motor_type' must be an argument of the test function.") + + mock_motor(monkeypatch, motor_type) + return func(*args, **kwargs) + + return wrapper + + +def mock_robot(monkeypatch, robot_type): + if robot_type not in available_robots: + raise ValueError( + f"The camera type '{robot_type}' is not valid. Expected one of these '{available_robots}" + ) + + if robot_type in ["koch", "koch_bimanual"]: + mock_camera(monkeypatch, "opencv") + mock_motor(monkeypatch, "dynamixel") + elif robot_type == "aloha": + mock_camera(monkeypatch, "intelrealsense") + mock_motor(monkeypatch, "dynamixel") + else: + raise NotImplementedError("Implement mocking logic for new robot.") -def mock_cameras(request, camera_type="all"): - # TODO(rcadene): Redesign the mocking tests - monkeypatch = request.getfixturevalue("monkeypatch") + # To run calibration without user input + mock_builtins_input(monkeypatch) + + +def mock_camera(monkeypatch, camera_type): + if camera_type not in available_cameras: + raise ValueError( + f"The motor type '{camera_type}' is not valid. Expected one of these '{available_cameras}" + ) - if camera_type in ["opencv", "all"]: + if camera_type == "opencv": try: import cv2 @@ -335,7 +368,7 @@ def mock_cameras(request, camera_type="all"): traceback.print_exc() pytest.skip("To avoid skipping tests mocking opencv cameras, run `pip install opencv-python`.") - if camera_type in ["intelrealsense", "all"]: + elif camera_type == "intelrealsense": try: import pyrealsense2 as rs @@ -357,37 +390,52 @@ def mock_cameras(request, camera_type="all"): pytest.skip( "To avoid skipping tests mocking intelrealsense cameras, run `pip install pyrealsense2`." ) + else: + raise NotImplementedError("Implement mocking logic for new camera.") -def mock_motors(request): - # TODO(rcadene): Redesign the mocking tests - monkeypatch = request.getfixturevalue("monkeypatch") +def mock_motor(monkeypatch, motor_type): + if motor_type not in available_motors: + raise ValueError( + f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" + ) - try: - import dynamixel_sdk + if motor_type == "dynamixel": + try: + import dynamixel_sdk + + from tests.mock_dynamixel import ( + MockGroupSyncRead, + MockGroupSyncWrite, + MockPacketHandler, + MockPortHandler, + mock_convert_to_bytes, + ) - from tests.mock_dynamixel import ( - MockGroupSyncRead, - MockGroupSyncWrite, - MockPacketHandler, - MockPortHandler, - mock_convert_to_bytes, - ) + monkeypatch.setattr(dynamixel_sdk, "GroupSyncRead", MockGroupSyncRead) + monkeypatch.setattr(dynamixel_sdk, "GroupSyncWrite", MockGroupSyncWrite) + monkeypatch.setattr(dynamixel_sdk, "PacketHandler", MockPacketHandler) + monkeypatch.setattr(dynamixel_sdk, "PortHandler", MockPortHandler) + + # Import dynamixel AFTER mocking dynamixel_sdk to use mocked classes + from lerobot.common.robot_devices.motors import dynamixel + + # TODO(rcadene): remove need to mock `convert_to_bytes` by implemented the inverse transform + # `convert_bytes_to_value` + monkeypatch.setattr(dynamixel, "convert_to_bytes", mock_convert_to_bytes) + except ImportError: + traceback.print_exc() + pytest.skip("To avoid skipping tests mocking dynamixel motors, run `pip install dynamixel-sdk`.") + else: + raise NotImplementedError("Implement mocking logic for new motor.") - monkeypatch.setattr(dynamixel_sdk, "GroupSyncRead", MockGroupSyncRead) - monkeypatch.setattr(dynamixel_sdk, "GroupSyncWrite", MockGroupSyncWrite) - monkeypatch.setattr(dynamixel_sdk, "PacketHandler", MockPacketHandler) - monkeypatch.setattr(dynamixel_sdk, "PortHandler", MockPortHandler) - # Import dynamixel AFTER mocking dynamixel_sdk to use mocked classes - from lerobot.common.robot_devices.motors import dynamixel +def mock_builtins_input(monkeypatch): + def print_text(text=None): + if text is not None: + print(text) - # TODO(rcadene): remove need to mock `convert_to_bytes` by implemented the inverse transform - # `convert_bytes_to_value` - monkeypatch.setattr(dynamixel, "convert_to_bytes", mock_convert_to_bytes) - except ImportError: - traceback.print_exc() - pytest.skip("To avoid skipping tests mocking dynamixel motors, run `pip install dynamixel-sdk`.") + monkeypatch.setattr("builtins.input", print_text) def make_robot(robot_type: str, overrides: list[str] | None = None) -> Robot: @@ -424,43 +472,3 @@ def make_motors_bus(motor_type: str, **kwargs) -> MotorsBus: else: raise ValueError(f"The motor type '{motor_type}' is not valid.") - - -def is_robot_available(robot_type): - try: - from lerobot.common.robot_devices.robots.factory import make_robot - - config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) - robot_cfg = init_hydra_config(config_path) - robot = make_robot(robot_cfg) - robot.connect() - del robot - return True - except Exception: - traceback.print_exc() - print(f"\nA {robot_type} robot is not available.") - return False - - -def is_camera_available(camera_type): - try: - camera = make_camera(camera_type) - camera.connect() - del camera - return True - except Exception: - traceback.print_exc() - print(f"\nA {camera_type} camera is not available.") - return False - - -def is_motor_available(motor_type): - try: - motors_bus = make_motors_bus(motor_type) - motors_bus.connect() - del motors_bus - return True - except Exception: - traceback.print_exc() - print(f"\nA {motor_type} motor is not available.") - return False From 89b2b7397e7b83b61d0e4c21a7b84110e62082e9 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 16:31:23 +0200 Subject: [PATCH 29/53] fix unit tests --- tests/test_motors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_motors.py b/tests/test_motors.py index 07cca187d..6c1bace96 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -52,7 +52,7 @@ def _test_configure_motors_all_ids_1(motor_type): del motors_bus -def _test_motors_bus(request, motor_type, mock): +def _test_motors_bus(motor_type): motors_bus = make_motors_bus(motor_type) # Test reading and writting before connecting raises an error From e66900e3875eb875465e3b562f1367947affdc9e Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 16:35:37 +0200 Subject: [PATCH 30/53] mock_motor instead of require_mock_motor --- tests/test_motors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_motors.py b/tests/test_motors.py index 6c1bace96..6dc617ee8 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -32,7 +32,7 @@ from lerobot import available_motors from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import make_motors_bus, mock_builtins_input, require_mock_motor, require_motor +from tests.utils import make_motors_bus, mock_builtins_input, mock_motor, require_mock_motor, require_motor def _test_configure_motors_all_ids_1(motor_type): @@ -112,8 +112,8 @@ def _test_motors_bus(motor_type): @pytest.mark.parametrize("motor_type", available_motors) -@require_mock_motor def test_find_port_mock(monkeypatch, motor_type): + mock_motor(monkeypatch, motor_type) from lerobot.common.robot_devices.motors.dynamixel import find_port # To run find_port without user input From 7450adc72b1f5fb834a891874e696cadf5abf059 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 16:40:24 +0200 Subject: [PATCH 31/53] no more require_mock_motor --- tests/test_motors.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_motors.py b/tests/test_motors.py index 6dc617ee8..262a7b649 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -32,7 +32,7 @@ from lerobot import available_motors from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import make_motors_bus, mock_builtins_input, mock_motor, require_mock_motor, require_motor +from tests.utils import make_motors_bus, mock_builtins_input, mock_motor, require_motor def _test_configure_motors_all_ids_1(motor_type): @@ -131,8 +131,8 @@ def test_find_port(request, motor_type): @pytest.mark.parametrize("motor_type", available_motors) -@require_mock_motor def test_configure_motors_all_ids_1_mock(monkeypatch, motor_type): + mock_motor(monkeypatch, motor_type) _test_configure_motors_all_ids_1(motor_type) @@ -143,8 +143,8 @@ def test_configure_motors_all_ids_1(request, motor_type): @pytest.mark.parametrize("motor_type", available_motors) -@require_mock_motor def test_motors_bus_mock(monkeypatch, motor_type): + mock_motor(monkeypatch, motor_type) _test_motors_bus(motor_type) From 8da08935d457048782b84b0f1a254db57e95c84b Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Thu, 26 Sep 2024 16:45:04 +0200 Subject: [PATCH 32/53] move mock_motor in test_motors.py --- tests/test_motors.py | 39 +++++++++++++++++++++++++++++++- tests/utils.py | 54 ++------------------------------------------ 2 files changed, 40 insertions(+), 53 deletions(-) diff --git a/tests/test_motors.py b/tests/test_motors.py index 262a7b649..fcf33a3eb 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -26,13 +26,14 @@ # TODO(rcadene): add compatibility with other motors bus import time +import traceback import numpy as np import pytest from lerobot import available_motors from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import make_motors_bus, mock_builtins_input, mock_motor, require_motor +from tests.utils import make_motors_bus, mock_builtins_input, require_motor def _test_configure_motors_all_ids_1(motor_type): @@ -152,3 +153,39 @@ def test_motors_bus_mock(monkeypatch, motor_type): @require_motor def test_motors_bus(request, motor_type): _test_motors_bus(motor_type) + + +def mock_motor(monkeypatch, motor_type): + if motor_type not in available_motors: + raise ValueError( + f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" + ) + + if motor_type == "dynamixel": + try: + import dynamixel_sdk + + from tests.mock_dynamixel import ( + MockGroupSyncRead, + MockGroupSyncWrite, + MockPacketHandler, + MockPortHandler, + mock_convert_to_bytes, + ) + + monkeypatch.setattr(dynamixel_sdk, "GroupSyncRead", MockGroupSyncRead) + monkeypatch.setattr(dynamixel_sdk, "GroupSyncWrite", MockGroupSyncWrite) + monkeypatch.setattr(dynamixel_sdk, "PacketHandler", MockPacketHandler) + monkeypatch.setattr(dynamixel_sdk, "PortHandler", MockPortHandler) + + # Import dynamixel AFTER mocking dynamixel_sdk to use mocked classes + from lerobot.common.robot_devices.motors import dynamixel + + # TODO(rcadene): remove need to mock `convert_to_bytes` by implemented the inverse transform + # `convert_bytes_to_value` + monkeypatch.setattr(dynamixel, "convert_to_bytes", mock_convert_to_bytes) + except ImportError: + traceback.print_exc() + pytest.skip("To avoid skipping tests mocking dynamixel motors, run `pip install dynamixel-sdk`.") + else: + raise NotImplementedError("Implement mocking logic for new motor.") diff --git a/tests/utils.py b/tests/utils.py index a21891bf7..4bd474135 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -314,22 +314,8 @@ def wrapper(*args, **kwargs): return wrapper -def require_mock_motor(func): - @wraps(func) - def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture - monkeypatch = kwargs.get("monkeypatch") - motor_type = kwargs.get("motor_type") - - if monkeypatch is None: - raise ValueError("The 'monkeypatch' fixture must be an argument of the test function.") - if motor_type is None: - raise ValueError("The 'motor_type' must be an argument of the test function.") - - mock_motor(monkeypatch, motor_type) - return func(*args, **kwargs) - - return wrapper +def mock_motor(**kwargs): + pass def mock_robot(monkeypatch, robot_type): @@ -394,42 +380,6 @@ def mock_camera(monkeypatch, camera_type): raise NotImplementedError("Implement mocking logic for new camera.") -def mock_motor(monkeypatch, motor_type): - if motor_type not in available_motors: - raise ValueError( - f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" - ) - - if motor_type == "dynamixel": - try: - import dynamixel_sdk - - from tests.mock_dynamixel import ( - MockGroupSyncRead, - MockGroupSyncWrite, - MockPacketHandler, - MockPortHandler, - mock_convert_to_bytes, - ) - - monkeypatch.setattr(dynamixel_sdk, "GroupSyncRead", MockGroupSyncRead) - monkeypatch.setattr(dynamixel_sdk, "GroupSyncWrite", MockGroupSyncWrite) - monkeypatch.setattr(dynamixel_sdk, "PacketHandler", MockPacketHandler) - monkeypatch.setattr(dynamixel_sdk, "PortHandler", MockPortHandler) - - # Import dynamixel AFTER mocking dynamixel_sdk to use mocked classes - from lerobot.common.robot_devices.motors import dynamixel - - # TODO(rcadene): remove need to mock `convert_to_bytes` by implemented the inverse transform - # `convert_bytes_to_value` - monkeypatch.setattr(dynamixel, "convert_to_bytes", mock_convert_to_bytes) - except ImportError: - traceback.print_exc() - pytest.skip("To avoid skipping tests mocking dynamixel motors, run `pip install dynamixel-sdk`.") - else: - raise NotImplementedError("Implement mocking logic for new motor.") - - def mock_builtins_input(monkeypatch): def print_text(text=None): if text is not None: From a7350d9b65a9399f81de236f8188259b83ca81e7 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 12:02:14 +0200 Subject: [PATCH 33/53] add mock=False --- .../robot_devices/cameras/intelrealsense.py | 66 +++++-- .../common/robot_devices/cameras/opencv.py | 71 ++++++-- .../common/robot_devices/motors/dynamixel.py | 63 +++++-- tests/conftest.py | 11 +- tests/mock_dynamixel.py | 13 +- tests/mock_intelrealsense.py | 64 ++++--- tests/mock_opencv.py | 29 ++- tests/test_cameras.py | 52 ++---- tests/test_control_robot.py | 84 +++------ tests/test_motors.py | 117 +++--------- tests/test_robots.py | 28 +-- tests/utils.py | 172 +++++------------- 12 files changed, 347 insertions(+), 423 deletions(-) diff --git a/lerobot/common/robot_devices/cameras/intelrealsense.py b/lerobot/common/robot_devices/cameras/intelrealsense.py index 4a9310acb..5d6045242 100644 --- a/lerobot/common/robot_devices/cameras/intelrealsense.py +++ b/lerobot/common/robot_devices/cameras/intelrealsense.py @@ -14,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 ( @@ -29,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_intelrealsense 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(): + serial_number = int(device.get_info(RSCameraInfo(SERIAL_NUMBER_INDEX))) camera_ids.append(serial_number) if raise_when_empty and len(camera_ids) == 0: @@ -65,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_opencv 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})" @@ -104,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, @@ -150,6 +164,7 @@ 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"]: @@ -231,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 @@ -246,22 +262,35 @@ def connect(self): f"IntelRealSenseCamera({self.camera_index}) is already connected." ) - config = rs.config() + if self.mock: + from tests.mock_intelrealsense 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: profile = self.camera.start(config) is_camera_open = True @@ -282,7 +311,7 @@ def connect(self): raise OSError(f"Can't access IntelRealSenseCamera({self.camera_index}).") - color_stream = profile.get_stream(rs.stream.color) + 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() @@ -343,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_opencv 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: diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index a2a4407e2..9ff47b6a6 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -13,7 +13,6 @@ from pathlib import Path from threading import Thread -import cv2 import numpy as np from PIL import Image @@ -24,10 +23,6 @@ ) from lerobot.common.utils.utils import capture_timestamp_utc -# Use 1 thread to avoid blocking the main thread. Especially useful during data collection -# when other threads are used to save the images. -cv2.setNumThreads(1) - # The maximum opencv device index depends on your operating system. For instance, # if you have 3 cameras, they should be associated to index 0, 1, and 2. This is the case # on MacOS. However, on Ubuntu, the indices are different like 6, 16, 23. @@ -36,7 +31,7 @@ MAX_OPENCV_INDEX = 60 -def find_camera_indices(raise_when_empty=False, max_index_search_range=MAX_OPENCV_INDEX): +def find_camera_indices(raise_when_empty=False, max_index_search_range=MAX_OPENCV_INDEX, mock=False): if platform.system() == "Linux": # Linux uses camera ports print("Linux detected. Finding available camera indices through scanning '/dev/video*' ports") @@ -51,9 +46,14 @@ def find_camera_indices(raise_when_empty=False, max_index_search_range=MAX_OPENC ) possible_camera_ids = range(max_index_search_range) + if mock: + from tests.mock_opencv import VideoCapture + else: + from cv2 import VideoCapture + camera_ids = [] for camera_idx in possible_camera_ids: - camera = cv2.VideoCapture(camera_idx) + camera = VideoCapture(camera_idx) is_open = camera.isOpened() camera.release() @@ -78,19 +78,25 @@ def save_image(img_array, camera_index, frame_index, images_dir): def save_images_from_cameras( - images_dir: Path, camera_ids: list[int] | None = None, fps=None, width=None, height=None, record_time_s=2 + images_dir: Path, + camera_ids: list[int] | None = None, + fps=None, + 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) print("Connecting cameras") cameras = [] for cam_idx in camera_ids: - camera = OpenCVCamera(cam_idx, fps=fps, width=width, height=height) + camera = OpenCVCamera(cam_idx, fps=fps, width=width, height=height, mock=mock) camera.connect() print( f"OpenCVCamera({camera.camera_index}, fps={camera.fps}, width={camera.width}, " @@ -156,6 +162,7 @@ class OpenCVCameraConfig: width: int | None = None height: int | None = None color_mode: str = "rgb" + mock: bool = False def __post_init__(self): if self.color_mode not in ["rgb", "bgr"]: @@ -215,6 +222,7 @@ def __init__(self, camera_index: int, config: OpenCVCameraConfig | None = None, self.width = config.width self.height = config.height self.color_mode = config.color_mode + self.mock = config.mock self.camera = None self.is_connected = False @@ -235,11 +243,31 @@ def connect(self): if self.is_connected: raise RobotDeviceAlreadyConnectedError(f"OpenCVCamera({self.camera_index}) is already connected.") + if self.mock: + from tests.mock_opencv import ( + CAP_PROP_FPS, + CAP_PROP_FRAME_HEIGHT, + CAP_PROP_FRAME_WIDTH, + VideoCapture, + ) + else: + import cv2 + from cv2 import ( + CAP_PROP_FPS, + CAP_PROP_FRAME_HEIGHT, + CAP_PROP_FRAME_WIDTH, + VideoCapture, + ) + + # Use 1 thread to avoid blocking the main thread. Especially useful during data collection + # when other threads are used to save the images. + cv2.setNumThreads(1) + camera_idx = f"/dev/video{self.camera_index}" if platform.system() == "Linux" else self.camera_index with self.lock: # First create a temporary camera trying to access `camera_index`, # and verify it is a valid camera by calling `isOpened`. - tmp_camera = cv2.VideoCapture(camera_idx) + tmp_camera = VideoCapture(camera_idx) is_camera_open = tmp_camera.isOpened() # Release camera to make it accessible for `find_camera_indices` tmp_camera.release() @@ -262,18 +290,18 @@ def connect(self): # Note: For some unknown reason, calling `isOpened` blocks the camera which then # needs to be re-created. with self.lock: - self.camera = cv2.VideoCapture(camera_idx) + self.camera = VideoCapture(camera_idx) if self.fps is not None: - self.camera.set(cv2.CAP_PROP_FPS, self.fps) + self.camera.set(CAP_PROP_FPS, self.fps) if self.width is not None: - self.camera.set(cv2.CAP_PROP_FRAME_WIDTH, self.width) + self.camera.set(CAP_PROP_FRAME_WIDTH, self.width) if self.height is not None: - self.camera.set(cv2.CAP_PROP_FRAME_HEIGHT, self.height) + self.camera.set(CAP_PROP_FRAME_HEIGHT, self.height) - actual_fps = self.camera.get(cv2.CAP_PROP_FPS) - actual_width = self.camera.get(cv2.CAP_PROP_FRAME_WIDTH) - actual_height = self.camera.get(cv2.CAP_PROP_FRAME_HEIGHT) + actual_fps = self.camera.get(CAP_PROP_FPS) + actual_width = self.camera.get(CAP_PROP_FRAME_WIDTH) + actual_height = self.camera.get(CAP_PROP_FRAME_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): @@ -327,7 +355,12 @@ def read(self, temporary_color_mode: str | None = None) -> np.ndarray: # However, Deep Learning framework such as LeRobot uses RGB format as default to train neural networks, # so we convert the image color from BGR to RGB. if requested_color_mode == "rgb": - color_image = cv2.cvtColor(color_image, cv2.COLOR_BGR2RGB) + if self.mock: + from tests.mock_opencv import COLOR_BGR2RGB, cvtColor + else: + from cv2 import COLOR_BGR2RGB, cvtColor + + color_image = cvtColor(color_image, COLOR_BGR2RGB) h, w, _ = color_image.shape if h != self.height or w != self.width: diff --git a/lerobot/common/robot_devices/motors/dynamixel.py b/lerobot/common/robot_devices/motors/dynamixel.py index 491963fed..b6dd8affd 100644 --- a/lerobot/common/robot_devices/motors/dynamixel.py +++ b/lerobot/common/robot_devices/motors/dynamixel.py @@ -8,17 +8,6 @@ import numpy as np import tqdm -from dynamixel_sdk import ( - COMM_SUCCESS, - DXL_HIBYTE, - DXL_HIWORD, - DXL_LOBYTE, - DXL_LOWORD, - GroupSyncRead, - GroupSyncWrite, - PacketHandler, - PortHandler, -) from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError from lerobot.common.utils.utils import capture_timestamp_utc @@ -154,6 +143,8 @@ NUM_READ_RETRY = 10 NUM_WRITE_RETRY = 10 +COMM_SUCCESS = 0 + def convert_degrees_to_steps(degrees: float | np.ndarray, models: str | list[str]) -> np.ndarray: """This function converts the degree range to the step range for indicating motors rotation. @@ -166,7 +157,17 @@ def convert_degrees_to_steps(degrees: float | np.ndarray, models: str | list[str return steps -def convert_to_bytes(value, bytes): +def convert_to_bytes(value, bytes, mock=False): + if mock: + return value + + from dynamixel_sdk import ( + DXL_HIBYTE, + DXL_HIWORD, + DXL_LOBYTE, + DXL_LOWORD, + ) + # Note: No need to convert back into unsigned int, since this byte preprocessing # already handles it for us. if bytes == 1: @@ -333,9 +334,11 @@ def __init__( motors: dict[str, tuple[int, str]], extra_model_control_table: dict[str, list[tuple]] | None = None, extra_model_resolution: dict[str, int] | None = None, + mock=False, ): self.port = port self.motors = motors + self.mock = mock self.model_ctrl_table = deepcopy(MODEL_CONTROL_TABLE) if extra_model_control_table: @@ -359,6 +362,11 @@ def connect(self): f"DynamixelMotorsBus({self.port}) is already connected. Do not call `motors_bus.connect()` twice." ) + if self.mock: + from tests.mock_dynamixel import PacketHandler, PortHandler + else: + from dynamixel_sdk import PacketHandler, PortHandler + self.port_handler = PortHandler(self.port) self.packet_handler = PacketHandler(PROTOCOL_VERSION) @@ -392,10 +400,17 @@ def connect(self): self.configure_motors() def reconnect(self): + if self.mock: + from tests.mock_dynamixel import PacketHandler, PortHandler + else: + from dynamixel_sdk import PacketHandler, PortHandler + self.port_handler = PortHandler(self.port) self.packet_handler = PacketHandler(PROTOCOL_VERSION) + if not self.port_handler.openPort(): raise OSError(f"Failed to open port '{self.port}'.") + self.is_connected = True def are_motors_configured(self): @@ -781,6 +796,11 @@ def revert_calibration(self, values: np.ndarray | list, motor_names: list[str] | return values def _read_with_motor_ids(self, motor_models, motor_ids, data_name): + if self.mock: + from tests.mock_dynamixel import GroupSyncRead + else: + from dynamixel_sdk import GroupSyncRead + return_list = True if not isinstance(motor_ids, list): return_list = False @@ -817,6 +837,11 @@ def read(self, data_name, motor_names: str | list[str] | None = None): start_time = time.perf_counter() + if self.mock: + from tests.mock_dynamixel import GroupSyncRead + else: + from dynamixel_sdk import GroupSyncRead + if motor_names is None: motor_names = self.motor_names @@ -876,6 +901,11 @@ def read(self, data_name, motor_names: str | list[str] | None = None): return values def _write_with_motor_ids(self, motor_models, motor_ids, data_name, values): + if self.mock: + from tests.mock_dynamixel import GroupSyncWrite + else: + from dynamixel_sdk import GroupSyncWrite + if not isinstance(motor_ids, list): motor_ids = [motor_ids] if not isinstance(values, list): @@ -885,7 +915,7 @@ def _write_with_motor_ids(self, motor_models, motor_ids, data_name, values): addr, bytes = self.model_ctrl_table[motor_models[0]][data_name] group = GroupSyncWrite(self.port_handler, self.packet_handler, addr, bytes) for idx, value in zip(motor_ids, values, strict=True): - data = convert_to_bytes(value, bytes) + data = convert_to_bytes(value, bytes, self.mock) group.addParam(idx, data) comm = group.txPacket() @@ -903,6 +933,11 @@ def write(self, data_name, values: int | float | np.ndarray, motor_names: str | start_time = time.perf_counter() + if self.mock: + from tests.mock_dynamixel import GroupSyncWrite + else: + from dynamixel_sdk import GroupSyncWrite + if motor_names is None: motor_names = self.motor_names @@ -937,7 +972,7 @@ def write(self, data_name, values: int | float | np.ndarray, motor_names: str | ) for idx, value in zip(motor_ids, values, strict=True): - data = convert_to_bytes(value, bytes) + data = convert_to_bytes(value, bytes, self.mock) if init_group: self.group_writers[group_key].addParam(idx, data) else: diff --git a/tests/conftest.py b/tests/conftest.py index ac1d0b76a..826c6bf43 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -62,8 +62,8 @@ def is_camera_available(camera_type): del camera return True except Exception: - traceback.print_exc() print(f"\nA {camera_type} camera is not available.") + traceback.print_exc() return False @@ -83,3 +83,12 @@ def is_motor_available(motor_type): traceback.print_exc() print(f"\nA {motor_type} motor is not available.") return False + + +@pytest.fixture +def patch_builtins_input(monkeypatch): + def print_text(text=None): + if text is not None: + print(text) + + monkeypatch.setattr("builtins.input", print_text) diff --git a/tests/mock_dynamixel.py b/tests/mock_dynamixel.py index a3063e63e..6d0ed20e5 100644 --- a/tests/mock_dynamixel.py +++ b/tests/mock_dynamixel.py @@ -5,19 +5,20 @@ from the original classes and functions (e.g. return types might be None instead of boolean). """ -from dynamixel_sdk import COMM_SUCCESS +# from dynamixel_sdk import COMM_SUCCESS DEFAULT_BAUDRATE = 9_600 +COMM_SUCCESS = 0 # tx or rx packet communication success -def mock_convert_to_bytes(value, bytes): +def convert_to_bytes(value, bytes): # TODO(rcadene): remove need to mock `convert_to_bytes` by implemented the inverse transform # `convert_bytes_to_value` del bytes # unused return value -class MockPortHandler: +class PortHandler: def __init__(self, port): self.port = port # factory default baudrate @@ -39,14 +40,14 @@ def setBaudRate(self, baudrate): # noqa: N802 self.baudrate = baudrate -class MockPacketHandler: +class PacketHandler: def __init__(self, protocol_version): del protocol_version # unused # Use packet_handler.data to communicate across Read and Write self.data = {} -class MockGroupSyncRead: +class GroupSyncRead: def __init__(self, port_handler, packet_handler, address, bytes): self.packet_handler = packet_handler @@ -71,7 +72,7 @@ def getData(self, index, address, bytes): # noqa: N802 return self.packet_handler.data[index][address] -class MockGroupSyncWrite: +class GroupSyncWrite: def __init__(self, port_handler, packet_handler, address, bytes): self.packet_handler = packet_handler self.address = address diff --git a/tests/mock_intelrealsense.py b/tests/mock_intelrealsense.py index c1d6add21..b9c6ccf96 100644 --- a/tests/mock_intelrealsense.py +++ b/tests/mock_intelrealsense.py @@ -3,33 +3,33 @@ import numpy as np -class MockStream(enum.Enum): +class RSStream(enum.Enum): color = 0 depth = 1 -class MockFormat(enum.Enum): +class RSFormat(enum.Enum): rgb8 = 0 z16 = 1 -class MockConfig: +class RSConfig: def enable_device(self, device_id: str): self.device_enabled = device_id def enable_stream( - self, stream_type: MockStream, width=None, height=None, color_format: MockFormat = None, fps=None + self, stream_type: RSStream, width=None, height=None, color_format: RSFormat = None, fps=None ): self.stream_type = stream_type # Overwrite default values when possible self.width = 848 if width is None else width self.height = 480 if height is None else height - self.color_format = MockFormat.rgb8 if color_format is None else color_format + self.color_format = RSFormat.rgb8 if color_format is None else color_format self.fps = 30 if fps is None else fps -class MockColorProfile: - def __init__(self, config: MockConfig): +class RSColorProfile: + def __init__(self, config: RSConfig): self.config = config def fps(self): @@ -42,32 +42,32 @@ def height(self): return self.config.height -class MockColorStream: - def __init__(self, config: MockConfig): +class RSColorStream: + def __init__(self, config: RSConfig): self.config = config def as_video_stream_profile(self): - return MockColorProfile(self.config) + return RSColorProfile(self.config) -class MockProfile: - def __init__(self, config: MockConfig): +class RSProfile: + def __init__(self, config: RSConfig): self.config = config - def get_stream(self, color_format: MockFormat): + def get_stream(self, color_format: RSFormat): del color_format # unused - return MockColorStream(self.config) + return RSColorStream(self.config) -class MockPipeline: +class RSPipeline: def __init__(self): self.started = False self.config = None - def start(self, config: MockConfig): + def start(self, config: RSConfig): self.started = True self.config = config - return MockProfile(self.config) + return RSProfile(self.config) def stop(self): if not self.started: @@ -77,22 +77,22 @@ def stop(self): def wait_for_frames(self, timeout_ms=50000): del timeout_ms # unused - return MockFrames(self.config) + return RSFrames(self.config) -class MockFrames: - def __init__(self, config: MockConfig): +class RSFrames: + def __init__(self, config: RSConfig): self.config = config def get_color_frame(self): - return MockColorFrame(self.config) + return RSColorFrame(self.config) def get_depth_frame(self): - return MockDepthFrame(self.config) + return RSDepthFrame(self.config) -class MockColorFrame: - def __init__(self, config: MockConfig): +class RSColorFrame: + def __init__(self, config: RSConfig): self.config = config def get_data(self): @@ -102,15 +102,15 @@ def get_data(self): return data -class MockDepthFrame: - def __init__(self, config: MockConfig): +class RSDepthFrame: + def __init__(self, config: RSConfig): self.config = config def get_data(self): return np.ones((self.config.height, self.config.width), dtype=np.uint16) -class MockDevice: +class RSDevice: def __init__(self): pass @@ -120,9 +120,15 @@ def get_info(self, camera_info) -> str: return "123456789" -class MockContext: +class RSContext: def __init__(self): pass def query_devices(self): - return [MockDevice()] + return [RSDevice()] + + +class RSCameraInfo: + def __init__(self, serial_number): + del serial_number + pass diff --git a/tests/mock_opencv.py b/tests/mock_opencv.py index 8a89c0b5c..31d808cbe 100644 --- a/tests/mock_opencv.py +++ b/tests/mock_opencv.py @@ -1,20 +1,31 @@ from functools import cache -import cv2 import numpy as np +CAP_PROP_FPS = 5 +CAP_PROP_FRAME_WIDTH = 3 +CAP_PROP_FRAME_HEIGHT = 4 +COLOR_BGR2RGB = 4 + @cache def _generate_image(width: int, height: int): return np.random.randint(0, 256, size=(height, width, 3), dtype=np.uint8) -class MockVideoCapture: +def cvtColor(color_image, color_convertion): # noqa: N802 + if color_convertion == COLOR_BGR2RGB: + return color_image[:, :, [2, 1, 0]] + else: + raise NotImplementedError(color_convertion) + + +class VideoCapture: def __init__(self, *args, **kwargs): self._mock_dict = { - cv2.CAP_PROP_FPS: 30, - cv2.CAP_PROP_FRAME_WIDTH: 640, - cv2.CAP_PROP_FRAME_HEIGHT: 480, + CAP_PROP_FPS: 30, + CAP_PROP_FRAME_WIDTH: 640, + CAP_PROP_FRAME_HEIGHT: 480, } self._is_opened = True @@ -32,17 +43,17 @@ def get(self, propId: int) -> float: # noqa: N803 raise RuntimeError("Camera is not opened") value = self._mock_dict[propId] if value == 0: - if propId == cv2.CAP_PROP_FRAME_HEIGHT: + if propId == CAP_PROP_FRAME_HEIGHT: value = 480 - elif propId == cv2.CAP_PROP_FRAME_WIDTH: + elif propId == CAP_PROP_FRAME_WIDTH: value = 640 return value def read(self): if not self._is_opened: raise RuntimeError("Camera is not opened") - h = self.get(cv2.CAP_PROP_FRAME_HEIGHT) - w = self.get(cv2.CAP_PROP_FRAME_WIDTH) + h = self.get(CAP_PROP_FRAME_HEIGHT) + w = self.get(CAP_PROP_FRAME_WIDTH) ret = True return ret, _generate_image(width=w, height=h) diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 2adeb92e7..72da248d6 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -24,9 +24,8 @@ import numpy as np import pytest -from lerobot import available_cameras from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import make_camera, require_camera, require_mock_camera +from tests.utils import TEST_CAMERA_TYPES, make_camera, require_camera # Maximum absolute difference between two consecutive images recored by a camera. # This value differs with respect to the camera. @@ -37,7 +36,9 @@ def compute_max_pixel_difference(first_image, second_image): return np.abs(first_image.astype(float) - second_image.astype(float)).max() -def _test_camera(camera_type): +@pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) +@require_camera +def test_camera(request, camera_type, mock): """Test assumes that `camera.read()` returns the same image when called multiple times in a row. So the environment should not change (you shouldnt be in front of the camera) and the camera should not be moving. @@ -47,8 +48,11 @@ def _test_camera(camera_type): # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs + if camera_type == "opencv" and not mock: + pytest.skip("TODO(rcadene): fix test for opencv physical camera") + # Test instantiating - camera = make_camera(camera_type) + camera = make_camera(camera_type, mock=mock) # Test reading, async reading, disconnecting before connecting raises an error with pytest.raises(RobotDeviceNotConnectedError): @@ -62,7 +66,7 @@ def _test_camera(camera_type): del camera # Test connecting - camera = make_camera(camera_type) + camera = make_camera(camera_type, mock=mock) camera.connect() assert camera.is_connected assert camera.fps is not None @@ -102,12 +106,12 @@ def _test_camera(camera_type): assert camera.thread is None # Test disconnecting with `__del__` - camera = make_camera(camera_type) + camera = make_camera(camera_type, mock=mock) camera.connect() del camera # Test acquiring a bgr image - camera = make_camera(camera_type, color_mode="bgr") + camera = make_camera(camera_type, color_mode="bgr", mock=mock) camera.connect() assert camera.color_mode == "bgr" bgr_color_image = camera.read() @@ -120,7 +124,7 @@ def _test_camera(camera_type): # TODO(rcadene): Add a test for a camera that supports fps=60 # Test width and height can be set - camera = make_camera(camera_type, fps=30, width=1280, height=720) + camera = make_camera(camera_type, fps=30, width=1280, height=720, mock=mock) camera.connect() assert camera.fps == 30 assert camera.width == 1280 @@ -133,41 +137,19 @@ def _test_camera(camera_type): del camera # Test not supported width and height raise an error - camera = make_camera(camera_type, fps=30, width=0, height=0) + camera = make_camera(camera_type, fps=30, width=0, height=0, mock=mock) with pytest.raises(OSError): camera.connect() del camera -def _test_save_images_from_cameras(tmpdir, camera_type): +@pytest.mark.parametrize("camera_type, mock", TEST_CAMERA_TYPES) +@require_camera +def test_save_images_from_cameras(tmpdir, request, camera_type, mock): # TODO(rcadene): refactor if camera_type == "opencv": from lerobot.common.robot_devices.cameras.opencv import save_images_from_cameras elif camera_type == "intelrealsense": from lerobot.common.robot_devices.cameras.intelrealsense import save_images_from_cameras - save_images_from_cameras(tmpdir, record_time_s=1) - - -@pytest.mark.parametrize("camera_type", available_cameras) -@require_mock_camera -def test_camera_mock(monkeypatch, camera_type): - _test_camera(camera_type) - - -@pytest.mark.parametrize("camera_type", available_cameras) -@require_camera -def test_camera(request, camera_type): - _test_camera(camera_type) - - -@pytest.mark.parametrize("camera_type", available_cameras) -@require_mock_camera -def test_save_images_from_cameras_mock(tmpdir, monkeypatch, camera_type): - _test_save_images_from_cameras(tmpdir, camera_type) - - -@pytest.mark.parametrize("camera_type", available_cameras) -@require_camera -def test_save_images_from_cameras(tmpdir, request, camera_type): - _test_save_images_from_cameras(tmpdir, camera_type) + save_images_from_cameras(tmpdir, record_time_s=1, mock=mock) diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index 9940c8885..752380047 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -27,33 +27,44 @@ import pytest -from lerobot import available_robots from lerobot.common.policies.factory import make_policy from lerobot.common.utils.utils import init_hydra_config from lerobot.scripts.control_robot import calibrate, get_available_arms, record, replay, teleoperate from tests.test_robots import make_robot -from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, require_mock_robot, require_robot +from tests.utils import DEFAULT_CONFIG_PATH, DEVICE, TEST_ROBOT_TYPES, require_robot -def _test_teleoperate(robot_type): - robot = make_robot(robot_type) +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) +@require_robot +def test_teleoperate(request, robot_type, mock): + robot = make_robot(robot_type, mock=mock) teleoperate(robot, teleop_time_s=1) teleoperate(robot, fps=30, teleop_time_s=1) teleoperate(robot, fps=60, teleop_time_s=1) del robot -def _test_calibrate(robot_type): - robot = make_robot(robot_type) +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) +@require_robot +def test_calibrate(request, robot_type, mock): + if mock: + request.getfixturevalue("patch_builtins_input") + + robot = make_robot(robot_type, mock=mock) calibrate(robot, arms=get_available_arms(robot)) del robot -def _test_record_without_cameras(tmpdir, robot_type): +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) +@require_robot +def test_record_without_cameras(tmpdir, request, robot_type, mock): + if mock: + request.getfixturevalue("patch_builtins_input") + root = Path(tmpdir) repo_id = "lerobot/debug" - robot = make_robot(robot_type, overrides=["~cameras"]) + robot = make_robot(robot_type, overrides=["~cameras"], mock=mock) record( robot, fps=30, @@ -68,14 +79,19 @@ def _test_record_without_cameras(tmpdir, robot_type): ) -def _test_record_and_replay_and_policy(tmpdir, robot_type): +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) +@require_robot +def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): + if mock: + request.getfixturevalue("patch_builtins_input") + env_name = "koch_real" policy_name = "act_koch_real" root = Path(tmpdir) repo_id = "lerobot/debug" - robot = make_robot(robot_type) + robot = make_robot(robot_type, mock=mock) dataset = record( robot, fps=30, @@ -115,51 +131,3 @@ def _test_record_and_replay_and_policy(tmpdir, robot_type): ) del robot - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_mock_robot -def test_teleoperate_mock(monkeypatch, robot_type): - _test_teleoperate(robot_type) - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot -def test_teleoperate(request, robot_type): - _test_teleoperate(robot_type) - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_mock_robot -def test_calibrate_mock(monkeypatch, robot_type): - _test_calibrate(robot_type) - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot -def test_calibrate(request, robot_type): - _test_calibrate(robot_type) - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_mock_robot -def test_record_without_cameras_mock(tmpdir, monkeypatch, robot_type): - _test_record_without_cameras(tmpdir, robot_type) - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot -def test_record_without_cameras(tmpdir, request, robot_type): - _test_record_without_cameras(tmpdir, robot_type) - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_mock_robot -def test_record_and_replay_and_policy_mock(tmpdir, monkeypatch, robot_type): - _test_record_and_replay_and_policy(tmpdir, robot_type) - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot -def test_record_and_replay_and_policy(tmpdir, request, robot_type): - _test_record_and_replay_and_policy(tmpdir, robot_type) diff --git a/tests/test_motors.py b/tests/test_motors.py index fcf33a3eb..f3f5650cf 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -26,20 +26,35 @@ # TODO(rcadene): add compatibility with other motors bus import time -import traceback import numpy as np import pytest -from lerobot import available_motors +from lerobot.common.robot_devices.motors.dynamixel import find_port from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import make_motors_bus, mock_builtins_input, require_motor +from tests.utils import TEST_MOTOR_TYPES, make_motors_bus, require_motor -def _test_configure_motors_all_ids_1(motor_type): +@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) +@require_motor +def test_find_port_mock(request, motor_type, mock): + if mock: + request.getfixturevalue("patch_builtins_input") + with pytest.raises(OSError): + find_port() + else: + find_port() + + +@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) +@require_motor +def test_configure_motors_all_ids_1_mock(request, motor_type, mock): + if mock: + request.getfixturevalue("patch_builtins_input") + input("Are you sure you want to re-configure the motors? Press enter to continue...") # This test expect the configuration was already correct. - motors_bus = make_motors_bus(motor_type) + motors_bus = make_motors_bus(motor_type, mock=mock) motors_bus.connect() motors_bus.write("Baud_Rate", [0] * len(motors_bus.motors)) motors_bus.set_bus_baudrate(9_600) @@ -47,14 +62,19 @@ def _test_configure_motors_all_ids_1(motor_type): del motors_bus # Test configure - motors_bus = make_motors_bus(motor_type) + motors_bus = make_motors_bus(motor_type, mock=mock) motors_bus.connect() assert motors_bus.are_motors_configured() del motors_bus -def _test_motors_bus(motor_type): - motors_bus = make_motors_bus(motor_type) +@pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) +@require_motor +def test_motors_bus(request, motor_type, mock): + if mock: + request.getfixturevalue("patch_builtins_input") + + motors_bus = make_motors_bus(motor_type, mock=mock) # Test reading and writting before connecting raises an error with pytest.raises(RobotDeviceNotConnectedError): @@ -68,7 +88,7 @@ def _test_motors_bus(motor_type): del motors_bus # Test connecting - motors_bus = make_motors_bus(motor_type) + motors_bus = make_motors_bus(motor_type, mock=mock) motors_bus.connect() # Test connecting twice raises an error @@ -110,82 +130,3 @@ def _test_motors_bus(motor_type): time.sleep(1) new_values = motors_bus.read("Present_Position") assert (new_values == values).all() - - -@pytest.mark.parametrize("motor_type", available_motors) -def test_find_port_mock(monkeypatch, motor_type): - mock_motor(monkeypatch, motor_type) - from lerobot.common.robot_devices.motors.dynamixel import find_port - - # To run find_port without user input - mock_builtins_input(monkeypatch) - with pytest.raises(OSError): - find_port() - - -@pytest.mark.parametrize("motor_type", available_motors) -@require_motor -def test_find_port(request, motor_type): - from lerobot.common.robot_devices.motors.dynamixel import find_port - - find_port() - - -@pytest.mark.parametrize("motor_type", available_motors) -def test_configure_motors_all_ids_1_mock(monkeypatch, motor_type): - mock_motor(monkeypatch, motor_type) - _test_configure_motors_all_ids_1(motor_type) - - -@pytest.mark.parametrize("motor_type", available_motors) -@require_motor -def test_configure_motors_all_ids_1(request, motor_type): - _test_configure_motors_all_ids_1(motor_type) - - -@pytest.mark.parametrize("motor_type", available_motors) -def test_motors_bus_mock(monkeypatch, motor_type): - mock_motor(monkeypatch, motor_type) - _test_motors_bus(motor_type) - - -@pytest.mark.parametrize("motor_type", available_motors) -@require_motor -def test_motors_bus(request, motor_type): - _test_motors_bus(motor_type) - - -def mock_motor(monkeypatch, motor_type): - if motor_type not in available_motors: - raise ValueError( - f"The motor type '{motor_type}' is not valid. Expected one of these '{available_motors}" - ) - - if motor_type == "dynamixel": - try: - import dynamixel_sdk - - from tests.mock_dynamixel import ( - MockGroupSyncRead, - MockGroupSyncWrite, - MockPacketHandler, - MockPortHandler, - mock_convert_to_bytes, - ) - - monkeypatch.setattr(dynamixel_sdk, "GroupSyncRead", MockGroupSyncRead) - monkeypatch.setattr(dynamixel_sdk, "GroupSyncWrite", MockGroupSyncWrite) - monkeypatch.setattr(dynamixel_sdk, "PacketHandler", MockPacketHandler) - monkeypatch.setattr(dynamixel_sdk, "PortHandler", MockPortHandler) - - # Import dynamixel AFTER mocking dynamixel_sdk to use mocked classes - from lerobot.common.robot_devices.motors import dynamixel - - # TODO(rcadene): remove need to mock `convert_to_bytes` by implemented the inverse transform - # `convert_bytes_to_value` - monkeypatch.setattr(dynamixel, "convert_to_bytes", mock_convert_to_bytes) - except ImportError: - traceback.print_exc() - pytest.skip("To avoid skipping tests mocking dynamixel motors, run `pip install dynamixel-sdk`.") - else: - raise NotImplementedError("Implement mocking logic for new motor.") diff --git a/tests/test_robots.py b/tests/test_robots.py index d3160804b..93c1a1fe6 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -28,22 +28,26 @@ import pytest import torch -from lerobot import available_robots +from lerobot.common.robot_devices.robots.manipulator import ManipulatorRobot from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError -from tests.utils import make_robot, require_mock_robot, require_robot +from tests.utils import TEST_ROBOT_TYPES, make_robot, require_robot -def _test_robot(tmpdir, robot_type, mock): +@pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) +@require_robot +def test_robot(tmpdir, request, robot_type, mock): # TODO(rcadene): measure fps in nightly? # TODO(rcadene): test logs # TODO(rcadene): add compatibility with other robots - from lerobot.common.robot_devices.robots.manipulator import ManipulatorRobot if robot_type == "aloha" and mock: # To simplify unit test, we do not rerun manual calibration for Aloha mock=True. # Instead, we use the files from '.cache/calibration/aloha_default' overrides_calibration_dir = None else: + if mock: + request.getfixturevalue("patch_builtins_input") + # Create an empty calibration directory to trigger manual calibration tmpdir = Path(tmpdir) calibration_dir = tmpdir / robot_type @@ -72,7 +76,7 @@ def _test_robot(tmpdir, robot_type, mock): del robot # Test connecting (triggers manual calibration) - robot = make_robot(robot_type, overrides=overrides_calibration_dir) + robot = make_robot(robot_type, overrides=overrides_calibration_dir, mock=mock) robot.connect() assert robot.is_connected @@ -84,7 +88,7 @@ def _test_robot(tmpdir, robot_type, mock): del robot # Test teleop can run - robot = make_robot(robot_type, overrides=overrides_calibration_dir) + robot = make_robot(robot_type, overrides=overrides_calibration_dir, mock=mock) if overrides_calibration_dir is not None: robot.calibration_dir = calibration_dir robot.connect() @@ -133,15 +137,3 @@ def _test_robot(tmpdir, robot_type, mock): for name in robot.cameras: assert not robot.cameras[name].is_connected del robot - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_mock_robot -def test_robot_mock(tmpdir, monkeypatch, robot_type): - _test_robot(tmpdir, robot_type, mock=True) - - -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot -def test_robot(tmpdir, request, robot_type): - _test_robot(tmpdir, robot_type, mock=False) diff --git a/tests/utils.py b/tests/utils.py index 4bd474135..51356edf8 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -15,7 +15,7 @@ # limitations under the License. import os import platform -import traceback +from copy import copy from functools import wraps import pytest @@ -207,14 +207,17 @@ def wrapper(*args, **kwargs): # Access the pytest request context to get the mockeypatch fixture request = kwargs.get("request") robot_type = kwargs.get("robot_type") + mock = kwargs.get("mock") if robot_type is None: raise ValueError("The 'robot_type' must be an argument of the test function.") if request is None: raise ValueError("The 'request' fixture must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") # Run test with a real robot. Skip test if robot connection fails. - if not request.getfixturevalue("is_robot_available"): + if not mock and not request.getfixturevalue("is_robot_available"): pytest.skip(f"A {robot_type} robot is not available.") return func(*args, **kwargs) @@ -227,13 +230,16 @@ def require_camera(func): def wrapper(*args, **kwargs): request = kwargs.get("request") camera_type = kwargs.get("camera_type") + mock = kwargs.get("mock") if request is None: raise ValueError("The 'request' fixture must be an argument of the test function.") if camera_type is None: raise ValueError("The 'camera_type' must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") - if not request.getfixturevalue("is_camera_available"): + if not mock and not request.getfixturevalue("is_camera_available"): pytest.skip(f"A {camera_type} camera is not available.") return func(*args, **kwargs) @@ -247,13 +253,16 @@ def wrapper(*args, **kwargs): # Access the pytest request context to get the mockeypatch fixture request = kwargs.get("request") motor_type = kwargs.get("motor_type") + mock = kwargs.get("mock") if request is None: raise ValueError("The 'request' fixture must be an argument of the test function.") if motor_type is None: raise ValueError("The 'motor_type' must be an argument of the test function.") + if mock is None: + raise ValueError("The 'mock' variable must be an argument of the test function.") - if not request.getfixturevalue("is_motor_available"): + if not mock and not request.getfixturevalue("is_motor_available"): pytest.skip(f"A {motor_type} motor is not available.") return func(*args, **kwargs) @@ -261,134 +270,37 @@ def wrapper(*args, **kwargs): return wrapper -def require_mock_robot(func): - """ - Decorator over test function to mock the robot - - The decorated function must have two arguments `monkeypatch` and `robot_type`. - - Example of usage: - ```python - @pytest.mark.parametrize( - "robot_type", ["koch", "aloha"] - ) - @require_robot - def test_require_robot(request, robot_type): - pass - ``` - """ - - @wraps(func) - def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture - monkeypatch = kwargs.get("monkeypatch") - robot_type = kwargs.get("robot_type") - - if monkeypatch is None: - raise ValueError("The 'monkeypatch' fixture must be an argument of the test function.") - - if robot_type is None: - raise ValueError("The 'robot_type' must be an argument of the test function.") - - mock_robot(monkeypatch, robot_type) - return func(*args, **kwargs) - - return wrapper - - -def require_mock_camera(func): - @wraps(func) - def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture - monkeypatch = kwargs.get("monkeypatch") - camera_type = kwargs.get("camera_type") - - if monkeypatch is None: - raise ValueError("The 'monkeypatch' fixture must be an argument of the test function.") - if camera_type is None: - raise ValueError("The 'camera_type' must be an argument of the test function.") - - mock_camera(monkeypatch, camera_type) - return func(*args, **kwargs) - - return wrapper - - -def mock_motor(**kwargs): - pass - - -def mock_robot(monkeypatch, robot_type): - if robot_type not in available_robots: - raise ValueError( - f"The camera type '{robot_type}' is not valid. Expected one of these '{available_robots}" - ) - - if robot_type in ["koch", "koch_bimanual"]: - mock_camera(monkeypatch, "opencv") - mock_motor(monkeypatch, "dynamixel") - elif robot_type == "aloha": - mock_camera(monkeypatch, "intelrealsense") - mock_motor(monkeypatch, "dynamixel") - else: - raise NotImplementedError("Implement mocking logic for new robot.") - - # To run calibration without user input - mock_builtins_input(monkeypatch) - - -def mock_camera(monkeypatch, camera_type): - if camera_type not in available_cameras: - raise ValueError( - f"The motor type '{camera_type}' is not valid. Expected one of these '{available_cameras}" - ) - - if camera_type == "opencv": - try: - import cv2 - - from tests.mock_opencv import MockVideoCapture - - monkeypatch.setattr(cv2, "VideoCapture", MockVideoCapture) - except ImportError: - traceback.print_exc() - pytest.skip("To avoid skipping tests mocking opencv cameras, run `pip install opencv-python`.") - - elif camera_type == "intelrealsense": - try: - import pyrealsense2 as rs - - from tests.mock_intelrealsense import ( - MockConfig, - MockContext, - MockFormat, - MockPipeline, - MockStream, - ) - - monkeypatch.setattr(rs, "config", MockConfig) - monkeypatch.setattr(rs, "pipeline", MockPipeline) - monkeypatch.setattr(rs, "stream", MockStream) - monkeypatch.setattr(rs, "format", MockFormat) - monkeypatch.setattr(rs, "context", MockContext) - except ImportError: - traceback.print_exc() - pytest.skip( - "To avoid skipping tests mocking intelrealsense cameras, run `pip install pyrealsense2`." - ) - else: - raise NotImplementedError("Implement mocking logic for new camera.") - - -def mock_builtins_input(monkeypatch): - def print_text(text=None): - if text is not None: - print(text) - - monkeypatch.setattr("builtins.input", print_text) +def make_robot(robot_type: str, overrides: list[str] | None = None, mock=False) -> Robot: + if mock: + overrides = [] if overrides is None else copy(overrides) + + if robot_type == "koch": + overrides.append("+leader_arms.main.mock=true") + overrides.append("+follower_arms.main.mock=true") + overrides.append("+cameras.laptop.mock=true") + overrides.append("+cameras.phone.mock=true") + + elif robot_type == "koch_bimanual": + overrides.append("+leader_arms.left.mock=true") + overrides.append("+leader_arms.right.mock=true") + overrides.append("+follower_arms.left.mock=true") + overrides.append("+follower_arms.right.mock=true") + overrides.append("+cameras.laptop.mock=true") + overrides.append("+cameras.phone.mock=true") + + elif robot_type == "aloha": + overrides.append("+leader_arms.left.mock=true") + overrides.append("+leader_arms.right.mock=true") + overrides.append("+follower_arms.left.mock=true") + overrides.append("+follower_arms.right.mock=true") + overrides.append("+cameras.cam_high.mock=true") + overrides.append("+cameras.cam_low.mock=true") + overrides.append("+cameras.cam_left_wrist.mock=true") + overrides.append("+cameras.cam_right_wrist.mock=true") + else: + raise NotImplementedError(robot_type) -def make_robot(robot_type: str, overrides: list[str] | None = None) -> Robot: config_path = ROBOT_CONFIG_PATH_TEMPLATE.format(robot=robot_type) robot_cfg = init_hydra_config(config_path, overrides) robot = make_robot_from_cfg(robot_cfg) From bf7e906b70d1425e66400ec882c1520dd060f8e4 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 12:11:48 +0200 Subject: [PATCH 34/53] add +COLOR_RGB2BGR --- tests/mock_opencv.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/mock_opencv.py b/tests/mock_opencv.py index 31d808cbe..a3e19e550 100644 --- a/tests/mock_opencv.py +++ b/tests/mock_opencv.py @@ -5,6 +5,7 @@ CAP_PROP_FPS = 5 CAP_PROP_FRAME_WIDTH = 3 CAP_PROP_FRAME_HEIGHT = 4 +COLOR_RGB2BGR = 4 COLOR_BGR2RGB = 4 @@ -14,7 +15,7 @@ def _generate_image(width: int, height: int): def cvtColor(color_image, color_convertion): # noqa: N802 - if color_convertion == COLOR_BGR2RGB: + if color_convertion in [COLOR_RGB2BGR, COLOR_BGR2RGB]: return color_image[:, :, [2, 1, 0]] else: raise NotImplementedError(color_convertion) From 81f17d505ef6a7fd40c8ad86cd3c20cd7408d812 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 12:21:06 +0200 Subject: [PATCH 35/53] if not '~cameras' in overrides --- tests/utils.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 51356edf8..b013a6b41 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -277,26 +277,29 @@ def make_robot(robot_type: str, overrides: list[str] | None = None, mock=False) if robot_type == "koch": overrides.append("+leader_arms.main.mock=true") overrides.append("+follower_arms.main.mock=true") - overrides.append("+cameras.laptop.mock=true") - overrides.append("+cameras.phone.mock=true") + if "~cameras" not in overrides: + overrides.append("+cameras.laptop.mock=true") + overrides.append("+cameras.phone.mock=true") elif robot_type == "koch_bimanual": overrides.append("+leader_arms.left.mock=true") overrides.append("+leader_arms.right.mock=true") overrides.append("+follower_arms.left.mock=true") overrides.append("+follower_arms.right.mock=true") - overrides.append("+cameras.laptop.mock=true") - overrides.append("+cameras.phone.mock=true") + if "~cameras" not in overrides: + overrides.append("+cameras.laptop.mock=true") + overrides.append("+cameras.phone.mock=true") elif robot_type == "aloha": overrides.append("+leader_arms.left.mock=true") overrides.append("+leader_arms.right.mock=true") overrides.append("+follower_arms.left.mock=true") overrides.append("+follower_arms.right.mock=true") - overrides.append("+cameras.cam_high.mock=true") - overrides.append("+cameras.cam_low.mock=true") - overrides.append("+cameras.cam_left_wrist.mock=true") - overrides.append("+cameras.cam_right_wrist.mock=true") + if "~cameras" not in overrides: + overrides.append("+cameras.cam_high.mock=true") + overrides.append("+cameras.cam_low.mock=true") + overrides.append("+cameras.cam_left_wrist.mock=true") + overrides.append("+cameras.cam_right_wrist.mock=true") else: raise NotImplementedError(robot_type) From e499d607421500a61d608127d1a392e0ebfea3b0 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 12:29:58 +0200 Subject: [PATCH 36/53] fix unit test --- tests/test_control_robot.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index 752380047..407a32907 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -37,6 +37,9 @@ @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) @require_robot def test_teleoperate(request, robot_type, mock): + if mock: + request.getfixturevalue("patch_builtins_input") + robot = make_robot(robot_type, mock=mock) teleoperate(robot, teleop_time_s=1) teleoperate(robot, fps=30, teleop_time_s=1) @@ -85,6 +88,9 @@ def test_record_and_replay_and_policy(tmpdir, request, robot_type, mock): if mock: request.getfixturevalue("patch_builtins_input") + if robot_type == "aloha": + pytest.skip("TODO(rcadene): enable test once aloha_real and act_aloha_real are merged") + env_name = "koch_real" policy_name = "act_koch_real" From 0352c61b00b6edea082858be38500d136946ce79 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 13:44:41 +0200 Subject: [PATCH 37/53] Add more exception except --- tests/conftest.py | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 826c6bf43..a3460e567 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,6 +17,7 @@ import traceback import pytest +from serial import SerialException from lerobot import available_cameras, available_motors, available_robots from lerobot.common.utils.utils import init_hydra_config @@ -43,9 +44,22 @@ def is_robot_available(robot_type): robot.connect() del robot return True - except Exception: + + except ModuleNotFoundError as e: + print(f"\nA {robot_type} robot is not available.") + print(f"\nInstall module '{e.name}'") traceback.print_exc() + return False + + except SerialException: + traceback.print_exc() + print(f"\nA {robot_type} robot is not available.") + print("\nNo physical motors bus detected.") + return False + + except Exception: print(f"\nA {robot_type} robot is not available.") + traceback.print_exc() return False @@ -61,11 +75,24 @@ def is_camera_available(camera_type): camera.connect() del camera return True + + except ModuleNotFoundError as e: + print(f"\nA {camera_type} camera is not available.") + print(f"\nInstall module '{e.name}'") + traceback.print_exc() + return False + except Exception: print(f"\nA {camera_type} camera is not available.") + print("\nNo physical camera detected.") traceback.print_exc() return False + # except Exception as e: + # print(f"\nA {camera_type} camera is not available.") + # traceback.print_exc() + # return False + @pytest.fixture def is_motor_available(motor_type): @@ -79,9 +106,22 @@ def is_motor_available(motor_type): motors_bus.connect() del motors_bus return True - except Exception: + + except ModuleNotFoundError as e: + print(f"\nA {motor_type} motor is not available.") + print(f"\nInstall module '{e.name}'") + traceback.print_exc() + return False + + except SerialException: + print(f"\nA {motor_type} motor is not available.") + print("\nNo physical motors bus detected.") traceback.print_exc() + return False + + except Exception: print(f"\nA {motor_type} motor is not available.") + traceback.print_exc() return False From c704eb94c06900c322ac1aa6dd5ee76a896eb92b Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 13:54:32 +0200 Subject: [PATCH 38/53] improve except --- tests/conftest.py | 50 +++++++++++++++-------------------------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index a3460e567..386441e5d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -45,20 +45,14 @@ def is_robot_available(robot_type): del robot return True - except ModuleNotFoundError as e: + except Exception as e: print(f"\nA {robot_type} robot is not available.") - print(f"\nInstall module '{e.name}'") - traceback.print_exc() - return False - except SerialException: - traceback.print_exc() - print(f"\nA {robot_type} robot is not available.") - print("\nNo physical motors bus detected.") - return False + if isinstance(e, ModuleNotFoundError): + print(f"\nInstall module '{e.name}'") + elif isinstance(e, SerialException): + print("\nNo physical motors bus detected.") - except Exception: - print(f"\nA {robot_type} robot is not available.") traceback.print_exc() return False @@ -76,23 +70,17 @@ def is_camera_available(camera_type): del camera return True - except ModuleNotFoundError as e: + except Exception as e: print(f"\nA {camera_type} camera is not available.") - print(f"\nInstall module '{e.name}'") - traceback.print_exc() - return False - except Exception: - print(f"\nA {camera_type} camera is not available.") - print("\nNo physical camera detected.") + if isinstance(e, ModuleNotFoundError): + print(f"\nInstall module '{e.name}'") + elif isinstance(e, ValueError) and "camera_index" in e.args[0]: + print("\nNo physical camera detected.") + traceback.print_exc() return False - # except Exception as e: - # print(f"\nA {camera_type} camera is not available.") - # traceback.print_exc() - # return False - @pytest.fixture def is_motor_available(motor_type): @@ -107,20 +95,14 @@ def is_motor_available(motor_type): del motors_bus return True - except ModuleNotFoundError as e: + except Exception as e: print(f"\nA {motor_type} motor is not available.") - print(f"\nInstall module '{e.name}'") - traceback.print_exc() - return False - except SerialException: - print(f"\nA {motor_type} motor is not available.") - print("\nNo physical motors bus detected.") - traceback.print_exc() - return False + if isinstance(e, ModuleNotFoundError): + print(f"\nInstall module '{e.name}'") + elif isinstance(e, SerialException): + print("\nNo physical motors bus detected.") - except Exception: - print(f"\nA {motor_type} motor is not available.") traceback.print_exc() return False From 3f9f3dd0279649b7f07e8afb192f5fa30d1ad6ff Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 14:57:32 +0200 Subject: [PATCH 39/53] Add pyserial --- poetry.lock | 7 ++++--- pyproject.toml | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/poetry.lock b/poetry.lock index d8165371e..ad641ac9b 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. [[package]] name = "absl-py" @@ -2406,6 +2406,7 @@ description = "Nvidia JIT LTO Library" optional = false python-versions = ">=3" files = [ + {file = "nvidia_nvjitlink_cu12-12.5.82-py3-none-manylinux2014_aarch64.whl", hash = "sha256:98103729cc5226e13ca319a10bbf9433bbbd44ef64fe72f45f067cacc14b8d27"}, {file = "nvidia_nvjitlink_cu12-12.5.82-py3-none-manylinux2014_x86_64.whl", hash = "sha256:f9b37bc5c8cf7509665cb6ada5aaa0ce65618f2332b7d3e78e9790511f111212"}, {file = "nvidia_nvjitlink_cu12-12.5.82-py3-none-win_amd64.whl", hash = "sha256:e782564d705ff0bf61ac3e1bf730166da66dd2fe9012f111ede5fc49b64ae697"}, ] @@ -3220,7 +3221,7 @@ files = [ name = "pyserial" version = "3.5" description = "Python Serial Port Extension" -optional = true +optional = false python-versions = "*" files = [ {file = "pyserial-3.5-py2.py3-none-any.whl", hash = "sha256:c4451db6ba391ca6ca299fb3ec7bae67a5c55dde170964c7a14ceefec02f2cf0"}, @@ -4585,4 +4586,4 @@ xarm = ["gym-xarm"] [metadata] lock-version = "2.0" python-versions = ">=3.10,<3.13" -content-hash = "c9c3beac71f760738baf2fd169378eefdaef7d3a9cd068270bc5190fbefdb42a" +content-hash = "4e86e7b1eba959daff1edbc7d41df690e82e065f75de2cf0cde705d596565004" diff --git a/pyproject.toml b/pyproject.toml index f46e39da1..9ffaa648b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,6 +67,7 @@ pynput = {version = ">=1.7.7", optional = true} # TODO(rcadene, salibert): 71.0.1 has a bug setuptools = {version = "!=71.0.1", optional = true} pyrealsense2 = {version = ">=2.55.1.6486", markers = "sys_platform != 'darwin'", optional = true} +pyserial = ">=3.5" [tool.poetry.extras] From da1888a378f2fce3266dcaa733e3c56980eea595 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 14:59:17 +0200 Subject: [PATCH 40/53] revert to all tests --- .github/workflows/test.yml | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a123eb658..f7090c59a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -54,37 +54,12 @@ jobs: - name: Test with pytest run: | - pytest tests/test_motors.py -v --cov=./lerobot --durations=0 \ - -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ - -W ignore::UserWarning:torch.utils.data.dataloader:558 \ - -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ - && rm -rf tests/outputs outputs - - - name: Test with pytest - run: | - pytest tests/test_cameras.py -v --cov=./lerobot --durations=0 \ - -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ - -W ignore::UserWarning:torch.utils.data.dataloader:558 \ - -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ - && rm -rf tests/outputs outputs - - - name: Test with pytest - run: | - pytest tests/test_robots.py -v --cov=./lerobot --durations=0 \ - -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ - -W ignore::UserWarning:torch.utils.data.dataloader:558 \ - -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ - && rm -rf tests/outputs outputs - - - name: Test with pytest - run: | - pytest tests/test_control_robot.py -v --cov=./lerobot --durations=0 \ + pytest tests -v --cov=./lerobot --durations=0 \ -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ -W ignore::UserWarning:torch.utils.data.dataloader:558 \ -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ && rm -rf tests/outputs outputs - pytest-minimal: name: Pytest (minimal install) runs-on: ubuntu-latest From 675d4286c8f81578bf35ee50d36b4f980825548f Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 16:20:00 +0200 Subject: [PATCH 41/53] add --- .github/workflows/test.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f7090c59a..99a83c62d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,6 +11,7 @@ on: - ".github/**" - "poetry.lock" - "Makefile" + - ".cache/**" push: branches: - main @@ -21,6 +22,7 @@ on: - ".github/**" - "poetry.lock" - "Makefile" + - ".cache/**" jobs: pytest: @@ -34,6 +36,10 @@ jobs: with: lfs: true # Ensure LFS files are pulled + - name: Ensure required file is fetched + run: | + git checkout origin/main -- .cache/calibration/aloha_default/left_follower.json + - name: Install apt dependencies run: sudo apt-get update && sudo apt-get install -y libegl1-mesa-dev ffmpeg From 76cc47956ac4afab2a49c7116b3eeb9b2b4edf04 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 16:21:27 +0200 Subject: [PATCH 42/53] add --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 99a83c62d..d8d24f598 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -38,6 +38,7 @@ jobs: - name: Ensure required file is fetched run: | + git fetch origin main git checkout origin/main -- .cache/calibration/aloha_default/left_follower.json - name: Install apt dependencies From 50a979d6de92d1ee911aca99ff7dbd688dace54b Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 16:33:58 +0200 Subject: [PATCH 43/53] Check if file exists --- .github/workflows/test.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d8d24f598..4ff82456d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -35,12 +35,21 @@ jobs: - uses: actions/checkout@v4 with: lfs: true # Ensure LFS files are pulled + fetch-depth: 0 # Ensure the full history is fetched, not just the latest commit - name: Ensure required file is fetched run: | git fetch origin main git checkout origin/main -- .cache/calibration/aloha_default/left_follower.json + - name: Check if file exists + run: | + if [ -f .cache/calibration/aloha_default/left_follower.json ]; then + echo "File exists"; + else + echo "File does not exist"; + fi + - name: Install apt dependencies run: sudo apt-get update && sudo apt-get install -y libegl1-mesa-dev ffmpeg From 9dea00ee9e465f8dbc94ceb342f9595ef474e0d1 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 16:39:53 +0200 Subject: [PATCH 44/53] retest --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4ff82456d..bee8c99c3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -40,11 +40,11 @@ jobs: - name: Ensure required file is fetched run: | git fetch origin main - git checkout origin/main -- .cache/calibration/aloha_default/left_follower.json + git checkout origin/main -- .cache/calibration/aloha_default - name: Check if file exists run: | - if [ -f .cache/calibration/aloha_default/left_follower.json ]; then + if [ -f .cache/calibration/aloha_default/right_follower.json ]; then echo "File exists"; else echo "File does not exist"; From 2e694fcf8f014af9ac9146d4db3962a693d06378 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 16:56:53 +0200 Subject: [PATCH 45/53] test --- .github/workflows/test.yml | 2 +- tests/test_robots.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bee8c99c3..7215110ba 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -70,7 +70,7 @@ jobs: - name: Test with pytest run: | - pytest tests -v --cov=./lerobot --durations=0 \ + pytest tests/test_cameras.py tests/test_control_robot.py tests/test_motors.py tests/test_robots.py -v --cov=./lerobot --durations=0 \ -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ -W ignore::UserWarning:torch.utils.data.dataloader:558 \ -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ diff --git a/tests/test_robots.py b/tests/test_robots.py index 93c1a1fe6..0b33d8ea3 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -44,6 +44,8 @@ def test_robot(tmpdir, request, robot_type, mock): # To simplify unit test, we do not rerun manual calibration for Aloha mock=True. # Instead, we use the files from '.cache/calibration/aloha_default' overrides_calibration_dir = None + + assert Path(".cache/calibration/aloha_default/left_follower.json").exists() else: if mock: request.getfixturevalue("patch_builtins_input") From 88c2ed419e7dd1642fc33714b2c93220a5d37a45 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 17:03:27 +0200 Subject: [PATCH 46/53] fix unit tests --- .github/workflows/test.yml | 13 ------------- tests/test_control_robot.py | 8 ++++++-- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7215110ba..0eba0c260 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -37,19 +37,6 @@ jobs: lfs: true # Ensure LFS files are pulled fetch-depth: 0 # Ensure the full history is fetched, not just the latest commit - - name: Ensure required file is fetched - run: | - git fetch origin main - git checkout origin/main -- .cache/calibration/aloha_default - - - name: Check if file exists - run: | - if [ -f .cache/calibration/aloha_default/right_follower.json ]; then - echo "File exists"; - else - echo "File does not exist"; - fi - - name: Install apt dependencies run: sudo apt-get update && sudo apt-get install -y libegl1-mesa-dev ffmpeg diff --git a/tests/test_control_robot.py b/tests/test_control_robot.py index 407a32907..fc679528e 100644 --- a/tests/test_control_robot.py +++ b/tests/test_control_robot.py @@ -49,11 +49,15 @@ def test_teleoperate(request, robot_type, mock): @pytest.mark.parametrize("robot_type, mock", TEST_ROBOT_TYPES) @require_robot -def test_calibrate(request, robot_type, mock): +def test_calibrate(tmpdir, request, robot_type, mock): if mock: request.getfixturevalue("patch_builtins_input") - robot = make_robot(robot_type, mock=mock) + tmpdir = Path(tmpdir) + calibration_dir = tmpdir / robot_type + overrides_calibration_dir = [f"calibration_dir={calibration_dir}"] + + robot = make_robot(robot_type, overrides=overrides_calibration_dir, mock=mock) calibrate(robot, arms=get_available_arms(robot)) del robot From cc5c6231796f4894cca36850d03169f1cc6b9e6d Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 17:12:40 +0200 Subject: [PATCH 47/53] test --- .github/workflows/test.yml | 2 +- tests/test_motors.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0eba0c260..d331188e1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -57,7 +57,7 @@ jobs: - name: Test with pytest run: | - pytest tests/test_cameras.py tests/test_control_robot.py tests/test_motors.py tests/test_robots.py -v --cov=./lerobot --durations=0 \ + pytest tests/test_robots.py -v --cov=./lerobot --durations=0 \ -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ -W ignore::UserWarning:torch.utils.data.dataloader:558 \ -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ diff --git a/tests/test_motors.py b/tests/test_motors.py index f3f5650cf..672234071 100644 --- a/tests/test_motors.py +++ b/tests/test_motors.py @@ -37,7 +37,7 @@ @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) @require_motor -def test_find_port_mock(request, motor_type, mock): +def test_find_port(request, motor_type, mock): if mock: request.getfixturevalue("patch_builtins_input") with pytest.raises(OSError): @@ -48,7 +48,7 @@ def test_find_port_mock(request, motor_type, mock): @pytest.mark.parametrize("motor_type, mock", TEST_MOTOR_TYPES) @require_motor -def test_configure_motors_all_ids_1_mock(request, motor_type, mock): +def test_configure_motors_all_ids_1(request, motor_type, mock): if mock: request.getfixturevalue("patch_builtins_input") From 2c9defabdd3e6855f5562572aba36b706144db71 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 17:15:21 +0200 Subject: [PATCH 48/53] test --- .github/workflows/test.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d331188e1..5fbfe7a8e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -35,7 +35,6 @@ jobs: - uses: actions/checkout@v4 with: lfs: true # Ensure LFS files are pulled - fetch-depth: 0 # Ensure the full history is fetched, not just the latest commit - name: Install apt dependencies run: sudo apt-get update && sudo apt-get install -y libegl1-mesa-dev ffmpeg @@ -57,7 +56,7 @@ jobs: - name: Test with pytest run: | - pytest tests/test_robots.py -v --cov=./lerobot --durations=0 \ + pytest tests/test_control_robot.py tests/test_robots.py -v --cov=./lerobot --durations=0 \ -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ -W ignore::UserWarning:torch.utils.data.dataloader:558 \ -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ From bc479cb2d40244c6415d79ac0919908877fa5721 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 17:22:51 +0200 Subject: [PATCH 49/53] test --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5fbfe7a8e..5f22a8b87 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -56,7 +56,7 @@ jobs: - name: Test with pytest run: | - pytest tests/test_control_robot.py tests/test_robots.py -v --cov=./lerobot --durations=0 \ + pytest tests/test_motors.py tests/test_robots.py -v --cov=./lerobot --durations=0 \ -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ -W ignore::UserWarning:torch.utils.data.dataloader:558 \ -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ From 0e63f7c1b59a03a08a3fda3e6fda50ed1c5581f5 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 17:42:48 +0200 Subject: [PATCH 50/53] test --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5f22a8b87..cdb39742e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -56,7 +56,7 @@ jobs: - name: Test with pytest run: | - pytest tests/test_motors.py tests/test_robots.py -v --cov=./lerobot --durations=0 \ + pytest tests/test_cameras.py tests/test_motors.py tests/test_control_robot.py tests/test_robots.py -v --cov=./lerobot --durations=0 \ -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ -W ignore::UserWarning:torch.utils.data.dataloader:558 \ -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ From 83cfe60783f4b9ecc6bc25a4bf4a499fd3b5c426 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Fri, 27 Sep 2024 17:46:49 +0200 Subject: [PATCH 51/53] tests --- .github/workflows/test.yml | 2 +- tests/test_robots.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cdb39742e..953b1e4c3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -56,7 +56,7 @@ jobs: - name: Test with pytest run: | - pytest tests/test_cameras.py tests/test_motors.py tests/test_control_robot.py tests/test_robots.py -v --cov=./lerobot --durations=0 \ + pytest tests -v --cov=./lerobot --durations=0 \ -W ignore::DeprecationWarning:imageio_ffmpeg._utils:7 \ -W ignore::UserWarning:torch.utils.data.dataloader:558 \ -W ignore::UserWarning:gymnasium.utils.env_checker:247 \ diff --git a/tests/test_robots.py b/tests/test_robots.py index 0b33d8ea3..93c1a1fe6 100644 --- a/tests/test_robots.py +++ b/tests/test_robots.py @@ -44,8 +44,6 @@ def test_robot(tmpdir, request, robot_type, mock): # To simplify unit test, we do not rerun manual calibration for Aloha mock=True. # Instead, we use the files from '.cache/calibration/aloha_default' overrides_calibration_dir = None - - assert Path(".cache/calibration/aloha_default/left_follower.json").exists() else: if mock: request.getfixturevalue("patch_builtins_input") From 5c73bec91388f2be2e0e24325b1fa32d25208794 Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Sat, 28 Sep 2024 13:11:17 +0200 Subject: [PATCH 52/53] Address Jess comments --- .../robot_devices/cameras/intelrealsense.py | 9 ++++---- .../common/robot_devices/cameras/opencv.py | 14 +++++------- .../common/robot_devices/motors/dynamixel.py | 22 +++++++++---------- tests/{mock_opencv.py => mock_cv2.py} | 0 ...ock_dynamixel.py => mock_dynamixel_sdk.py} | 0 ...intelrealsense.py => mock_pyrealsense2.py} | 0 tests/utils.py | 5 +++-- 7 files changed, 23 insertions(+), 27 deletions(-) rename tests/{mock_opencv.py => mock_cv2.py} (100%) rename tests/{mock_dynamixel.py => mock_dynamixel_sdk.py} (100%) rename tests/{mock_intelrealsense.py => mock_pyrealsense2.py} (100%) diff --git a/lerobot/common/robot_devices/cameras/intelrealsense.py b/lerobot/common/robot_devices/cameras/intelrealsense.py index 5d6045242..c8bb5f070 100644 --- a/lerobot/common/robot_devices/cameras/intelrealsense.py +++ b/lerobot/common/robot_devices/cameras/intelrealsense.py @@ -33,7 +33,7 @@ def find_camera_indices(raise_when_empty=True, mock=False) -> list[int]: connected to the computer. """ if mock: - from tests.mock_intelrealsense import ( + from tests.mock_pyrealsense2 import ( RSCameraInfo, RSContext, ) @@ -82,7 +82,7 @@ def save_images_from_cameras( camera_ids = find_camera_indices(mock=mock) if mock: - from tests.mock_opencv import COLOR_RGB2BGR, cvtColor + from tests.mock_cv2 import COLOR_RGB2BGR, cvtColor else: from cv2 import COLOR_RGB2BGR, cvtColor @@ -263,7 +263,7 @@ def connect(self): ) if self.mock: - from tests.mock_intelrealsense import ( + from tests.mock_pyrealsense2 import ( RSConfig, RSFormat, RSPipeline, @@ -373,7 +373,7 @@ 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": if self.mock: - from tests.mock_opencv import COLOR_RGB2BGR, cvtColor + from tests.mock_cv2 import COLOR_RGB2BGR, cvtColor else: from cv2 import COLOR_RGB2BGR, cvtColor @@ -430,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()): diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index 9ff47b6a6..5c2f6460d 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -47,7 +47,7 @@ def find_camera_indices(raise_when_empty=False, max_index_search_range=MAX_OPENC possible_camera_ids = range(max_index_search_range) if mock: - from tests.mock_opencv import VideoCapture + from tests.mock_cv2 import VideoCapture else: from cv2 import VideoCapture @@ -244,24 +244,24 @@ def connect(self): raise RobotDeviceAlreadyConnectedError(f"OpenCVCamera({self.camera_index}) is already connected.") if self.mock: - from tests.mock_opencv import ( + from tests.mock_cv2 import ( CAP_PROP_FPS, CAP_PROP_FRAME_HEIGHT, CAP_PROP_FRAME_WIDTH, VideoCapture, ) else: - import cv2 from cv2 import ( CAP_PROP_FPS, CAP_PROP_FRAME_HEIGHT, CAP_PROP_FRAME_WIDTH, VideoCapture, + setNumThreads, ) # Use 1 thread to avoid blocking the main thread. Especially useful during data collection # when other threads are used to save the images. - cv2.setNumThreads(1) + setNumThreads(1) camera_idx = f"/dev/video{self.camera_index}" if platform.system() == "Linux" else self.camera_index with self.lock: @@ -356,7 +356,7 @@ def read(self, temporary_color_mode: str | None = None) -> np.ndarray: # so we convert the image color from BGR to RGB. if requested_color_mode == "rgb": if self.mock: - from tests.mock_opencv import COLOR_BGR2RGB, cvtColor + from tests.mock_cv2 import COLOR_BGR2RGB, cvtColor else: from cv2 import COLOR_BGR2RGB, cvtColor @@ -409,10 +409,6 @@ def async_read(self): num_tries += 1 if num_tries > self.fps * 2: raise TimeoutError("Timed out waiting for async_read() to start.") - # if num_tries > self.fps and (self.thread.ident is None or not self.thread.is_alive()): - # raise Exception( - # "The thread responsible for `self.async_read()` took too much time to start. There might be an issue. Verify that `self.thread.start()` has been called." - # ) def disconnect(self): if not self.is_connected: diff --git a/lerobot/common/robot_devices/motors/dynamixel.py b/lerobot/common/robot_devices/motors/dynamixel.py index b6dd8affd..0c3145028 100644 --- a/lerobot/common/robot_devices/motors/dynamixel.py +++ b/lerobot/common/robot_devices/motors/dynamixel.py @@ -143,8 +143,6 @@ NUM_READ_RETRY = 10 NUM_WRITE_RETRY = 10 -COMM_SUCCESS = 0 - def convert_degrees_to_steps(degrees: float | np.ndarray, models: str | list[str]) -> np.ndarray: """This function converts the degree range to the step range for indicating motors rotation. @@ -363,7 +361,7 @@ def connect(self): ) if self.mock: - from tests.mock_dynamixel import PacketHandler, PortHandler + from tests.mock_dynamixel_sdk import PacketHandler, PortHandler else: from dynamixel_sdk import PacketHandler, PortHandler @@ -401,7 +399,7 @@ def connect(self): def reconnect(self): if self.mock: - from tests.mock_dynamixel import PacketHandler, PortHandler + from tests.mock_dynamixel_sdk import PacketHandler, PortHandler else: from dynamixel_sdk import PacketHandler, PortHandler @@ -797,9 +795,9 @@ def revert_calibration(self, values: np.ndarray | list, motor_names: list[str] | def _read_with_motor_ids(self, motor_models, motor_ids, data_name): if self.mock: - from tests.mock_dynamixel import GroupSyncRead + from tests.mock_dynamixel_sdk import COMM_SUCCESS, GroupSyncRead else: - from dynamixel_sdk import GroupSyncRead + from dynamixel_sdk import COMM_SUCCESS, GroupSyncRead return_list = True if not isinstance(motor_ids, list): @@ -838,9 +836,9 @@ def read(self, data_name, motor_names: str | list[str] | None = None): start_time = time.perf_counter() if self.mock: - from tests.mock_dynamixel import GroupSyncRead + from tests.mock_dynamixel_sdk import COMM_SUCCESS, GroupSyncRead else: - from dynamixel_sdk import GroupSyncRead + from dynamixel_sdk import COMM_SUCCESS, GroupSyncRead if motor_names is None: motor_names = self.motor_names @@ -902,9 +900,9 @@ def read(self, data_name, motor_names: str | list[str] | None = None): def _write_with_motor_ids(self, motor_models, motor_ids, data_name, values): if self.mock: - from tests.mock_dynamixel import GroupSyncWrite + from tests.mock_dynamixel_sdk import COMM_SUCCESS, GroupSyncWrite else: - from dynamixel_sdk import GroupSyncWrite + from dynamixel_sdk import COMM_SUCCESS, GroupSyncWrite if not isinstance(motor_ids, list): motor_ids = [motor_ids] @@ -934,9 +932,9 @@ def write(self, data_name, values: int | float | np.ndarray, motor_names: str | start_time = time.perf_counter() if self.mock: - from tests.mock_dynamixel import GroupSyncWrite + from tests.mock_dynamixel_sdk import COMM_SUCCESS, GroupSyncWrite else: - from dynamixel_sdk import GroupSyncWrite + from dynamixel_sdk import COMM_SUCCESS, GroupSyncWrite if motor_names is None: motor_names = self.motor_names diff --git a/tests/mock_opencv.py b/tests/mock_cv2.py similarity index 100% rename from tests/mock_opencv.py rename to tests/mock_cv2.py diff --git a/tests/mock_dynamixel.py b/tests/mock_dynamixel_sdk.py similarity index 100% rename from tests/mock_dynamixel.py rename to tests/mock_dynamixel_sdk.py diff --git a/tests/mock_intelrealsense.py b/tests/mock_pyrealsense2.py similarity index 100% rename from tests/mock_intelrealsense.py rename to tests/mock_pyrealsense2.py diff --git a/tests/utils.py b/tests/utils.py index b013a6b41..69a1743c7 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -204,7 +204,7 @@ def test_require_robot(request, robot_type): @wraps(func) def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture + # Access the pytest request context to get the is_robot_available fixture request = kwargs.get("request") robot_type = kwargs.get("robot_type") mock = kwargs.get("mock") @@ -228,6 +228,7 @@ def wrapper(*args, **kwargs): def require_camera(func): @wraps(func) def wrapper(*args, **kwargs): + # Access the pytest request context to get the is_camera_available fixture request = kwargs.get("request") camera_type = kwargs.get("camera_type") mock = kwargs.get("mock") @@ -250,7 +251,7 @@ def wrapper(*args, **kwargs): def require_motor(func): @wraps(func) def wrapper(*args, **kwargs): - # Access the pytest request context to get the mockeypatch fixture + # Access the pytest request context to get the is_motor_available fixture request = kwargs.get("request") motor_type = kwargs.get("motor_type") mock = kwargs.get("mock") From 95a708cd05c6923adef846743fa2153e127d26dd Mon Sep 17 00:00:00 2001 From: Remi Cadene Date: Sat, 28 Sep 2024 15:41:15 +0200 Subject: [PATCH 53/53] Fix slow fps --- lerobot/common/robot_devices/cameras/opencv.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index 5c2f6460d..35e548ca5 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -401,9 +401,9 @@ def async_read(self): num_tries = 0 while True: - with self.lock: - if self.color_image is not None: - return self.color_image + # Do not use `with self.lock` here, as it reduces fps + if self.color_image is not None: + return self.color_image time.sleep(1 / self.fps) num_tries += 1