Skip to content

Conversation

Woojin-Crive
Copy link
Member

Description

This pull request introduces significant updates to the open_manipulator_bringup and open_manipulator_description packages to enhance the initialization and control of the manipulator's joints. The most important changes include the updates to the ROS2 control parameters for better hardware interaction.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @Woojin-Crive, I'm Gemini Code Assist1! 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 introduces updates for the new OM-Y Follower hardware, focusing on enhancing joint initialization and control. The changes primarily involve modifications to the open_manipulator_bringup and open_manipulator_description packages. A key aspect is the update of ROS2 control parameters to improve hardware interaction, including switching to action-based control for smoother trajectories and adjusting dynamixel parameters.

Highlights

  • Action-based Control: The init_position_for_follower.py script is updated to use action clients for FollowJointTrajectory, replacing the previous publisher-based approach. This allows for smoother, more controlled joint movements using quintic polynomial trajectory generation.
  • New Script for Packing Position: A new script, pack_position_for_follower.py, is added, mirroring the functionality of init_position_for_follower.py but with slightly different target positions. Both scripts now implement a step-by-step movement approach.
  • Dynamixel Parameter Adjustments: Several XACRO files in open_manipulator_description are modified to adjust Dynamixel hardware parameters, including changing error_timeout_sec to error_timeout_ms and updating baud rates and drive modes for the follower and leader systems.
  • URDF Macro Updates: The URDF XACRO files are updated to include the new end unit and follower system configurations, streamlining the robot description process.

Changelog

