ros2 / ros2/rclcpp

Open Client API for inheritance

Open
#305 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement question
Dominant language
C++
Stars
805
Forks
564
Avg merge
1d 17h
Merged PRs (30d)
27

Description

Hi
Like in https://github.com/ros2/rclcpp/pull/258 where Node/Publisher API is opened for inheritance shouldn't we open the client API also for inheritance?

Probably the reason behind Node/Publisher API being allowed to inherit may not apply Client API. Anyway the reason I do this suggestion is for the mocking/unit testing perspective.

For example. Now I could pass in a publisher into my node class and mock publish() calls of the said publisher in the unit tests of my node class. (To verify whether my node class correctly called the corresponding methods in the publisher interface). So if we had the async_send_request call
https://github.com/ros2/rclcpp/blob/master/rclcpp/include/rclcpp/client.hpp#L239
in the Client API also marked as virtual we could extend the same capability for the client objects as well.

I should admit that my experience with C++ and the ros2 core is limited. So what I propose above may not be the correct way to achieve the said requirement. Or there may be strong reasons against that warrant such a change.

If the proposed change is acceptable way to do this I would be more than happy to try to implement it and open a pull request.

Please let me know if any additional information is required.

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.

Research direction

Start with rclcpp/include/rclcpp/client.hpp at the async_send_request declaration and compare the inheritance changes discussed in ros2/rclcpp#258. Determine with maintainers whether opening the Client API and making async_send_request virtual is an acceptable mocking approach; the issue is complete only once that design is agreed and the required implementation and tests are defined.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.