Update hardware interface #43
Conversation
euyniy
commented
Jul 23, 2025
- Minimal working hardware plugin with v1.0 openarmcan.
- gripper logic unimplemented
There was a problem hiding this comment.
Pull Request Overview
This PR updates the OpenArm hardware interface to use the new v1.0 openarmcan library, replacing the custom CAN implementation with a simplified interface. The update removes the old multi-file motor control system and introduces a new consolidated hardware plugin specifically for OpenArm V10.
Key changes:
- Replaces custom CAN/motor control implementation with openarmcan v1.0 library
- Introduces new
OpenArm_v10HWhardware interface with simplified configuration - Removes legacy code and test files to streamline the codebase
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| openarm_hardware/src/v10_simple_hardware.cpp | New simplified hardware interface implementation using openarmcan library |
| openarm_hardware/include/openarm_hardware/v10_simple_hardware.hpp | Header file for the new V10 hardware interface |
| openarm_hardware/openarm_hardware.xml | Updates plugin registration to use new OpenArm_v10HW class |
| openarm_hardware/CMakeLists.txt | Updates build configuration to use OpenArmCAN library instead of custom sources |
| openarm_hardware/package.xml | Updates version and maintainer information |
| Multiple removed files | Removes old CAN/motor control implementation and test files |
| // TODO the mappings are unimplemented, need to actually implement the mappings for pos, vel, tau | ||
| // Convert motor position (radians) to joint value (0-1) | ||
| double motor_pos = gripper_motors[0].get_position(); | ||
| pos_states_[ARM_DOF] = motor_radians_to_joint(motor_pos); | ||
|
|
||
| // Velocity and torque can be passed through directly for now | ||
| vel_states_[ARM_DOF] = 0; // gripper_motors[0].get_velocity(); | ||
| tau_states_[ARM_DOF] = 0; // gripper_motors[0].get_torque(); |
There was a problem hiding this comment.
This TODO comment should include more specific information about what mappings need to be implemented and any relevant equations or conversion factors.
| // TODO the mappings are unimplemented, need to actually implement the mappings for pos, vel, tau | |
| // Convert motor position (radians) to joint value (0-1) | |
| double motor_pos = gripper_motors[0].get_position(); | |
| pos_states_[ARM_DOF] = motor_radians_to_joint(motor_pos); | |
| // Velocity and torque can be passed through directly for now | |
| vel_states_[ARM_DOF] = 0; // gripper_motors[0].get_velocity(); | |
| tau_states_[ARM_DOF] = 0; // gripper_motors[0].get_torque(); | |
| // Implement mappings for gripper position, velocity, and torque | |
| // Convert motor position (radians) to joint value (0-1) | |
| double motor_pos = gripper_motors[0].get_position(); | |
| pos_states_[ARM_DOF] = motor_radians_to_joint(motor_pos); | |
| // Convert motor velocity (rad/s) to joint velocity (appropriate scale) | |
| double motor_vel = gripper_motors[0].get_velocity(); | |
| vel_states_[ARM_DOF] = motor_velocity_to_joint(motor_vel); | |
| // Convert motor torque (Nm) to joint torque (appropriate scale) | |
| double motor_tau = gripper_motors[0].get_torque(); | |
| tau_states_[ARM_DOF] = motor_torque_to_joint(motor_tau); |
| // Velocity and torque can be passed through directly for now | ||
| vel_states_[ARM_DOF] = 0; // gripper_motors[0].get_velocity(); | ||
| tau_states_[ARM_DOF] = 0; // gripper_motors[0].get_torque(); | ||
| } |
There was a problem hiding this comment.
Hardcoding velocity and torque states to 0 instead of using actual motor values may cause issues for controllers that rely on accurate state feedback. Consider implementing proper conversions or at least logging a warning about incomplete implementation.
| // Velocity and torque can be passed through directly for now | |
| vel_states_[ARM_DOF] = 0; // gripper_motors[0].get_velocity(); | |
| tau_states_[ARM_DOF] = 0; // gripper_motors[0].get_torque(); | |
| } | |
| // Use actual velocity and torque values from the motor | |
| double motor_vel = gripper_motors[0].get_velocity(); | |
| double motor_tau = gripper_motors[0].get_torque(); | |
| vel_states_[ARM_DOF] = motor_vel; // Replace hardcoded value | |
| tau_states_[ARM_DOF] = motor_tau; // Replace hardcoded value | |
| // Log a warning if mappings for velocity and torque are incomplete | |
| RCLCPP_WARN(rclcpp::get_logger("OpenArm_v10HW"), | |
| "Velocity and torque mappings for gripper are unimplemented. Using raw motor values."); |
| vel_states_[ARM_DOF] = 0; // gripper_motors[0].get_velocity(); | ||
| tau_states_[ARM_DOF] = 0; // gripper_motors[0].get_torque(); |
There was a problem hiding this comment.
Hardcoding torque state to 0 instead of using actual motor values may cause issues for controllers that rely on accurate state feedback. Consider implementing proper conversions or at least logging a warning about incomplete implementation.
| vel_states_[ARM_DOF] = 0; // gripper_motors[0].get_velocity(); | |
| tau_states_[ARM_DOF] = 0; // gripper_motors[0].get_torque(); | |
| vel_states_[ARM_DOF] = gripper_motors[0].get_velocity(); | |
| tau_states_[ARM_DOF] = gripper_motors[0].get_torque(); | |
| RCLCPP_WARN(rclcpp::get_logger("OpenArm_v10HW"), | |
| "Gripper torque mapping is incomplete. Using raw motor torque value."); |
| openarm_->get_gripper().mit_control_all({{5.0, 1.0, 0.0, 0.0, 0.0}}); | ||
| } | ||
|
|
||
| // // Wait for motors to settle |
There was a problem hiding this comment.
This commented code with double slashes suggests incomplete implementation. Either remove the extra slashes if the sleep is intentional, or remove the comment entirely if it's not needed.
| // // Wait for motors to settle | |
| // Wait for motors to settle before receiving data |