Skip to content

docs: comprehensive docstring updates for JointLimiterInterface - #3412

Open
Rudyy75 wants to merge 11 commits into
ros-controls:masterfrom
Rudyy75:fix/jointlimiter-init-docstring
Open

docs: comprehensive docstring updates for JointLimiterInterface#3412
Rudyy75 wants to merge 11 commits into
ros-controls:masterfrom
Rudyy75:fix/jointlimiter-init-docstring

Conversation

@Rudyy75

@Rudyy75 Rudyy75 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Description

Initially addresses #3410 by updating the @brief for the init overload that accepts soft_joint_limits.

Per maintainer feedback, expanded the scope to unify, standardize, and complete the Doxygen docstrings (@param, @return) across the rest of the JointLimiterInterface APIs.

Fixes #3410
Related to #3391

Is this user-facing behavior change?

Did you use Generative AI?

Yes, Claude Sonnet 4.6

Additional Information

TODOs

To send us a pull request, please:

  • Fork the repository.
  • Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
  • Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
  • Commit to your fork using clear commit messages.
  • Send a pull request, answering any default questions in the pull request interface.
  • Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

@saikishor saikishor added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Jun 24, 2026

/**
* Wrapper init method that accepts the joint names and their limits directly
* @brief Initialize joints from directly provided names, limits, and soft joint limits.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a detailed overview of the parameters and also for the other parts of the codebase

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the docstring with @param entries for all parameters and checked the rest of the codebase for similar issues.

@Rudyy75 Rudyy75 mentioned this pull request Jun 24, 2026
@Rudyy75
Rudyy75 requested a review from saikishor June 25, 2026 17:11
@mergify

mergify Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

This pull request is in conflict. Could you fix it @Rudyy75?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates Doxygen documentation for joint_limits::JointLimiterInterface init/configure APIs to better reflect the actual parameters and return values, addressing the mismatch called out in issue #3410.

Changes:

  • Add missing @return documentation for the primary init(...) overload.
  • Update the init(...) overload docstring to explicitly include soft_joint_limits and document its parameters/return.
  • Expand Doxygen docs for init(...) node/lifecycle-node overloads and add brief docs for configure(...) and reset_internals().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread joint_limits/include/joint_limits/joint_limiter_interface.hpp
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.56%. Comparing base (0b09bf4) to head (c8a6217).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3412   +/-   ##
=======================================
  Coverage   89.56%   89.56%           
=======================================
  Files         168      168           
  Lines       21419    21419           
  Branches     1662     1662           
=======================================
  Hits        19184    19184           
  Misses       1533     1533           
  Partials      702      702           
Flag Coverage Δ
unittests 89.56% <ø> (ø)

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

Files with missing lines Coverage Δ
...s/include/joint_limits/joint_limiter_interface.hpp 51.51% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@Rudyy75

Rudyy75 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

I have resolved the merge conflicts and updated all the docstrings with @param and @return descriptions as requested. Let me know if anything else is needed.

@Rudyy75

Rudyy75 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Hey @saikishor, I have resolved all the threads and updated the docstrings as requested. Christoph has also approved. Could you take a look and approve?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread joint_limits/include/joint_limits/joint_limiter_interface.hpp
@Rudyy75 Rudyy75 changed the title fix: update docstring for JointLimiterInterface::init overload to include soft_joint_limits docs: comprehensive docstring updates for JointLimiterInterface Jul 20, 2026
@Rudyy75

Rudyy75 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Hey @saikishor, I've updated the PR title and description to match the expanded documentation scope you requested. Ready for your final look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted.

Projects

Status: Needs review

Development

Successfully merging this pull request may close these issues.

Inaccurate init method description in JointLimiterInterface.

4 participants