Skip to content

Conversation

@juanjqo
Copy link
Member

@juanjqo juanjqo commented Feb 27, 2025

Hi @dqrobotics/developers,

This PR adds the abstract class DQ_CoppeliaSimInterface. This class was originally proposed and discussed here, and it is based on the current C++ implementation. This abstract class will enable the implementation of the concrete class DQ_CoppeliaSimInterfaceZMQ, which is under development.

UML:

drawing

Updated UML:

drawing

Kind regards,

Juancho

@bvadorno
Copy link
Member

bvadorno commented Mar 5, 2025

Thanks, @juanjqo. The class definition looks good to me.

@ffasilva, could you please review the code?

Many thanks,
Bruno

Copy link
Member

@ffasilva ffasilva left a comment

Choose a reason for hiding this comment

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

Hi @juanjqo,

Thank you for the pull request! Most of my comments are cosmetic, but the nomenclatures of the methods for getting and setting joint positions, velocities, and torques might require some discussion.

Kind regards,
Frederico

juanjqo added 3 commits March 10, 2025 11:01
…posed by Frederico. This includes the use of lowercase style for all arguments in the methods, and keep the same spacing between them.
…_joint_positions to match the style of the other methods.
@juanjqo juanjqo marked this pull request as draft March 10, 2025 11:05
…o set_joint_target_forces and get_joint_forces to comply the style of the set/get methods related to joint positions, and velocities.
@juanjqo juanjqo marked this pull request as ready for review March 10, 2025 11:13
Copy link
Member

@ffasilva ffasilva left a comment

Choose a reason for hiding this comment

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

Hi @juanjqo,

I've added some more minor comments, and I agree with your reasoning for keeping the set/get_joint information name convention for the methods.

I believe we're pretty much good to go here.

Kind regards,
Frederico

@juanjqo
Copy link
Member Author

juanjqo commented Mar 11, 2025

Hi @juanjqo,

I've added some more minor comments, and I agree with your reasoning for keeping the set/get_joint information name convention for the methods.

I believe we're pretty much good to go here.

Kind regards, Frederico

Thanks, @ffasilva. I updated the PR to take your comments into account.

@ffasilva
Copy link
Member

Hi @juanjqo,

Thank you! I have nothing else to add.

Kind regards,
Frederico

@ffasilva
Copy link
Member

Hi @bvadorno,

We finished the review. Shall we approve this PR?

Kind regards,
Frederico

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