fix(parallel_gripper): fix effort and velocity interface lookup

[AkshayArjun]

Description

Fix effort and velocity command interface lookup in on_activate().

Previously, the controller searched for interfaces using hardcoded strings
"set_gripper_max_effort" and "set_gripper_max_velocity". This meant effort and velocity
interfaces were silently never populated for any other hardware using
standard interface names.("Controller will claim the specified effort interface, e.g. robotiq_85_left_knuckle_joint/max_effort_interface.", "Controller will claim the speed specified interface, e.g. robotiq_85_left_knuckle_joint/max_effort_interface.", -> the previous code would work for these, but not for soemthing like joint/effort_max or something generic)

The fix uses interface.get_name() compared against
params_.max_effort_interface and params_.max_velocity_interface (
exactly what was declared in command_interface_configuration() and claimed
from hardware, so this should now work for any generically mentioned names in the yaml file.

Also adds missing std::nullopt resets for effort and speed interfaces in
on_deactivate().

Relates to #1229

Is this user-facing behavior change?

Yes. Effort and velocity commands will now actually be written to hardware
when max_effort_interface and max_velocity_interface are configured.
Previously they were silently ignored.

Did you use Generative AI?

Yes, Claude (Anthropic) was used to help understand the codebase and
discuss the changes. The actual code changes were made manually.

Additional Information

This is the second of multiple PRs addressing issue #1229.
Tests added to verify effort and velocity interface activation and
deactivation behavior.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.89%. Comparing base (83ef85c) to head (2e3f0b5).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2318      +/-   ##
==========================================
+ Coverage   86.82%   86.89%   +0.06%     
==========================================
  Files         148      148              
  Lines       16097    16156      +59     
  Branches     1361     1360       -1     
==========================================
+ Hits        13976    14038      +62     
+ Misses       1615     1614       -1     
+ Partials      506      504       -2     

Flag	Coverage Δ
unittests	86.89% <100.00%> (+0.06%)	⬆️

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

Files with missing lines	Coverage Δ
...roller/parallel_gripper_action_controller_impl.hpp	29.95% <100.00%> (+4.20%)	⬆️
...ntroller/test/test_parallel_gripper_controller.cpp	98.13% <100.00%> (+1.90%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[christophfroehlich]

Thank you. Can you fix also the parameter description here? this does not seem to be correct

[AkshayArjun]

Thank you. Can you fix also the parameter description here? this does not seem to be correct.

Yup, just made the changes to the yaml file, must be updated now.

[tpoignonec]

Hi, I also ran into the issue for a project.
Was about to open a PR before I saw that one that is essentially the same.

Is there a remaining issue that prevent merging?

[christophfroehlich]

Is there a remaining issue that prevent merging?

Your review would help!

[tpoignonec]

Maybe we should fail on missing required interfaces

  if (!params_.max_effort_interface.empty() && !joint_effort_command_interface_.has_value())
  {
    RCLCPP_ERROR(
      get_node()->get_logger(), "Expected max effort command interface `%s` not found",
      params_.max_effort_interface.c_str());
    return controller_interface::CallbackReturn::FAILURE;
  }
  if (!params_.max_velocity_interface.empty() && !joint_speed_command_interface_.has_value())
  {
    RCLCPP_ERROR(
      get_node()->get_logger(), "Expected max velocity command interface `%s` not found",
      params_.max_velocity_interface.c_str());
    return controller_interface::CallbackReturn::FAILURE;
  }

[AkshayArjun]

Yes, that defenietly seems necessary. Ill update the file with the suggested changes. Thanks for the review.

[tpoignonec]

Test case for missing interfaces?

TEST_F(GripperControllerTest, SetupInvalidLimitInterfacesFails)
{
  this->SetUpController("test_gripper_controller_with_effort_and_velocity");

  this->controller_->get_node()->set_parameter({"max_effort_interface", "joint/invalid_max_effort_interface"});

  ASSERT_TRUE(configure_succeeds(controller_));
  // activate fails
  ASSERT_FALSE(activate_succeeds(controller_));
}

[MarqRazz]

@pac48 initially set these up and I don't believe they were made to be a command interface. It was a way for you to set the max gripping force so that it stops closing at that point regardless of the position command you give. Is that still the case with these changes? The PR description sounds like it does this but I just wanted to double check.