Skip to content

Conversation

@juanjqo
Copy link
Member

@juanjqo juanjqo commented Dec 20, 2021

Status:

  • Finished

Changes before we can update:

General

  • Add the first version of the contributing file with the instructions for future contributors.

  • The automatic documentation is not a priority in this pull request. Therefore, the original goal "Make Automatic documentation (C++ Doxygen documentation Problem in jacobian method #1)" will be addressed in future versions.

  • Add DQ_SerialManipulatorMDH()

DQ_SerialManipulator

  • Add get_{lower,upper}_{q,q_dot}_limit

  • Add set_{lower,upper}_{q,q_dot}_limit

  • Make it abstract.

DQ_SerialManipulatorDH

  • Update it to support the changes in DQ_SerialManipulator

  • Unify the {get,set}_{thetas, as, ds, alphas, types}

  • Make a compatible pose_jacobian_derivative

  • Deprecate the DQ_SerialManipulatorDH(MatrixXd, string) constructor.

DQ_Kinematics

  • Update it to support the changes in DQ_SerialManipulator

…er_q_limit_, lower_q_dot_limit_, upper_q_dot_limit_
…more because DQ_SerialManipulator requires to use it to plot the robot.
…atorDH. Now the method is in DQ_SerialManipulator as in CPP.
…the dim_configuration_space parameter instead the matrix A and the DH convention.
Juan Jose Quiroz Omaña added 21 commits June 1, 2022 09:43
…Furthermore, Fixed the bug that prevented printing the error message when the user does not use arguments in the constructor.
…Furthermore, Fixed the bug that prevented printing the error message when the user does not use arguments in the constructor.
… Furthermore, Fixed the bug that prevented printing the error message when the user does not use arguments in the constructor.
…nstructor. Now we have dh_matrix instead. This could avoid confunsions with the parameter A of the DH matrix.
@bvadorno
Copy link
Member

Hi @juanjqo,

@mmmarinho and I concluded that this pull request is unfeasible to double-check and has several elements that are likely to be rejected.

Therefore,

  1. restart the procedure with individual changes (several changes of the same type are allowed on the same pull request);
  2. do not unnecessarily change any internal implementation that is working correctly without prior approval;
  3. and include a clear and concise rationale behind each pull request.

Many thanks,
Bruno

@bvadorno bvadorno closed this Nov 25, 2022
@bvadorno
Copy link
Member

Oh, by the way, @juanjqo, may you include the aforementioned information (i.e., items 1, 2, and 3) in the instructions for pull requests? I guess a good place would be the README.md files in each repository.

@juanjqo
Copy link
Member Author

juanjqo commented Nov 25, 2022

Oh, by the way, @juanjqo, may you include the aforementioned information (i.e., items 1, 2, and 3) in the instructions for pull requests? I guess a good place would be the README.md files in each repository.

@bvadorno Sure!

@juanjqo
Copy link
Member Author

juanjqo commented Nov 25, 2022

Hi @juanjqo,

@mmmarinho and I concluded that this pull request is unfeasible to double-check and has several elements that are likely to be rejected.

Therefore,

  1. restart the procedure with individual changes (several changes of the same type are allowed on the same pull request);
  2. do not unnecessarily change any internal implementation that is working correctly without prior approval;
  3. and include a clear and concise rationale behind each pull request.

Many thanks, Bruno

@bvadorno I'll do that!

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