Skip to content

Conversation

juanjqo
Copy link
Member

@juanjqo juanjqo commented May 8, 2025

Hi @dqrobotics/developers,

This PR implements setter and getter methods for the DH parameters in the DQ_SerialManipulatorDH and DQ_SerialManipulatorMDH classes, which are already available in the MATLAB version. This PR aims to complete one of the pending tasks for the 23.04 release.

drawing

List of Modifications:

  • New enum class: DQ_ParameterDH in the DQ_SerialManipulator class.
namespace DQ_robotics
{
enum class DQ_ParameterDH
{
    THETA,
    D,
    A,
    ALPHA
};
}
  • The DQ_SerialManipulatorDH and DQ_SerialManipulatorMDH classes now have the methods
    VectorXd get_parameters(const DQ_ParameterDH& parameter_type) const;
    double   get_parameter(const DQ_ParameterDH& parameter_type,
                           const int& to_ith_link) const;
    void set_parameters(const DQ_ParameterDH& parameter_type,
                        const VectorXd& vector_parameters);
    void set_parameter(const DQ_ParameterDH& parameter_type,
                       const int& to_ith_link,
                       const double& parameter);

Usage:

    auto robot = FrankaEmikaPandaRobot::kinematics();
    robot.set_parameter(DQ_ParameterDH::D, 2, 0.523);

    // Update: Using char/string style is now allowed, as in Matlab
    robot.set_parameters("THETA", VectorXd::Zero(7));

Example:

I proposed an example in dqrobotics/cpp-examples#20.

Kind regards,

Juancho

@juanjqo juanjqo marked this pull request as ready for review May 8, 2025 15:21
@juanjqo juanjqo marked this pull request as draft May 8, 2025 15:21
@juanjqo juanjqo marked this pull request as ready for review May 8, 2025 15:41
@juanjqo juanjqo marked this pull request as draft May 8, 2025 15:49
@mmmarinho
Copy link
Member

Hi @juanjqo. Thanks for this. I noticed you changed this to draft so I hope this comment helps, I don't mean to imply I think this is the finished product. My memory is fuzzy but I remember DQ_ParameterDH being its own class, not necessarily a nested class of DQ_SerialManipulator.

I think this architecture you proposed would be great (and cleaner) in cpp and Python. My concern is that we'll soon get some issues raised that languages differ, because matlab does not have the trailing DQ_SerialManipulator which is effectively just behaving as a namespace.

@juanjqo
Copy link
Member Author

juanjqo commented May 8, 2025

Hi @juanjqo. Thanks for this. I noticed you changed this to draft so I hope this comment helps, I don't mean to imply I think this is the finished product. My memory is fuzzy but I remember DQ_ParameterDH being its own class, not necessarily a nested class of DQ_SerialManipulator.

I think this architecture you proposed would be great (and cleaner) in cpp and Python. My concern is that we'll soon get some issues raised that languages differ, because matlab does not have the trailing DQ_SerialManipulator which is effectively just behaving as a namespace.

Hi @mmmarinho, thanks for your feedback. I am going to implement the required modifications taking into account your comments.

@juanjqo juanjqo marked this pull request as ready for review May 8, 2025 21:27
@mmmarinho
Copy link
Member

Thanks, @juanjqo.

Cool stuff. I think we’re just missing this case you listed in the MATLAB version. I’m replicating it below.

robot.set_parameters("THETA", [1 1 1 1 1 1 1])
robot.get_parameter ("THETA", 2) ; 
robot.set_parameter ("THETA", 2, 0.01)

We can support this in cpp as well by not using enum class and having a suitable std::string constructor. Surprisingly, no dubious practices needed.

This will cause an implicit conversion when a std::string is used as argument and solve the situation transparently for the user.

@mmmarinho
Copy link
Member

PS: before you throw a keyboard at me, here’s an example we can start from:
https://stackoverflow.com/questions/21295935/can-a-c-enum-class-have-methods

@juanjqo juanjqo marked this pull request as draft May 12, 2025 14:09
@juanjqo
Copy link
Member Author

juanjqo commented May 12, 2025

Hi @mmmarinho, I updated the DQ_ParameterDH class to support the char/string style in the setter and getter methods. I updated the example to test it.

    auto robot = FrankaEmikaPandaRobot::kinematics();
    robot.set_parameter("THETA", 2, 0.01);
    std::cout<<robot.get_parameter("THETA", 2)<<std::endl;

PS: There is a good chance that I misunderstood your instructions. Please tell me if my modifications are close to what you were thinking.

Kind regards,

Juancho

@juanjqo juanjqo marked this pull request as ready for review May 12, 2025 14:45
@mmmarinho mmmarinho self-assigned this May 12, 2025
@mmmarinho
Copy link
Member

@juanjqo seems good to me!

Could I please ask you to document in doxygen style all the methods, including the new constructions etc? You’ve done it for most but maybe not for all. Apologies if my phone isn’t showing it correctly.

@juanjqo
Copy link
Member Author

juanjqo commented May 13, 2025

@juanjqo seems good to me!

Could I please ask you to document in doxygen style all the methods, including the new constructions etc? You’ve done it for most but maybe not for all. Apologies if my phone isn’t showing it correctly.

@mmmarinho I added the documentation for the constructors. =)

@mmmarinho mmmarinho merged commit 68f21ab into dqrobotics:master May 13, 2025
3 of 6 checks passed
@juanjqo juanjqo deleted the set_get_dh branch May 14, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants