Skip to content

Conversation

fracapuano
Copy link
Collaborator

What this does

Adds the interface all robots have to interface the motors through the bus instead of directly to MockRobot too. This improves testing coverage because before this RL tests (how I have found out of this issue) could not run properly. See the MRE for more details

Say you want to test the RL setup using a mock robot (for testing purposes, or for development without a robot).
The current setup for mock robots does not adequately support testing because it does not implement the same mechanism to instantiate/retrieve the motors that we instead rely on for actual robots. This PR fixes this, which is a bug I have found out while developing self-contained tutorials for how to use RL in lerobot.

MRE

from tests.mocks.mock_robot import MockRobotConfig
from lerobot.teleoperators.keyboard import KeyboardTeleopConfig
from lerobot.envs.configs import HILSerlRobotEnvConfig, HILSerlProcessorConfig
from lerobot.rl.gym_manipulator import make_robot_env

# Robot and environment configuration
robot_cfg = MockRobotConfig(n_motors=3)
teleop_cfg = KeyboardTeleopConfig(mock=True)
processor_cfg = HILSerlProcessorConfig(control_mode="keyboard")

env_cfg = HILSerlRobotEnvConfig(
    robot=robot_cfg,
    teleop=teleop_cfg,
    processor=processor_cfg
)

# Create robot environment
env, teleop_device = make_robot_env(env_cfg)  # as this currently stands, this will raise!

Error trace ⬇️ (after git checkout main)

Traceback (most recent call last):
  File "/Users/fracapuano/Desktop/lerobot/prova.py", line 18, in <module>
    env, teleop_device = make_robot_env(env_cfg)  # as this currently stands, this will raise!
  File "/Users/fracapuano/Desktop/lerobot/src/lerobot/rl/gym_manipulator.py", line 344, in make_robot_env
    env = RobotEnv(
  File "/Users/fracapuano/Desktop/lerobot/src/lerobot/rl/gym_manipulator.py", line 153, in __init__
    self._joint_names = [f"{key}.pos" for key in self.robot.bus.motors]
AttributeError: 'MockRobot' object has no attribute 'bus'

After checking out to this branch, this bug is resolves and the mock robot is correctly wrapped in the robot environment by RL environments.

@fracapuano fracapuano added the bug Something isn’t working correctly label Sep 28, 2025
@fracapuano fracapuano changed the title Add MockMotorBus to MockRobot (bug) Add MockMotorBus to MockRobot Sep 28, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a critical bug in MockRobot by adding a MockMotorBus interface to match the actual robot implementation. The issue prevented RL environments from properly initializing with mock robots because they expected a .bus attribute that didn't exist.

  • Adds MockMotorsBus initialization to provide the same motor interface as real robots
  • Creates proper Motor objects for each configured motor in the mock
  • Maintains backward compatibility by preserving the existing .motors attribute

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

motor_name = f"motor_{i + 1}"
mock_motors[motor_name] = Motor(
id=i + 1,
model="model_1", # Use model_1 which exists in MockMotorsBus tables
Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

The hardcoded model name 'model_1' and its comment suggest tight coupling with MockMotorsBus internal implementation. Consider making this configurable or using a constant to avoid potential issues if MockMotorsBus changes its supported models.

Copilot uses AI. Check for mistakes.

norm_mode=MotorNormMode.RANGE_M100_100,
)

self.bus = MockMotorsBus("/dev/dummy-port", mock_motors)
Copy link
Preview

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

The hardcoded '/dev/dummy-port' path should be extracted to a constant or made configurable to improve maintainability and make the mock's behavior more explicit.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant