ros-controls / control_toolbox

This package contains several C++ classes useful in writing controllers.
https://control.ros.org
BSD 3-Clause "New" or "Revised" License
121 stars 97 forks source link

Add lowpass filter #152

Closed guihomework closed 1 year ago

guihomework commented 1 year ago

This is a rework of PR #115 for the low-pass filter part, now using generate_parameter_library.

guihomework commented 1 year ago

xmlcheck complained about angle brackets, to type of class to instantiate. since there is no cli for pluginlib in ros2 yet, to quickly test if the <> would be usable in the command line, a pluginlib instantiation should be tested within the tests

guihomework commented 1 year ago

Test passes with the escaped angle brackets.

destogl commented 1 year ago

@bmagyar can we branch-out here for humble and rolling?, i.e., we need one foxy branch that holds current code for foxy and galactic.

guihomework commented 1 year ago

If I were allowed to wish something: a short README would help guiding new users

Do you mean a doc similar to the one in the Gravity Compensation "filter" https://github.com/ros-controls/control_toolbox/blob/f181fe76279860ff9b1c9f3d9edde927da29703b/src/control_filters/README.md or more a general description of the role of control filters and why they are there and how to use them ?

christophfroehlich commented 1 year ago

If I were allowed to wish something: a short README would help guiding new users

Do you mean a doc similar to the one in the Gravity Compensation "filter" https://github.com/ros-controls/control_toolbox/blob/f181fe76279860ff9b1c9f3d9edde927da29703b/src/control_filters/README.md or more a general description of the role of control filters and why they are there and how to use them ?

I mean a doc similar to the Gravity filter -> it will be short for this one. A general readme for this repo is out of scope for this PR IMHO.

guihomework commented 1 year ago

@christophfroehlich Thanks for the clarification. Another question: I am new to PR that require rebasing+changes, when the reviewer does merges of master in the PR. What should I do on my side ? rebase my initial branch (before your merge from master) on top of maaster, commit the requested changes and push or now add the requested changes over your merged-commit. I am used to always keep history of branches clean and straight.

christophfroehlich commented 1 year ago

I also prefer having a clean history, but in the end everything gets squashed in a single commit on master -> I'd suggest that you just commit on top of the last merge-commit.

guihomework commented 1 year ago

This makes sense. When I kept branches clean, it was to keep all commits in the master as well.

guihomework commented 1 year ago

If I were allowed to wish something: a short README would help guiding new users, as well as adding some function API documentation inside the header (like in all other functions of this repository).

both done now.