search

fkie / multimaster_fkie

ROS stack with FKIE packages for multi-robot (discovering, synchronizing and management GUI)
BSD 3-Clause "New" or "Revised" License
268 stars 108 forks source link

Configuration option to ignore only topic publisher or subscribers #21

Closed adamantivm closed 9 years ago

adamantivm commented 9 years ago

This is mostly a request for review to the approach in general. Would be happy to hear if this type of addition is palatable or if a different approach is recommended.

This PR adds two new configuration parameters: ignore_publishers and ignore_subscribers. If either of those are set, then subscribers or publishers in particular for the listed topics will be ignored.

We needed this enhancement for a particular set-up in which we need to avoid propagating subscribers of a particular topic (/clock) while at the same time allowing the propagation of publishers of this same topic.

atiderko commented 9 years ago

Interesting use case, it new for me ;-)

Basically, it is the right way to add this two parameter. I would prefer to append the new parameter to the lists in load(), to_list() and from_list() methods of FilterInterface class. Otherwise, the compatibility is broken with older versions. Because the filter list is used to request master state from remote master_discovery.

The whole functionality I can check after I am back from vacation...

adamantivm commented 9 years ago

@atiderko thanks for the quick review. I moved the new parameters to the end of the lists on the methods you mentioned and also did some clean-up. I hope it is ready to marge now. No rush.