Open Client API for inheritance
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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