Bump 1.4.5

[yun-goon]

No description provided.

[gemini-code-assist]

Hello @yun-goon, 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

Hello! Gemini here, providing a summary of this pull request to help everyone get up to speed.

This PR primarily focuses on bumping the package version to 1.4.5, incorporating several improvements and fixes from recent development, including changes originally intended for 1.4.4. The core changes involve updates to the CMake build configuration, significant enhancements to the Dynamixel communication layer (adding support for fast read protocols and direct/indirect addressing for both read and write operations), improved error handling, and better synchronization between joint states and commands. It also adds support for a new sensor model (sensorxel_joy) and refines the parameter initialization process, including per-device torque enable control instead of a global setting.

Overall, this looks like a solid update improving robustness, performance, and flexibility of the dynamixel_hardware_interface.

Highlights

• Version Bump and Changelog Update: The package version is updated to 1.4.5, and the changelog is updated to reflect the changes included in this release, as well as the previous 1.4.4 release.
• CMake Build System Updates: The CMakeLists.txt file is updated to find and link against rclcpp_lifecycle and deprecate the use of ament_include_dependency in favor of target_link_libraries for dependencies.
• Dynamixel Communication Enhancements: The Dynamixel class sees significant changes, including safer destructor handling, support for both GroupSyncRead/GroupBulkRead and GroupFastSyncRead/GroupFastBulkRead, and logic to attempt fast read protocols first with fallback to normal read. It also adds explicit handling for direct and indirect read/write addresses.
• Per-Device Torque Enable: The hardware interface now supports enabling/disabling torque on a per-Dynamixel basis via parameters, replacing the previous global torque enable setting.
• Improved Error Handling: Error handling is improved, particularly around reading Dynamixel model files (now throwing exceptions) and during communication setup and read/write operations, with better logging of specific errors.
• New Sensor Model Support: The sensorxel_joy model is added to the dynamixel.model list and its control table is defined in a new sensorxel_joy.model file.
• Enhanced Interface Mapping and Synchronization: A new MapInterfaces helper function is introduced to generalize the mapping between transmission/Dynamixel interfaces and joint/ROS2 interfaces. The joint state-command synchronization logic is also updated to use this mapping and be more robust.
• Refined Parameter Initialization: The initItems function is refactored to write parameters in a specific order (Operating Mode, then Limits, then others) and incorporates the new per-device torque enable logic.

Changelog

Click here to see the changelog

• CHANGELOG.rst
  • Deprecate ament_include_dependency usage in CMakeLists.txt
  • Added proper command interface support with ROS2-Dynamixel interface mapping
  • Improved error handling and robustness throughout the codebase
  • Implemented per-device torque enable control (replacing global control)
  • Added support for new sensor model (sensorxel_joy)
  • Enhanced joint state-command synchronization
  • Improved parameter initialization organization
  • Added robust error handling for model file reading
• package.xml
  • Bumped version to 1.4.5

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.

---

Bits flow down the wire,
Dynamixels obey desire.
States read, commands sent,
Robot moves, time well spent.

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 significant improvements to the Dynamixel hardware interface, including support for new sensor models, enhanced error handling, per-device torque control, and a more flexible interface mapping mechanism. The refactoring of the transmission matrix calculations using the new MapInterfaces function and the removal of hardcoded interface indices are particularly good changes that improve maintainability. The addition of fast read protocols (sync and bulk) with a fallback mechanism is also a valuable feature for potentially improving communication performance.

However, the review identified several critical and high-severity issues related to memory management and error handling that need to be addressed before merging. There are also some medium-severity issues regarding code complexity and blocking operations that should be considered for improvement.

Summary of Findings

• Memory Leaks: The param_write_value buffer allocated with new in SetDxlValueToBulkWrite is not deallocated, leading to a memory leak on every call. Additionally, the group write handlers (group_sync_write_, group_bulk_write_) are declared as raw pointers but are not allocated in the functions that attempt to use them (SetSyncWriteItemAndHandler, SetBulkWriteItemAndHandler, SetSyncWriteHandler, SetBulkWriteHandler), which will result in null pointer dereferences.
• Missing Error Handling for String Conversions: Functions like stoi and stod are used to parse parameters from strings (retryWriteItem, initItems, initRevoluteToPrismaticParam) without handling potential exceptions if the string is not a valid number. This can cause the node to crash.
• Use of Raw Pointers for Handlers: Raw pointers (dynamixel::Group*, dynamixel::PortHandler*) are used for communication handlers. While cleanup is present in the destructor, using smart pointers (std::unique_ptr) is the preferred modern C++ approach for automatic memory management and safety.
• Blocking Operations: Blocking loops with usleep or std::this_thread::sleep_for are used in start, CommReset, and set_dxl_torque_srv_callback. Blocking the main thread can negatively impact the real-time performance and responsiveness of the ROS2 node. Consider using non-blocking approaches or timers.
• Complex Fast Read Fallback Logic: The logic to attempt fast read protocols for a fixed number of attempts before permanently falling back adds complexity. While it aims for robustness, it might be hard to debug and potentially not optimal in all noisy environments. Consider if a simpler dynamic switching or a single attempt fallback is sufficient.
• Matrix Allocation Style: C-style dynamic allocation (double**) is used for the transmission matrices. Using std::vector<std::vector<double>> or a dedicated matrix class would be more idiomatic C++ and leverage RAII for safer memory management.
• Interface Mapping Robustness: The MapInterfaces function assumes that interface_name_vec and value_ptr_vec have matching sizes for each handler. Although checks are added in export_state_interfaces and export_command_interfaces, adding a size check within MapInterfaces itself would make the function more robust against potential mismatches.
• Error Reporting in Bulk Write Setup: The error message for gap detection in direct bulk write items in SetBulkWriteItemAndHandler uses fprintf(stderr, ...). While functional, using RCLCPP_ERROR_STREAM would integrate better with the ROS2 logging system.
• Fast Read Permanent Switch Heuristic: The decision to permanently switch to fast read after the first success might not be ideal if the communication quality fluctuates. (Low severity, noted in comments but not requiring changes based on settings).
• Minor Performance in Item Initialization: The item initialization functions involve multiple passes over the component information and parameter lists. While likely not a major bottleneck, there might be minor opportunities for optimization by reducing the number of iterations. (Low severity, noted in comments but not requiring changes based on settings).

Merge Readiness

This pull request introduces valuable features and improvements but currently contains critical memory leaks and high-severity issues related to error handling that must be resolved. The medium-severity issues should also be addressed to improve code quality and robustness. Therefore, I recommend that this pull request not be merged until the identified issues are fixed. Please have other reviewers examine the changes once the issues are addressed, as I am unable to approve the pull request myself.

[Woojin-Crive]

LGTM

[robotpilot]

LGTM