Custom QoS for inputs and outputs

[domire8]

One limitation we currently have with component inputs and outputs is that we can't specify custom QoS settings for the publisher/subscriber. This can be quite crucial when communicating with other devices that provide non default QoS.

@eeberhard @buschbapti @yrh012

[eeberhard]

There is always a tradeoff between wrapper functions that provide convenience and that restrict customization. I think adding more options or overloading the helper function gets cumbersome at some point, and the mechanism to add "raw" pubs and subs always exists.

Maybe you could summarize again all the things our add_input / add_output helpers do that the user would have to repeat manually. I can think of a few off the top of my head but there may be more:

• bind and translate message to shared pointer attribute
• publish on step
• optional user callback on subscription
• dynamic topic name with parameter

Then we can see what the tradeoff is for either adding more options to our helper function or just documenting this behavior to allow manual reproduction / customization

[domire8]

Here are the differences:

declare_input("tcp_state"); // for topic parameter creation
tcp_sub_ = create_subscription<urinterfaces::msg::TCPState>(
    get_parameter_value<std::string>("tcp_state_topic"), rclcpp::QoS(10).best_effort(),
    [this](const std::shared_ptr<urinterfaces::msg::TCPState> msg) { this->tcp_state_callback(msg); });

vs.

add_input("cartesian_state", tcp_state_);

in the very simplest case. In other words, if not using our helpers, the user has to take care of topic parameter, callback in case of subscription / publishing for publisher, and any kind of translation.

Technically, and additional argument with default value for add_input/add_output wouldn't really hurt but at the same time, we have to store this somewhere because the configuration of the underlying pub/sub is not immediate.

[eeberhard]

we have to store this somewhere

If we change the structure of what is stored for a given input/output, it seems to me it could this be ABI breaking and then should wait for a breaking release.

an additional argument with default value for add_input/add_output wouldn't really hurt

Another thing to consider is that this whole opinionated library is about facilitating standard plug and play connections between componets - having the common EncodedState message type and standard QoS enables that under the hood. While it should be possible to use any arbitrary message type or QoS, having more options increases the space for incompatibility.

Ultimately if we add helpers for different QoS we have to reflect that also in the component description schema and ultimately in studio also (also not an issue, just worth mentioning)

So overall I'm ok with it as a new feature as long as it is intentionally rolled out across the stack, and that we test for ABI breaking behavior first.