Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for All Dynamixel X-Series Actuators Including XC330 Series #3

Merged
merged 7 commits into from
Dec 27, 2024

Conversation

Woojin-Crive
Copy link
Contributor

Overview

This pull request adds support for all Dynamixel X-Series actuators by including their respective .model files. Specifically, it addresses the need for the XC330 series, as raised in #2

Key Changes

  1. Added .model files for:
    • XC330 series actuators
    • Other X-Series actuators for broader compatibility
  2. Modified Indirect Address Write and Indirect Data Write for the entire X-Series .model files:
    • Adjusted the range of indirect read/write to account for actuators with limited indirect range.

Notes

  • Additional feedback or testing requests are welcome.

@Woojin-Crive Woojin-Crive added the enhancement New feature or request label Dec 19, 2024
@Woojin-Crive
Copy link
Contributor Author

Woojin-Crive commented Dec 19, 2024

The dynamixel.model file should also be updated to reflect the changes. It will be updated soon.

@@ -63,5 +63,5 @@ Address Size Data Name
224 1 Indirect Data 1
168 2 Indirect Address Read
Copy link
Member

@HPaper HPaper Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Woojin-Crive Is a 26-byte length for the read data sufficient? Was there a specific reason for changing it?

Copy link
Contributor Author

@Woojin-Crive Woojin-Crive Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HPaper dynamixel_hardware_interface(open manipulator parameters) is now reading the following data: Present Position, Present Velocity, Present Current, Torque Enable, and Present Input Voltage, totaling 13 bytes. Consequently, I've allocated a 13-byte read data space. For writing, the package handles Goal Position and Goal Current(option), requiring up to 6 bytes. Given that the X*-330 series Dynamixels have an indirect address space of 20 bytes, I've configured the read data space without any margin to accommodate these requirements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Woojin-Crive
While it's true that the Open Manipulator requires 13 bytes of read data and has been configured accordingly, the dynamixel_hardware_interface is not designed solely for that package. Therefore, limiting the read space to 13 bytes is not ideal.

For the X*-330 series, where only 20 bytes of indirect address space are available, such a configuration might be necessary. However, for other Dynamixels, reducing the read space unnecessarily does not seem appropriate.

Additionally, since the write area generally requires less space compared to the read area, it would be better to position the write indirect area at the beginning of the address space.

Lastly, it might be necessary to add code that generates an error when adding read data to the indirect address exceeds the provided space.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Woojin-Crive @sunghowoo
Therefore, I will make the necessary corrections and submit an updated commit for this PR. Please review it once it's updated.

Copy link
Member

@HPaper HPaper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a 26-byte length for the read data sufficient? Was there a specific reason for changing it?

Signed-off-by: Sungho Woo <[email protected]>
Signed-off-by: Sungho Woo <[email protected]>
Copy link
Collaborator

@sunghowoo sunghowoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have verified that the open manipulator's X and Y are working properly.

@sunghowoo sunghowoo merged commit 945fc5c into main Dec 27, 2024
@Woojin-Crive Woojin-Crive deleted the feature-dxl-x-support branch January 10, 2025 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants