Ideas for changes to the migration guide about design

[mjcarroll]

@sharronliu There is currently an equivalent in rcl with work on-going to expose it in rclcpp: ros2/rclcpp#628

[sharronliu]

I have a question, in ROS we have getNumSubscribers() (ros::Publisher) which is used to check if nobody subscribing the publisher just stop any processing beneath, to avoid waste of computation resources.

Is there any equivalence in ROS2? If no, destroying a subscription and making the publisher being aware of that is useful API.

[dirk-thomas]

+1 for passing the queue_size.
+1 for publishing const references.

Regarding the name I think the new names (create / destroy) make sense when the handle is maintained by the user manually. May be the advertise call could be used in a way where you get a shared pointer and when you let it go out of scope it destroys itself.

[tfoote]

I would suggest we provide the aliased functions in a backwards compatibility header which can be planned to be deprecated in the future.

[esteve]

+1 to all three proposals and to include them in a separate header file.

[wjwwood]

I would suggest we provide the aliased functions in a backwards compatibility header which can be planned to be deprecated in the future.

Unfortunately, the "aliases" in question are methods, so you cannot conditionally include them without having a subclass of Node and having the user use that subclass instead.

[wjwwood]

I'll open separate pull requests as soon as I get a chance.

[ugocupcic]

+1 on all of these. Just a question, why not use the current shutdown instead of destroy_publisher ? (it might not be the most descriptive so I don't mind destroy_publisher either, just asking)

[wjwwood]

Sorry, I wasn't clear, there's no destroy_publisher in C++ at the moment, though there could be. The create_publisher name comes from C where there is both, but in C that's because there is no destructor. Right now (this is something we want to revisit) the Publisher and Subscription classes are scoped.

[dornhege]

+1 to everything.

Regarding the names: I would have picked create/destroy for versions that create a shared pointer, i.e., the opposite as Dirk. Maybe it's best to only have one version of the function to avoid confusion. I'd prefer the old names.

Regarding the deprecation: One could #include a "ros1_node_aliases.h" in the public section of the Node class. It's not pretty, but would work.

[jbohren]

+1 To all of @wjwwood's suggestions. I also think create_* and destroy_* functions usually imply heap allocation and then transferring ownership. With respect to consistent API design, I understand the desire for consistency, but the Node::advertise() and Node::subscribe() member functions don't really have an equivalent in C, so the analog to create_* and destroy_* are really the direct forms of creating publishers and subscribers with their constructors, and passing in a reference to a Node (i.e. Publisher::Publisher and Subscription::Subscription.

@wjwwood While we're talking about this, why has Subscriber been renamed Subscription?

[wjwwood]

Regarding the names: I would have picked create/destroy for versions that create a shared pointer, i.e., the opposite as Dirk. Maybe it's best to only have one version of the function to avoid confusion. I'd prefer the old names.

Both cases would be required to return a shared_ptr, the only difference would be what the Node class would hold, a shared_ptr or a weak_ptr. By holding a shared_ptr the Node would prevent the object from going out of scope until someone explicitly "removed" it. If it held only a weak_ptr then if the user let the object go out of scope then it would be implicitly "removed", "unsubscribed", "unadvertised", or otherwise undone.

So I actually agree with @dirk-thomas, if there is both create_ and destroy_ then the object probably shouldn't be implicitly undone when it goes out of scope, but if there is only "advertise" then it might make sense for it to go out of scope and get undone implicitly. This is, however, not how ROS 1 behaves. The ROS 1 Publisher is actually pretty strange and subtly broken (See: ros/ros_comm#450). The ROS 1 Subscriber makes more sense, you create it using a node, but then undo it using a method on itself and/or by letting it go out of scope. So Publishers are not scoped and singletons and Subscribers are scoped.

The other reason that, at the time, I avoided advertise in favor of create/destroy was that I thought we might have different kinds of publishers, e.g. Publisher, ScopedPublisher, NamedPublisher, ReliablePublisher, etc. That would have made advertise awkwardly overused, imo, but it turns out that we may not have anything but Publisher, so that doesn't really matter anymore.

Regarding the deprecation: One could #include a "ros1_node_aliases.h" in the public section of the Node class. It's not pretty, but would work.

The idea of putting them in a different header was to make them optional, requiring that header to be included in order to make those aliases available. I don't see how people could optionally include the header into the node class. Maybe you could explain what you mean?

I also think create_* and destroy_* functions usually imply heap allocation and then transferring ownership.

Actually, I think it is more accurate to say that create and destroy most often correspond to allocation and initialization as well as shutdown and deallocation. Transfer of ownership is implied even by malloc and so is always implied when allocation is involved. create and destroy functions are essentially the equivalent to new and delete in C++ which conflates allocation and construction as well as destruction and deallocation.

With respect to consistent API design, I understand the desire for consistency, but the Node::advertise() and Node::subscribe() member functions don't really have an equivalent in C

There is no reason that we couldn't have an "advertise"/"destroy_publisher" and "subscribe"/"destroy_subscription" pair in C.

so the analog to create_* and destroy_* are really the direct forms of creating publishers and subscribers with their constructors, and passing in a reference to a Node (i.e. Publisher::Publisher and Subscription::Subscription.

Actually much more happens than just creating publisher/subscription objects and passing a reference to the node. But in fact it would be possible to make the constructor of Publisher, for example, do all the work or to have a free function create_publisher(node) -> Publisher. However, in order to mimic the ROS 1 API and because Node needs a reference to the Publisher anyways (this is actually more relevant for subscriptions) we kept to having these be a method of the node class.

@wjwwood While we're talking about this, why has Subscriber been renamed Subscription?

In ROS 1 "Subscriber" doesn't subscribe to anything. By the time you get a "Subscriber" the subscribing has already been done. At that point you have a "Subscription", which you can modify, delete, etc... If you got a "Subscriber" and then had to call "subscribe" on it, then it would be aptly named. This is aesthetic, but is a good example of something that I think we should take the opportunity to fix because it makes more sense and if we do not consider to do it now then we never will. I'm also pretty sure the use of Subscriber in ROS 1 is an artifact of them having already used "Subscription" in the internal implementation and not wanting to reuse the name in the user facing API: http://docs.ros.org/jade/api/roscpp/html/subscription_8h.html

[jbohren]

With respect to consistent API design, I understand the desire for consistency, but the Node::advertise() and Node::subscribe() member functions don't really have an equivalent in C

There is no reason that we couldn't have an "advertise"/"destroy_publisher" and "subscribe"/"destroy_subscription" pair in C.

I'm specifically talking about the Node:: member functions having no analog in C.

so the analog to create_* and destroy_* are really the direct forms of creating publishers and subscribers with their constructors, and passing in a reference to a Node (i.e. Publisher::Publisher and Subscription::Subscription.

Actually much more happens than just creating publisher/subscription objects and passing a reference to the node. But in fact it would be possible to make the constructor of Publisher, for example, do all the work or to have a free function create_publisher(node) -> Publisher.

Right because the publisher handle needs to come from somewhere.

However, in order to mimic the ROS 1 API and because Node needs a reference to the Publisher anyways (this is actually more relevant for subscriptions) we kept to having these be a method of the node class.

Why does mimicking the ROS1 API preclude you from making a Publisher constructor which takes the same arguments as Node::advertise with the addition of a non-const reference to a Node?

@wjwwood While we're talking about this, why has Subscriber been renamed Subscription?

In ROS 1 "Subscriber" doesn't subscribe to anything. By the time you get a "Subscriber" the subscribing has already been done. At that point you have a "Subscription", which you can modify, delete, etc... If you got a "Subscriber" and then had to call "subscribe" on it, then it would be aptly named.

This seems like an unnecessary change to me, especially because you could also make the linguistic argument that a "subscriber" is an entity which has subscribed to something. If I subscribe to a magazine, I would be considered a "subscriber" even though I'm not "subscribing" repeatedly after the fact.

[wjwwood]

I'm specifically talking about the Node:: member functions having no analog in C.

I guess you mean there is no analogue because the C version of advertise cannot be automatically deadvertised? Otherwise you could have in C rclc_advertise(rclc_node_t * node, const char * topic, ...) -> rclc_publisher_t, with a matching rclc_destroy_publisher(rclc_publisher_t * publisher).

Why does mimicking the ROS1 API preclude you from making a Publisher constructor which takes the same arguments as Node::advertise with the addition of a non-const reference to a Node?

Well it doesn't preclude it, I just said that such a constructor (or free function) would be possible. But we wanted to mimic the ROS 1 API, and the Publisher in ROS 1 doesn't have such a constructor:

http://docs.ros.org/jade/api/roscpp/html/classros_1_1Publisher.html

In ROS 1 Publishers are created from the node, and so, for now, it is the same with ROS 2.

This seems like an unnecessary change to me, especially because you could also make the linguistic argument that a "subscriber" is an entity which has subscribed to something. If I subscribe to a magazine, I would be considered a "subscriber" even though I'm not "subscribing" repeatedly after the fact.

Like I said it's aesthetic, so we can reconsider it, but as for the rational goes I'll stand by my logic. A subscriber can be someone or something that subscribed in the past or will subscribe in the future, and so in this case you could call the Node a subscriber or the user a subscriber, but I don't think it is appropriate to call the thing the node gives to the user a subscriber. I would instead call that a subscription. Likening it to your example, if you subscribe to a magazine you're a subscriber and if you do so through a middleman you could consider them to be a subscriber to the upstream magazine company as well. But you wouldn't say, "the magazine company acknowledged my subscriber" nor would you call the magazine company and ask to cancel or modify your subscriber.

Obviously this is just a small thing, but I think subscription makes more sense.

[jbohren]

I'm specifically talking about the Node:: member functions having no analog in C.

I guess you mean there is no analogue because the C++ version of advertise cannot be automatically deadvertised? Otherwise you could have in C rclc_advertise(rclc_node_t * node, const char * topic, ...) -> rclc_publisher_t, with a matching rclc_destroy_publisher(rclc_publisher_t * publisher).

I was simply highlighting the lack of object-oriented operations in C.

Why does mimicking the ROS1 API preclude you from making a Publisher constructor which takes the same arguments as Node::advertise with the addition of a non-const reference to a Node?

Well it doesn't preclude it, I just said that such a constructor (or free function) would be possible. But we wanted to mimic the ROS 1 API, and the Publisher in ROS 1 doesn't have such a constructor:

Right, I was just making the point that if you wanted to have similar APIs between C and C++, then a natural way to do it would be to create constructors for Publisher and Subscri[ber|ption] which took the similar arguments to create_publisher() and create_subscri[ber|ption] in C.

This seems like an unnecessary change to me, especially because you could also make the linguistic argument that a "subscriber" is an entity which has subscribed to something. If I subscribe to a magazine, I would be considered a "subscriber" even though I'm not "subscribing" repeatedly after the fact.

Like I said it's aesthetic, so we can reconsider it, but as for the rational goes I'll stand by my logic
Obviously this is just a small thing, but I think subscription makes more sense.

And if it's such a small thing, then why change it? I don't think I've ever encountered a new user who was confused by the name of the ROS subscriber.

[wjwwood]

And if it's such a small thing, then why change it? I don't think I've ever encountered a new user who was confused by the name of the ROS subscriber.

I didn't claim it was a point of confusion for new users, but rather that it's conceptually inconsistent. I develop with the mind set that anything worth doing is worth doing right and if I see a way for something to make more sense or be more correct I'm going to change it. If all things are otherwise equal, then I'll pick the least disruptive choice. For example, I didn't perceive any benefit for doing a constructor which takes a Node reference over a method on the node class which serves as a factory. Since C++ didn't force me, I chose to stick with the ROS 1 style until some good reason came up for a change. In C, the langue forced us to change the pattern.

Obviously this is best effort (and this issue is about capturing areas where we could have done better), but I've given you my rational for the name change. And it's fine to disagree with my rational, but I think this is a case of bike shedding because there a whole bunch of complex changes which have compelling justifications which no one comments on (and which I would really like feedback on) and instead we're spending our time arguing about things like the class name.

For example, the ROS 2 Subscription class is in a different namespace, has different behavior and lifecycle semantics, different methods, and is returned as a shared pointer rather than as a class which wraps a shared pointer to the implementation. Given all of that, the name being Subscription versus Subscriber is really insignificant. If we find a way to make all of those other things like ROS 1, then I'd consider talking about the name in more detail.

If you feel strongly about the name, open an issue on this design doc with reasons why keeping the name makes more sense and we'll address it when we have time.

[wjwwood]

Right, I was just making the point that if you wanted to have similar APIs between C and C++, then a natural way to do it would be to create constructors for Publisher and Subscri[ber|ption] which took the similar arguments to create_publisher() and create_subscri[ber|ption] in C.

It's also natural to emulate class methods by this pattern:

class Node {
...
    // Implicit first argument is `this`
    Publisher::SharedPtr advertise(...);
}

// node emulates `this`
publisher_t * advertise(node_t * node, ...);
void destroy_publisher(publisher_t * publisher);

Which is what we did for C so that it mimics C++ as much as possible. It's sort of a bidirectional influence.

[wjwwood]

Ok, I've addressed the easy points of having a signature of create_publisher/create_subscription which take a queue size rather than a full QoS profile, and the ability to publish const references of messages. I still plan to open a pull request to discuss the changing of Subscription to Subscriber, but first I'm looking at the behavior of Publisher and Subscription objects which affect the create_publisher/advertise and create_subscription/subscribe discussion.

---

So first I'll talk a little bit about how to present Publishers and Subscriptions in ROS 2. Then I'll talk a little bit about the possibilities for the different API functions (create_publisher/advertise and create_subscription/subscribe).

Publisher and Subscription Interfaces

In ROS 1 a publisher is described as:

(from http://wiki.ros.org/roscpp/Overview/Publishers%20and%20Subscribers)

ros::Publisher is reference counted internally -- this means that copying them is very fast, and does not create a "new" version of the ros::Publisher. Once all copies of a ros::Publisher are destructed the topic will be shutdown. There are some exceptions to this:

• ros::shutdown() is called -- this shuts down all publishers (and everything else).
• ros::Publisher::shutdown() is called. This will shutdown this topic, unless there have been...
• Multiple calls to NodeHandle::advertise() for the same topic, with the same message type. In this case all the ros::Publishers on a specific topic are treated as copies of each other.
  ros::Publisher implements the ==, != and < operators, and it is possible to use them in std::map, std::set, etc.

You can retrieve the topic of a publisher with the ros::Publisher::getTopic() method.

The third point there will be changing in ROS 2, based on our experience with ros/ros_comm#450. I think the right approach there is not to have advertise return singletons, but rather each call to advertise produces a new publisher which does not share a queue with any other publishers, even if they're on the same topic.

For Subscribers in ROS 1:

(from http://docs.ros.org/api/roscpp/html/classros_1_1NodeHandle.html#a317fe4c05919e0bf3fb5162ccb2f7c28)

Returns:
On success, a Subscriber that, when all copies of it go out of scope, will unsubscribe from this topic. On failure, an empty Subscriber ...

So just like Publishers, Subscribers in ROS 1 are scoped as well.

The question is how to present Publishers (and Subscriptions) to users in ROS 2. Currently both are returned within a shared_ptr. The idea was that we could use smart pointers to convey the semantics of a their life cycle to the user rather than having an internally reference counted object of our own. Also in ROS 2, the Publisher and Subscription objects returned to the user are held internally as a weak_ptr, therefore if the user lets the shared pointers go out of scope they effectively unadvertised or unsubscribe them.

Pros of this approach:

• For those familiar with std::shared_ptr, the semantics are clear on how they can copy and share the result.
• The implementation is simple because we're reusing standard interfaces to model ownership.

Cons:

• It still isn't clear what happens if the shared pointer to the Publisher or Subscription goes out of scope without reading documentation.
• It doesn't avoid needing a separate valid state bool in the Publisher or Subscription, since the they could be invalid even if they haven't been destroyed, i.e. just having a valid shared_ptr to a Publisher is not sufficient to know if it is valid.
  • It could be invalid if the related Node has been destroyed or if rclcpp::shutdown is called or for example if we allow the publisher to be passed to an unadvertise option or if it has an unadvertise/shutdown method on it.
• Could be confusing to users who have not dealt with smart pointers before.

The alternative approach for how we're doing this in ROS 2 would be to emulate ROS 1 and provide a class which wraps up this ownership. This class would hold a shared pointer to a common implementation instance of the Publisher and when copied the shared pointer would also be copied. This class would be simple, basically only providing the same interface as the internal publisher and implementing it by delegating all work to the implementation, i.e. basically the pimpl pattern with a shared implementation pointer.

Pros of this approach:

• Simple, non-pointer interface for novice C++ users.
• Provides a single check for whether or not the publish is valid, rather than checking for a valid pointer and then for a valid publisher/subscription state.
• Provides the benefits of the pimpl pattern (which we might have done anyways).
• Could (optionally) still be returned as a shared_ptr to hint at how to copy/store it.
• More similar to ROS 1, so potentially less changes needed to migrate.

Cons:

• Doesn't implicitly convey what happens when this object is let out of scope.
• Doesn't implicitly convey the copy semantics of the class.
• Requires lots of boiler plate in the API to expose the implementation's API through the wrapping class, e.g. like in ROS 1: https://github.com/ros/ros_comm/blob/008a61f8b7614cfdc4675b2bad6353ca228374ce/clients/roscpp/src/libros/publisher.cpp#L112-L120

There may be other pros/cons that I missed, but I think that's a reasonable summary.

Scoping

Another direction of discussion is whether or not the Publishers and Subscriptions should be scoped. It is a reasonable observation that most people never use a ROS 1 Subscriber object after creating it and just keep it around to prevent the subscription from going out of scope and never allow it to do so until the program is finished. Therefore, it would be convenient to subscribe without having to keep the result around. For publishers this is not as useful since the purpose of a publisher is to use it to call publish on, and so you need to keep the publisher around anyways in order to use it.

Currently in ROS 2 we use the name create_publisher which creates a scoped publisher object. This isn't very consistent since in C the result of create_publisher is not scoped, but must be passed to a matching destroy_publisher call.

We could have a version of create_publisher/create_subscription which returns a non-scoped object. The unadvertising/unsubscribing would then only happen it the resulting object is passed to a matching destroy_* function or if the containing node is shutdown. The danger with this (in my opinion) is that users might create a subscription with this, hold on to it, then later drop it thinking this will unsubscribe it but instead it does not and lives on as a zombie subscription until the node is shutdown.

Summary (tl;dr)

Questions:

• Should we provide a reference counting wrapper class for Publishers and Subscriptions like in ROS 1?
  • The alternative is to continue to return a "simple" Publisher/Subscription class wrapped in a shared_ptr.
  • Follow up: Should we have a way to create one as a shared_ptr as well?
• Should we rename create_publisher to advertise in C++?
  • Follow up: Should the object returned from advertise be scoped?
• Should we rename create_subscription to subscribe in C++?
  • Follow up: Should the object returned from subscribe be scoped?
• Should we have a version of create_publisher and create_subscription which return non-scoped versions of the respective classes?
  • These non-scoped versions would either only be destroyed when the node is shutdown or if the result is passed to the matching destroy_* function.

Are there other options to consider?

My initial answer to each of these would be:

• Yes, because it provides a simpler interface and we'd probably do pimpl anyways and it's more similar to ROS 1's interface.
  • Follow up: yes, because you often see people having global pointers to publishers anyways and having an option to get the result already within a smart pointer would be useful.
• Yes, rename to advertise.
  • Follow up: Yes, it should be scoped as in ROS 1.
• Yes, rename to subscribe.
  • Follow up: Yes, it should continue to be scoped.
• +0 I can see how this might be useful because it allows for "subscribe and forget it" and it provides consistency with the C api, but it is at risk of unnecessarily complicating the API and confusing users.

I look forward to hearing what others think about these questions and whether or not I've framed the problem adequately.

[tfoote]

I'd lean toward the shared pointer interface. As we're planning to step forward to using c++11 throughout I don't think that we should design our API assuming that people don't know how to use the built in shared_ptr. We already use and expose shared_ptr and unique_ptr in the publish and subscribe callbacks, thus if you are going to use the publisher and subscription objects you will need to understand them. Also as I expect there will be demand for the shared_ptr interface we would then be providing a duplicate interface, where for the beginner a single interface will be easier to learn. Similarly we won't have to document our own lifecycle semantics.

If we're using the shared_ptr interface we don't need to consider a scoped vs unscoped version. It's just the shared_ptr scope which again simplifies things.

I do agree that the double check for validity is a bit of a pain, but a trivial helper function which both checks the dereference and validity would make that a one line solution.

+1 to the renames, again no need for scope discussions if using the smart pointers.

[dirk-thomas]

Imo both APIs have their use cases and I don't think we should make that decision for the user. I would argue for having pairs of create / destroy functions (close to the C API).

Beside that we can have separate functions returning smart pointers. These can reuse the create functions to obtain the instance and the deleter can invoke the corresponding destroy function.

This allows users to choose which API fits their use case best and we only have one implementation to maintain since the secondary API is simply syntactic sugar on top.

Regarding the exact naming for ROS 1 compatibility / similarity I think we need to consider many more aspects before making decision on this. Therefore I would avoid renaming stuff now without having a clear picture of the many pending migration questions.

[wjwwood]

If we're using the shared_ptr interface we don't need to consider a scoped vs unscoped version. It's just the shared_ptr scope which again simplifies things.

@tfoote I was trying to point out in my previous post that just using a shared_ptr doesn't resolve the scoped vs unscoped behavior (at least as far as I can see), since you don't know what happens when you release the shared_ptr you were given. If the node is holding a weak_prt it will be destroyed, but if the node holds a shared_ptr it is not.

Imo both APIs have their use cases and I don't think we should make that decision for the user. I would argue for having pairs of create / destroy functions (close to the C API).

@dirk-thomas What type do these functions return?

Regarding the exact naming for ROS 1 compatibility / similarity I think we need to consider many more aspects before making decision on this. Therefore I would avoid renaming stuff now without having a clear picture of the many pending migration questions.

I'm not sure what outstanding issues we need to resolve before considering the name of these functions. Could you elaborate?

[dirk-thomas]

As long as we don't decide on how the migration path should / will look like (similar API, shim to map ROS 1 API to ROS 2 API, etc.) I would avoid spending time on any renaming / refactoring. We should do that only once - when we have a a clear vision of the path forward.

[wjwwood]

As long as we don't decide on how the migration path should / will look like (similar API, shim to map ROS 1 API to ROS 2 API, etc.) I would avoid spending time on any renaming / refactoring. We should do that only once - when we have a a clear vision of the path forward.

Whether or not we have a library shim is orthogonal to how close the new API is to the old one, because the shim is always a temporary solution and eventually code will be converted to the new API. When that happens, the extent to which the API's are similar will help. Additionally, I pointed out several reasons in the previous comments (and during the meeting) why the the advertise name makes more sense, beyond the argument that it is more similar to the ROS 1 API. For example, the create_publisher function while named similarly to the C API's version, does not behave the same nor does it have a corresponding destroy_publisher. That's why I'm proposing the name change now. I guess another way to look at it is, that if we make this change, and later decide for what ever reason API parity isn't required, then I wouldn't change it back to create_publisher, i.e. create_publisher is not preferable to advertise in my opinion.

So, after your comment I'm still wondering what we need to figure out or what precisely is unclear about the migration path which impacts our ability to consider the questions I asked before?

[jack-oquin]

I agree. The migration guide tells what must change to convert a ROS 1 program to the ROS 2 interfaces. That is a separate topic than the ROS 1 API shim (which is also a good idea).

William is advocating a cleaner interface. That it happens to be more like ROS 1 is not the only benefit.

[gbiggs]

I think that if there is going to be a ROS 1 API shim, which will provide an approach to using ROS 2 with an API that is familiar to a ROS 1 user, then there is an opportunity to propose clean APIs that also follow the conventions of the languages being used.

I don't know what the proposed reason is for making the C++ API mimic the C API, but my opinion is that it is better to make the C++ API use C++ conventions even if that means it works differently to the C API. This means things like relying on construction/destruction as much as possible rather than create/destroy functions (which are easy to forget to call). If the objects involved obey sensible lifetime rules then the behaviour should be intuitive for a C++ developer.

The DDS C++ API uses the C-style approach, and it is icky (to me).

Having said that, the questions that @wjwwood raised depend on whether we see a Node object as the central repository of all resources and on which everything is done, or we see a set of objects with their own lifetimes that the developer manages themselves. Personally, I like the latter as C++ class design best practice says not to make monster classes, and in general it has less of a hidden magic feeling about it.

If the former approach is taken, then I mostly agree with @wjwwood. However, I think (if I have got this right) that there is an issue with providing smart_ptrs to Publisher/Subscription objects when they are being managed by the node: If the node goes out of scope, the smart_ptr will ensure that the Publisher/Subscription does not die, but will hang around in some weird half-life because it probably needs other resources that are now gone.

If the latter is the approach taken, then my answers to the questions would be:

Should we provide a reference counting wrapper class for Publishers and Subscriptions like in ROS 1? The alternative is to continue to return a "simple" Publisher/Subscription class wrapped in a shared_ptr.

I prefer using shared_ptrs. It's a simpler design.

Follow up: Should we have a way to create one as a shared_ptr as well?

If the Publisher/Subscription classes are created via a factory function, then it should return a shared_ptr. If they are constructed directly, then an easy-to-remember way to grab a shared_ptr to the resulting object is probably nice to have.

Should we rename create_publisher to advertise in C++?

I like "advertise" more, but it doesn't imply creating an object that the developer must then take care of. It implies that something magic happens in the background and a topic is advertised until I tell it to stop being advertised, irrespective of the life time of any objects that may or may not be created. If the developer is responsible for the lifetime of the publisher/subscription, then a create_* name makes more sense as a factory function. If it's all hidden like it was in ROS 1, then advertise makes more sense.

Follow up: Should the object returned from advertise be scoped?

Yes.

Should we rename create_subscription to subscribe in C++?

Same as "advertise".

Follow up: Should the object returned from subscribe be scoped?

Yes.

Should we have a version of create_publisher and create_subscription which return non-scoped versions of the respective classes? These non-scoped versions would either only be destroyed when the node is shutdown or if the result is passed to the matching destroy_* function.

No, because that means hidden background magic.

I think it should be possible to have a Publisher/Subscription that goes out of scope destroy itself by having it hold a reference to the parent object that created it, and call the necessary clean-up functions on itself via that during destruction. I don't think that would violate any of the best practices about destructors.