Add Haply device API with force feedback and teleoperation demo #3873
+932
−2
Conversation
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
github-actions
bot
added
isaac-mimic
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR introduces support for Haply haptic devices to Isaac Lab's teleoperation ecosystem, enabling 6-DOF robot control with bidirectional force feedback. The implementation adds a new HaplyDevice class following the existing DeviceBase pattern used by keyboard, gamepad, and spacemouse interfaces. The device communicates with Haply SDK via WebSocket in a separate thread to receive position/orientation from Inverse3 and VerseGrip hardware while sending force feedback commands. A standalone demo script (haply_teleoperation.py) demonstrates real-time teleoperation of a Franka Panda robot using differential IK control, mapping the Haply workspace to robot end-effector poses with contact-based force feedback. The changes integrate cleanly into the existing device factory pattern without modifying core teleoperation infrastructure.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/init.py | 5/5 | Adds Haply device documentation and imports following existing device pattern |
| source/isaaclab/isaaclab/devices/haply/init.py | 5/5 | Creates new Haply module initialization exporting HaplyDevice and HaplyDeviceCfg |
| source/isaaclab/isaaclab/devices/teleop_device_factory.py | 5/5 | Registers HaplyDeviceCfg in device factory mapping for instantiation |
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 3.5/5 | Implements core Haply device controller with WebSocket communication and force feedback (hardcoded availability flag, potential connection logic issue) |
| scripts/demos/haply_teleoperation.py | 3/5 | Adds teleoperation demo with Franka Panda (hardcoded transforms, potential joint index conflict at line 318-320, force feedback always computed but conditionally applied) |
Confidence score: 3/5
- This PR requires careful review due to implementation issues in core device logic and the demo script
- Score lowered due to: (1) hardcoded WEBSOCKETS_AVAILABLE flag that doesn't respect import failures in se3_haply.py, (2) connection logic using OR condition may allow operation with incomplete device setup, (3) potential joint index conflict in haply_teleoperation.py where joint 7 (index 6) could be set twice based on arm DOF, (4) hardcoded workspace mappings without validation, (5) no graceful WebSocket thread shutdown mechanism
- Pay close attention to source/isaaclab/isaaclab/devices/haply/se3_haply.py (lines 18-20, 81-82, 264-266, 292-294) and scripts/demos/haply_teleoperation.py (lines211, 318-320) for potential runtime issues
Sequence Diagram
sequenceDiagram
participant User
participant HaplyDevice
participant WebSocket
participant SimulationContext
participant Robot
participant IKController
participant ContactSensor
User->>HaplyDevice: Initialize connection (websocket_uri)
HaplyDevice->>WebSocket: Connect to Haply SDK
WebSocket-->>HaplyDevice: Connection established
HaplyDevice->>HaplyDevice: Start background thread
User->>SimulationContext: Start simulation
SimulationContext->>Robot: Initialize Franka Panda
SimulationContext->>ContactSensor: Setup finger sensors
loop Simulation Loop
User->>HaplyDevice: Move handle / Press buttons
HaplyDevice->>WebSocket: Request device state
WebSocket-->>HaplyDevice: Return position, orientation, buttons
HaplyDevice->>HaplyDevice: Cache data with lock
SimulationContext->>HaplyDevice: advance()
HaplyDevice-->>SimulationContext: Return [pos, quat, buttons]
SimulationContext->>SimulationContext: apply_haply_to_robot_mapping()
SimulationContext->>IKController: compute(target_pos, target_quat)
IKController-->>SimulationContext: Return joint_pos_des
SimulationContext->>Robot: set_joint_position_target()
Robot->>Robot: Update joint positions
alt Force Feedback Enabled
ContactSensor->>ContactSensor: Measure net_forces_w
SimulationContext->>ContactSensor: Get finger contact forces
ContactSensor-->>SimulationContext: Return force vectors
SimulationContext->>SimulationContext: Calculate feedback_force
SimulationContext->>HaplyDevice: set_force_feedback(fx, fy, fz)
HaplyDevice->>WebSocket: Send force command
WebSocket-->>User: Apply haptic feedback
end
SimulationContext->>SimulationContext: step()
end
User->>SimulationContext: Close application
SimulationContext->>HaplyDevice: Cleanup
HaplyDevice->>WebSocket: Disconnect
5 files reviewed, 6 comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This review covers only the changes made since the last review, not the entire PR. The latest changes involve two focused updates: (1) removal of a quaternion conversion comment from the teleoperation demo script, and (2) comprehensive removal of all inline comments and section dividers from the HaplyDevice class implementation. The comment removal in se3_haply.py reduces code verbosity by eliminating explanatory text for operations like "Apply sensitivity scaling" and "Detect rising edge", while preserving all docstrings and functional logic. This makes the code more concise but relies entirely on method names and docstrings for understanding the WebSocket communication, threading, and force feedback implementation. The demo script change is minimal, removing a redundant comment about quaternion conversion. Both changes align with a code style preference for self-documenting code over inline annotations, though this may impact readability for contributors unfamiliar with haptic device protocols or the Haply WebSocket API.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| scripts/demos/haply_teleoperation.py | 4/5 | Removed quaternion conversion comment on line 117; purely cosmetic change with no logic modifications |
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 4/5 | Stripped all inline comments and section dividers while preserving docstrings; no functional changes but reduced code documentation |
Confidence score: 4/5
- This PR is safe to merge with minimal risk as the changes are purely cosmetic comment removals with no functional modifications
- Score reflects that while the changes reduce inline documentation, all core logic, docstrings, and functionality remain intact; no new bugs are introduced by these changes
- No files require special attention as both changes are straightforward comment removals; previous review concerns about logic issues (lines 317-319 data race) and performance (deep copying at 200Hz) remain unaddressed but are not affected by these latest changes
2 files reviewed, 2 comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This review covers only the changes made since the last review, not the entire PR. The most recent changes refactor the Haply teleoperation demo by removing the --enable_force_feedback CLI flag and making force feedback always-on. The se3_haply.py device API was enhanced with proper cleanup mechanisms, improved connection monitoring with timeout tracking, and optimized data processing by removing unnecessary deep copies. Key structural improvements include extracting workspace mapping constants to module-level scope, adding a close() method for graceful device shutdown with try-finally cleanup, and fixing the WebSocket message format to use proper list structure. The connection logic was changed from OR to AND semantics (requiring both Inverse3 and VerseGrip devices), which may affect partial-operation scenarios. The refactoring indicates maturation from experimental to production-ready feature status.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| scripts/demos/haply_teleoperation.py | 3/5 | Removes force feedback CLI flag, extracts workspace constants, adds try-finally cleanup; joint position overwrite issue on line 318-319 remains unresolved from previous review |
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 4/5 | Adds optional websockets import handling, implements close() method, improves connection monitoring with timeout tracking, removes deep copies in high-frequency loop; connection logic changed from OR to AND requiring both devices |
Confidence score: 3/5
- This PR has robustness improvements but contains an unresolved logic issue and a potentially breaking connection requirement change that needs verification
- Score reflects: (1) the unresolved joint position overwrite issue in haply_teleoperation.py line 318-319 that was flagged in previous reviews but not fixed, which may discard IK-computed values for joint 7; (2) the connection logic change from OR to AND semantics in se3_haply.py line 275-277 that now requires BOTH Inverse3 and VerseGrip devices to be connected, which is a breaking change that may not be intentional and needs verification; (3) the PR description still references the removed
--enable_force_feedbackflag in the usage example, creating documentation inconsistency - Pay close attention to haply_teleoperation.py lines 318-320for the joint position assignment logic and se3_haply.py lines 275-277 for the device connection requirements
2 files reviewed, 2 comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This review covers only changes made since the last review, focusing on thread management and cleanup improvements in the Haply device WebSocket connection handling. The changes address previous feedback by adding explicit thread joining with timeout in the close() method and ensuring _websocket_thread is initialized before the thread starts to prevent race conditions. The WebSocket thread is now non-daemon to enable proper cleanup through join(), though this requires that close() is called explicitly to avoid hanging the process at exit.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 3/5 | Improved WebSocket thread cleanup with explicit join and timeout handling; changed thread to non-daemon which requires explicit cleanup to prevent hangs |
Confidence score: 3/5
- This PR addresses previous feedback on thread cleanup but introduces a potential process hang if
close()is not called before exit - Score reflects improved cleanup logic offset by the non-daemon thread change requiring explicit cleanup, plus unaddressed thread safety concerns between
advance()andreset()methods accessing shared_prev_buttonsstate without synchronization - Pay close attention to the thread joining logic in
close()(lines 165-168) and verify thatclose()is reliably called in all exit paths; also verify thread safety of_prev_buttonsaccess betweenadvance()andreset()methods
1 file reviewed, no comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This review covers only the changes made since the last review, not the entire PR. The latest update adds thread-safe button state tracking to the Haply haptic device teleoperation API. A new _button_lock threading lock was introduced to protect concurrent access to the _prev_buttons dictionary, which tracks button states for rising-edge detection. The lock is used in both the reset() method and the advance() method where button press events are detected. This addresses a race condition where the WebSocket thread (running _websocket_loop()) updates button states while the main thread reads them in advance() for edge detection. The implementation follows the existing synchronization pattern used for force_lock and data_lock in the same class. Additionally, the cleanup logic in __del__ was enhanced to warn about daemon thread conversion if graceful shutdown fails.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 3/5 | Added _button_lock for thread-safe button state tracking to prevent race conditions during rising-edge detection |
Confidence score: 3/5
- This PR requires careful review due to potential issues with the thread safety implementation and callback execution within locked sections
- Score reflects concerns about holding locks during callback invocation (lines 236-245) which could cause deadlocks if callbacks take significant time or attempt to acquire the same lock, and the lack of explicit timeout or error handling for lock acquisition
- Pay close attention to the button event detection logic in
advance()method where callbacks are invoked while holding_button_lock, as this could block the WebSocket thread if callbacks are slow
1 file reviewed, 1 comment
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This review covers changes made since the last review, focusing on recent modifications to the Haply device controller. The refactor removes the connection_timeout parameter and time-based "data freshness" checks (get_device_state(), _is_data_fresh(), last_data_time) in favor of simpler binary connection state tracking. A critical logic change switches the connection check from OR to AND, now requiring both Inverse3 and VerseGrip devices to be connected simultaneously. Thread safety has been improved with a dedicated _connected_lock separating connection state from data synchronization, and the _button_lock was removed to allow callbacks to execute outside locks, addressing previous deadlock concerns. However, the refactor introduces race conditions where cached_data is accessed without holding data_lock, and array indexing operations lack bounds checking which could cause IndexErrors.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 2/5 | Refactored device connection management removing timeout logic, changed connection check from OR to AND, improved lock separation but introduced race conditions in cached_data access and missing bounds checks on array indexing |
Confidence score: 2/5
- This PR has significant thread-safety issues that could cause runtime failures or race conditions in production
- Score reflects critical bugs: (1)
cached_dataaccessed withoutdata_lockat lines 331-334 during connection status updates, (2) Array indexing at lines 288-289 without bounds checking could raise IndexError if WebSocket returns empty arrays, (3) Connection status check at lines 206-217 is separated from data access creating a window where device could disconnect between checks - Pay close attention to lines 288-289 (array indexing), lines 331-334 (unlocked cached_data access), and lines 206-217 (connection check race condition). The AND logic change at line 332-333 should also be verified as intentional behavior change requiring both devices
1 file reviewed, 2 comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This review covers only the changes made since the last review, not the entire PR. The developer addressed critical race conditions and thread safety issues in the Haply device teleoperation implementation. The primary fix involves consolidating connection status checks under proper lock synchronization: the advance() method now reads connection state inside data_lock to prevent stale reads, while the WebSocket loop computes connection status locally before atomically updating both cached_data and the self.connected flag. Additional improvements include safer handling of empty device data arrays to prevent IndexError exceptions, and consistent connection state resets in exception handlers under appropriate locks. These changes maintain the existing two-lock design pattern while ensuring thread-safe state transitions between the WebSocket thread and the main simulation thread.
Important Files Changed
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 4/5 | Fixed race condition by moving connection checks inside data_lock; improved empty list handling; made connection status updates atomic |
Confidence score: 4/5
- This PR is safer to merge than previous versions but still has some residual threading concerns that warrant attention.
- Score reflects successful resolution of the critical race condition in connection status checking, proper synchronization of state updates, and improved exception safety. However, one point deducted because the callback invocation inside
_button_lock(raised in previous reviews) remains unaddressed, which could potentially block the simulation loop or cause deadlocks if callbacks acquire other locks or perform time-consuming operations. - Pay close attention to the button callback logic within the lock in
_update_button_callbacks()- consider testing scenarios where button callbacks interact with other locked resources or take significant execution time.
1 file reviewed, no comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Adds Haply haptic device (Inverse3 + VerseGrip) API and teleoperation demo for robotic manipulation with bidirectional force feedback.
Key Changes:
- New
HaplyDeviceclass inisaaclab/devices/haply/with WebSocket-based communication (200Hz) - Teleoperation demo showing Franka Panda control with IK, gripper control, and force feedback
- Integration into device factory pattern for seamless device switching
- Position/orientation tracking from hardware devices with button callbacks
Issues Found:
- Critical race condition in
se3_haply.py:207-214:advance()readscached_datawithout holdingdata_lock, causing potential data corruption when WebSocket thread updates the same data - Callback deadlock risk in
se3_haply.py:217-224: Button callbacks invoked while holding_button_lock(though current code doesn't use_button_lock, the pattern in_prev_buttonsupdate could block simulation if callbacks are slow) - IndexError risk in
se3_haply.py:287-288: Empty device arrays from SDK will raiseIndexErroron[0]access - Breaking compatibility change in
se3_haply.py:334: Connection logic changed from OR to AND, now requires both devices instead of allowing partial operation - Minor: Joint assignment pattern in demo overwrites IK solution for joint 7
Confidence Score: 2/5
- Not safe to merge - contains critical race condition that can cause data corruption in multi-threaded environment
- Score of 2 reflects the race condition in
advance()method accessingcached_datawithout proper locking (lines 207-214 in se3_haply.py). This is a critical thread-safety issue where the main simulation thread reads device data while the WebSocket thread concurrently updates it, potentially causing torn reads or inconsistent state. The IndexError risk on empty arrays and the breaking compatibility change (OR→AND) are also concerning. source/isaaclab/isaaclab/devices/haply/se3_haply.pyrequires immediate attention for race condition fix. Theadvance()method must acquiredata_lockbefore readingcached_data.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 2/5 | Core Haply device implementation with several critical issues: race condition in advance() accessing cached_data without lock (line 207-214), callback deadlock risk with button state updates happening inside lock (line 217-224), IndexError risk on empty device arrays (line 287-288), and changed device connection logic from OR to AND breaking backwards compatibility (line 334) |
| scripts/demos/haply_teleoperation.py | 3/5 | Teleoperation demo with IK-based control and force feedback. Contains joint index mismatch where joint 7 (index 6) is set manually after IK solution includes it (line 318-319), and force feedback computed unconditionally every iteration (line 332-336) |
Sequence Diagram
sequenceDiagram
participant User
participant Demo as haply_teleoperation.py
participant Device as HaplyDevice
participant WS as WebSocket Thread
participant SDK as Haply SDK
participant Robot as Franka Robot
participant Sensors as Contact Sensors
User->>Demo: Start teleoperation demo
Demo->>Device: HaplyDevice(cfg)
Device->>WS: Start WebSocket thread
WS->>SDK: Connect via WebSocket
SDK-->>WS: Connection established
WS->>SDK: Receive device data (200Hz)
SDK-->>WS: {inverse3: [], wireless_verse_grip: []}
WS->>Device: Update cached_data (position, orientation, buttons)
loop Simulation Loop (200Hz)
Demo->>Device: advance()
Device->>Device: Read cached_data (potential race condition)
Device->>Device: Check button callbacks
Device-->>Demo: [x,y,z, qx,qy,qz,qw, btn_a, btn_b, btn_c]
Demo->>Demo: Apply coordinate mapping
Demo->>Demo: Compute IK for arm joints
Demo->>Robot: set_joint_position_target()
Robot->>Sensors: Measure contact forces
Sensors-->>Demo: net_forces_w
Demo->>Device: set_force_feedback(fx, fy, fz)
Device->>WS: Update feedback_force
WS->>SDK: Send force command via WebSocket
SDK-->>WS: Acknowledgment
end
User->>Demo: Stop simulation
Demo->>Device: close()
Device->>WS: running = False
WS->>SDK: Close WebSocket connection
Device->>Device: Wait for thread termination
5 files reviewed, 1 comment
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: mingxueg-nv <mingxueg@nvidia.com>
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Added comprehensive Haply device API for teleoperation with force feedback. The implementation provides WebSocket-based communication with Haply Inverse3 and VerseGrip devices, enabling real-time position tracking, orientation sensing, button input, and bidirectional force feedback at 200Hz.
Key changes:
- New
HaplyDeviceclass with thread-safe data handling using multiple locks - WebSocket client running in dedicated thread for async communication
- Device connection monitoring with timeout detection and auto-reconnection
- Force feedback support sent to Inverse3 device
- Button event callbacks with rising edge detection
- Proper cleanup and thread termination in
close()method
Latest commit (243b8ae) addresses critical race condition by adding .copy() calls inside the data lock when reading position and quaternion arrays, preventing potential data corruption from concurrent access.
Confidence Score: 4/5
- Safe to merge with minor style improvements recommended
- The latest commit fixed the critical race condition identified in previous review. Thread safety mechanisms are properly implemented with appropriate locks. Connection handling and error recovery are robust. Remaining comments are style/performance suggestions that don't affect correctness.
- No files require special attention - the critical race condition has been resolved
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 4/5 | New Haply device API with WebSocket communication, force feedback, and thread-safe data handling. Latest commit fixes critical race condition. |
Sequence Diagram
sequenceDiagram
participant App as Application
participant HD as HaplyDevice
participant WSThread as WebSocket Thread
participant HaplySDK as Haply SDK Server
App->>HD: __init__(cfg)
HD->>HD: Initialize locks & data structures
HD->>WSThread: Start WebSocket thread
WSThread->>HaplySDK: Connect (WebSocket)
loop Every 1/data_rate seconds (200Hz default)
HaplySDK->>WSThread: Send device state (JSON)
WSThread->>WSThread: Parse position, quaternion, buttons
WSThread->>HD: Update cached_data (with data_lock)
WSThread->>HD: Update connection status
WSThread->>HaplySDK: Send force feedback (with force_lock)
end
App->>HD: advance()
HD->>HD: Check connection status
HD->>HD: Copy position, quaternion, buttons (with data_lock)
HD->>HD: Trigger button callbacks if pressed
HD->>App: Return torch.Tensor[10]
App->>HD: set_force_feedback(fx, fy, fz)
HD->>HD: Update feedback_force (with force_lock)
App->>HD: close()
HD->>HD: Set running=False, reset force
HD->>WSThread: Wait for thread termination
WSThread->>HaplySDK: Disconnect
1 file reviewed, no comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds comprehensive Haply haptic device support for robotic teleoperation with bidirectional force feedback. The implementation includes a WebSocket-based device controller (se3_haply.py) that connects to Haply Inverse3 (position tracking) and VerseGrip (orientation/buttons) devices, plus a complete teleoperation demo (haply_teleoperation.py) showing Franka Panda arm control with contact-based force feedback.
Key additions:
HaplyDeviceclass with async WebSocket communication, thread-safe data handling, and force feedback API- Teleoperation demo with IK-based position control, workspace mapping, and contact sensor force feedback
- Comprehensive test coverage with proper mocking for device constructors
Issues identified:
- Critical joint indexing bug in demo (line 317): IK computes 6 joints but
[:-1]slice incorrectly handles assignment - Previous comments highlighted race conditions in
se3_haply.py(data_lock usage, button callback timing) and connection logic issues
Confidence Score: 2/5
- This PR has critical bugs that will cause incorrect robot motion and previous unresolved race conditions
- Score reflects one new critical logic error in joint assignment (line 317) that will cause incorrect IK joint application, plus multiple unresolved issues from previous review including race conditions in se3_haply.py that could cause thread safety problems and data corruption. The test coverage is excellent but doesn't catch the runtime logic bug.
- scripts/demos/haply_teleoperation.py requires immediate fix to joint indexing logic before merge
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| scripts/demos/haply_teleoperation.py | 3/5 | New teleoperation demo with Haply device - has joint indexing issue where IK output for 6 joints is assigned but joint 7 is also referenced causing potential mismatch |
| source/isaaclab/test/devices/test_device_constructors.py | 5/5 | Comprehensive test coverage added for Haply device with proper mocking of WebSocket connections and device states |
Sequence Diagram
sequenceDiagram
participant User as User/Haply Device
participant WS as WebSocket Thread
participant Device as HaplyDevice
participant Demo as haply_teleoperation.py
participant IK as IK Controller
participant Robot as Franka Robot
participant Sensor as Contact Sensors
Note over User,WS: Device Initialization
User->>WS: Connect via WebSocket
WS->>Device: Receive position (Inverse3)
WS->>Device: Receive orientation & buttons (VerseGrip)
Device->>Device: Cache data with lock
Note over Demo,Robot: Teleoperation Loop
Demo->>Device: advance()
Device->>Demo: Return [pos, quat, buttons]
Demo->>Demo: Map Haply workspace to robot frame
Demo->>IK: set_command(target_pos, ee_quat)
IK->>Demo: Compute joint positions (6 joints)
Demo->>Robot: set_joint_position_target()
Robot->>Robot: Execute motion (5 sim steps)
Note over Sensor,Device: Force Feedback Path
Robot->>Sensor: Contact forces on fingers
Sensor->>Demo: Read net_forces_w
Demo->>Demo: Compute feedback_force (clamped)
Demo->>Device: set_force_feedback(fx, fy, fz)
Device->>Device: Store in feedback_force with lock
WS->>Device: Get feedback_force
WS->>User: Send cursor_force command to Inverse3
2 files reviewed, 1 comment
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Adds Haply haptic device teleoperation support with bidirectional force feedback for robotic manipulation. The implementation includes a WebSocket-based device API (HaplyDevice) and a complete demo script for controlling a Franka Panda arm.
Key Changes:
- New
HaplyDeviceAPI providing position tracking (Inverse3), orientation/buttons (VerseGrip), and force feedback - Teleoperation demo with IK control for 6-DOF arm positioning, button-based gripper/rotation control, and contact-based haptic feedback
- Thread-safe WebSocket communication with proper connection monitoring and auto-reconnect
- Workspace safety limits and coordinate mapping from Haply to robot frame
Recent Fix:
- Latest commit (13c4047) correctly fixed IK joint index issue - now properly assigns all 6 IK-computed joints to
panda_joint1-6viaarm_joint_indices
Minor Issues:
- VerseGrip orientation data computed but unused (intentional design for button-based rotation)
- Python list vs numpy array inconsistency for
haply_initial_pos - Previous comments cover threading, race conditions, and error handling in device API
Confidence Score: 4/5
- This PR is safe to merge with minor style improvements recommended
- The recent commit fixed the critical IK joint index issue. The implementation is functionally correct with proper error handling, thread safety in the device API, and workspace safety limits. Previous review comments identified race conditions and threading issues in the device API that should be addressed, but the demo script itself is solid. The unused orientation data is a design choice rather than a bug.
- No files require special attention - the IK index bug was already fixed in commit 13c4047
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| scripts/demos/haply_teleoperation.py | 4/5 | Teleoperation demo correctly fixed IK joint index issue; orientation from VerseGrip unused but this appears intentional for button-based rotation control |
Sequence Diagram
sequenceDiagram
participant User
participant HaplyDevice
participant WebSocket
participant Simulation
participant Robot
participant IKController
participant ContactSensor
User->>HaplyDevice: Initialize(websocket_uri, pos_sensitivity)
HaplyDevice->>WebSocket: Connect to Haply SDK
WebSocket-->>HaplyDevice: Connection established
HaplyDevice->>HaplyDevice: Start WebSocket thread
WebSocket-->>HaplyDevice: Device data (Inverse3 + VerseGrip)
HaplyDevice-->>User: Device ready
loop Teleoperation Loop
Simulation->>HaplyDevice: advance()
HaplyDevice-->>Simulation: [pos, quat, buttons]
Simulation->>Simulation: apply_haply_to_robot_mapping()
Simulation->>Simulation: Handle button events (gripper, rotation)
Simulation->>IKController: set_command(target_pos, ee_quat)
IKController->>Robot: Get current_joint_pos, jacobian
Robot-->>IKController: Joint states
IKController-->>Simulation: joint_pos_des (6 joints)
Simulation->>Robot: set_joint_position_target()
loop 5 simulation steps
Simulation->>Simulation: write_data_to_sim()
Simulation->>Simulation: step()
end
Simulation->>Simulation: update(sim_dt)
Simulation->>ContactSensor: Get finger forces
ContactSensor-->>Simulation: net_forces_w
Simulation->>Simulation: Calculate feedback_force
Simulation->>HaplyDevice: set_force_feedback(fx, fy, fz)
HaplyDevice->>WebSocket: Send force command to Inverse3
WebSocket-->>User: Apply haptic feedback
end
1 file reviewed, 2 comments
rwiltz
reviewed
Oct 30, 2025
|
|
||
| return torch.tensor(command, dtype=torch.float32, device=self._sim_device) | ||
|
|
||
| def set_force_feedback(self, force_x: float, force_y: float, force_z: float): |
There was a problem hiding this comment.
It would be nice if we generalized this and added it to the DeviceBase interface, so that this teleop device could be used in the exisiting teleop/record_demos scripts.
There was a problem hiding this comment.
@rwiltz thank you for your suggestion, I have added this interface into DeviceBase, the input is force from contact sensor, set to fx, fy, fz, and the output is customed to adapt to different force feedback devices.
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds support for Haply haptic devices (Inverse3 + VerseGrip) to Isaac Lab, enabling teleoperation with bidirectional force feedback.
Key changes:
- New
HaplyDeviceclass with WebSocket-based communication at 200Hz - Position tracking from Inverse3 device with configurable sensitivity
- Orientation and 3-button input from VerseGrip device
- Force feedback support with safety limits (default 2.0N)
- Demo script showing Franka Panda manipulation with IK control and contact-based haptic feedback
- Integration with device factory and comprehensive test coverage
- Added
set_force_feedback()method toDeviceBaseinterface
Architecture:
The implementation uses a background thread for WebSocket communication, maintaining cached device state that's read by the main simulation loop. Force feedback is computed from contact sensors and sent back to the device with safety clamping.
Concerns:
Several threading synchronization issues were identified in previous comments that could lead to race conditions or data corruption in the se3_haply.py implementation. The teleoperation demo also has a joint indexing inconsistency that needs verification.
Confidence Score: 3/5
- Approve with caution - core functionality is sound but threading issues need attention before production use
- Score reflects well-designed API and comprehensive test coverage, but critical race conditions in WebSocket thread synchronization (advance() method accessing cached_data outside lock) and potential IndexError from empty device arrays need resolution. Demo script also has joint indexing questions. These issues won't prevent basic functionality but could cause intermittent failures under real-world conditions.
source/isaaclab/isaaclab/devices/haply/se3_haply.pyrequires careful review of threading logic (lines 209-214, 275, 288, 331).scripts/demos/haply_teleoperation.pyneeds verification of joint index assignments (lines 317-324).
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 3/5 | Implements WebSocket-based Haply device interface with position/orientation tracking and force feedback. Several critical threading issues noted in previous comments need resolution. |
| scripts/demos/haply_teleoperation.py | 3/5 | Teleoperation demo integrating Haply with Franka Panda robot via IK control. Some indexing issues with joint assignments flagged in previous comments. |
| source/isaaclab/isaaclab/devices/device_base.py | 5/5 | Added set_force_feedback method to base device interface with proper default implementation. Clean abstraction for force feedback support. |
| source/isaaclab/test/devices/test_device_constructors.py | 4/5 | Comprehensive test coverage for HaplyDevice including mocked WebSocket connections, force feedback limits, and factory integration tests. |
Sequence Diagram
sequenceDiagram
participant User
participant Demo as haply_teleoperation.py
participant Device as HaplyDevice
participant WebSocket as WebSocket Thread
participant HaplySDK as Haply SDK
participant Robot as Franka Panda
participant Sensor as Contact Sensors
User->>Demo: Start teleoperation demo
Demo->>Device: HaplyDevice(cfg)
Device->>WebSocket: Start WebSocket thread
WebSocket->>HaplySDK: Connect ws://localhost:10001
HaplySDK-->>WebSocket: Connection established
WebSocket->>HaplySDK: Receive device data (Inverse3 + VerseGrip)
WebSocket->>Device: Update cached_data (position, quaternion, buttons)
loop Simulation Loop (200Hz)
Demo->>Device: advance()
Device-->>Demo: [pos, quat, buttons] tensor
Demo->>Demo: Map Haply coordinates to robot workspace
Demo->>Demo: Compute IK for joints 1-6
Demo->>Demo: Set joint 7 rotation from Button C
Demo->>Demo: Set gripper from Buttons A/B
Demo->>Robot: set_joint_position_target()
Robot->>Robot: Execute motion (5 sim steps)
Robot->>Sensor: Update contact forces
Sensor-->>Demo: net_forces_w (left + right fingers)
Demo->>Device: set_force_feedback(fx, fy, fz)
Device->>Device: Clip forces to limit_force
Device->>WebSocket: Update feedback_force
WebSocket->>HaplySDK: Send force command to Inverse3
HaplySDK->>User: Apply haptic feedback
end
User->>Demo: Stop demo
Demo->>Device: close()
Device->>WebSocket: running = False
WebSocket->>HaplySDK: Disconnect
WebSocket-->>Device: Thread terminated
8 files reviewed, no comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Adds Haply haptic device support (Inverse3 + VerseGrip) for robot teleoperation with bidirectional force feedback. Implementation includes device API, WebSocket-based communication, and a Franka Panda teleoperation demo.
Major Changes:
- New
HaplyDeviceclass implementingDeviceBaseinterface with WebSocket communication for position/orientation tracking and force feedback - Teleoperation demo using IK control for 6-DOF arm positioning with manual joint 7 rotation control
- Added optional
set_force_feedback()method toDeviceBasefor haptic devices - Comprehensive test coverage with WebSocket and async mocking
Issues Found:
- Critical: Demo calls non-existent
haply_device.close()method (line 376) - will crash at cleanup - Hardcoded
WEBSOCKETS_AVAILABLE = Truedespite import that can fail (se3_haply.py:19) - Button callbacks executed inside lock, risking deadlock if callbacks are slow (se3_haply.py:224-225)
- IK computes 6 joints but joint assignment logic needs verification against Panda's actual joint configuration
Confidence Score: 2/5
- Not safe to merge - will crash at cleanup due to missing close() method
- Critical bug in demo (AttributeError from missing close() method) blocks normal cleanup. Additionally, hardcoded WEBSOCKETS_AVAILABLE flag and lock management issues in button callbacks pose reliability concerns.
- scripts/demos/haply_teleoperation.py (missing close() method) and source/isaaclab/isaaclab/devices/haply/se3_haply.py (hardcoded flag, callback lock issues)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| scripts/demos/haply_teleoperation.py | 2/5 | Teleoperation demo with critical AttributeError (missing close() method), potential joint indexing issues, and unused target_quat |
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 3/5 | Core Haply device implementation with hardcoded WEBSOCKETS_AVAILABLE flag and button callback execution inside lock |
| source/isaaclab/isaaclab/devices/device_base.py | 5/5 | Added optional force feedback interface to base class - clean implementation with default no-op behavior |
| source/isaaclab/test/devices/test_device_constructors.py | 4/5 | Comprehensive test coverage for Haply device with proper mocking of WebSocket and async operations |
Sequence Diagram
sequenceDiagram
participant User
participant Demo as haply_teleoperation.py
participant Device as HaplyDevice
participant WS as WebSocket Thread
participant SDK as Haply SDK
participant Robot as Franka Robot
participant Sensor as Contact Sensors
User->>Demo: Start demo
Demo->>Device: Create HaplyDevice(cfg)
Device->>WS: Start WebSocket thread
WS->>SDK: Connect to ws://localhost:10001
SDK-->>WS: Connection established
WS->>SDK: Receive device data (Inverse3 + VerseGrip)
WS->>Device: Update cached_data (position, quaternion, buttons)
Device-->>Demo: Device ready
loop Simulation Loop (200Hz)
Demo->>Device: advance()
Device->>Device: Lock data_lock
Device->>Device: Copy position, quaternion, buttons
Device->>Device: Unlock data_lock
Device->>Device: Check button callbacks
Device-->>Demo: Return tensor [pos(3), quat(4), buttons(3)]
Demo->>Demo: Apply coordinate mapping
Demo->>Demo: Compute IK for joints 1-6
Demo->>Robot: Set joint targets (IK + joint7 + gripper)
Demo->>Robot: Step simulation (5 steps)
Robot->>Sensor: Update contact sensors
Sensor-->>Demo: Get contact forces
Demo->>Device: set_force_feedback(fx, fy, fz)
Device->>Device: Lock force_lock
Device->>Device: Update feedback_force
Device->>Device: Unlock force_lock
WS->>Device: Lock force_lock
WS->>Device: Copy feedback_force
WS->>Device: Unlock force_lock
WS->>SDK: Send force command to Inverse3
end
Demo->>Device: close()
Note over Demo,Device: AttributeError: no close() method!
Device->>Device: __del__ called
Device->>WS: Set running=False
WS->>SDK: Disconnect WebSocket
9 files reviewed, 1 comment
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Adds Haply haptic device integration with bidirectional force feedback for robotic teleoperation. The implementation includes a WebSocket-based device API (HaplyDevice) and a demonstration script showing real-time teleoperation of a Franka Panda arm with IK control.
Key changes:
- New
HaplyDeviceclass with WebSocket communication thread for position/orientation tracking and force feedback - Teleoperation demo with differential IK control, coordinate mapping, and contact-based force feedback
- Button-based gripper and end-effector rotation control
Issues identified in previous comments:
- Critical: Race conditions in
se3_haply.pywithdata_lockandcached_dataaccess (lines 209, 331) - Critical: Joint indexing mismatch in demo - IK computes 6 joints but assignment expects 7 (line 317)
- Critical: Missing error handling for WebSocket import and empty device arrays (IndexError risks)
- Important: Thread lifecycle issues in
__del__and connection status logic changed from OR to AND - Multiple style improvements for performance and code clarity
Confidence Score: 2/5
- This PR has multiple critical issues that need resolution before merging
- Score reflects critical race conditions in device implementation, joint indexing bugs in the demo, and missing error handling for WebSocket operations. The core functionality is sound but thread safety and array bounds checking must be addressed to prevent runtime errors and data corruption during high-frequency teleoperation.
- Primary attention needed on
source/isaaclab/isaaclab/devices/haply/se3_haply.pyfor race conditions (lines 209, 331) and connection logic (line 275), andscripts/demos/haply_teleoperation.pyfor joint indexing fix (line 327)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| scripts/demos/haply_teleoperation.py | 3/5 | New teleoperation demo with IK control logic. Previous comments identified critical joint indexing issue on line 327 and unused target_quat variable. Force feedback logic is sound but could be optimized. |
Sequence Diagram
sequenceDiagram
participant User as Haply Device<br/>(Inverse3 + VerseGrip)
participant WS as WebSocket Thread
participant Device as HaplyDevice
participant Demo as haply_teleoperation.py
participant IK as IK Controller
participant Robot as Franka Robot
participant Sensors as Contact Sensors
Note over User,Sensors: Initialization Phase
Demo->>Device: HaplyDevice(cfg)
Device->>WS: Start WebSocket thread (daemon)
WS->>User: Connect to Haply SDK
Demo->>Robot: Initialize to default pose
Demo->>IK: Create DifferentialIKController
Note over User,Sensors: Main Control Loop (200Hz)
loop Every Simulation Step
Demo->>Device: advance()
Device->>WS: Read cached_data (with data_lock)
WS-->>Device: position, quaternion, buttons
Device-->>Demo: haply_data [pos, quat, buttons]
Demo->>Demo: Process button inputs<br/>(gripper, rotation)
Demo->>Demo: apply_haply_to_robot_mapping()<br/>(coordinate transform)
Demo->>Robot: Get current EE pose & jacobian
Robot-->>Demo: ee_pos_w, ee_quat_w, jacobian
Demo->>IK: set_command(target_pos, ee_quat)
Demo->>IK: compute(ee_pos, ee_quat, jacobian, joint_pos)
IK-->>Demo: joint_pos_des (6 joints)
Demo->>Robot: set_joint_position_target()<br/>(joints 0-5 from IK, joint 6 manual, gripper)
Demo->>Robot: step simulation (5x)
Robot->>Sensors: Update contact forces
Sensors-->>Demo: left_finger_forces, right_finger_forces
Demo->>Demo: Calculate total_contact_force
Demo->>Device: set_force_feedback(fx, fy, fz)
Device->>WS: Update force_feedback (with force_lock)
WS->>User: Send force feedback to device
end
1 file reviewed, 2 comments
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: mingxueg <mingxueg@nvidia.com>
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds a new teleoperation demo (scripts/demos/haply_teleoperation.py) that integrates Haply haptic devices (Inverse3 + VerseGrip) with Isaac Lab's Franka Panda robot arm. The implementation provides real-time robot control with bidirectional force feedback via WebSocket communication.
Key changes:
- New teleoperation script with IK-based position control for 6-DOF arm manipulation
- Button-based gripper control (open/close) and end-effector rotation
- Contact sensor integration for haptic force feedback to the operator
- Workspace limits and coordinate mapping between Haply and robot frames
Critical issues identified:
- IK joint assignment logic error that may cause incorrect robot movement (see previous comments on line 317/327)
- Race conditions in the underlying
HaplyDeviceimplementation affecting data consistency and connection status checks - Missing cleanup handling (
close()method doesn't exist) - Several unused variables that could cause confusion
Confidence Score: 2/5
- This PR has critical logic errors in joint control and race conditions that could cause unexpected robot behavior
- Score reflects multiple critical issues: (1) IK joint assignment error on line 327 where 6 IK joints are assigned but joint 7 is also set separately, potentially causing incorrect robot motion; (2) Race conditions in
HaplyDevice.advance()where data_lock is released before accessing copied array references; (3) Missingclose()method causing AttributeError at cleanup; (4) Force feedback updates happen every iteration without checking if device supports it. The underlying device implementation also has connection logic changed from OR to AND which breaks backward compatibility. These issues need resolution before merge. - Pay close attention to
scripts/demos/haply_teleoperation.py- the IK joint assignment logic (lines 315-329) needs careful review and testing. The underlyingsource/isaaclab/isaaclab/devices/haply/se3_haply.pyalso has race conditions that impact this demo.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| scripts/demos/haply_teleoperation.py | 2/5 | New teleoperation demo with Haply device integration. Contains critical logic errors in IK joint assignment and potential race conditions in force feedback. Multiple style issues with unused variables. |
Sequence Diagram
sequenceDiagram
participant User
participant TeleopScript as haply_teleoperation.py
participant HaplyDevice as HaplyDevice
participant WebSocket as WebSocket Thread
participant HaplySDK as Haply SDK
participant IKController as IKController
participant Robot as Franka Robot
participant ContactSensors as Contact Sensors
User->>TeleopScript: Start demo
TeleopScript->>HaplyDevice: Initialize(cfg)
HaplyDevice->>WebSocket: Start WebSocket thread
WebSocket->>HaplySDK: Connect ws://localhost:10001
HaplySDK-->>WebSocket: Connection established
WebSocket->>HaplySDK: Request device data
HaplySDK-->>WebSocket: Device data (Inverse3 + VerseGrip)
WebSocket->>HaplyDevice: Update cached_data
HaplyDevice-->>TeleopScript: Device ready
TeleopScript->>Robot: Initialize robot state
TeleopScript->>IKController: Initialize IK controller
loop Simulation Loop
TeleopScript->>HaplyDevice: advance()
HaplyDevice->>HaplyDevice: Read cached_data (position, quaternion, buttons)
HaplyDevice-->>TeleopScript: Return [pos(3), quat(4), buttons(3)]
TeleopScript->>TeleopScript: Process button inputs (gripper, rotation)
TeleopScript->>TeleopScript: apply_haply_to_robot_mapping()
TeleopScript->>IKController: compute(target_pos, jacobian)
IKController-->>TeleopScript: joint_pos_des (6 joints)
TeleopScript->>Robot: set_joint_position_target(joints 1-6, joint 7, gripper)
Robot->>Robot: Execute motion (5 sim steps)
Robot->>ContactSensors: Update contact forces
ContactSensors-->>TeleopScript: net_forces_w (left + right fingers)
TeleopScript->>HaplyDevice: set_force_feedback(force_x, force_y, force_z)
HaplyDevice->>HaplyDevice: Update feedback_force (with lock)
par WebSocket Loop (async)
WebSocket->>HaplyDevice: Read feedback_force (with lock)
WebSocket->>HaplySDK: Send force command to Inverse3
HaplySDK->>User: Apply haptic feedback
end
end
User->>TeleopScript: Stop demo
TeleopScript->>HaplyDevice: Cleanup (__del__)
HaplyDevice->>WebSocket: Stop thread
WebSocket->>HaplySDK: Disconnect
1 file reviewed, no comments
- IK joint assignment is correct (6 from IK + joint7 from button) - No race conditions (.copy() ensures thread safety) - close() merged into __del__() (consistent with other devices) - Force feedback updates are necessary for haptic devices - AND logic correct (Haply needs both Inverse3 + VerseGrip) - Add orientation_sensitivity to tests - Remove unused quaternion return value
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Adds Haply haptic device API (HaplyDevice class in se3_haply.py) for teleoperation with force feedback, integrated with a Franka Panda demo script that uses IK control and contact sensors for bidirectional haptic feedback.
Key Changes:
- WebSocket-based communication with Haply SDK for Inverse3 (position + force feedback) and VerseGrip (orientation + buttons)
- Real-time teleoperation at 200Hz with force clipping for safety
- Demo script maps Haply workspace to robot end-effector with IK control for 6 joints
- Contact sensors on gripper fingers provide force feedback to the operator
Critical Issues:
- Race condition in
advance()method wherecached_dataarrays are accessed after lock release (se3_haply.py:209-214) - Invalid quaternion scaling by
orientation_sensitivitybreaks normalization (se3_haply.py:214) - Joint indexing mismatch between IK output (6 joints) and array assignment (se3_haply.py:317-320)
Other Issues:
- Missing dependency handling for websockets import
- Callback invocations within locks may cause deadlock
- IndexError risk when device arrays are empty
- Thread cleanup incomplete in
__del__
Confidence Score: 2/5
- This PR has multiple critical issues including race conditions, invalid quaternion operations, and joint indexing mismatches that will cause runtime errors
- Score reflects three critical issues that need resolution: (1) race condition in data access that can corrupt position/quaternion data during teleoperation, (2) quaternion scaling that violates normalization requirements and will cause undefined rotation behavior, (3) joint indexing bug where IK computes 6 joints but assignment logic expects 7. Additional concerns include missing error handling for empty device arrays, potential deadlocks from callbacks inside locks, and incomplete thread cleanup
- Pay close attention to
source/isaaclab/isaaclab/devices/haply/se3_haply.py- critical race condition inadvance()method (lines 209-217) and invalid quaternion scaling must be fixed. Also reviewscripts/demos/haply_teleoperation.pyjoint assignment logic (lines 319-320)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 2/5 | New Haply device API with WebSocket-based teleoperation and force feedback. Critical race condition in data access and invalid quaternion scaling found, plus several threading and error handling issues |
| scripts/demos/haply_teleoperation.py | 3/5 | Teleoperation demo integrating Haply device with Franka robot using IK control. Has joint indexing issue and minor inefficiencies in force feedback computation |
Sequence Diagram
sequenceDiagram
participant User
participant HaplySDK
participant HaplyDevice
participant WSThread
participant TeleopDemo
participant IKController
participant Robot
participant ContactSensor
User->>TeleopDemo: Start demo
TeleopDemo->>HaplyDevice: Initialize(cfg)
HaplyDevice->>WSThread: Start WebSocket thread
WSThread->>HaplySDK: Connect(websocket_uri)
HaplySDK-->>WSThread: Connection established
WSThread->>HaplySDK: Receive device data
HaplySDK-->>WSThread: {inverse3, verse_grip}
WSThread->>HaplyDevice: Update cached_data (position, quaternion, buttons)
loop Simulation Loop
TeleopDemo->>HaplyDevice: advance()
HaplyDevice-->>TeleopDemo: [x, y, z, qx, qy, qz, qw, btn_a, btn_b, btn_c]
TeleopDemo->>TeleopDemo: apply_haply_to_robot_mapping()
TeleopDemo->>IKController: compute(target_pos, ee_quat, jacobian)
IKController-->>TeleopDemo: joint_pos_des[6 joints]
TeleopDemo->>Robot: set_joint_position_target()
Robot->>Robot: Execute motion
Robot->>ContactSensor: Update contact forces
ContactSensor-->>TeleopDemo: net_forces_w
TeleopDemo->>HaplyDevice: set_force_feedback(fx, fy, fz)
HaplyDevice->>WSThread: Update feedback_force
WSThread->>HaplySDK: send({set_cursor_force})
HaplySDK-->>User: Apply force feedback
end
2 files reviewed, 1 comment
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: mingxueg <mingxueg@nvidia.com>
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This update addresses critical issues from the previous review of the Haply device API implementation. Key improvements include:
- Fixed websockets import handling with proper try-except block (previously hardcoded to
True) - Improved thread cleanup in
__del__with explicit join and 2s timeout - Resolved race condition by copying all data inside
data_lock(lines 213-217) - Removed invalid quaternion multiplication by
orientation_sensitivity - Maintains AND logic for dual-device connection requirement (both Inverse3 and VerseGrip)
- Button callbacks now invoked outside data lock to prevent blocking
The implementation provides a robust WebSocket-based interface for teleoperation with force feedback, handling real-time data streaming at 200Hz between the Haply SDK and Isaac Lab.
Confidence Score: 4/5
- This PR is safe to merge with minimal risk - addresses all critical threading and concurrency issues from previous review
- The code fixes all critical bugs identified in the previous review (race conditions, import handling, thread cleanup, quaternion math). Thread safety is now properly implemented with correct lock usage. Minor style suggestions remain but don't affect correctness.
- No files require special attention - all critical issues have been resolved
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 4/5 | Addresses most critical issues from previous review - proper websocket import handling, thread cleanup, data locking, and quaternion handling now correct |
Sequence Diagram
sequenceDiagram
participant App as Teleoperation Demo
participant HD as HaplyDevice
participant WSThread as WebSocket Thread
participant SDK as Haply SDK
participant I3 as Inverse3 Device
participant VG as VerseGrip Device
App->>HD: __init__(cfg)
HD->>HD: Start WebSocket thread
HD->>WSThread: _start_websocket_thread()
WSThread->>SDK: Connect to ws://localhost:10001
loop Every 1/200s (5ms)
SDK->>WSThread: Send device state (JSON)
WSThread->>WSThread: Parse position from Inverse3
WSThread->>WSThread: Parse orientation & buttons from VerseGrip
WSThread->>HD: Update cached_data (with data_lock)
Note over HD,WSThread: Force feedback path
App->>HD: set_force_feedback(fx, fy, fz)
HD->>HD: Store in feedback_force (with force_lock)
WSThread->>HD: Read feedback_force (with force_lock)
WSThread->>SDK: Send cursor force command
SDK->>I3: Apply force feedback
end
loop Simulation Loop
App->>HD: advance()
HD->>HD: Read cached_data (with data_lock)
HD->>HD: Apply pos_sensitivity scaling
HD->>HD: Detect button press edges
HD->>App: Return [pos(3), quat(4), buttons(3)]
App->>App: Compute IK for target position
App->>App: Calculate force feedback from sensors
App->>HD: set_force_feedback(fx, fy, fz)
end
App->>HD: __del__()
HD->>HD: Set running = False
HD->>WSThread: Wait for thread (join timeout=2s)
WSThread->>SDK: Close WebSocket connection
1 file reviewed, no comments
|
Hi, @rwiltz , @kellyguo11, I resolved the most comments, please help review this PR again, thanks🙏 |
rwiltz
reviewed
Oct 31, 2025
| """ | ||
| raise NotImplementedError | ||
|
|
||
| def set_force_feedback(self, force_x: float, force_y: float, force_z: float) -> Any: |
There was a problem hiding this comment.
This is great, but can we make it a little more general since we expect more devices to be coming soon? Maybe we can add something like:
# -----------------------------
# Optional force/torque feedback
# -----------------------------
def push_force(self, forces: torch.Tensor, names: list[str] | None = None, frame: str = "world") -> None:
"""Push one or more 3D force vectors to the device (optional; default no-op).
Args:
forces: Tensor of shape (N, 3) with forces [fx, fy, fz].
names: Optional labels for each force channel (e.g., ["ee", "finger_1"]).
frame: Frame of the vectors: "world" (default) or "device".
Note:
Devices that support haptics should override this method to forward
forces to their hardware API. Devices that do not support haptics
may ignore this call.
"""
return # no-op by default
def push_wrench(self, wrenches: torch.Tensor, names: list[str] | None = None, frame: str = "world") -> None:
"""Push one or more 6D wrenches to the device (optional; default no-op).
Args:
wrenches: Tensor of shape (N, 6) with [fx, fy, fz, tx, ty, tz].
names: Optional labels for each wrench channel (e.g., ["ee", "finger_1"]).
frame: Frame of the vectors: "world" (default) or "device".
Note:
Devices that support haptics should override this method. Devices that
do not support haptics may ignore this call.
"""
return # no-op by default`
```
Does that make sense?
There was a problem hiding this comment.
Also do we need a "clear_force" call to reset all the forces?
There was a problem hiding this comment.
If we just update your function to def push_force(self, forces: torch.Tensor, names: list[str] | None = None, frame: str = "world") -> None: we can merge this and add the rest later incrementally
There was a problem hiding this comment.
Hi, @rwiltz, thank you for your good point. I'll use push_force and test with office's haply device tomorrow - will push the commit once verified.
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Adds comprehensive Haply haptic device support (Inverse3 + VerseGrip) for teleoperation with force feedback. The implementation includes a WebSocket-based device driver with multi-threaded communication at 200Hz, a teleoperation demo using IK control with a Franka Panda robot, and integration with the existing device factory pattern.
Key additions:
HaplyDeviceclass with position/orientation tracking, button inputs, and bidirectional force feedback- Real-time WebSocket communication thread with connection monitoring and automatic reconnection
- Demo script showcasing end-effector control with contact force feedback from finger sensors
- Comprehensive test coverage with mocked WebSocket connections
- Force limiting (2N default) for safety
Issues requiring attention:
- Multiple threading synchronization issues exist in
se3_haply.py(see previous comments about race conditions on lines 209, 331) - Invalid quaternion multiplication by
orientation_sensitivityon line 213 (quaternions must remain normalized) - Connection logic changed from OR to AND (line 275) - breaks backward compatibility if only one device is available
- IK joint indexing in demo needs clarification - computes 6 joints but handles 7-joint arm
orientation_sensitivityparameter is stored but never actually used
Confidence Score: 2/5
- This PR has critical threading race conditions and logic errors that could cause runtime failures or undefined behavior in production teleoperation.
- Score reflects multiple critical issues: race conditions in thread synchronization (lines 209, 331 of se3_haply.py) where data is accessed outside locks, invalid quaternion operations that break rotation math, and connection logic that may fail unexpectedly. While the feature implementation is comprehensive with good test coverage, the threading bugs and mathematical errors need resolution before safe deployment in a real-time teleoperation context.
- Pay close attention to
source/isaaclab/isaaclab/devices/haply/se3_haply.pyfor race condition fixes andscripts/demos/haply_teleoperation.pyfor joint indexing clarification
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 2/5 | New Haply device driver with WebSocket communication. Has multiple critical race conditions in thread synchronization, invalid quaternion multiplication, and connection logic issues that need resolution. |
| scripts/demos/haply_teleoperation.py | 3/5 | Teleoperation demo with IK control. Has joint indexing mismatch where IK computes 6 joints but joint 7 handling is unclear, and unused orientation computation. |
| source/isaaclab/test/devices/test_device_constructors.py | 4/5 | Comprehensive test coverage for Haply device including mock WebSocket connections, force feedback, and configuration validation. |
Sequence Diagram
sequenceDiagram
participant User
participant Demo as haply_teleoperation.py
participant HaplyDev as HaplyDevice
participant WSThread as WebSocket Thread
participant HaplySDK as Haply SDK
participant IKCtrl as IK Controller
participant Robot as Franka Robot
User->>Demo: Start teleoperation demo
Demo->>HaplyDev: HaplyDevice(cfg)
HaplyDev->>WSThread: Start WebSocket thread
WSThread->>HaplySDK: Connect via WebSocket
HaplySDK-->>WSThread: Connection established
loop Every 200Hz (data_rate)
WSThread->>HaplySDK: Request device data
HaplySDK-->>WSThread: Position, orientation, buttons
WSThread->>HaplyDev: Update cached_data (with data_lock)
WSThread->>HaplySDK: Send force feedback
end
loop Simulation Loop
Demo->>HaplyDev: advance()
HaplyDev->>HaplyDev: Read cached_data (with data_lock)
HaplyDev-->>Demo: [pos, quat, buttons] tensor
Demo->>Demo: apply_haply_to_robot_mapping()
Demo->>IKCtrl: compute(target_pos, jacobian)
IKCtrl-->>Demo: joint_pos_des[6 joints]
Demo->>Demo: Set joint 7 (rotation angle)
Demo->>Robot: set_joint_position_target()
Demo->>Robot: Get contact forces from sensors
Demo->>HaplyDev: set_force_feedback(fx, fy, fz)
HaplyDev->>HaplyDev: Update feedback_force (with force_lock)
end
9 files reviewed, 1 comment
| # Store configuration | ||
| self.websocket_uri = cfg.websocket_uri | ||
| self.pos_sensitivity = cfg.pos_sensitivity | ||
| self.orientation_sensitivity = cfg.orientation_sensitivity |
There was a problem hiding this comment.
style: orientation_sensitivity is stored but never used in the advance() method (line 213-214). Either remove it or apply it to quaternion data if scaling is needed.
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds support for Haply haptic devices (Inverse3 + VerseGrip) to IsaacLab for teleoperation with bidirectional force feedback. The implementation includes a WebSocket-based device API (HaplyDevice), a Franka Panda teleoperation demo with IK control, and comprehensive unit tests.
Key Changes:
- New
HaplyDeviceclass insource/isaaclab/isaaclab/devices/haply/se3_haply.pyproviding SE(3) control with real-time position tracking, orientation sensing, button inputs, and force feedback at 200Hz - Demo script
scripts/demos/haply_teleoperation.pyshowcasing robot manipulation with workspace mapping, IK-based motion control, and contact-based force feedback - Unit tests in
test_device_constructors.pyvalidating device initialization, configuration, and force limiting - Added
websocketsdependency to extension requirements
Critical Issues Found:
- Race condition in
se3_haply.py:209-214: Data copied outside lock protection after release - Invalid quaternion scaling on line 214 breaks normalization constraint
- Missing try-except for websockets import (hardcoded
WEBSOCKETS_AVAILABLE = True) - Connection logic changed from OR to AND (line 275), breaking backward compatibility
- Button callbacks invoked inside lock (line 225), potential for deadlock
- IK joint indexing mismatch in demo (6 joints from IK applied incorrectly to 7-joint array)
Recommendations:
Address the race condition and quaternion scaling issues before merge as these will cause runtime failures and undefined rotation behavior.
Confidence Score: 2/5
- This PR has critical threading issues and logic errors that will cause runtime failures and undefined behavior
- Score of 2 reflects multiple critical issues: (1) race condition accessing
cached_dataarrays outside lock protection will cause data corruption in high-frequency loops, (2) invalid quaternion multiplication breaks rotation math entirely, (3) missing import error handling will fail on systems without websockets, (4) IK joint indexing mismatch causes incorrect motion, (5) callback invocation inside locks risks deadlock. These are not potential issues but definite bugs that will manifest in production. source/isaaclab/isaaclab/devices/haply/se3_haply.pyrequires immediate attention for race conditions (lines 209-214), quaternion handling (line 214), import handling (line 20), and connection logic (line 275).scripts/demos/haply_teleoperation.pyneeds IK indexing fix (line 319).
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 2/5 | Added Haply device API with WebSocket communication and force feedback. Contains critical race conditions (data access outside locks), invalid quaternion scaling, missing import error handling, and connection logic issues that need resolution. |
| scripts/demos/haply_teleoperation.py | 3/5 | Teleoperation demo with IK control and force feedback. Has joint indexing mismatch (6-joint IK applied to 7-joint array), force computation happens unconditionally, and minor code style issues. |
| source/isaaclab/test/devices/test_device_constructors.py | 4/5 | Added unit tests for Haply device constructors with proper mocking. Tests cover config validation, force limiting, and reset functionality. Well-structured test implementation. |
Sequence Diagram
sequenceDiagram
participant Demo as haply_teleoperation.py
participant Haply as HaplyDevice
participant WS as WebSocket Thread
participant SDK as Haply SDK
participant Robot as Franka Robot
participant Sensor as Contact Sensors
Note over Demo,SDK: Initialization Phase
Demo->>Haply: Create HaplyDevice(cfg)
Haply->>WS: Start WebSocket thread
WS->>SDK: Connect ws://localhost:10001
SDK-->>WS: Connection established
WS->>SDK: Receive device data
SDK-->>WS: {inverse3, wireless_verse_grip}
WS->>Haply: Cache position, orientation, buttons
Haply-->>Demo: Device ready
Note over Demo,Sensor: Main Teleoperation Loop (200Hz)
loop Every simulation step
Demo->>Haply: advance()
Haply->>Haply: Lock data_lock
Haply->>Haply: Copy position * pos_sensitivity
Haply->>Haply: Copy quaternion
Haply->>Haply: Copy buttons (a, b, c)
Haply->>Haply: Release data_lock
Haply-->>Demo: [pos, quat, buttons] tensor
Demo->>Demo: Map Haply to robot workspace
Demo->>Demo: Compute IK (6 joints)
Demo->>Demo: Apply joint targets (7 joints + gripper)
Demo->>Robot: set_joint_position_target()
Robot->>Robot: Execute motion (5 steps)
Demo->>Sensor: Read left_finger_contact_sensor
Demo->>Sensor: Read right_finger_contact_sensor
Sensor-->>Demo: net_forces_w
Demo->>Demo: Average contact forces
Demo->>Haply: push_force(total_contact_force)
Haply->>Haply: Lock force_lock
Haply->>Haply: Clip forces to [-2.0, 2.0]N
Haply->>Haply: Update feedback_force
Haply->>Haply: Release force_lock
end
Note over WS,SDK: Background Force Feedback (200Hz)
loop Async WebSocket loop
WS->>SDK: Receive state update
SDK-->>WS: {position, orientation, buttons}
WS->>Haply: Lock data_lock and update cached_data
WS->>Haply: Lock force_lock and read feedback_force
WS->>SDK: Send set_cursor_force command
SDK->>SDK: Apply force to Inverse3 device
WS->>WS: Sleep 1/data_rate (5ms)
end
3 files reviewed, no comments
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds comprehensive Haply haptic device support to Isaac Lab, enabling real-time teleoperation with bidirectional force feedback. The implementation introduces a new HaplyDevice class that communicates with Haply hardware (Inverse3 + VerseGrip) via WebSocket, along with a complete demo showcasing robotic manipulation with IK control.
Key changes:
- Added
push_force()andpush_wrench()interfaces toDeviceBasefor force feedback support - Implemented
HaplyDevicewith WebSocket-based communication, position/orientation tracking, and force feedback - Created teleoperation demo with Franka Panda robot, IK control, and contact-based haptic feedback
- Added comprehensive unit tests with proper mocking of async operations
Critical issues identified:
- Race condition in
se3_haply.py:207-214where data is copied inside lock but accessed outside (previous comment addresses this) - Connection status logic changed from OR to AND on line 275, requiring both devices (previous comment questions backward compatibility)
- Multiple threading and cleanup issues flagged in previous comments
Architecture:
The system uses a producer-consumer pattern with a background WebSocket thread updating cached device data at 200Hz, while the main simulation loop reads this data through thread-safe locks and sends force feedback back to the hardware.
Confidence Score: 3/5
- This PR introduces valuable haptic teleoperation functionality but contains critical race conditions and threading issues that should be resolved before merging
- Score of 3 reflects the presence of several critical issues flagged in previous comments: race condition in
advance()data access (line 207-214), connection status logic changes potentially breaking backward compatibility (line 275), and multiple threading/cleanup concerns. While the feature is well-tested and architecturally sound, these concurrency issues could cause intermittent failures in production use. The device_base.py interface additions are clean (5/5), but the core implementation needs fixes. source/isaaclab/isaaclab/devices/haply/se3_haply.pyrequires immediate attention for race conditions and threading issues.scripts/demos/haply_teleoperation.pyhas minor issues with joint assignment logic that should be verified.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/devices/device_base.py | 5/5 | Added push_force and push_wrench methods to DeviceBase interface for force feedback support - clean, well-documented API additions with sensible no-op defaults |
| source/isaaclab/isaaclab/devices/haply/se3_haply.py | 2/5 | New Haply device implementation with WebSocket communication and force feedback - contains critical race conditions in advance() and connection status checks, plus several style/cleanup issues flagged in previous comments |
| scripts/demos/haply_teleoperation.py | 3/5 | New teleoperation demo with IK control and force feedback - contains logic error in joint assignment (line 313) and some unused variables, but overall functional implementation |
| source/isaaclab/test/devices/test_device_constructors.py | 4/5 | Added comprehensive unit tests for HaplyDevice constructor with proper mocking of WebSocket and threading - thorough test coverage including force limiting and reset functionality |
Sequence Diagram
sequenceDiagram
participant User as User/Haply Device
participant HaplySDK as Haply SDK (WebSocket)
participant HaplyDevice as HaplyDevice
participant Demo as haply_teleoperation.py
participant IK as IK Controller
participant Robot as Franka Robot
participant Sensor as Contact Sensors
Note over User,HaplySDK: Initialization Phase
User->>HaplySDK: Connect Inverse3 + VerseGrip
Demo->>HaplyDevice: __init__(cfg)
HaplyDevice->>HaplySDK: WebSocket connect (ws://localhost:10001)
HaplySDK-->>HaplyDevice: Connection established
HaplyDevice->>HaplyDevice: Start WebSocket thread
HaplySDK-->>HaplyDevice: Device IDs (Inverse3, VerseGrip)
HaplyDevice-->>Demo: Device ready
Note over User,Robot: Teleoperation Loop (200Hz)
loop Every simulation step
User->>HaplySDK: Move device, press buttons
HaplySDK->>HaplyDevice: WebSocket data: {position, orientation, buttons}
HaplyDevice->>HaplyDevice: Cache data (with data_lock)
Demo->>HaplyDevice: advance()
HaplyDevice->>HaplyDevice: Read cached_data (with data_lock)
HaplyDevice-->>Demo: [x,y,z, qx,qy,qz,qw, btn_a,btn_b,btn_c]
Demo->>Demo: apply_haply_to_robot_mapping()
Demo->>Demo: Handle button events (gripper, rotation)
Demo->>IK: compute(target_pos, current_state, jacobian)
IK-->>Demo: joint_pos_des (6 joints)
Demo->>Demo: Assemble joint targets (IK + rotation + gripper)
Demo->>Robot: set_joint_position_target()
Robot->>Robot: Execute motion
Robot->>Sensor: Contact forces
Sensor-->>Demo: net_forces_w (left + right finger)
Demo->>Demo: Calculate total_contact_force
Demo->>HaplyDevice: push_force(total_contact_force)
HaplyDevice->>HaplyDevice: Update feedback_force (with force_lock)
HaplyDevice->>HaplySDK: WebSocket send: set_cursor_force
HaplySDK->>User: Apply haptic feedback
end
4 files reviewed, no comments
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.
Description
Add Haply haptic device teleoperation support for robotic manipulation with force feedback.
Type of change
Haply device documentation and usage
Usage (make sure your Haply device is connected):
Screenshots
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there