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.

Sign up for GitHub

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

Jump to bottom

Ported LowPass and GravityCompensation filters iirob_filters repository #115

Closed
wants to merge 15 commits into from
Closed

Ported LowPass and GravityCompensation filters iirob_filters repository #115

wants to merge 15 commits into from

Conversation

destogl
Copy link
Member

@destogl destogl commented Sep 23, 2021

  • using the dynamic parametrization as implemented in existing gravity_compensation filter
  • added testcases for loading parameters

destogl and others added 2 commits June 12, 2021 17:05
@destogl
Added initial files of control filters
e8794ee
@dzumkeller
Ported low_pass_filter from iirob_filters:
c41eb9a 
- using the dynamic parametrization as implemented in existing gravity_compensation filter
- added testcases for loading parameters
destogl
destogl commented Sep 23, 2021
View reviewed changes
Copy link
Member Author

@destogl destogl left a comment •
edited
Loading

Choose a reason for hiding this comment

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

there are few simplifications to do.

to do:

  • plugin loading test as for controllers
  • Filter loading from Param-server
  • FilterChain loading from Param-server (see Admittance controller)

control_filters.xml Outdated Show resolved Hide resolved
include/control_filters/low_pass_filter.hpp Outdated Show resolved Hide resolved
include/control_filters/low_pass_filter.hpp Outdated Show resolved Hide resolved
include/control_filters/low_pass_filter.hpp Outdated Show resolved Hide resolved
include/control_filters/low_pass_filter.hpp Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
include/control_filters/low_pass_filter.hpp Outdated Show resolved Hide resolved
include/control_filters/low_pass_filter.hpp Outdated Show resolved Hide resolved
include/control_filters/low_pass_filter.hpp Outdated Show resolved Hide resolved
test/control_filters/test_low_pass_filter.cpp Show resolved Hide resolved
@dzumkeller
Update low_pass_filter.hpp
ee8a6ff 
removed parameter_namespace and added override flag
dzumkeller
dzumkeller previously approved these changes Sep 30, 2021
View reviewed changes
dzumkeller and others added 9 commits September 30, 2021 18:31
@dzumkeller @destogl
Apply suggestions from code review
f0ec3be 
Co-authored-by: Denis Štogl <destogl@users.noreply.github.com>
@dzumkeller
added new testcase after review
ff01bf1
@dzumkeller
Merge branch 'add-control-filters-gravity-compensation' of https://gi…
2cecfd6 
…thub.com/StoglRobotics-forks/control_toolbox into add-control-filters-gravity-compensation
@destogl
Added initial files of control filters
79aeeb2
@dzumkeller @destogl
Ported low_pass_filter from iirob_filters:
1ea8b37 
- using the dynamic parametrization as implemented in existing gravity_compensation filter
- added testcases for loading parameters
@dzumkeller @destogl
added new testcase after review
06391d8
@dzumkeller @destogl
Update low_pass_filter.hpp
1cebf05 
removed parameter_namespace and added override flag
@dzumkeller @destogl
Apply suggestions from code review
c0f365a 
Co-authored-by: Denis Štogl <destogl@users.noreply.github.com>
@destogl
Update version of gravity compensation to use parameter_handler and o…
8b391c6 
…ptimize code.
@destogl destogl force-pushed the add-control-filters-gravity-compensation branch from 2cecfd6 to 8b391c6 Compare October 29, 2021 15:44
@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2021 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (add-control-filters-gravity-compensation@e8794ee). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head d7462e9 differs from pull request most recent head 978e590. Consider uploading reports for the commit 978e590 to get more accurate results
Impacted file tree graph

@@                             Coverage Diff                             @@
##             add-control-filters-gravity-compensation     #115   +/-   ##
===========================================================================
  Coverage                                            ?   29.11%           
===========================================================================
  Files                                               ?      440           
  Lines                                               ?    30360           
  Branches                                            ?    15600           
===========================================================================
  Hits                                                ?     8840           
  Misses                                              ?     7920           
  Partials                                            ?    13600
Flag Coverage Δ
unittests 29.11% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8794ee...978e590. Read the comment docs.

@destogl destogl mentioned this pull request Oct 30, 2021
Closed
@destogl destogl changed the base branch from add-control-filters-gravity-compensation to ros2-master November 16, 2021 10:27
@destogl destogl changed the title Ported low_pass_filter from iirob_filters: Ported LowPass and GravityCompensation filters iirob_filters repository Dec 3, 2021
@destogl destogl marked this pull request as draft December 3, 2021 08:21
destogl
destogl commented Jun 8, 2022
View reviewed changes
src/control_filters/gravity_compensation.cpp
tf2::doTransform(parameters_->cog_, cog_transformed, transform_cog_);

// Compensate for gravity force
temp_force_transformed.vector.z += parameters_->force_z_;
Copy link
Member Author

Choose a reason for hiding this comment

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

We should always subtract the parameter value independently if it is positive or negative.

guihomework
guihomework reviewed Jan 20, 2023
View reviewed changes
include/control_filters/gravity_compensation.hpp
{
world_frame_ = string_parameters_[0].second;
RCUTILS_LOG_INFO_NAMED(logger_name_.c_str(), "World frame: %s", world_frame_.c_str());
sensor_frame_ = string_parameters_[0].second;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
sensor_frame_ = string_parameters_[0].second;
sensor_frame_ = string_parameters_[1].second;

guihomework
guihomework reviewed Jan 20, 2023
View reviewed changes
include/control_filters/gravity_compensation.hpp
RCUTILS_LOG_INFO_NAMED(logger_name_.c_str(), "World frame: %s", world_frame_.c_str());
sensor_frame_ = string_parameters_[0].second;
RCUTILS_LOG_INFO_NAMED(logger_name_.c_str(), "Sensor frame: %s", sensor_frame_.c_str());
force_frame_ = string_parameters_[0].second;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
force_frame_ = string_parameters_[0].second;
force_frame_ = string_parameters_[2].second;

This was referenced Mar 7, 2023
Merged
Open
@destogl
Copy link
Member Author

destogl commented Mar 13, 2023

closed in favor of #152 and #153

@destogl destogl closed this Mar 13, 2023
@destogl destogl deleted the add-control-filters-gravity-compensation branch March 13, 2023 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guihomework guihomework guihomework left review comments

@dzumkeller dzumkeller dzumkeller left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@destogl @codecov-commenter @guihomework @dzumkeller