Codecov Report

❌ Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.19%. Comparing base (e3c31f1) to head (7a83fa2).

@@            Coverage Diff             @@
##           master    #2294      +/-   ##
==========================================
+ Coverage   85.16%   85.19%   +0.03%     
==========================================
  Files         145      145              
  Lines       15407    15394      -13     
  Branches     1322     1322              
==========================================
- Hits        13121    13115       -6     
+ Misses       1797     1794       -3     
+ Partials      489      485       -4     

Thanks for the quick replies everyone!
@christophfroehlich updated the docs/comments
@saikishor I added a new test for this parameter. I also confirmed (locally) that if I set the parameter to false, the test fails which is good 😄

In thinking about these features more, I wonder whether it really makes sense to initialize state_current_ to command interfaces. I definitely want last_command_state_ set correctly, and ultimate what I want is that when you first update the commands, you set them to the command not the measured state. The tests seem to be only testing internal state of the controller (last_commanded_state_), not what actually gets bound to the hardware interface (I think joint_command_interface_). Does anyone have thoughts on tihs?

Not sure about the CI failures, looks related to this but not sure if I did something wrong or need to somehow update something to get CI to pass.

Not sure about the CI failures, looks related to this but not sure if I did something wrong or need to somehow update something to get CI to pass.

tl;dr not related ;)

You are right. I already released ros2_control, but the build farm had an outtake this week and the binaries are still not available on ros-testing. Windows build fails because of the removal done by #2016 until we have released this.

@destogl since it seems you wrote the initial implementation, maybe you want to check that this logic change makes sense

should I rebase on master? or will that trigger needless CI and/or dismiss your approval?

Please never do a rebase if there was already any review in the PR, see the hint in the PR template you certainly have read ;)
Updating via merge via UI without having merge conflicts normally doesn't dismiss approvals. But let's wait for another review, then re-trigger CI before merging.

friendly ping in hopes of a review @saikishor

@saikishor thanks! Changes included.
@christophfroehlich since review was dismissed

I clicked "rerequest review", since it looked like the new commit means old approvals are dismissed. Sorry if that generated extra emails/notifications!

@saikishor is there a bot that can backport to Jazzy/Kilted...? I am planning to do a backport to Humble but I am pretty sure that has to be manual since the code has changed a lot.

@saikishor is there a bot that can backport to Jazzy/Kilted...? I am planning to do a backport to Humble but I am pretty sure that has to be manual since the code has changed a lot.

I think there shouldn't be problem backporting it, as it only affects the case when the interface is only available as a command but not as a state. @christophfroehlich what do you think?

Hmm slight correction -- I believe it effects also the case where you have the interface only as state and not command. That's how I discovered this in the first place. At least in humble, if you had position command but position,velocity state, the controller would initialize from the state not command.

do you need it on humble? if it is too much effort, I would not backport it
users can always compile the rolling version on humble if needed

I would like it in humble. It shouldn't take more than an hour for me to do the backport. My current workaround, unbelievably, is to remove my velocity state interface 😆

Compiling rolling on humble is possible, yes, but I think I wouldn't do it. It involves also bringing many other repos (ros2 control, ros2 control cmake, pal statistics, ...) from rolling as well.