Skip to content

Conversation

@juanjqo
Copy link
Member

@juanjqo juanjqo commented May 16, 2025

Hi @dqrobotics/developers ,

This PR adds a new example to test the new setter and getter methods in the DQ_SerialManipulator class related to the joint types. The example requires dqrobotics/cpp#70.

Kind regards,

Juancho

… getter methods related to the joint types in the DQ_SerialManipulator class.
Copy link
Member

@mmmarinho mmmarinho left a comment

Choose a reason for hiding this comment

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

Please check that one comment. I’m happy to give more info if you need.

}

template<typename T>
void perform_tests(const std::shared_ptr<T>& robot, const std::string& msg, const bool& is_denso){
Copy link
Member

Choose a reason for hiding this comment

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

Hi @juanjqo.

This is a bit strange. You don’t need the flag if you’re using a template. You can check the type and switch accordingly. That will be more scalable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @mmmarinho, thanks for your feedback. I removed the flag. Now, I am checking the type using the following approach

    auto denso = std::dynamic_pointer_cast<DQ_SerialManipulatorDenso>(robot);
    if (denso)
        target_joint_type = DQ_JointType::REVOLUTE;

I wonder if this is the right strategy.

Kind regards,

Juancho

Copy link
Member

@mmmarinho mmmarinho May 18, 2025

Choose a reason for hiding this comment

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

Hi @juanjqo,

This would be a good example:
https://github.com/SmartArmStack/sas_common/blob/cc092fd6f156c4cabb6dd48d0476261c1a639067/include/sas_common/sas_common.hpp#L59

The thing to keep in mind with templates is to always try to solve things at compilation time. Your check is in runtime so it wouldn’t fully take advantage of the template capabilities.

That example is C++17 compliant I believe. If we agree that it’s time to update our standard then it’s fine.

Otherwise you can use template specialization.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @juanjqo, I implemented the modifications. =)

@juanjqo juanjqo marked this pull request as draft May 19, 2025 09:45
@juanjqo juanjqo marked this pull request as ready for review May 19, 2025 14:37
@mmmarinho mmmarinho merged commit ca20a78 into dqrobotics:master May 19, 2025
@juanjqo juanjqo deleted the joint_type branch May 22, 2025 08:47
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