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 implementation for friction parameters #2805

Merged
merged 12 commits into from
Mar 7, 2022
Merged

Conversation

sgiraz
Copy link
Contributor

@sgiraz sgiraz commented Feb 17, 2022

What's new in this PR:

  • In ITorqueControl.h have been added 4 new friction parameters: viscousUp, viscousDown, coulombUp, coulombDown.
  • the definitions of all the get/set functions have been updated accordingly.
  • A new ControlTorqueTest source has been added to the test folder in order to validate the implementation of the new parameters

Notes:

  • The implementation has been tested successfully with the new test provided with this PR.

True positive detection ✅

image

False positive detection ✅

image

@update-docs
Copy link

update-docs bot commented Feb 17, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update the release notes by adding a file in doc/release/<target_branch>, based on your changes.

@sgiraz sgiraz temporarily deployed to code-analysis February 17, 2022 23:00 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 17, 2022 23:00 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 17, 2022 23:00 Inactive
Copy link
Member

@randaz81 randaz81 left a comment

Choose a reason for hiding this comment

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

  • measurement units conversion layer (missing in ImplementTorqueControl.cpp) to be implemented and tested
  • changelog (missing)

@sgiraz
Copy link
Contributor Author

sgiraz commented Feb 21, 2022

  • measurement units conversion layer (missing in ImplementTorqueControl.cpp) to be implemented and tested
  • changelog (missing)

Done! During the tests, I verified that the conversion for each new parameter is performed correctly.

cc @randaz81

@sgiraz sgiraz temporarily deployed to code-analysis February 21, 2022 10:38 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 21, 2022 10:38 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 21, 2022 10:38 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 21, 2022 16:16 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 21, 2022 16:16 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 22, 2022 16:24 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 22, 2022 16:24 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 22, 2022 16:24 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 23, 2022 14:11 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 23, 2022 14:11 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 24, 2022 09:41 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 24, 2022 09:41 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 24, 2022 09:41 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 24, 2022 09:51 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 24, 2022 09:51 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 28, 2022 14:41 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 28, 2022 14:41 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis February 28, 2022 14:41 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis March 2, 2022 17:49 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis March 2, 2022 17:49 Inactive
@sgiraz sgiraz temporarily deployed to code-analysis March 2, 2022 17:49 Inactive
@pattacini
Copy link
Member

Hi @randaz81

Can we merge the PR? It's kind of blocking all the other PR's we have in the pipeline.

@randaz81
Copy link
Member

randaz81 commented Mar 7, 2022

Merged even if there are some errors in the CI, that should be hopefully fixed by #2811

Copy link
Member

@randaz81 randaz81 left a comment

Choose a reason for hiding this comment

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

PR ok, some errors in the CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants