feat(tests): Add MockMotorBus to MockRobot#2081
Conversation
There was a problem hiding this comment.
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
MockMotorsBusinitialization to provide the same motor interface as real robots - Creates proper
Motorobjects for each configured motor in the mock - Maintains backward compatibility by preserving the existing
.motorsattribute
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
imstevenpmwork
left a comment
There was a problem hiding this comment.
LGTM, thanks !
I think the reason why MockRobot doesn't have the motor bus is because conceptually, not all Robot will have one. However, this seems like a simple addition without breaking any assumptions.
I see, makes a lot of sense! I think |
What this does
Adds the interface all robots have to interface the motors through the bus instead of directly to
MockRobottoo. 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 detailsSay 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
Error trace ⬇️ (after
git checkout main)After checking out to this branch, this bug is resolves and the mock robot is correctly wrapped in the robot environment by RL environments.