From 732a12108e2ec5a5cdbf6d2ce8393e6e6f8dac32 Mon Sep 17 00:00:00 2001 From: Lin Junrong Date: Fri, 31 Jul 2026 22:12:18 +0800 Subject: [PATCH] fix(so_follower): check the observation for None before copying it (#4255) Four kinematic processor steps read the observation as observation = self.transition.get(TransitionKey.OBSERVATION).copy() if observation is None: raise ValueError("Joints observation is require for computing robot kinematics") so `.copy()` runs first and the guard below it is unreachable. A transition without an observation raises `AttributeError: 'NoneType' object has no attribute 'copy'` instead of the intended message. That transition is not hypothetical: `RobotProcessorPipeline.process_action` builds one with `create_transition(action=action)`, which sets `TransitionKey.OBSERVATION` to None. Reads the value first, checks it, then copies. Affects EEReferenceAndDelta, InverseKinematicsEEToJoints, GripperVelocityToJoint and InverseKinematicsRLStep. Adds a parametrised regression test covering all four; each fails with the AttributeError if the fix is reverted. Co-authored-by: Steven Palma --- .../so_follower/robot_kinematic_processor.py | 24 +++++++---- .../robots/test_robot_kinematic_processor.py | 40 +++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/lerobot/robots/so_follower/robot_kinematic_processor.py b/src/lerobot/robots/so_follower/robot_kinematic_processor.py index 50519d5e6..98bb54f67 100644 --- a/src/lerobot/robots/so_follower/robot_kinematic_processor.py +++ b/src/lerobot/robots/so_follower/robot_kinematic_processor.py @@ -77,11 +77,13 @@ class EEReferenceAndDelta(RobotActionProcessorStep): _command_when_disabled: np.ndarray | None = field(default=None, init=False, repr=False) def action(self, action: RobotAction) -> RobotAction: - observation = self.transition.get(TransitionKey.OBSERVATION).copy() + raw_observation = self.transition.get(TransitionKey.OBSERVATION) - if observation is None: + if raw_observation is None: raise ValueError("Joints observation is require for computing robot kinematics") + observation = raw_observation.copy() + if self.use_ik_solution and "IK_solution" in self.transition.get(TransitionKey.COMPLEMENTARY_DATA): q_raw = self.transition.get(TransitionKey.COMPLEMENTARY_DATA)["IK_solution"] else: @@ -311,10 +313,12 @@ class InverseKinematicsEEToJoints(RobotActionProcessorStep): "Missing required end-effector pose components: ee.x, ee.y, ee.z, ee.wx, ee.wy, ee.wz, ee.gripper_pos must all be present in action" ) - observation = self.transition.get(TransitionKey.OBSERVATION).copy() - if observation is None: + raw_observation = self.transition.get(TransitionKey.OBSERVATION) + if raw_observation is None: raise ValueError("Joints observation is require for computing robot kinematics") + observation = raw_observation.copy() + q_raw = np.array( [float(v) for k, v in observation.items() if isinstance(k, str) and k.endswith(".pos")], dtype=float, @@ -391,13 +395,15 @@ class GripperVelocityToJoint(RobotActionProcessorStep): discrete_gripper: bool = False def action(self, action: RobotAction) -> RobotAction: - observation = self.transition.get(TransitionKey.OBSERVATION).copy() + raw_observation = self.transition.get(TransitionKey.OBSERVATION) gripper_vel = action.pop("ee.gripper_vel") - if observation is None: + if raw_observation is None: raise ValueError("Joints observation is require for computing robot kinematics") + observation = raw_observation.copy() + q_raw = np.array( [float(v) for k, v in observation.items() if isinstance(k, str) and k.endswith(".pos")], dtype=float, @@ -583,10 +589,12 @@ class InverseKinematicsRLStep(ProcessorStep): "Missing required end-effector pose components: ee.x, ee.y, ee.z, ee.wx, ee.wy, ee.wz, ee.gripper_pos must all be present in action" ) - observation = new_transition.get(TransitionKey.OBSERVATION).copy() - if observation is None: + raw_observation = new_transition.get(TransitionKey.OBSERVATION) + if raw_observation is None: raise ValueError("Joints observation is require for computing robot kinematics") + observation = raw_observation.copy() + q_raw = np.array( [float(v) for k, v in observation.items() if isinstance(k, str) and k.endswith(".pos")], dtype=float, diff --git a/tests/robots/test_robot_kinematic_processor.py b/tests/robots/test_robot_kinematic_processor.py index c8dd4c77a..7a8183c21 100644 --- a/tests/robots/test_robot_kinematic_processor.py +++ b/tests/robots/test_robot_kinematic_processor.py @@ -17,9 +17,14 @@ import pytest from lerobot.configs import FeatureType, PipelineFeatureType, PolicyFeature +from lerobot.processor.converters import create_transition from lerobot.robots.so_follower.robot_kinematic_processor import ( + EEReferenceAndDelta, ForwardKinematicsJointsToEEAction, ForwardKinematicsJointsToEEObservation, + GripperVelocityToJoint, + InverseKinematicsEEToJoints, + InverseKinematicsRLStep, ) MOTOR_NAMES = ["shoulder_pan", "shoulder_lift", "elbow_flex", "wrist_flex", "wrist_roll", "gripper"] @@ -43,3 +48,38 @@ def test_fk_feature_schema(step_cls, bucket, feature_type): out = step_cls(kinematics=None, motor_names=MOTOR_NAMES).transform_features(features)[bucket] assert set(out) == EE_KEYS assert {feature.type for feature in out.values()} == {feature_type} + + +EE_ACTION = dict.fromkeys(EE_KEYS, 0.0) + + +@pytest.mark.parametrize( + ("step", "action"), + [ + ( + EEReferenceAndDelta(kinematics=None, end_effector_step_sizes={}, motor_names=MOTOR_NAMES), + dict(EE_ACTION), + ), + (InverseKinematicsEEToJoints(kinematics=None, motor_names=MOTOR_NAMES), dict(EE_ACTION)), + (GripperVelocityToJoint(), {**EE_ACTION, "ee.gripper_vel": 0.0}), + (InverseKinematicsRLStep(kinematics=None, motor_names=MOTOR_NAMES), dict(EE_ACTION)), + ], + ids=[ + "ee_reference_and_delta", + "inverse_kinematics_ee_to_joints", + "gripper_velocity_to_joint", + "inverse_kinematics_rl_step", + ], +) +def test_missing_observation_raises_value_error(step, action): + """A transition without an observation must surface the documented ValueError. + + `RobotProcessorPipeline.process_action` builds its transition with + `create_transition(action=...)`, which sets `TransitionKey.OBSERVATION` to None. + These steps used to call `.copy()` on that before the None check, so the guard + below them was unreachable and an AttributeError escaped instead. + """ + transition = create_transition(action=action) + + with pytest.raises(ValueError, match="Joints observation"): + step(transition)