API review

Proposer: Josh F

Present at review:

• List reviewers

This is a review of the following additions to message_filters

API Additions

Chain

doc/api/message_filters/html/c++/classmessage__filters_1_1Chain.html

message_filters::Chain allows you to store a dynamic chain of filters. For example:

   1 void myCallback(const MsgConstPtr& msg)
   2 {
   3 }
   4 
   5 Chain<Msg> c;
   6 c.addFilter(boost::shared_ptr<Subscriber<Msg> >(new Subscriber<Msg>));
   7 c.addFilter(boost::shared_ptr<TimeSequencer<Msg> >(new TimeSequencer<Msg>));
   8 c.registerCallback(myCallback);

It is also possible to pass bare pointers in, which will not be automatically deleted when Chain is destructed:

   1 Chain<Msg> c;
   2 Subscriber<Msg> s;
   3 c.addFilter(&s);
   4 c.registerCallback(myCallback);

It also includes a base class, ChainBase (doc/api/message_filters/html/c++/classmessage__filters_1_1ChainBase.html), which allows you to store the chain itself in a type-independent manner.

Chain only works with single-input/single-output (simple) filters.

In addition to adding a filter you can also retrieve one added previously:

   1 Chain<Msg> c;
   2 size_t sub_index = c.addFilter(boost::shared_ptr<Subscriber<Msg> >(new Subscriber<Msg>));
   3 
   4 boost::shared_ptr<Subscriber<Msg> > sub = c.getFilter<Subscriber<Msg> >(sub_index);

Policy-based Synchronizer and two policies (ExactTime, ApproximateTime)

This is a replacement for the current TimeSynchronizer class. It is currently in the message_filters::synchro namespace (sync is reserved by the sync() system call).

Synchronizer and friends are in a nested namespace because I can see other message filters (like the sequencer) moving to follow this pattern, and I don't want their policy classes colliding. If people have suggestions for other names for the namespace I'd like to hear them, since I'm not a big fan of synchro

The driving force behind changing TimeSynchronizer to be policy based is the creation of Romain's ApproximateTimeSynchronizer that duplicated TimeSynchronizer's code until we could integrate them together.

The equivalent of:

   1 typedef message_filters::TimeSynchronizer<Msg, Msg> Sync2;
   2 Sync2 sync(1);

is:

   1 typedef message_filters::synchro::ExactTime<Msg, Msg> Policy2;
   2 typedef message_filters::synchro::Synchronizer<Policy2> Sync2;
   3 Sync2 sync(Policy2(1));

Using the new approximate time policy:

   1 typedef message_filters::synchro::ApproximateTime<Msg, Msg> Policy2;
   2 typedef message_filters::synchro::Synchronizer<Policy2> Sync2;
   3 Sync2 sync(Policy2(1));

Approximate Time Policy

ApproximateTime policy algorithm description

Drop callback

This is in a separate category because it adds functionality to what used to be the TimeSynchronizer.

You can register a drop callback in the same way you register a normal one, but with registerDropCallback. This callback is called whenever a set of messages are dropped.

Use of traits instead of directly accessing message.header.*

All filters now use the ros::message_traits::TimeStamp trait to retrieve the timestamp from a message. This means you can now specialize that trait if you like.

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.

• (Romain) One issue is that the definition is a little more verbose, and the users has to know what a "Policy" is. Would it make sense and is it possible to typedef

message_filters::synchro::Synchronizer<message_filters::synchro::ExactTime<...> >

• (Romain) Given that you managed to make a larger part of the code non-repetitive (i.e. it does not depend on the number of template parameters), what would it take to remove the restriction of 9 messages. Sure, I can't think of anything that would require it...

• (Romain) This is probably not the place for this, but would it be possible to synchronize on the availability of tf as well? I.e. a mix between Synchronizer and TransformListener. I had a case with the pan-tilt table where that might have been useful (I since found a better way that does not need this).

to message_filters::synchro::ExactSynchronizer<...>?

Meeting agenda

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

Conclusion

Package status change mark change manifest)

• Action items that need to be taken.

• Major issues that need to be resolved

• Check sync:: namespace again

• Possibly pull Synchronizer up into message_filters namespace

• Fix TimeSynchronizer constructors to call connectInput() and check the unit tests

• Look into convenience constructor for policy creation

• Move Synchronizer drop callback into the ExactTime policy

• Implement queue size in ApproximateTime policy