Click here to see the changelog
  • open_manipulator_bringup/scripts/init_position_for_follower.py
    • Replaced publisher with action client for FollowJointTrajectory.
    • Implemented step-by-step movement using smooth quintic polynomial trajectories.
    • Added feedback and goal response callbacks for action client.
    • Initialized target positions based on current joint states.
    • Reduced error threshold for smoother motion.
    • Added logging for current and target positions, and current step.
  • open_manipulator_bringup/scripts/pack_position_for_follower.py
    • Added a new script that mirrors the functionality of init_position_for_follower.py but with slightly different target positions.
    • Implements step-by-step movement using smooth quintic polynomial trajectories.
    • Includes action client for FollowJointTrajectory and related callbacks.
  • open_manipulator_description/ros2_control/open_manipulator_x_system.ros2_control.xacro
    • Changed error_timeout_sec to error_timeout_ms and set the value to 500.
    • Removed ros_update_freq parameter.
  • open_manipulator_description/ros2_control/open_manipulator_y_end_unit.ros2_control.xacro
    • Added a new XACRO file to define the ROS2 control parameters for the OM-Y end unit, including Dynamixel hardware interface settings and GPIO configurations.
  • open_manipulator_description/ros2_control/open_manipulator_y_follower_system.ros2_control.xacro
    • Changed port_name to /dev/ttyAMA2 and baud_rate to 6250000.
    • Changed error_timeout_sec to error_timeout_ms and set the value to 500.
    • Reduced the number of joints and transmissions to 6.
    • Removed ros_update_freq parameter.
    • Removed joint limits from joint definitions.
    • Modified drive modes and homing offsets for specific joints.
    • Replaced dxl7 GPIO with omy_hat GPIO, configuring controller parameters.
  • open_manipulator_description/ros2_control/open_manipulator_y_leader_system.ros2_control.xacro
    • Changed error_timeout_sec to error_timeout_ms and set the value to 500.
    • Removed ros_update_freq parameter.
    • Added reboot and torque setting services.
    • Added drive mode and operating mode parameters to the gpio definitions.
  • open_manipulator_description/ros2_control/open_manipulator_y_system.ros2_control.xacro
    • Changed error_timeout_sec to error_timeout_ms and set the value to 500.
    • Removed ros_update_freq parameter.
  • open_manipulator_description/urdf/om_y/open_manipulator_y.urdf.xacro
    • Removed conditional inclusion of follower URDF.
    • Included open_manipulator_y_end_unit.ros2_control.xacro.
  • open_manipulator_description/urdf/om_y_follower/open_manipulator_y_follower.urdf.xacro
    • Created a new XACRO file for the OM-Y follower URDF, including necessary gazebo and ros2 control macros.
  • open_manipulator_description/urdf/om_y_leader/open_manipulator_y_leader.urdf.xacro
    • Added use_sim, use_fake_hardware, and fake_sensor_commands arguments.
    • Included open_manipulator_y_leader_system.ros2_control.xacro.
  • open_manipulator_moveit_config/config/om_y_follower/initial_positions.yaml
    • Created a new YAML file for initial joint positions of the OM-Y follower.
  • open_manipulator_moveit_config/config/om_y_follower/joint_limits.yaml
    • Created a new YAML file for joint limits of the OM-Y follower.
  • open_manipulator_moveit_config/config/om_y_follower/kinematics.yaml
    • Created a new YAML file for kinematics configuration of the OM-Y follower.
  • open_manipulator_moveit_config/config/om_y_follower/moveit.rviz
    • Created a new RViz configuration file for the OM-Y follower.
  • open_manipulator_moveit_config/config/om_y_follower/moveit_controllers.yaml
    • Created a new YAML file for MoveIt controllers configuration of the OM-Y follower.
  • open_manipulator_moveit_config/config/om_y_follower/moveit_servo.yaml
    • Created a new YAML file for MoveIt Servo configuration of the OM-Y follower.
  • open_manipulator_moveit_config/config/om_y_follower/ompl_planning.yaml
    • Created a new YAML file for OMPL planning configuration of the OM-Y follower.
  • open_manipulator_moveit_config/config/om_y_follower/open_manipulator_y_follower.srdf
    • Created a new SRDF file for the OM-Y follower, defining groups, states, and collision settings.
  • open_manipulator_moveit_config/config/om_y_leader/initial_positions.yaml
    • Updated the description of the initial positions YAML file for the OM-Y leader.
  • open_manipulator_moveit_config/config/om_y_leader/open_manipulator_y_leader.srdf
    • Updated the robot name in the SRDF file for the OM-Y leader.
    • Corrected the base link name in the disable collisions section.
  • open_manipulator_moveit_config/config/om_y_leader/ros2_controllers_leader.yaml
    • Added kinetic friction scalars and torque scaling factors to the ros2 controllers configuration.
  • open_manipulator_moveit_config/launch/move_group.launch.py
    • Added om_y_leader to the list of valid ROBOT_MODEL values.
    • Added a case for om_y_leader to set the urdf_file and urdf_folder variables.
  • open_manipulator_moveit_config/launch/moveit_rviz.launch.py
    • Added om_y_leader to the list of valid ROBOT_MODEL values.
    • Added a case for om_y_leader to set the urdf_file and urdf_folder variables.
  • ros2_controller/om_gravity_compensation_controller/include/gravity_compensation_controller/gravity_compensation_controller.hpp
    • Replaced relative includes with bracketed includes.
    • Replaced gravity_compensation_controller_parameters.hpp with om_gravity_compensation_controller/gravity_compensation_controller_parameters.hpp.
    • Removed dof_ member variable.
    • Added previous_velocities_ member variable.
  • ros2_controller/om_gravity_compensation_controller/src/gravity_compensation_controller.cpp
    • Replaced relative includes with bracketed includes.
    • Calculated joint accelerations from velocity using finite difference.
    • Populated joint accelerations from state interfaces.
    • Updated previous velocities for next iteration.
    • Replaced dof_ with n_joints_ in error messages.
    • Replaced relative includes with bracketed includes.
  • ros2_controller/om_gravity_compensation_controller/src/gravity_compensation_controller_parameters.yaml
    • Set default values for unloaded_effort_offsets and unloaded_effort_thresholds to 0.0.
    • Set default values for static_friction_scalars and static_friction_velocity_thresholds to 0.0.
    • Set default values for torque_scaling_factors to 1.0.
    • Set default values for kinetic_friction_torque_scalars to 3.0.
  • ros2_controller/om_joint_trajectory_command_broadcaster/include/joint_trajectory_command_broadcaster/joint_trajectory_command_broadcaster.hpp
    • Replaced relative includes with bracketed includes.
    • Replaced joint_trajectory_command_broadcaster_parameters.hpp with om_joint_trajectory_command_broadcaster/joint_trajectory_command_broadcaster_parameters.hpp.
  • ros2_controller/om_joint_trajectory_command_broadcaster/src/joint_trajectory_command_broadcaster.cpp
    • Replaced relative includes with bracketed includes.
  • ros2_controller/om_spring_actuator_controller/include/spring_actuator_controller/spring_actuator_controller.hpp
    • Replaced relative includes with bracketed includes.
    • Replaced spring_actuator_controller_parameters.hpp with om_spring_actuator_controller/spring_actuator_controller_parameters.hpp.
  • ros2_controller/om_spring_actuator_controller/src/spring_actuator_controller.cpp
    • Replaced relative includes with bracketed includes.
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 XACRO's embrace,
Robots rise with parameters,
A structured dance.

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.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces updates to the open_manipulator_bringup and open_manipulator_description packages, focusing on enhancing joint initialization and control, particularly for the new OM-Y Follower hardware. The changes include updates to ROS2 control parameters and the introduction of new scripts for position initialization. Overall, the changes seem well-structured and address the intended improvements.

