[Documentation] [TitleIndex] [WordIndex

API review

Proposer: Patrick

Present at review:

Question / concerns / comments

Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.

Patrick

This API review is for the stack as a whole, as each plugin package has the same API structure and there isn't that much of it.

Plugin parameters

I'm making a breaking change to how publisher plugins retrieve parameters (for example, the compressed publisher has parameters for JPEG quality, etc.). The old behavior is documented in detail here and described briefly below. Possible options for retrieving parameters (happens for every image published/received):

  1. Do a searchParam starting from the namespace of the image topic. This is how publisher plugins work in boxturtle/cturtle. This provides a lot of flexibility, but my impression is that it's little used and that a searchParam on every publication hammers the master.

    • Blaise: I am not certain that this hammers the master. getParam should not go to the master more than once AFAIK, at least for roscpp Josh hasn't done anything to dispell that impression that I have. I would be unpleasantly surprised if searchParam was bypassing that optimization. This is one that should definitely get checked with Josh (and Ken for rospy).

    • Blaise: I'm worried that using searchParam may make it really easy to accidentally pull in unintended parameters, unless the plugin parameters have names that makes it very clear that they apply to a particular plugin. For instance, compression_level, is sure to cause collisions at some point, whereas theora_transport_compression_level would probably be okay.

    • Blaise: Is there ever a need to check these parameters at more than 1Hz or so?

    • Radu: I agree with Blaise about the parameter search. I ran into some nasty similar problems lately. One workaround is to make the search really explicit by making sure the parameter names are somewhat unique, but that's not guaranteed to work on the long run I think.

  2. Do a getParamCached in the node private namespace (by default, can be changed through image_transport::TransportHints). This is how subscriber plugins are already defined to work. It should (I think?) be easier on the master, it's simpler to have all plugins work this way, and it's forward-compatible with option 3.

    • Blaise What is the advantage of this over the current method? This means that you need to know the node that is publishing to configure the transport, and it means that a given node can only have one set of transport parameters for all its image publications.

  3. Use dynamic_reconfigure. I like this the best, but dynamic_reconfigure wasn't designed with this use case in mind. Ideally dynamic_reconfigure would coalesce each plugin's parameters with those of the node, but I don't think this is currently possible.

    • Blaise Even if dynamic_reconfigure could coalesce all the node parameters, I would find it preferable to group each plugin's parameters into a separate dynamic_reconfigure namespace. Otherwise you increase the number of parameters that the user is confronted with when seeing the node, and they already tend to find that there are too many.

So for Diamondback I'm choosing option 2 for all plugins, possibly switching to option 3 if dynamic_reconfigure is up to it.

ogg_saver in theora_image_transport

I'm changing the topic remap on the command line to be more consistent with other image subscriber nodes, docs. Probably I can fix the problem of not knowing the frame rate.

libtheora

I'm planning to replace this 3rd-party package with a rosdep before the Diamondback release, so ignore it. It only exists due to Ubuntu's libtheora-dev package being broken up through Jaunty, and Jaunty gets end-of-lifed in October.

Meeting agenda

To be filled out by proposer based on comments gathered during API review period

Conclusion

Stack status change mark change manifest)



2019-11-09 12:45