Skip to content
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

Add LinearHolonomicTask #43

Merged
merged 2 commits into from Mar 15, 2023
Merged

Conversation

shbang91
Copy link
Contributor

I closed the old PR and instead made cleaner commit history.

Here, the major changes are the following:

  1. Removed the draco3 robot description
  2. Added a general class of LinearHolonomicTask
  3. Added a child class of LinearHolonomicTask called JointCouplingTask

I am looking forward to seeing any feedback from you.

Thank you!

Copy link
Owner

@stephane-caron stephane-caron left a comment

Choose a reason for hiding this comment

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

Thank you for this improvement! The PR is starting to look good. I just have one technical point re. the computation of the error in the base LinearHolonomicTask.

pink/tasks/linear_holonomic_task.py Outdated Show resolved Hide resolved
examples/draco3.py Outdated Show resolved Hide resolved
pink/tasks/linear_holonomic_task.py Outdated Show resolved Hide resolved
pink/tasks/linear_holonomic_task.py Show resolved Hide resolved
pink/tasks/linear_holonomic_task.py Outdated Show resolved Hide resolved
pink/tasks/linear_holonomic_task.py Outdated Show resolved Hide resolved
pink/tasks/linear_holonomic_task.py Outdated Show resolved Hide resolved
pink/tasks/joint_coupling_task.py Show resolved Hide resolved
pink/tasks/joint_coupling_task.py Outdated Show resolved Hide resolved
@stephane-caron
Copy link
Owner

Would you be up for adding unit tests for the new tasks in the tests/ folder?

You could take inspiration from test_posture_task.py to spin e.g. test_linear_holonomic_task.py and test_joint_coupling_task.py.

@shbang91
Copy link
Contributor Author

Would you be up for adding unit tests for the new tasks in the tests/ folder?

Yes, I can make those unit tests.

@shbang91
Copy link
Contributor Author

shbang91 commented Mar 14, 2023

Based on your comments above, I've made some changes and added unit tests. Please feel free to give me any feedback you might have.
By the way, I've tried to edit the documentation, but I don't know exactly how it works, so maybe I need to leave it to you 😥

@stephane-caron stephane-caron merged commit 214c426 into stephane-caron:master Mar 15, 2023
@stephane-caron
Copy link
Owner

LGTM. I took care of the documentation, fixed the issue mypy had when verifying the weights matrix, and merged this PR. @shbang91 thank you for your contribution 😃

@stephane-caron stephane-caron changed the title added LinearHolonomicTask Add LinearHolonomicTask Mar 30, 2023
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.

None yet

2 participants