Enhance ProcessDirectReadData to handle item names for specific conversions #64

Woojin-Crive · 2025-07-08T06:22:00Z

Enhance ProcessDirectReadData to handle item names for specific conversions

… conversions in Dynamixel class

gemini-code-assist

Summary of Changes

Hello @Woojin-Crive, 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 modifies the Dynamixel class to enhance data processing for both reading and writing operations. It introduces item name handling within the ProcessDirectReadData function to allow for specific data conversions based on the item being processed. Additionally, it updates the SetDxlValueToBulkWrite function to include conversion logic for goal-related parameters, ensuring accurate data transmission to Dynamixel servos.

Highlights

Enhancement: The ProcessDirectReadData function is enhanced to handle item names for specific conversions, allowing for more precise data processing based on the item being read.
Conversion Logic: Specific conversion logic is added within ProcessDirectReadData for 'Present Position', 'Present Velocity', and 'Present Current', utilizing dxl_info_ methods to convert raw data to meaningful units (radians, RPS, and effort, respectively).
Bulk Write Handling: The SetDxlValueToBulkWrite function is updated to include conversion logic for 'Goal Position', 'Goal Velocity', and 'Goal Current' when writing data, ensuring that the correct values are sent to the Dynamixel servos.
Parameter Addition: The ProcessDirectReadData function now takes item_names as a parameter, allowing the function to determine how to convert the raw data.

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 enhances the data processing logic to handle specific conversions based on item names. My review focuses on improving maintainability by removing redundant code and avoiding magic strings.

gemini-code-assist · 2025-07-08T06:26:20Z

