Bump 1.4.3 #33

yun-goon · 2025-04-14T00:16:29Z

No description provided.

Signed-off-by: Wonho Yun <ywh@robotis.com>

Modified Build Error

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

This pull request bumps the version of the dynamixel_hardware_interface package from 1.4.2 to 1.4.3. It includes a fix for build errors, updates the changelog, and modifies the dynamixel.cpp and dynamixel_hardware_interface.cpp files to improve error message formatting and remove unused variables. Additionally, the ConvertValueRPMToVelocityRPS and ConvertVelocityRPSToValueRPM functions in dynamixel_info.hpp are updated to remove the unnecessary id parameter.

Highlights

Version Bump: The package version is bumped from 1.4.2 to 1.4.3.
Build Error Fix: A fix is implemented to resolve build errors.
Code Improvements: Minor code improvements are made in dynamixel.cpp and dynamixel_hardware_interface.cpp.
Function Signature Update: The ConvertValueRPMToVelocityRPS and ConvertVelocityRPSToValueRPM functions in dynamixel_info.hpp are updated to remove the unnecessary id parameter.

Changelog

Click here to see the changelog

CHANGELOG.rst
- Added version 1.4.3 changelog entry.
- Indicated that build errors were fixed.
- Added Wonho Yun as a contributor.
include/dynamixel_hardware_interface/dynamixel/dynamixel_info.hpp
- Removed the id parameter from ConvertValueRPMToVelocityRPS and ConvertVelocityRPSToValueRPM functions.
package.xml
- Updated the package version to 1.4.3.
src/dynamixel/dynamixel.cpp
- Added a default case to the DxlErrorToString function to return "UNKNOWN_ERROR".
- Modified fprintf statements in ProcessReadCommunication to use %ld for printing the size of read_data_list_.
- Removed the id parameter when calling ConvertValueRPMToVelocityRPS in ProcessReadData.
- Removed the id parameter when calling ConvertVelocityRPSToValueRPM in SetDxlValueToSyncWrite.
- Removed the id parameter when calling ConvertVelocityRPSToValueRPM in SetDxlValueToBulkWrite.
src/dynamixel_hardware_interface.cpp
- Added [[maybe_unused]] attribute to the previous_state parameter in on_activate and on_deactivate functions.
- Added [[maybe_unused]] attribute to the time and period parameters in read and write functions.
- Added [[maybe_unused]] attribute to the request parameter in reboot_dxl_srv_callback function.

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 version is bumped,
Bugs squashed, code is trumped,
Clean and clear now.

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.3 and includes fixes for build errors. The changes look good overall, with some minor improvements suggested below.

Summary of Findings

Log message formatting: In src/dynamixel/dynamixel.cpp, the format specifier %d is used for read_data_list_.size(), which is of type size_t. It's better to use %ld for size_t to avoid potential issues on different architectures.
Unused parameter: In src/dynamixel_hardware_interface.cpp, several functions have unused parameters. Using [[maybe_unused]] is a good practice to suppress warnings, but ensure that these parameters are indeed not needed. If they are needed in the future, the code will need to be updated.
Redundant id parameter: In include/dynamixel_hardware_interface/dynamixel/dynamixel_info.hpp and src/dynamixel/dynamixel.cpp, the id parameter is removed from ConvertValueRPMToVelocityRPS and ConvertVelocityRPSToValueRPM functions. Ensure that this change does not affect the functionality of the code.
Missing default case in switch statement: In src/dynamixel/dynamixel.cpp, a default case is added to the DxlErrorToString function to handle unknown error codes. This is a good practice to prevent unexpected behavior.

Merge Readiness

The pull request is almost ready to be merged. The changes address build errors and include good practices like adding a default case to the switch statement. However, before merging, it's recommended to address the log message formatting in src/dynamixel/dynamixel.cpp and verify that the removal of the id parameter from the conversion functions does not affect the code's functionality. I am unable to approve this pull request, and users should have others review and approve this code before merging.

gemini-code-assist · 2025-04-14T00:16:55Z

src/dynamixel/dynamixel.cpp

