Skip to content

Enable JTC for hardware having offset from state measurements#189

Merged
bmagyar merged 10 commits intoros-controls:masterfrom
destogl:jtc-hw-state-has-offset
Jul 23, 2021
Merged

Enable JTC for hardware having offset from state measurements#189
bmagyar merged 10 commits intoros-controls:masterfrom
destogl:jtc-hw-state-has-offset

Conversation

@destogl
Copy link
Copy Markdown
Member

@destogl destogl commented May 28, 2021 •
edited
Loading

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)

@destogl destogl self-assigned this May 28, 2021
@destogl destogl mentioned this pull request May 28, 2021
Closed
@destogl destogl mentioned this pull request May 28, 2021
Merged
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 28, 2021

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 force-pushed the jtc-hw-state-has-offset branch from c2f506c to 13bd183 Compare June 25, 2021 09:52
@destogl destogl requested a review from v-lopez June 25, 2021 11:19
bmagyar
bmagyar approved these changes Jul 10, 2021
View reviewed changes
Copy link
Copy Markdown
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!

@destogl destogl mentioned this pull request Jul 12, 2021
Closed
@destogl @bmagyar
Apply suggestions from code review
818662c 
Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
v-lopez
v-lopez approved these changes Jul 17, 2021
View reviewed changes
Copy link
Copy Markdown
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
@destogl @bmagyar
Enable JTC for hardware having offset from state measurements (ros-co…
46e02bc 
…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
@christophfroehlich christophfroehlich mentioned this pull request Oct 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bmagyar bmagyar bmagyar approved these changes

@jordan-palacios jordan-palacios Awaiting requested review from jordan-palacios

+1 more reviewer

@v-lopez v-lopez v-lopez approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@destogl destogl

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@destogl @codecov-commenter @v-lopez @bmagyar