Claude/fix ur7e robot connection zwkk5 - #81
Open
Angelicahsiao wants to merge 24 commits into
Open
Conversation
Adds MultiDofGripper and MultiDofGripperConfig that publish a Float64MultiArray with N joint targets and read N joint indices from a JointState topic, normalizing each joint independently against per-joint min/max bounds.
- RobotConfig: has_effort_feedback flag (default False, opt-in per robot) - Robot: extract effort from JointState msg when flag is set - Robot: add _ros_msg_to_joint_effort() helper and current_joint_effort property https://claude.ai/code/session_01JjRhmpJGHK4phM65xkjwtM
from_yaml whitelisted fields and silently dropped use_gripper_command_action and max_effort, so any action-based gripper (Robotiq) loaded via yaml fell back to topic publishing. Now both fields are read (defaults False / 10.0). Affects the existing 2F-85 config too. https://claude.ai/code/session_01JjRhmpJGHK4phM65xkjwtM
…_yaml from_yaml built its config dict by hand and dropped use_gripper_command_action and max_effort, so a YAML setting use_gripper_command_action: true was ignored and the gripper fell back to topic publishing (Float64MultiArray). For a GripperActionController (action-only, e.g. UR Robotiq) the commands then went nowhere - 0 action clients, gripper never moved. Parse both fields from YAML. https://claude.ai/code/session_01JjRhmpJGHK4phM65xkjwtM
- RobotConfig: has_effort_feedback flag (default False, opt-in per robot) - Robot: extract effort from JointState msg when flag is set - Robot: add _ros_msg_to_joint_effort() helper and current_joint_effort property https://claude.ai/code/session_01JjRhmpJGHK4phM65xkjwtM
…tion-Zwkk5 Claude/fix ur7e robot connection zwkk5
Claude/add dg3f crisp gym id5lh
Adds the continuous 6D rotation representation (first two rows of the rotation matrix, pytorch3d/UMI convention; Zhou et al. 2019) to OrientationRepresentation and Pose.to_array via to_pos_rotation_6d_array. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147TJ8VwLLTD6mHLHhh3GC5
HANDOFF.md documents what must not change silently (ROTATION_6D first-two-rows convention, GripperConfig.from_yaml field parity, target_frame/TCP contract), the agreed registry-based extensibility direction with the sensor registry as the pattern to copy, known open bugs, the ROS-coupling map, and process rules. CLAUDE.md points to it and mirrors the crisp_gym session rules. https://claude.ai/code/session_0147TJ8VwLLTD6mHLHhh3GC5
Robot.home() hardcoded the literal "joint_trajectory_controller" both in the switch call and in the FollowJointTrajectory action path, breaking robots whose trajectory controller is named differently. - RobotConfig: new home_controller_name field (default keeps the legacy name). Note: joint_trajectory_controller_name could NOT be reused — despite its name it is the parameter-client target of the streaming joint controller (default "joint_impedance_controller"). - JointTrajectoryControllerClient: controller_name parameter for the action path (default unchanged). - Robot: home() switches to config.home_controller_name; the JTC client is constructed with it. https://claude.ai/code/session_0147TJ8VwLLTD6mHLHhh3GC5
- Gripper.reboot/_set_torque_holding: service_is_ready was referenced without calling it (bound method always truthy -> the not-ready guard could never fire). Added the missing parentheses; MultiDofGripper already did this correctly. - GripperConfig.from_yaml: replaced the manually-enumerated field dict with cls(**data) — dataclass defaults apply, unknown YAML keys raise instead of being silently dropped, and the behavior now matches Gripper.from_yaml (single load path; this manual dict lost fields twice before). All bundled gripper YAML keys verified against the dataclass. - robot/__init__.py: export PandaConfig, URConfig, DynaArmConfig (existed but were missing from imports and __all__). - Docstrings: RobotConfig attributes now document home_controller_name, the misleading joint_trajectory_controller_name, has_effort_feedback; JointTrajectoryControllerClient class Args covers use_prefix and controller_name. https://claude.ai/code/session_0147TJ8VwLLTD6mHLHhh3GC5
Three .pyc files under crisp_py/config/__pycache__ were tracked in git — including a stale factory.cpython-311.pyc whose source module no longer exists — and showed up as perpetual uncommitted modifications. Removed from tracking and replaced the per-directory .gitignore entries with a global __pycache__/ + *.pyc rule. https://claude.ai/code/session_0147TJ8VwLLTD6mHLHhh3GC5
Torque holding and reboot are Dynamixel-style services (the historical default enable_torque_service points at dynamixel_hardware_interface), but ManipulatorBaseEnv.reset() calls gripper.enable_torque() on EVERY gripper — crashing UR+Robotiq setups after the service_is_ready() guard fix made the missing-service check actually work. New GripperConfig.torque_interface tri-state: None (default) -> best-effort: warn and skip when the service is missing false -> declared unsupported: skip silently true -> required: raise when the service is missing Gripper.reboot/_set_torque_holding and MultiDofGripper honor it; Dynamixel behavior with the service present is unchanged. Robotiq 2F-85 and Franka Hand configs declare torque_interface: false. Stub-tested: silent skip (false), warn-not-raise (None + missing service, the reported crash), raise (true + missing), unchanged call (service present), reboot parity, YAML round-trip. https://claude.ai/code/session_0147TJ8VwLLTD6mHLHhh3GC5
- parameters_client.wait_until_ready: wait for ALL three services (the 'not A and not B and not C' chain returned as soon as ANY ONE was ready, so get/set/list could hit a not-ready service). - Gripper.is_valid: lower-bound check normalized the already-normalized self.value; use the raw _value so out-of-range raw readings are caught. - Gripper._callback_joint_state: bounds-guard the joint index (wrong topic raised IndexError in the executor thread, silently killing the subscription); mirror MultiDofGripper's guard. - DummyCameraConfig: subclass CameraConfig (dataclass) so it carries crop_width/crop_height — Camera(config=None) previously raised AttributeError on the first image — and inherits validation. - Camera image callback: drop undecodable frames with a throttled warning instead of raising in the executor thread. - joint_trajectory_controller_client: preserve fractional goal times (int(seconds) truncated 2.5s->2s, <1s->0s); sleep in busy-wait loops; raise on rejected goals instead of deref'ing a None handle. - controller_switcher: sleep in the four busy-wait future loops.
robot.py: - add _pose_lock (RLock) guarding _current_pose/_target_pose across the executor callbacks, publish timer, user-thread readers, set_target, reset_targets and home() interpolation (previously unsynchronized). - current_joint_effort warns (throttled) when returning zeros because no effort was ever received — a silent all-zeros misconfig was indistinguishable from a real zero-torque reading. - shutdown() destroys the node this robot created (tracked via _owns_node) before the global rclpy.shutdown(), and documents the global semantics. parameters_client: replace synchronous client.call() (deadlock-prone with an externally spun node, no timeout) with a call_async + timeout helper used by list/get/set. controller_switcher: switch_controller now fails fast with a clear error when the controller_manager services are unavailable instead of hanging in the first service call. camera: add is_stale property (staleness was detected but not exposed; is_ready/current_image keep serving the last frame after an outage). camera_config: document/validate the (HEIGHT, WIDTH) resolution order — the code unpacks (h, w) and the camera-info fallback stores (msg.height, msg.width); square shipped configs masked the ambiguity.
- robot.py: single _joint_field_to_array helper replaces the three byte-identical JointState field parsers (positions/velocities/efforts). - gripper.py: remove dead ros_msg_to_gripper_value (unused; its own TODO admitted the duplication). - gripper_config.py: __post_init__ rejects min_value == max_value (zero calibration range -> divide-by-zero in _normalize); inverted ranges (Robotiq) remain valid. No shipped config is affected. - robot_config.py: document use_admittance_controller, cartesian_admittance_controller_name, target_admittance_stiffness_topic. - HANDOFF: mark GripperConfig.from_yaml + robot __init__ export drift as FIXED, record the (HEIGHT, WIDTH) camera resolution convention, the _pose_lock rule, and the Pose +/- world-frame-delta order-sensitivity with a pointer to crisp_gym tests/test_pose_math.py.
Gripper unification (HANDOFF-agreed direction, modeled on the sensor
registry):
- New gripper/gripper_base.py: GripperBase(ABC) holds all shared
machinery — node creation with ownership tracking (shutdown destroys
its own node), spin thread, CallbackMonitor, and ONE tri-state
torque_interface implementation for reboot/enable/disable_torque.
Blocking service calls use call_async + timeout instead of the
deadlock-prone synchronous call.
- Gripper and MultiDofGripper subclass it; each keeps its public
value/target protocol byte-identical (scalar vs per-joint arrays).
Deliberately preserved divergences: is_open thresholds (0.1 vs 0.5)
and target None-vs-raise semantics.
- Behavior fix: MultiDofGripper now RAISES on torque_interface: true
with missing services (previously silently ignored) and its
MultiDofGripperConfig.__post_init__ runs the parent zero-range guard.
- @register_gripper registry; make_gripper dispatches on the gripper
YAML's optional 'type:' key ('gripper' default, 'multi_dof') or on the
config instance type. No type key -> plain Gripper, exactly as before;
GripperConfig.from_yaml tolerates/consumes the dispatch key.
Camera transport (un-hardcodes /compressed):
- CameraConfig: image_transport ('compressed' default | 'raw'),
compressed_topic_suffix, image_encoding; validated in __post_init__.
- Camera subscribes CompressedImage on topic+suffix (unchanged default)
or sensor_msgs/Image on the topic as-is; both routes share the guarded
decode + resize pipeline and honor image_encoding.
- RAW transport logs a warning that uncompressed frames cost far more
bandwidth/storage than compressed.
- The resolution/info-topic print is now a proper logger message.
Tests: tests/unit/test_gripper_registry_camera_transport.py (20 tests) —
registry contents, YAML type dispatch incl. unknown-type rejection,
config-instance dispatch, torque tri-state truth table on BOTH classes
(incl. the MultiDof true->raise fix), owned-vs-external node shutdown,
scalar/per-joint normalization parity, camera transport selection with
suffix override, raw-transport storage warning, invalid transport
rejection, decode routing. Runs with real rclpy (Mock nodes) or with
module stubs where ROS is unavailable.
HANDOFF: gripper/camera extensibility rows marked DONE.
A home_config whose length differs from the robot's joint count is silently rejected by the trajectory controller (invisible with blocking=False — the robot just never moves; observed when a 7-joint Franka HomeConfig was sent to a 6-joint UR). Raise a clear ValueError before sending instead.
crisp_py's pyproject declares scipy as a pip requirement. In the humble env (numpy 1.26 from robostack) pixi/uv installed the PyPI scipy wheel (built against numpy 2.x), whose f2py modules fail at runtime (ImportError: cannot import name '_spropack' from scipy.sparse.linalg._propack). robostack provides scipy transitively, but uv only counts DIRECT conda deps as satisfying a pypi requirement, so it layered the pip wheel on top. Add scipy as a direct conda dep in the humble feature so the conda build (matching numpy 1.26) is used and no pip scipy is installed. Humble-only by design: the jazzy feature keeps the upstream decision (pip scipy + a version warning, PR learnsyslab#63); this does not touch it. Requires a fresh solve (rm pixi.lock) to drop the stale pip scipy.
The GripperBase extraction moved the ROS node machinery (rclpy, CallbackMonitor)
into gripper_base.py, so the mock patches must target that module — the old
'crisp_py.gripper.gripper.rclpy' patches raised AttributeError (28 failures).
Repoint all patches to gripper_base via shared _RCLPY/_MONITOR constants.
Also update two contracts the tests still encoded from before earlier changes:
* GripperConfig.min_value/max_value are required (no silent 0/1 default), so
an empty YAML must raise TypeError — assert that instead of expecting
defaults.
* reboot/torque are tri-state (torque_interface None/True/False) and issue
every call via call_async (a sync client call deadlocks an externally spun
node). Rewrite the service tests to cover best-effort-skip, required-raise,
and capability-absent paths, and mock service_is_ready as the method it is.
Gripper registry refactor, robustness fixes, and dependency corrections
Mirrors crisp_gym's Dockerfile/docker-compose.yml: a minimal pixi image (runtime + X11/GL libs) with the repo bind-mounted and the pixi env created on first start via 'pixi install -e humble'. Adds a Docker section to the README covering the sibling ~/workspace layout, build/run commands, and the ROS_DOMAIN_ID / activation-script caveat. https://claude.ai/code/session_01JjRhmpJGHK4phM65xkjwtM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.