-
Notifications
You must be signed in to change notification settings - Fork 13
This commit fixes the bug #76 #84
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
Conversation
[DQ_SerialManipulator] Added the property joint_types. Added the methods get_supported_joint_types(), check_joint_types() and, new methods to set and get the joint types. [DQ_SerialManipulatorDH, MDH] Updated the subclasses to comply with the modifications made in DQ_SerialManipulator.
bvadorno
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @juanjqo,
The modification is overall fine, although there are some minor issues to solve before I approve it.
Best wishes,
Bruno
|
|
||
| function set_joint_types(obj, joint_types) | ||
| % SET_JOINT_TYPES(joint_types) sets the joint types. | ||
| % 'joint_types' the vector that contains the joint types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'joint_types' is a vector containing the joint types.
| obj.check_joint_types(); | ||
| end | ||
|
|
||
| function ret = get_joint_types(obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change it according to my comment for the set_joint_type method.
| end | ||
|
|
||
|
|
||
| function ret = get_supported_joint_types(~) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason for ignoring the arguments? (That is, for using ~)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @bvadorno, as discussed internally, I removed the '~'.
Current:
methods (Static, Access = protected)
function ret = get_supported_joint_types()
% This method returns the supported joint types.
ret = [DQ_JointType.REVOLUTE, DQ_JointType.PRISMATIC];
end
end
[DQ_SerialManipulator.m] Removed the '~' from the method signature. Furthermore, I defined as a static method. [DQ_SerialManipulatorDH, MDH] Changed method get_supported_join_types() to comply with the modification made in DQ_SerialManipulator.
[DQ_SerialManipulator, DQ_SerialManipulatorDH, MDH]
|
|
||
| function ret = get_joint_type(obj, ith_joint) | ||
| % GET_JOINT_TYPE(ith_joint) returns the joint type of the ith joint. | ||
| % GET_JOINT_TYPE(ith_joint) returns a vector containing the joint types. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@juanjqo, is get_joint_type returning a vector? I thought we were sticking with your initial implementation, in which get_joint_type would return just a double.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bvadorno I was a typo. You're right, get_joint_type returns a double.
|
Hi @juanjqo, there is just one outstanding clarification before merging this pull request. |
@dqrobotics/developers
Hi @bvadorno,
This PR proposes the following modifications to fix #76 :
typefrom subclassesDQ_SerialManipulatorDHandDQ_SerialManipulatorMDH. Added the protected propertyjoint_typesinDQ_SerialManipulator. Motivations:obj.joint_typesis a property common to all subclasses, likeobj.n_joints.set_joint_types(and get_joint_types, both added in this PR) there is no need to keepobj.joint_typesas a public property. (Currently, the user can freely get and set the propertyobj.type.UML:

Example of use:
Minimal example used in #76
Output:
Error using DQ_SerialManipulator/check_joint_types Unsupported joint types. Use valid joint types: DQ_JointType.REVOLUTE, DQ_JointType.PRISMATIC. Error in DQ_SerialManipulator/set_joint_types (line 146) obj.check_joint_types(); Error in DQ_SerialManipulatorDH (line 196) obj.set_joint_types(A(5,:)); Error in untitled2 (line 30) ax = DQ_SerialManipulatorDH(ax18_DH_matrix)Best regards,
Juancho