Summary of Findings

  • Action Client Usage: The transition from publishing JointTrajectory messages to using the FollowJointTrajectory action client is a significant change that should improve the control and feedback mechanisms. However, ensure that the action server is robust and handles potential errors gracefully.
  • Trajectory Generation: The implementation of quintic polynomial trajectory generation in create_smooth_trajectory is a good approach for smoother motion. Verify that the trajectory generation handles edge cases and joint limits correctly.
  • Error Handling and Recovery: The code includes some basic error handling, such as checking for action server availability. Consider adding more comprehensive error handling and recovery mechanisms, especially for hardware-related issues.
  • Parameter Updates: The changes in open_manipulator_description involve updating parameters like error_timeout_ms and baud_rate. Ensure that these parameters are correctly tuned for the new hardware and that they are configurable.

Merge Readiness

The pull request introduces significant updates and new features that appear to improve the control and initialization of the manipulator's joints. However, before merging, it's crucial to address the identified issues, particularly those related to error handling, trajectory generation edge cases, and parameter tuning. Given the high and critical severity comments, I recommend that the pull request not be merged until those are addressed (at a minimum). I am unable to directly approve the pull request, and users should have others review and approve this code before merging.

@robotpilot robotpilot self-assigned this Apr 4, 2025
@robotpilot robotpilot added the enhancement New feature or request label Apr 4, 2025
@robotpilot robotpilot moved this to 📝 Pull Request in Platform Apr 4, 2025
- Removed open-manipulator-y-cdc.rules and associated udev script.
- Renamed and updated initialization scripts for x and y positions.
- Added new launch files for packing and unpacking operations.
- Adjusted launch configurations for hardware components and initialization sequences.
- Improved structure and organization of scripts for better maintainability.
…figurations

- Added new script `init_position_y_follower.py` for follower initialization.
- Updated launch file to use the new follower script instead of the old one.
- Adjusted logging messages to reflect the changes in script names.
- Changed permissions for `pack_unpack_y.py` to make it executable.
@Woojin-Crive Woojin-Crive requested a review from sunghowoo April 9, 2025 01:35
@robotpilot robotpilot requested a review from yun-goon April 9, 2025 07:36
@robotpilot robotpilot assigned sunghowoo and unassigned robotpilot Apr 9, 2025
Copy link
Member

@yun-goon yun-goon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. It looks like it will become an even better OpenManipulator (OM)!

Copy link
Member

@sunghowoo sunghowoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your hard work in improving the OM functionality

@sunghowoo sunghowoo merged commit b316780 into main Apr 9, 2025
7 checks passed
@sunghowoo sunghowoo deleted the feature-omy branch April 9, 2025 10:07
@github-project-automation github-project-automation bot moved this from 📝 Pull Request to 🚩Done in Platform Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants