Bump 1.4.7 #52

robotpilot · 2025-06-19T23:00:01Z

No description provided.

…virtual DXL support - Added `comm_id` to `HandlerVarType` for better communication management. - Updated methods to utilize `comm_id` for reading and writing operations. - Introduced handling for virtual DXL components in GPIO initialization and state management. - Improved error handling and logging for GPIO parameter checks. - Updated model files to include new virtual DXL models.

…and model definitions

…y mechanism - Updated the maximum number of retries for communication operations from 10 to 5. - Added a new method `InitTorqueStates` to manage torque states during initialization. - Enhanced error logging for torque state management and communication issues. - Introduced a parameter to disable torque at initialization for improved safety.

…larity

- Changed goal_velocity type from int16_t to int32_t for better precision. - Replaced instances of ID with comm_id for improved readability and consistency across the codebase. - Added a check to return early if the write_data_list_ is empty in SetDxlValueToBulkWrite method.

- Added early return checks in ReadMultiDxlData and WriteMultiDxlData methods to handle empty data lists.

…lude file path

…r better readability and consistency

…ecessary operations

Add virtual_dxl feaure

gemini-code-assist

Summary of Changes

Hello @robotpilot, 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!

This pull request introduces significant enhancements to the dynamixel_hardware_interface by adding support for virtual Dynamixels and improving the robustness of communication. It refactors how Dynamixel IDs are managed, allowing for more complex and flexible hardware setups, and refines velocity conversions for greater accuracy. The changes also include general code improvements for reliability and maintainability.

Highlights

Virtual Dynamixel Support: Introduced the concept of 'virtual Dynamixels' (virtual_dxl) allowing for more flexible hardware configurations, including the ability to map multiple physical Dynamixel IDs to a single communication ID (comm_id). This is crucial for devices like the RCU (Robotis Control Unit) that manage multiple motors under one communication channel.
Communication Robustness: Implemented a retry mechanism (MAX_COMM_RETRIES) for individual Dynamixel read/write operations to enhance reliability. The system will also attempt to reboot a Dynamixel if a hardware error is detected during initial communication.
Enhanced ID Management: Refactored Dynamixel ID handling by introducing a comm_id alongside the physical id. This allows the system to communicate with multiple Dynamixels (or virtual ones) through a single communication channel, streamlining read/write operations, especially for sync and bulk protocols.
Improved Velocity Conversion: Added a velocity_unit parameter to Dynamixel model files and updated velocity conversion functions to utilize this new parameter. This provides more accurate and motor-specific velocity calculations.
Initialization and Torque Control: Added a disable_torque_at_init parameter, allowing users to configure whether the system should automatically disable Dynamixel torque during initialization if it's found to be enabled. This helps prevent unexpected motor movements on startup.
Code Modernization and Refactoring: Replaced usleep calls with std::this_thread::sleep_for for improved cross-platform compatibility and precision. The redundant retryWriteItem helper function was removed, with its logic integrated directly into the WriteItem method, simplifying the codebase.

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.

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

This pull request primarily adds support for virtual Dynamixels and RCU, involving significant changes to communication logic, handler initialization, and data structures. Key improvements include retry mechanisms for robustness and new model file support. A critical bug in handler initialization has been identified, along with several medium-severity issues related to maintainability and best practices. Addressing these will improve the code's reliability and clarity.

gemini-code-assist · 2025-06-19T23:03:41Z

include/dynamixel_hardware_interface/dynamixel/dynamixel.hpp


  // Set Dxl Option
-  DxlError SetOperatingMode(uint8_t id, uint8_t dynamixel_mode);
+  // DxlError SetOperatingMode(uint8_t id, uint8_t dynamixel_mode);


The method SetOperatingMode is commented out. If this method is no longer needed or deprecated, it should be removed entirely to avoid confusion and dead code. If it's commented out for temporary reasons (e.g., debugging or pending refactor), a TODO comment explaining the situation and when it might be reinstated would be beneficial for maintainability.

gemini-code-assist · 2025-06-19T23:03:41Z

include/dynamixel_hardware_interface/dynamixel/dynamixel_info.hpp

+  // bool GetDxlTypeInfo(
+  //   uint8_t id,
+  //   int32_t & value_of_zero_radian_position,
+  //   int32_t & value_of_max_radian_position,
+  //   int32_t & value_of_min_radian_position,
+  //   double & min_radian,
+  //   double & max_radian,
+  //   double & torque_constant,
+  //   double & velocity_unit);


The method GetDxlTypeInfo is commented out. Similar to SetOperatingMode, if this is a permanent removal, the commented code should be deleted. If it's temporary, a TODO comment should be added to clarify its status and future intentions. This improves code clarity and reduces clutter.

gemini-code-assist · 2025-06-19T23:03:41Z

include/dynamixel_hardware_interface/dynamixel_hardware_interface.hpp

    size_t inner_size,
    std::vector<HandlerVarType> & outer_handlers,
-    const std::vector<HandlerVarType> & inner_handlers,
+    std::vector<HandlerVarType> & inner_handlers,


The inner_handlers parameter in MapInterfaces was changed from const std::vector<HandlerVarType> & to std::vector<HandlerVarType> &. However, the implementation of MapInterfaces (as seen in the full file context) only reads from inner_handlers and does not modify it. It's a good practice to keep parameters const if they are not modified by the function, as this enhances code safety and readability by clearly stating the function's intent. Consider reverting this to const & if inner_handlers is indeed not modified.

Suggested change

std::vector<HandlerVarType> & inner_handlers,

const std::vector<HandlerVarType> & inner_handlers,

gemini-code-assist · 2025-06-19T23:03:42Z

src/dynamixel/dynamixel.cpp

+//   }
+
+//   return DxlError::OK;
+// }


The method SetOperatingMode is commented out. If this method is no longer needed or deprecated, it should be removed entirely. If it's commented out for temporary reasons, a TODO comment explaining the situation would be helpful for maintainability.

gemini-code-assist · 2025-06-19T23:03:42Z

src/dynamixel/dynamixel.cpp

+  fprintf(stderr, "MAX_COMM_RETRIES should be set to 1 or more\n");
+  return DxlError::ITEM_WRITE_FAIL;


This fprintf message, "MAX_COMM_RETRIES should be set to 1 or more\n", is potentially confusing or indicates dead code.
Given MAX_COMM_RETRIES is a #define 5, this line is only reached if all 5 retry attempts fail. The message seems to imply a configuration issue with MAX_COMM_RETRIES itself, rather than the exhaustion of retries.

Consider rephrasing to indicate that all retries failed, or removing it if the preceding per-retry error logs are sufficient.

gemini-code-assist · 2025-06-19T23:03:42Z

src/dynamixel/dynamixel.cpp

+  fprintf(stderr, "MAX_COMM_RETRIES should be set to 1 or more\n");
+  return DxlError::ITEM_READ_FAIL;


Similar to the WriteItem retry loop, this fprintf message, "MAX_COMM_RETRIES should be set to 1 or more\n", is potentially confusing. It's reached after all MAX_COMM_RETRIES (defined as 5) attempts have failed.

Consider clarifying the message to reflect that all retries were exhausted, or remove it if previous logs are adequate.

gemini-code-assist · 2025-06-19T23:03:42Z

src/dynamixel/dynamixel_info.cpp

+// bool DynamixelInfo::GetDxlTypeInfo(
+//   uint8_t id,
+//   int32_t & value_of_zero_radian_position,
+//   int32_t & value_of_max_radian_position,
+//   int32_t & value_of_min_radian_position,
+//   double & min_radian,
+//   double & max_radian,
+//   double & torque_constant,
+//   double & velocity_unit)
+// {
+//   value_of_zero_radian_position = dxl_info_[id].value_of_zero_radian_position;
+//   value_of_max_radian_position = dxl_info_[id].value_of_max_radian_position;
+//   value_of_min_radian_position = dxl_info_[id].value_of_min_radian_position;
+//   min_radian = dxl_info_[id].min_radian;
+//   max_radian = dxl_info_[id].max_radian;
+//   torque_constant = dxl_info_[id].torque_constant;
+//   velocity_unit = dxl_info_[id].velocity_unit;
+//   return true;
+// }


The method GetDxlTypeInfo is commented out. If this functionality is deprecated or no longer required, it's better to remove the commented code to keep the codebase clean. If it's a temporary measure, a TODO comment explaining the reason and expected resolution would be beneficial.

gemini-code-assist · 2025-06-19T23:03:42Z

src/dynamixel_hardware_interface.cpp

+    if (gpio.parameters.find("ID") == gpio.parameters.end()) {
+      RCLCPP_ERROR_STREAM(logger_, "ID is not found in gpio parameters");
+      exit(-1);
+    }
+    if (gpio.parameters.find("type") == gpio.parameters.end()) {
+      RCLCPP_ERROR_STREAM(logger_, "type is not found in gpio parameters");
+      exit(-1);
+    }


Using exit(-1) for configuration errors (missing 'ID' or 'type' parameters) can be quite abrupt for a ROS node. Consider using rclcpp::shutdown() followed by returning hardware_interface::CallbackReturn::ERROR from the on_init method. This allows for a more graceful shutdown within the ROS ecosystem and can be easier to manage and debug.

gemini-code-assist · 2025-06-19T23:03:42Z

src/dynamixel_hardware_interface.cpp

    transmission_to_joint_matrix_,
    dynamixel_hardware_interface::ros2_to_dxl_state_map,
-    "Present Position",
+    hardware_interface::HW_IF_POSITION,


Using hardware_interface::HW_IF_POSITION constant instead of the string literal "Present Position" for conversion_iface is a good improvement for consistency and maintainability.

Woojin-Crive and others added 21 commits June 5, 2025 10:22

feat: Add GPIO controller state handling and rcu model for ffw

ba695c9

refactor: Update Dynamixel interface for improved handler management …

28c35b6

…and model definitions

refactor: Improve formatting

3dae77e

refactor: Update position parameter in transmission calculation for c…

bb268c6

…larity

refactor: Remove unused retryWriteItem function from Dynamixel interface

ed1d1ee

refactor: Remove duplicate entries for Indirect Address in model file

6aebda4

refactor: Clarify documentation for Dynamixel motor ID array

d157ee4

feat: Add velocity unit handling to Dynamixel model processing

d88ff3b

feat: Add velocity unit parameter to multiple Dynamixel model files

6f696f4

refactor: Improve warning message formatting

00f7d86

refactor: Optimize early return checks in Dynamixel data handling

4392276

- Added early return checks in ReadMultiDxlData and WriteMultiDxlData methods to handle empty data lists.

fix: Enhance warning messages in Dynamixel model file handling to inc…

bf0452f

…lude file path

refactor: Replace sleep functions with std::this_thread::sleep_for fo…

6e1ed52

…r better readability and consistency

fix: Add thread header for dynamixel.cpp

44d3f8f

fix: Handle empty read data list in Dynamixel read operations

7e20f4e

fix: Add check for empty ID array in Sync Read Handler to prevent unn…

f9d5233

…ecessary operations

chore: Update version to 1.4.7 and add new features

1d790f7

Merge pull request #47 from ROBOTIS-GIT/feature-rcu

4be96ec

Add virtual_dxl feaure

robotpilot self-assigned this Jun 19, 2025

robotpilot added the bump Increase the version number to release label Jun 19, 2025

robotpilot added this to Platform Jun 19, 2025

robotpilot moved this to 📝 Pull Request in Platform Jun 19, 2025

gemini-code-assist bot reviewed Jun 19, 2025

View reviewed changes

robotpilot merged commit bddc63d into humble Jun 19, 2025
25 checks passed

github-project-automation bot moved this from 📝 Pull Request to 🚩Done in Platform Jun 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bump 1.4.7 #52

Bump 1.4.7 #52

Uh oh!

robotpilot commented Jun 19, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

gemini-code-assist bot Jun 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	std::vector<HandlerVarType> & inner_handlers,
	const std::vector<HandlerVarType> & inner_handlers,

		fprintf(stderr, "MAX_COMM_RETRIES should be set to 1 or more\n");
		return DxlError::ITEM_WRITE_FAIL;

		fprintf(stderr, "MAX_COMM_RETRIES should be set to 1 or more\n");
		return DxlError::ITEM_READ_FAIL;

Bump 1.4.7 #52

Bump 1.4.7 #52

Uh oh!

Conversation

robotpilot commented Jun 19, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants