Skip to content

More general initialization of state from command (backport #2294)#2357

Merged
christophfroehlich merged 2 commits into
kiltedfrom
mergify/bp/kilted/pr-2294
May 17, 2026
Merged

More general initialization of state from command (backport #2294)#2357
christophfroehlich merged 2 commits into
kiltedfrom
mergify/bp/kilted/pr-2294

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented May 15, 2026

Description

When operating a robot which does not always track desired positions accurately (e.g. torque controlled) you may sometimes want the JTC to initialize based on the current/latest command from the hardware interface, rather than the current measured state. This will ensure the "command" is unchanged when activating the controller. Otherwise, if you use measured state, the arm can move unexpectedly (usually, it will droop).

To handle this, JTC has set_last_command_interface_value_as_state_on_activation parameter. However, it's implementation is weird to me. Specifically, bool read_state_from_command_interfaces will return False if you have a velocity state interface but no velocity command interface. For me, this restriction seems unnecessary. You simply should need a state interface for each command interface, but not the other way around. Extra state interfaces are simply ignored.

Explanation of changes

Now, we always first call read_state_from_state_interfaces in on_activate.
Then, if the parameter is set, we update this state from the command interfaces.
Thus, we renamed read_state_from_command_interfaces to update_state_from_command_interface and change its behavior to only update fields that have command interfaces.
To minimize code duplication, I refactored out interface_has_values and assign_point_from_command_interface. This is the reason we have more deletions than additions :)

Also I updated the tests to reflect the desired change in behavior.

Fixes # (issue)

https://discourse.openrobotics.org/t/handling-missing-command-interfaces-in-read-state-from-command-interface/53551

Is this user-facing behavior change?

Yes!

Did you use Generative AI?

A bit of copilot autocompletion.


This is an automatic backport of pull request #2294 done by Mergify.

(cherry picked from commit f6ee8cb)

# Conflicts:
#	doc/release_notes.rst
@mergify mergify Bot added the conflicts label May 15, 2026
@mergify
Copy link
Copy Markdown
Contributor Author

mergify Bot commented May 15, 2026

Cherry-pick of f6ee8cb has failed:

On branch mergify/bp/kilted/pr-2294
Your branch is up to date with 'origin/kilted'.

You are currently cherry-picking commit f6ee8cb.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   joint_trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller.hpp
	modified:   joint_trajectory_controller/src/joint_trajectory_controller.cpp
	modified:   joint_trajectory_controller/src/joint_trajectory_controller_parameters.yaml
	modified:   joint_trajectory_controller/test/test_trajectory_controller.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   doc/release_notes.rst

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

❌ Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.81%. Comparing base (df4be38) to head (1a6a120).

Files with missing lines Patch % Lines
...ory_controller/src/joint_trajectory_controller.cpp 90.90% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           kilted    #2357      +/-   ##
==========================================
+ Coverage   85.77%   85.81%   +0.03%     
==========================================
  Files         152      152              
  Lines       15073    15060      -13     
  Branches     1293     1293              
==========================================
- Hits        12929    12923       -6     
+ Misses       1685     1682       -3     
+ Partials      459      455       -4     
Flag Coverage Δ
unittests 85.81% <94.11%> (+0.03%) ⬆️

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

Files with missing lines Coverage Δ
...jectory_controller/joint_trajectory_controller.hpp 50.00% <ø> (ø)
...ory_controller/test/test_trajectory_controller.cpp 99.75% <100.00%> (+<0.01%) ⬆️
...ory_controller/src/joint_trajectory_controller.cpp 83.96% <90.90%> (+0.29%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich merged commit 93cb66b into kilted May 17, 2026
13 of 15 checks passed
@christophfroehlich christophfroehlich deleted the mergify/bp/kilted/pr-2294 branch May 17, 2026 09:18
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.

2 participants