Bump 1.4.2

[robotpilot]

No description provided.

[gemini-code-assist]

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!

Summary of Changes

This pull request bumps the version of the dynamixel_hardware_interface package to 1.4.2. It includes adding OM-Y dynamixel model files, adding a function to enable torque, and fixing the configuration for OM-Y robots. It also includes some refactoring and communication improvements.

Highlights

• New Dynamixel Models: The PR adds .model files for omy_hat.model and omy_end.model, and updates dynamixel.model to include these new models. It also updates .gitignore to not ignore .model files.
• Torque Enable Functionality: A global torque enable parameter is added, along with functions to enable/disable torque for Dynamixel devices. This includes adding a torque_enable parameter and a global_torque_enable_ variable.
• Communication Improvements: The PR modifies the ReadMultiDxlData function to accept a period_ms parameter, and introduces ProcessReadCommunication and ProcessReadData functions to handle data processing and communication for SyncRead and BulkRead operations.
• Controller Items: The PR adds support for controller items, including initialization and handling of controller-specific parameters.
• Retry Write Item: The PR introduces a retryWriteItem function to retry writing an item to a Dynamixel device if the initial write fails.

Changelog

Click here to see the changelog

• .gitignore
  • Added exception to not ignore *.model files
• CHANGELOG.rst
  • Added version 1.4.2 changelog entry.
  • Included details about adding OM-Y dynamixel model files, adding a function to enable torque, and fixing the configuration for OM-Y robots.
  • Added Woojin Wie and Wonho Yun as contributors.
• include/dynamixel_hardware_interface/dynamixel/dynamixel.hpp
  • Added <functional> include.
  • Modified ReadMultiDxlData to accept a double period_ms parameter.
  • Modified GetDxlValueFromSyncRead to accept a double period_ms parameter.
  • Modified GetDxlValueFromBulkRead to accept a double period_ms parameter.
  • Added ProcessReadData function declaration.
  • Added ProcessReadCommunication function declaration.
• include/dynamixel_hardware_interface/dynamixel_hardware_interface.hpp
  • Added global_torque_enable_ variable.
  • Added controller_id_ and controller_item_ variables.
  • Added is_set_hdl_ variable.
  • Added initItems function declaration.
  • Added retryWriteItem function declaration.
  • Added InitControllerItems function declaration.
• package.xml
  • Bumped version to 1.4.2.
  • Added Wonho Yun as an author.
• param/dxl_model/dynamixel.model
  • Added omy_hat.model and omy_end.model to the list of available models.
• param/dxl_model/omy_end.model
  • Added new model file for omy_end.
• param/dxl_model/omy_hat.model
  • Added new model file for omy_hat.
• param/dxl_model/rh_p12_rn.model
  • Corrected value_of_min_radian_position and min_radian values.
• src/dynamixel/dynamixel.cpp
  • Added <functional> include.
  • Modified DynamixelDisable to return DxlError.
  • Modified ReadMultiDxlData to accept a double period_ms parameter.
  • Modified GetDxlValueFromSyncRead to accept a double period_ms parameter and use ProcessReadCommunication and ProcessReadData.
  • Corrected return error for SetBulkReadItemAndHandler.
  • Modified GetDxlValueFromBulkRead to accept a double period_ms parameter and use ProcessReadCommunication and ProcessReadData.
  • Added ProcessReadCommunication function implementation.
  • Added ProcessReadData function implementation.
• src/dynamixel_hardware_interface.cpp
  • Added parsing of torque_enable parameter.
  • Added support for controller type GPIOs.
  • Added initialization of controller items.
  • Modified start to use ReadMultiDxlData(0.0).
  • Added conditional torque enabling based on global_torque_enable_.
  • Modified read to use ReadMultiDxlData(period_ms).
  • Added InitControllerItems to CommReset.
  • Added retryWriteItem function implementation.
  • Added initItems function implementation.
  • Modified InitDxlItems to use initItems.
  • Removed static keyword from is_set_hdl and renamed to is_set_hdl_.

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.

