Bump 1.4.3 #34

yun-goon · 2025-04-14T00:17:45Z

No description provided.

Signed-off-by: Wonho Yun <[email protected]>

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 incremented to 1.4.3 in package.xml and the changelog is updated to reflect the new version and associated fixes.
Code Improvements: The ConvertValueRPMToVelocityRPS and ConvertVelocityRPSToValueRPM functions in dynamixel_info.hpp are updated to remove the unnecessary id parameter, simplifying their usage.
Error Message Formatting: The error messages in dynamixel.cpp are updated to use %ld for printing the size of read_data_list_, ensuring compatibility with different architectures and preventing potential format string vulnerabilities. Also, a default case is added to the DxlErrorToString function to handle unknown error codes.
Unused Variable Removal: The previous_state parameter is marked as [[maybe_unused]] in the on_activate and on_deactivate methods of DynamixelHardware in dynamixel_hardware_interface.cpp, and the time and period parameters are marked as [[maybe_unused]] in the read and write methods, indicating that these parameters are not currently used within the function bodies. Similarly, the request parameter is marked as [[maybe_unused]] in the reboot_dxl_srv_callback method.

Changelog

Click here to see the changelog

CHANGELOG.rst
- Added entry for version 1.4.3, noting the fix for build errors and the contributor (Wonho Yun).
include/dynamixel_hardware_interface/dynamixel/dynamixel_info.hpp
- Removed the id parameter from the 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 handle unknown error codes.
- Updated the format specifier in fprintf calls for SyncRead Tx/Rx Fail and BulkRead Tx/Rx Fail messages from %d to %ld to correctly print the size of read_data_list_.
- Removed the id parameter from calls to dxl_info_.ConvertValueRPMToVelocityRPS and dxl_info_.ConvertVelocityRPSToValueRPM.
src/dynamixel_hardware_interface.cpp
- Marked the previous_state parameter as [[maybe_unused]] in the on_activate and on_deactivate methods.
- Marked the time and period parameters as [[maybe_unused]] in the read and write methods.
- Marked the request parameter as [[maybe_unused]] in the reboot_dxl_srv_callback method.

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 ascends,
Bugs squashed, the build now mends,
Code flows anew.

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 it's worth considering if these parameters can be removed entirely to simplify the function signatures.
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 the removal of the id parameter does not affect the functionality of these functions.
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 when new error codes are introduced.

Merge Readiness

The pull request is almost ready for merging. Please address the medium and high severity issues identified in the review comments. After addressing these issues, the code should be in good shape to be merged. I am unable to approve this pull request, and recommend that others review and approve this code before merging.

gemini-code-assist · 2025-04-14T00:18:11Z

src/dynamixel/dynamixel.cpp

  if (is_sync) {
    dxl_comm_result = group_sync_read->txPacket();
    if (dxl_comm_result != COMM_SUCCESS) {
      fprintf(


Consider using static_cast<long> to explicitly cast read_data_list_.size() to long to avoid potential compiler warnings or errors. It is generally safer to use %ld for size_t as size_t is an unsigned type, and %d is for signed integers.

Suggested change

fprintf(

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

static_cast<long>(read_data_list_.size()), dxl_comm_result);

gemini-code-assist · 2025-04-14T00:18:11Z

src/dynamixel/dynamixel.cpp

    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",


Consider using static_cast<long> to explicitly cast read_data_list_.size() to long to avoid potential compiler warnings or errors. It is generally safer to use %ld for size_t as size_t is an unsigned type, and %d is for signed integers.

Suggested change

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

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

static_cast<long>(read_data_list_.size()), dxl_comm_result);

gemini-code-assist · 2025-04-14T00:18:11Z

src/dynamixel/dynamixel.cpp

  if (is_sync) {
    dxl_comm_result = group_sync_read->rxPacket();
    if (dxl_comm_result != COMM_SUCCESS) {
      fprintf(


Consider using static_cast<long> to explicitly cast read_data_list_.size() to long to avoid potential compiler warnings or errors. It is generally safer to use %ld for size_t as size_t is an unsigned type, and %d is for signed integers.

Suggested change

fprintf(

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

static_cast<long>(read_data_list_.size()), dxl_comm_result);

gemini-code-assist · 2025-04-14T00:18:11Z

src/dynamixel/dynamixel.cpp

    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",


Consider using static_cast<long> to explicitly cast read_data_list_.size() to long to avoid potential compiler warnings or errors. It is generally safer to use %ld for size_t as size_t is an unsigned type, and %d is for signed integers.

Suggested change

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

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

static_cast<long>(read_data_list_.size()), dxl_comm_result);

robotpilot

LGTM

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

Modified print type error

b1d8a28

Signed-off-by: Wonho Yun <[email protected]>

Modified the error about unused id

ebce0be

Signed-off-by: Wonho Yun <[email protected]>

Added switch's default option

99e1702

Signed-off-by: Wonho Yun <[email protected]>

Modified unused parameters adding maybe_unused

32b08ab

Signed-off-by: Wonho Yun <[email protected]>

Modified version

85b2763

Signed-off-by: Wonho Yun <[email protected]>

Modified Lint

cdb4d70

Signed-off-by: Wonho Yun <[email protected]>

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:17

yun-goon assigned robotpilot Apr 14, 2025

yun-goon added this to Platform Apr 14, 2025

github-project-automation bot moved this to 🌱 Todo in Platform Apr 14, 2025

yun-goon moved this from 🌱 Todo to 📝 Pull Request in Platform Apr 14, 2025

gemini-code-assist bot reviewed Apr 14, 2025

View reviewed changes

gemini-code-assist bot suggested changes Apr 14, 2025

View reviewed changes

robotpilot approved these changes Apr 14, 2025

View reviewed changes

robotpilot merged commit 1dba549 into jazzy Apr 14, 2025
25 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bump 1.4.3 #34

Bump 1.4.3 #34

Uh oh!

yun-goon commented Apr 14, 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 Apr 14, 2025

Uh oh!

gemini-code-assist bot Apr 14, 2025

Uh oh!

gemini-code-assist bot Apr 14, 2025

Uh oh!

gemini-code-assist bot Apr 14, 2025

Uh oh!

robotpilot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	fprintf(
	stderr, "SyncRead Tx Fail [Dxl Size : %ld] [Error code : %d]\n",
	static_cast<long>(read_data_list_.size()), dxl_comm_result);

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

	fprintf(
	stderr, "SyncRead Rx Fail [Dxl Size : %ld] [Error code : %d]\n",
	static_cast<long>(read_data_list_.size()), dxl_comm_result);

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

Bump 1.4.3 #34

Bump 1.4.3 #34

Uh oh!

Conversation

yun-goon commented Apr 14, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

gemini-code-assist bot Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

robotpilot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants