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

Enable JTC for hardware having offset from state measurements #189

Merged
merged 10 commits into from
Jul 23, 2021

Conversation

destogl
Copy link
Member

@destogl destogl commented May 28, 2021

This PR replaces the first part of #161.

The goal is short:

  • support "open-loop" control when hardware state is not exactly following the robot commands, i.e., there is systematic offset in the state measurements
  • add approach to change controller without "jumps" from the command to state values:
    • when started HW interface has to set command_interfaces to N aN
    • controller checks if command interfaces are NaN --> then is a controller started for the first time, so read states
    • if command interface have a value --> so a controller was running already (supports switching between controllers)

@codecov-commenter
Copy link

Codecov Report

Merging #189 (c5109c9) into master (e7f9962) will decrease coverage by 0.78%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #189      +/-   ##
==========================================
- Coverage   35.78%   34.99%   -0.79%     
==========================================
  Files         189       27     -162     
  Lines       17570     2709   -14861     
  Branches    11592     1803    -9789     
==========================================
- Hits         6287      948    -5339     
+ Misses        994      152     -842     
+ Partials    10289     1609    -8680     
Flag Coverage Δ
unittests 34.99% <ø> (-0.79%) ⬇️

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

Impacted Files Coverage Δ
...ler/test/test_load_joint_trajectory_controller.cpp
...s/test/test_load_joint_group_effort_controller.cpp
...ntroller/test/test_trajectory_controller_utils.hpp
...nt_state_controller/src/joint_state_controller.cpp
...ontrollers/src/joint_group_position_controller.cpp
...ory_controller/test/test_trajectory_controller.cpp
..._broadcaster/test/test_joint_state_broadcaster.cpp
...test/test_load_joint_group_velocity_controller.cpp
...ectory_controller/test/test_trajectory_actions.cpp
...ntroller/test/test_trajectory_controller_utils.hpp
... and 206 more

@destogl destogl requested a review from v-lopez June 25, 2021 11:19
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Mostly nitpick, otherwise looks good to me!

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
Copy link
Contributor

@v-lopez v-lopez left a comment

Choose a reason for hiding this comment

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

LGTM, just minor comment that can be ignored.

@bmagyar bmagyar merged commit 856e8e2 into ros-controls:master Jul 23, 2021
bmagyar added a commit to destogl/ros2_controllers that referenced this pull request Jul 28, 2021
…ntrols#189)

* Avoid "jumps" with states that have tracking error. All test are passing but separatelly. Is there some kind of timeout?

* Remove allow_integration_flag

* Add reading from command interfaces when restarting controller

* Rename variable according to review

* Use even more clear name

* Remove debug lines.

* Update documentation

* Apply suggestions from code review

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>

* Move methods to cpp file as per reviewers comment.

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
@destogl destogl deleted the jtc-hw-state-has-offset branch August 9, 2021 12:01
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.

4 participants