Address comments

lerobot/common/robot_devices/cameras/intelrealsense.py

@@ -288,7 +288,9 @@ class IntelRealSenseCamera:
         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}."
             )

lerobot/common/robot_devices/cameras/opencv.py

@@ -272,7 +272,9 @@ class OpenCVCamera:
         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}."
             )

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
 

tests/test_cameras.py

@@ -10,17 +10,19 @@ pytest -sx tests/test_cameras.py::test_camera
 
 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[opencv-False]'
-pytest -sx tests/test_cameras.py::test_camera[intelrealsense]
+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 'tests/test_cameras.py::test_camera[opencv-True]'
-pytest -sx -k "mocked_intelrealsense" tests/test_cameras.py::test_camera
+pytest -sx 'tests/test_cameras.py::test_camera[intelrealsense-True]'
 ```
 """
 
+import os
+
 import numpy as np
 import pytest
 
@@ -28,8 +30,8 @@ from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError,
 from tests.utils import TEST_CAMERA_TYPES, require_camera
 
 # Camera indices used for connecting physical cameras
-OPENCV_CAMERA_INDEX = 0
+OPENCV_CAMERA_INDEX = int(os.environ.get("LEROBOT_TEST_OPENCV_CAMERA_INDEX", 0))
-INTELREALSENSE_CAMERA_INDEX = 128422271614
+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

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.test_robots import make_robot
 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"
 

tests/test_motors.py

@@ -11,12 +11,12 @@ 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]
+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

tests/test_robots.py

@@ -10,18 +10,17 @@ 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-False]'
-pytest -sx tests/test_robots.py::test_robot[koch_bimanual]
+pytest -sx 'tests/test_robots.py::test_robot[koch_bimanual-False]'
-pytest -sx tests/test_robots.py::test_robot[aloha]
+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 'tests/test_robots.py::test_robot[koch-True]'
-pytest -sx -k "mocked_koch_bimanual" tests/test_robots.py::test_robot
+pytest -sx 'tests/test_robots.py::test_robot[koch_bimanual-True]'
-pytest -sx -k "mocked_aloha" tests/test_robots.py::test_robot
+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

tests/utils.py

@@ -30,9 +30,17 @@ DEFAULT_CONFIG_PATH = "lerobot/configs/default.yaml"
 
 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_ROBOT_TYPES = []
-TEST_CAMERA_TYPES = available_cameras + [f"mocked_{camera_type}" for camera_type in available_cameras]
+for robot_type in available_robots:
-TEST_MOTOR_TYPES = available_motors + [f"mocked_{motor_type}" for motor_type in available_motors]
+    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 require_robot(func):
         # 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 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.")
+        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 robot_type.startswith("mocked_"):
+        if mock:
-            kwargs["robot_type"] = robot_type.replace("mocked_", "")
             mock_cameras(request)
             mock_motors(request)
 
@@ -221,21 +230,22 @@ def require_camera(func):
         # 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 camera_type is None:
-            raise ValueError("The 'camera_type' must be an argument of the test function.")
-
-        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.")
+        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 camera_type.startswith("mocked_"):
+        if mock:
-            kwargs["camera_type"] = camera_type.replace("mocked_", "")
             mock_cameras(request)
 
         # Run test with a real robot. Skip test if robot connection fails.
@@ -255,21 +265,22 @@ def require_motor(func):
         # 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 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.")
+        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 motor_type.startswith("mocked_"):
+        if mock:
-            kwargs["motor_type"] = motor_type.replace("mocked_", "")
             mock_motors(request)
 
         # Run test with a real robot. Skip test if robot connection fails.