@@ -965,15 +967,15 @@ DxlError Dynamixel::ProcessReadCommunication(
    dxl_comm_result = group_sync_read->txPacket();
    if (dxl_comm_result != COMM_SUCCESS) {
      fprintf(
-        stderr, "SyncRead Tx Fail [Dxl Size : %d] [Error code : %d]\n",
+        stderr, "SyncRead Tx Fail [Dxl Size : %ld] [Error code : %d]\n",


Consider using %zu instead of %ld for size_t to be more specific.

Suggested change

stderr, "SyncRead Tx Fail [Dxl Size : %ld] [Error code : %d]\n",

stderr, "SyncRead Tx Fail [Dxl Size : %zu] [Error code : %d]\n",

gemini-code-assist · 2025-04-14T00:16:55Z

src/dynamixel/dynamixel.cpp

        read_data_list_.size(), dxl_comm_result);
      return DxlError::SYNC_READ_FAIL;
    }
  } else {
    dxl_comm_result = group_bulk_read->txPacket();
    if (dxl_comm_result != COMM_SUCCESS) {
      fprintf(
-        stderr, "BulkRead Tx Fail [Dxl Size : %d] [Error code : %d]\n",
+        stderr, "BulkRead Tx Fail [Dxl Size : %ld] [Error code : %d]\n",
        read_data_list_.size(), dxl_comm_result);


Consider using %zu instead of %ld for size_t to be more specific.

Suggested change

read_data_list_.size(), dxl_comm_result);

stderr, "BulkRead Tx Fail [Dxl Size : %zu] [Error code : %d]\n",

gemini-code-assist · 2025-04-14T00:16:55Z

src/dynamixel/dynamixel.cpp

@@ -989,15 +991,15 @@ DxlError Dynamixel::ProcessReadCommunication(
    dxl_comm_result = group_sync_read->rxPacket();
    if (dxl_comm_result != COMM_SUCCESS) {
      fprintf(
-        stderr, "SyncRead Rx Fail [Dxl Size : %d] [Error code : %d]\n",
+        stderr, "SyncRead Rx Fail [Dxl Size : %ld] [Error code : %d]\n",


Consider using %zu instead of %ld for size_t to be more specific.

Suggested change

stderr, "SyncRead Rx Fail [Dxl Size : %ld] [Error code : %d]\n",

stderr, "SyncRead Rx Fail [Dxl Size : %zu] [Error code : %d]\n",

gemini-code-assist · 2025-04-14T00:16:56Z

src/dynamixel/dynamixel.cpp

        read_data_list_.size(), dxl_comm_result);
      return DxlError::SYNC_READ_FAIL;
    }
  } else {
    dxl_comm_result = group_bulk_read->rxPacket();
    if (dxl_comm_result != COMM_SUCCESS) {
      fprintf(
-        stderr, "BulkRead Rx Fail [Dxl Size : %d] [Error code : %d]\n",
+        stderr, "BulkRead Rx Fail [Dxl Size : %ld] [Error code : %d]\n",
        read_data_list_.size(), dxl_comm_result);


Consider using %zu instead of %ld for size_t to be more specific.

Suggested change

read_data_list_.size(), dxl_comm_result);

stderr, "BulkRead Rx Fail [Dxl Size : %zu] [Error code : %d]\n",

robotpilot

LGTM

yun-goon and others added 7 commits April 10, 2025 11:12

Modified print type error

b1d8a28

Signed-off-by: Wonho Yun <ywh@robotis.com>

Modified the error about unused id

ebce0be

Signed-off-by: Wonho Yun <ywh@robotis.com>

Added switch's default option

99e1702

Signed-off-by: Wonho Yun <ywh@robotis.com>

Modified unused parameters adding maybe_unused

32b08ab

Signed-off-by: Wonho Yun <ywh@robotis.com>

Modified version

85b2763

Signed-off-by: Wonho Yun <ywh@robotis.com>

Modified Lint

cdb4d70

Signed-off-by: Wonho Yun <ywh@robotis.com>

Merge pull request #32 from ROBOTIS-GIT/feature-modified-build-error

541632c

Modified Build Error

yun-goon added the bump Increase the version number to release label Apr 14, 2025

yun-goon requested a review from robotpilot April 14, 2025 00:16

gemini-code-assist bot reviewed Apr 14, 2025

View reviewed changes

yun-goon assigned robotpilot Apr 14, 2025

gemini-code-assist bot reviewed Apr 14, 2025

View reviewed changes

robotpilot approved these changes Apr 14, 2025

View reviewed changes

robotpilot merged commit dd6df05 into humble Apr 14, 2025
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump 1.4.3 #33

Bump 1.4.3 #33

yun-goon commented Apr 14, 2025

gemini-code-assist bot left a comment

gemini-code-assist bot left a comment

gemini-code-assist bot Apr 14, 2025

gemini-code-assist bot Apr 14, 2025

gemini-code-assist bot Apr 14, 2025

gemini-code-assist bot Apr 14, 2025

robotpilot left a comment

	stderr, "SyncRead Tx Fail [Dxl Size : %ld] [Error code : %d]\n",
	stderr, "SyncRead Tx Fail [Dxl Size : %zu] [Error code : %d]\n",

	read_data_list_.size(), dxl_comm_result);
	stderr, "BulkRead Tx Fail [Dxl Size : %zu] [Error code : %d]\n",

	stderr, "SyncRead Rx Fail [Dxl Size : %ld] [Error code : %d]\n",
	stderr, "SyncRead Rx Fail [Dxl Size : %zu] [Error code : %d]\n",

	read_data_list_.size(), dxl_comm_result);
	stderr, "BulkRead Rx Fail [Dxl Size : %zu] [Error code : %d]\n",

Bump 1.4.3 #33

Bump 1.4.3 #33

Conversation

yun-goon commented Apr 14, 2025

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

gemini-code-assist bot Apr 14, 2025

Choose a reason for hiding this comment

gemini-code-assist bot Apr 14, 2025

Choose a reason for hiding this comment

gemini-code-assist bot Apr 14, 2025

Choose a reason for hiding this comment

gemini-code-assist bot Apr 14, 2025

Choose a reason for hiding this comment

robotpilot left a comment

Choose a reason for hiding this comment