Add approximate gripper position #49
Conversation
euyniy
commented
Jul 24, 2025
euyniy
commented
- Main change: Add approximate gripper position
- Minor change: Add more comments about TODOs and appropriate waits
There was a problem hiding this comment.
Pull Request Overview
This PR adds approximate gripper position mapping functionality to the OpenArm V10 hardware interface. The main change converts joint values to motor radians for proper gripper control, replacing placeholder logic with approximate position calculations.
- Implements gripper position mapping with approximate conversion between joint values (0-0.044m) and motor radians (0 to -1.0472 rad)
- Adds gripper-related constants for default control parameters and position mappings
- Improves code documentation with more detailed TODO comments and removes unnecessary callback mode settings
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| v10_simple_hardware.cpp | Implements gripper position mapping functions and updates control logic with new constants |
| v10_simple_hardware.hpp | Adds gripper-related constants for position mapping and control parameters |
Comments suppressed due to low confidence (1)
openarm_hardware/src/v10_simple_hardware.cpp:114
- [nitpick] The constant name GRIPPER_DEFAULT_KD conflicts with the existing naming pattern. The ARM constants use DEFAULT_KD (array), but this uses GRIPPER_DEFAULT_KD (scalar). Consider using a more consistent naming pattern like GRIPPER_KD to avoid confusion.
RCLCPP_ERROR(rclcpp::get_logger("OpenArm_v10HW"),
| @@ -105,6 +106,13 @@ class OpenArm_v10HW : public hardware_interface::SystemInterface { | |||
| const std::vector<double> DEFAULT_KD = {2.75, 2.5, 0.7, 0.4, | |||
| 0.7, 0.6, 0.5, 0.1}; | |||
|
|
|||
| const double GRIPPER_JOINT_0_POSITION = 0.044; | |||
| const double GRIPPER_JOINT_1_POSITION = 0.0; | |||
There was a problem hiding this comment.
The constant GRIPPER_JOINT_1_POSITION is defined but never used in the code. Consider removing unused constants to improve maintainability.
| const double GRIPPER_JOINT_1_POSITION = 0.0; |
Copilot uses AI. Check for mistakes.
| @@ -105,6 +106,13 @@ class OpenArm_v10HW : public hardware_interface::SystemInterface { | |||
| const std::vector<double> DEFAULT_KD = {2.75, 2.5, 0.7, 0.4, | |||
| 0.7, 0.6, 0.5, 0.1}; | |||
|
|
|||
| const double GRIPPER_JOINT_0_POSITION = 0.044; | |||
| const double GRIPPER_JOINT_1_POSITION = 0.0; | |||
| const double GRIPPER_MOTOR_0_RADIANS = 0.0; | |||
There was a problem hiding this comment.
The constant GRIPPER_MOTOR_0_RADIANS is defined but never used in the code. Consider removing unused constants to improve maintainability.
| const double GRIPPER_MOTOR_0_RADIANS = 0.0; |
Copilot uses AI. Check for mistakes.
|
|
||
| // // Wait for motors to settle | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(100)); | ||
| std::this_thread::sleep_for(std::chrono::microseconds(1000)); |
There was a problem hiding this comment.
The sleep duration of 1000 microseconds (1ms) appears to be a magic number. Consider defining this as a named constant to improve code maintainability and make the timing requirement clear.
Copilot uses AI. Check for mistakes.
