API review

Proposer: Josh Faust

Present at review:

• Vijay, Stu, Patrick, Jeremy, Eric, Brian

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.

• [Vijay] It looks like the "Connection" object is pretty much a thin wrapper around the boost::signals:connection object, along with a disconnect callback. This seems reasonable if we need to support registering a custom DisconnectFunction, but I think disconnection can be simplified to always being a boost::signals::connection::disconnect() call in the receiving filter.

  • [Vijay] We could simplify the interface by getting rid of Connection, and let the receiving filter deal with the boost::signals::connection object directly.

    • [Josh] Unfortunately not possible because of threading issues... ie. because we're using signals and not signals2 (which arrived in boost 1.39), the signal/connections are not thread-safe, so the disconnect call needs to be guarded by the correct mutex. Using a Connection type of our own also means that we should be able to migrate to another signals library less painfully if we want to.

  • [Vijay] Writing the disconnect() call is definitely one of the more confusing parts of creating a new filter. We could possibly inherit off of a MessageFilter base class that defines disconnect()

• [Vijay] The names of connect() and connectTo() don't make it very clear as to which is the input and which is the output. I would suggest something more symmetrical, like connectToInput() and connectToOutput()

• [Patrick] Specifying what needs to be synchronized at compile time is convenient for many uses but also pretty limiting. I am looking to fold stereo_view into image_view, my idea being to pass multiple topic names to image_view and have each of them displayed in synch with each other. This requires synchronizing topics dynamically.
  • [Jeremy] The special case which instead deals with a Vector of N messages of the same type shouldn't be too ugly (not that I'm volunteering to implement it). Where things get hairier is having N images, and N cam infos, etc.
• [Jeremy] We will eventually want a time sequencer that handles multiple types of message. What does the API look like to deal with multiple input callbacks of different type and multiple output callbacks of different types? Even for the same type, we might want to specify a different callback-per-topic.
• [Jeremy] Many of the old version of these had "timeout callbacks" of some form or another. A naive implementation of this shouldn't be too bad. Do we care about he ability to chain a timeout callback into another filter of some sort though?

Meeting agenda

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

• Might be useful to add something similar to TimeSynchronizer, but with a tolerance. E.g., give me a laser scan and a camera image that are timestamped near each other. No concrete use case yet; possible future enhancement

• topic_synchronizer2 supports dynamic addition of topics; TimeSynchronizer does not. Future enhancement.

• Multi-type TimeSequencer is a future enhancement

• Would be nice to connect a diagnostic callback to a filter, to be notified when things aren't going right (e.g., incoming data is falling off the back of the queue). Future enhancement.

Conclusion

Package status change mark change manifest)

• Action items that need to be taken.

• Major issues that need to be resolved

• connect vs. connectTo:

  • To attach a user's callback to a filter, use filter.registerCallback()

  • To attach upstream filters as inputs to a filter, use filter.connectInput()

• Common functionality for simple filters goes into SimplerFilter base class

  • Add setName() method to filter API (implement it in SimpleFilter, require complex filters to implement it themselves. This method stores a name to be used in debug output.