Support Current Control

[Woojin-Crive]

Changes

1. Add Torque Constant Parameter to DXL Model Files

   • Introduced 'torque_constant' parameter to xc330_t181.model, xc330_t288.model, and xh540_w150.model
   • Added specific torque constant values for each model:
     • xc330_t181: 0.0006709470296015791
     • xc330_t288: 0.0009674796739867748
     • xh540_w150: 0.0045

2. Enhance Transmission Command Calculation

   • Improved handling of 'Goal Current' interface in transmission commands
   • Added conditional checks for interface names before accumulating values
   • Ensures proper validation of hardware interface types

3. Unified Initialization Structure

   • Refactored initialization code for better organization
   • Standardized command interface handling
   • Improved support for multiple command interfaces

4. Support for Goal Current Control

   • Added support for 'Goal Current' control in OpenMANIPULATOR-Y
   • Introduced new models: xh540_w150 and xc330_t288
   • Enhanced current-to-effort conversion functionality

Implementation Details

• Added torque constant parameter to model files for accurate torque calculations
• Implemented proper interface validation in transmission command calculations
• Unified the structure of initialization code for better maintainability
• Enhanced current control support with proper conversion functions

Why

Improved Control and Compatibility

• Enables more precise torque control through accurate torque constants
• Provides better support for current-based control mode
• Improves code organization and maintainability
• Ensures proper validation of hardware interfaces

Let me know if any refinements are needed! 🚀

[gemini-code-assist]

Hello @Woojin-Crive, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request focuses on adding support for current control in the dynamixel hardware interface. It introduces a 'torque_constant' parameter to the DXL model files (xc330_t181.model, xc330_t288.model, and xh540_w150.model) and enhances the transmission command calculation to handle 'Goal Current' interface. The initialization structure is unified for better organization, and support for 'Goal Current' control is added, including current-to-effort conversion functionality. The changes also involve modifications to the dynamixel.cpp and dynamixel_info.cpp files to properly convert current to effort and retrieve the torque constant.

Highlights

• Torque Constant Parameter: Added 'torque_constant' parameter to xc330_t181.model, xc330_t288.model, and xh540_w150.model with specific values for each model to enable more precise torque control.
• Goal Current Control: Implemented support for 'Goal Current' control, including enhancements to transmission command calculation and current-to-effort conversion.
• Unified Initialization: Refactored and unified the initialization code structure for better maintainability and standardized command interface handling.

Changelog

Click here to see the changelog

• include/dynamixel_hardware_interface/dynamixel/dynamixel_info.hpp
  • Added torque_constant parameter to the DynamixelInfo::GetDxlTypeInfo method signature (line 82).
• include/dynamixel_hardware_interface/dynamixel_hardware_interface.hpp
  • Added GOAL_CURRENT_INDEX definition (line 52).
• param/dxl_model/xc330_t181.model
  • Added torque_constant parameter with a value of 0.0006709470296015791 (line 8).
• param/dxl_model/xc330_t288.model
  • Added torque_constant parameter with a value of 0.0009674796739867748 (line 8).
• param/dxl_model/xh540_w150.model
  • Added torque_constant parameter with a value of 0.0045 (line 8).
• src/dynamixel/dynamixel.cpp
  • Modified GetDxlValueFromSyncRead to convert 'Present Current' to effort using dxl_info_.ConvertCurrentToEffort (line 843).
  • Modified GetDxlValueFromBulkRead to convert 'Present Current' to effort using dxl_info_.ConvertCurrentToEffort (line 965).
• src/dynamixel/dynamixel_info.cpp
  • Modified GetDxlTypeInfo to retrieve torque_constant from dxl_info_ (line 160).
• src/dynamixel_hardware_interface.cpp
  • Modified InitDxlWriteItems to include 'Goal Current' interface (lines 714-728).
  • Modified CalcJointToTransmission to handle 'Goal Current' in transmission commands (lines 871-883).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

In realms of robotics, where motors reside,
Current control emerges, a powerful stride.
Torque constants defined, with precision and care,
Enhancing the interface, beyond compare.
With each line of code, a symphony plays,
As robots respond in intelligent ways.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request introduces support for current control in the dynamixel hardware interface, which includes adding a torque constant parameter to the DXL model files, enhancing transmission command calculation, unifying the initialization structure, and adding support for 'Goal Current' control. The changes seem well-structured and address the intended functionality. However, there are a few areas that could benefit from further review and refinement.

Summary of Findings

• Missing null check: There is a potential null pointer dereference in DynamixelHardware::CalcJointToTransmission if hdl_joint_commands_.at(j).interface_name_vec is empty. This could lead to a crash if the joint command interface is not properly initialized.
• Inconsistent Interface Handling: The InitDxlWriteItems function seems to be adding "Goal Position" and "Goal Current" interfaces unconditionally, which might not be appropriate for all Dynamixel configurations. This could lead to unexpected behavior if a Dynamixel doesn't support these interfaces.
• Magic Number: The constants PRESENT_POSITION_INDEX, PRESENT_VELOCITY_INDEX, PRESENT_EFFORT_INDEX, GOAL_POSITION_INDEX, and GOAL_CURRENT_INDEX are defined using #define. It would be better to use constexpr or enum class to improve type safety and readability.

Merge Readiness

The pull request introduces important functionality for current control. However, the potential null pointer dereference and inconsistent interface handling should be addressed before merging. I am unable to approve this pull request, and recommend that it not be merged until the critical and high severity issues are addressed, and that others review and approve this code before merging.

[sunghowoo]

Great improvements! The torque constant parameter addition and enhanced control support look solid. 🚀

[robotpilot]

@Woojin-Crive Before proceeding with the review, please ensure all CI checks and linting issues are resolved. Once these items are addressed, we can move forward with a comprehensive code review. Thank you for your cooperation.

[Woojin-Crive]

@robotpilot Thanks for your feedback. I've addressed the CI and lint issues and pushed the fixes. Please let me know if there's anything else you'd like me to adjust. Thanks again for your review!

[robotpilot]

Great! LGTM. Thanks, @Woojin-Crive :)