Public API in detail namespace - `rclcpp::node_interfaces::detail::NodeInterfacesSupport`

Open Beginner friendly
#3,281 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
70/100
Issue type
Documentation
Clarity
Mostly clear
Activity status
Active
Tech stack
cpp
Domain
api, documentation

Research direction

Start with the NodeInterfaces documentation linked in the issue and the documentation for rclcpp::node_interfaces::detail::NodeInterfacesSupport. Determine where the public-API status should be stated, then update the relevant documentation so users and maintainers can clearly tell whether this class is supported despite its detail namespace.

Written by the indexing model from the issue text.

Description

enhancement
Description

From what I can find in the documentation on NodeInterfaces, the rclcpp::node_interfaces::detail::NodeInterfacesSupport class is meant to be specialized by users and thus is part of the public interface of rclcpp, however, it is part of the detail namespace explicitly called out as not public interface.

Since it is the only way to use the class with custom interfaces and is explicitly called out in the docs, I would assume it is part of the public interface. Should we maybe mark it more clearly on the rclcpp::node_interfaces::detail::NodeInterfacesSupport itself (both for users and future changes?).

I don't know if there is any other reason for it to be in the detail namespace. This functionality was added in #2041 and was like this from the beginning.

Motivation

Depending on implementation details of library is wrong, so we should be clear on whether this is public API or not.

Design / Implementation Considerations

I don't think it is feasible to change the namespace of the type, as that would break any existing users. The path of the header may be changed and the old one kept for compatibility if deemed necessary.

The most likely resolution I can think of is just an explicit note in the documentation that the class is part of the public API despite the namespace, so that both users and maintainers know, how to approach it.

Additional Information

No response

Dominant language
C++
Stars
805
Forks
564
Avg merge
1d 18h
Merged PRs (30d)
26

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ros2/rclcpp

All issues in ros2/rclcpp

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.