---

A motor's hum, a gear's soft click,
Torque enabled, doing the trick.
New models arise,
Before our eyes,
Robots awaken, built brick by brick.

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 bumps the version to 1.4.2, adds OM-Y dynamixel model files, adds a function to enable torque, and fixes the configuration for OM-Y robots. The changes touch several files, including .gitignore, CHANGELOG.rst, header files, source files, package.xml, and model files. Overall, the changes seem reasonable, but there are some areas that could be improved.

Summary of Findings

• Error Handling in DynamixelDisable: The DynamixelDisable function in src/dynamixel/dynamixel.cpp could potentially mask errors. If writing "Torque Off" fails for one ID, the function continues processing other IDs but still returns DxlError::OK, even though an error occurred. This could lead to unexpected behavior where some motors are disabled while others are not, without the caller being aware of the failure.
• Missing Timeout Handling in ProcessReadCommunication: The ProcessReadCommunication function in src/dynamixel/dynamixel.cpp sets a packet timeout based on period_ms but doesn't seem to handle the case where the timeout expires during the rxPacket calls. This could lead to indefinite blocking if a Dynamixel device fails to respond.
• Inconsistent Error Handling in SetBulkReadItemAndHandler: The SetBulkReadItemAndHandler function in src/dynamixel/dynamixel.cpp returns DxlError::SYNC_READ_FAIL when it fails to set the BulkRead handler, which is inconsistent. It should return DxlError::BULK_READ_FAIL instead.
• Redundant Initialization of trying_connect and trying_cnt: In src/dynamixel_hardware_interface.cpp, the variables trying_connect and trying_cnt are initialized multiple times, which is redundant and can be simplified.

Merge Readiness

The pull request introduces important features and fixes, but there are some potential issues with error handling and timeout management that should be addressed before merging. Specifically, the error handling in DynamixelDisable and the timeout handling in ProcessReadCommunication should be reviewed and improved. Additionally, the inconsistent error code in SetBulkReadItemAndHandler should be corrected. I am unable to approve this pull request, and recommend that these issues be addressed before merging. Another reviewer should also look at this code before merging.

[gemini-code-assist]

The function returns DxlError::OK even if WriteItem fails for some of the IDs. This could mask errors and lead to unexpected behavior. It would be better to return an error if any of the write operations fail. Consider returning the first error that occurs, or aggregating the errors and returning a combined error status.

Suggested change

	DxlError result = DxlError::OK;
	for (auto it_id : id_arr) {
	if (torque_state_[it_id] == TORQUE_ON) {
	if (WriteItem(it_id, "Torque Enable", TORQUE_OFF) < 0) {
	fprintf(stderr, "[ID:%03d] Cannot write \"Torque Off\" command!\n", it_id);
	return DxlError::ITEM_WRITE_FAIL;
	result = DxlError::ITEM_WRITE_FAIL;
	} else {
	torque_state_[it_id] = TORQUE_OFF;
	fprintf(stderr, "[ID:%03d] Torque OFF\n", it_id);
	}
	}
	}
	return DxlError::OK;
	return result;
	DxlError Dynamixel::DynamixelDisable(std::vector<uint8_t> id_arr)
	{
	DxlError result = DxlError::OK;
	for (auto it_id : id_arr) {
	if (torque_state_[it_id] == TORQUE_ON) {
	DxlError write_result = WriteItem(it_id, "Torque Enable", TORQUE_OFF);
	if (write_result != DxlError::OK) {
	fprintf(stderr, "[ID:%03d] Cannot write \"Torque Off\" command!\n", it_id);
	result = (result == DxlError::OK) ? write_result : result; // Preserve the first error
	} else {
	torque_state_[it_id] = TORQUE_OFF;
	fprintf(stderr, "[ID:%03d] Torque OFF\n", it_id);
	}
	}
	}
	return result;
	}