src/dynamixel/dynamixel.cpp

  std::function<uint32_t(uint8_t, uint16_t, uint8_t)> get_data_func)
 {
  for (size_t item_index = 0; item_index < item_addrs.size(); item_index++) {
+    uint8_t ID = comm_id;


The ID variable is redundant as it's just a copy of comm_id. You can remove this line and use comm_id directly in the conversion functions below (lines 1677, 1681, 1685). This will make the code slightly cleaner.

gemini-code-assist · 2025-07-08T06:26:20Z

src/dynamixel/dynamixel.cpp

+    if (item_names[item_index] == "Present Position") {
+      *data_ptrs[item_index] = dxl_info_.ConvertValueToRadian(
+        ID,
+        static_cast<int32_t>(dxl_getdata));
+    } else if (item_names[item_index] == "Present Velocity") {
+      *data_ptrs[item_index] = dxl_info_.ConvertValueRPMToVelocityRPS(
+        ID,
+        static_cast<int32_t>(dxl_getdata));
+    } else if (item_names[item_index] == "Present Current") {
+      *data_ptrs[item_index] = dxl_info_.ConvertCurrentToEffort(
+        ID,
+        static_cast<int16_t>(dxl_getdata));
+    } else {
+      *data_ptrs[item_index] = static_cast<double>(dxl_getdata);
+    }


Using hardcoded strings like "Present Position", "Present Velocity", and "Present Current" can make the code harder to maintain and prone to typos. It's better to define these as static const string variables at a suitable scope (e.g., at the top of the file or in a shared header).

gemini-code-assist · 2025-07-08T06:26:20Z

src/dynamixel/dynamixel.cpp


      for (uint16_t item_index = 0; item_index < direct_info_write_[comm_id].cnt; item_index++) {
        double data = *it_write_data.item_data_ptr_vec.at(item_index);
+        uint8_t ID = comm_id;


The ID variable is redundant as it's just a copy of comm_id. You can remove this line and use comm_id directly in the conversion functions below.

gemini-code-assist · 2025-07-08T06:26:20Z

src/dynamixel/dynamixel.cpp

+        if (direct_info_write_[comm_id].item_name.at(item_index) == "Goal Position") {
+          int32_t goal_position = dxl_info_.ConvertRadianToValue(ID, data);
+          param_write_value[added_byte + 0] = DXL_LOBYTE(DXL_LOWORD(goal_position));
+          param_write_value[added_byte + 1] = DXL_HIBYTE(DXL_LOWORD(goal_position));
+          param_write_value[added_byte + 2] = DXL_LOBYTE(DXL_HIWORD(goal_position));
+          param_write_value[added_byte + 3] = DXL_HIBYTE(DXL_HIWORD(goal_position));
+        } else if (direct_info_write_[comm_id].item_name.at(item_index) == "Goal Velocity") {
+          int32_t goal_velocity = dxl_info_.ConvertVelocityRPSToValueRPM(ID, data);
+          param_write_value[added_byte + 0] = DXL_LOBYTE(DXL_LOWORD(goal_velocity));
+          param_write_value[added_byte + 1] = DXL_HIBYTE(DXL_LOWORD(goal_velocity));
+          param_write_value[added_byte + 2] = DXL_LOBYTE(DXL_HIWORD(goal_velocity));
+          param_write_value[added_byte + 3] = DXL_HIBYTE(DXL_HIWORD(goal_velocity));
+        } else if (direct_info_write_[comm_id].item_name.at(item_index) == "Goal Current") {
+          int16_t goal_current = dxl_info_.ConvertEffortToCurrent(ID, data);
+          param_write_value[added_byte + 0] = DXL_LOBYTE(goal_current);
+          param_write_value[added_byte + 1] = DXL_HIBYTE(goal_current);
+        } else {
+          if (size == 4) {
+            int32_t value = static_cast<int32_t>(data);
+            param_write_value[added_byte + 0] = DXL_LOBYTE(DXL_LOWORD(value));
+            param_write_value[added_byte + 1] = DXL_HIBYTE(DXL_LOWORD(value));
+            param_write_value[added_byte + 2] = DXL_LOBYTE(DXL_HIWORD(value));
+            param_write_value[added_byte + 3] = DXL_HIBYTE(DXL_HIWORD(value));
+          } else if (size == 2) {
+            int16_t value = static_cast<int16_t>(data);
+            param_write_value[added_byte + 0] = DXL_LOBYTE(value);
+            param_write_value[added_byte + 1] = DXL_HIBYTE(value);
+          } else if (size == 1) {
+            param_write_value[added_byte] = static_cast<uint8_t>(data);
+          }
        }


The strings "Goal Position", "Goal Velocity", and "Goal Current" are hardcoded. It's better to define these as static const string variables to avoid typos and improve maintainability.

…te model files for unit information

feat: Enhance ProcessDirectReadData to handle item names for specific…

5fe935a

… conversions in Dynamixel class

Woojin-Crive self-assigned this Jul 8, 2025

Woojin-Crive added this to Platform Jul 8, 2025

Woojin-Crive added the enhancement New feature or request label Jul 8, 2025

github-project-automation bot moved this to 🌱 Todo in Platform Jul 8, 2025

gemini-code-assist bot reviewed Jul 8, 2025

View reviewed changes

Woojin-Crive mentioned this pull request Jul 8, 2025

Needed Help with Dynamixel Hardware Interface and unstable motor values #62

Closed

Woojin-Crive moved this from 🌱 Todo to 📝 Pull Request in Platform Jul 8, 2025

feat: Introduce unit and sign type handling in DynamixelInfo and upda…

3dc9b00

…te model files for unit information

Woojin-Crive closed this Jul 16, 2025

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

Woojin-Crive deleted the feature-unit-handling branch July 23, 2025 00:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enhance ProcessDirectReadData to handle item names for specific conversions #64

Enhance ProcessDirectReadData to handle item names for specific conversions #64

Uh oh!

Woojin-Crive commented Jul 8, 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 Jul 8, 2025

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

gemini-code-assist bot Jul 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Enhance ProcessDirectReadData to handle item names for specific conversions #64

Enhance ProcessDirectReadData to handle item names for specific conversions #64

Uh oh!

Conversation

Woojin-Crive commented Jul 8, 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 Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants