roscpp: Cannot have multiple latched publishers on the same topic (ros ticket #2356)

[tfoote]

In rosplay, to actually mirror the appearance of multiple nodes, I need to create multiple latched publishers on the same topic.

The expected behavior would be for every advertiser to publish its own most recent message when a new subscriber shows up, which is what independent nodes would do. However, only a single message gets published.

trac data:

• Owner: straszheim
• Reporter: leibs
• Reported at: Tue Jan 19 03:50:03 2010
• URL: https://code.ros.org/trac/ros/ticket/2356

[tfoote]

[jfaust] This isn't how we spec'ed out latched publishers to work, and it's not completely trivial to change. Not going to happen for 1.0.

[tfoote]

[jfaust] Ken, I assume this wouldn't work in rospy either?

[tfoote]

[kwc] Right, the latch is stored with the publisher singleton, not the publisher instance

[tfoote]

[leibs] Pushing is fine. I don't think it's a very common use-case given that latched recording wasn't working at all previously and nobody was complaining.

Is there a reason that latched publishers were spec'ed to be per-topic rather than per-advertiser?

If I have multiple advertisers from within the same node how do their subscription callbacks interact? Does each get its own subscription callback at least?

[tfoote]

[jfaust] subscription callbacks work per advertiser, but latched topics are really meant more for topics that only a single publisher is publishing on (like the map).

[tfoote]

[tfoote] With people using nodelets more this can start to produce unexpected errors, and it would enable libraries from using latched topics. In particular we are looking at libraries which used latched topics for bulk updates with a parallel channel with delta updates.

[tfoote]

[jbohren] I also encountered this "feature" when I was trying to use latched publishers in smach last year.

[wjwwood]

This came up again in the context of recording /tf_static.

One solution would be to have rosbag advertise as a non-latched topic but keep the latest message for each publisher, watch for new subscribers to come up and when they do publish the latest message from each publisher. The upside is that this approach would ensure that all subscribers would receive the latest message from each publisher, but the downside is that existing subscribers will get repeated messages when new subscribers come up.

Short of refactoring how publishers and latching works I don't think we can fix this...

[mkhansenbot]

Any chance this will be fixed in an upcoming release?

[dirk-thomas]

This ticket has the milestone "untargeted" assigned which says the following in its description:

It is unlikely that the maintainers will have time to address these issues. Please provide pull requests if you want these issues to be addressed.

[racko]

I implemented a prototypical solution for this problem in the rosbag play context: #1004
What do you think about it? Is it worth pursuing?

[dirk-thomas]

Please see my comment on the PR: #1004 (comment)

[BenjaminPelletier]

For other people running into this issue, there's a pretty easy work-around with rospy to the poor behavior in this bug. Instead of using a rospy.Publisher, use a LatchPublisher as shown below. One gotcha is that, because of the underlying Publisher singleton, the queue_size must be large enough for all Publishers on the same topic, and each instantiated Publisher/LatchPublisher overwrites the queue_size specified for the previously-instantiated Publisher/LatchPublisher.

class LatchPublisher(rospy.Publisher, rospy.SubscribeListener):
    def __init__(self, name, data_class, tcp_nodelay=False, headers=None, queue_size=None):
        super(LatchPublisher, self).__init__(name, data_class=data_class, tcp_nodelay=tcp_nodelay, headers=headers, queue_size=queue_size, subscriber_listener=self, latch=False)
        self.message = None

    def publish(self, msg):
        self.message = msg
        super(LatchPublisher, self).publish(msg)

    def peer_subscribe(self, resolved_name, publish, publish_single):
        if self.message is not None:
            publish_single(self.message)

[NikolausDemmel]

This came up again in the context of recording /tf_static.

One solution would be to have rosbag advertise as a non-latched topic but keep the latest message for each publisher, watch for new subscribers to come up and when they do publish the latest message from each publisher. The upside is that this approach would ensure that all subscribers would receive the latest message from each publisher, but the downside is that existing subscribers will get repeated messages when new subscribers come up.

Coming here (again) from https://discourse.ros.org/t/request-for-comments-static-transform-mux/5907. I guess for all practical purposes the mux would actually work well enough. However, this solution suggested by @wjwwood to modify rosbag play for special handling of latchted topics also seems relatively simple and it sounds like it would also work well enough and maybe more seamlessly than with the mux. The mentioned "downside" could maybe be avoided by only sending out the cached messages to the new subscriber, not all. Not sure how easy the is to do with the existing roscpp API.

One thing to keep in mind is that AFAIR most, if not all, the tools that modify rosbags (rosbag filter, fix, ...) currently loose the connection info and also the "latched" flag.

[meyerj]

Missing messages when using tf_static or replaying bag files with latched topics are only symptoms of the underlying problem, namely that the current design of latching publishers is broken whenever modules using roscpp are linked together and run in the same process. This might not have been the case yet when this issue was opened in 2013, but nodelets, Gazebo, move_base planners or any other kind of plugins are examples where the author cannot know in advance in which process his code will finally run and using latched publishers or static transform publishers in those contexts must be considered as undefined behavior. Are there examples where it is actually desirable that only the message published last is stored and forwarded to new subscribers? If the two publishers would live in separate processes, subscribers also receive both messages.

Would a patch to roscpp be acceptable that moves the storage of the latched SerializedMessage instance from the Publication singleton to the individual Publisher instances? The publishers could handle the case when a new subscriber connects in a wrapped subscriber connect callback that first enqueues the cached message and then calls the actual user-provided callback. Or a separate callback that the publisher can register and that is called by the publication for each new subscriber link without using a callback queue. As a result, if there are multiple publishers of a latched topic in a single process, connecting subscribers will receive one message per publisher instead of only the last one - given that their queue size is large enough. Unfortunately such a patch would probably break the roscpp ABI, but because of the significant change in behavior it can only be introduced in a future release anyway.

A limitation of this solution is that either all or no topics in a process must be advertised as latching. In other cases the creation of an additional publisher should fail explicitly.

Once this design flaw is solved, it should be straight-forward to fix rosbag play by instantiating one publisher per connection. I did not check, maybe this is already the case. However, when recording and replaying topics that are published by a single node with multiple latching publishers, rosbag play can still not know which message originated from which publisher and hence not fully mimic the situation at the time of recording.

I did not think about the solution in all details and maybe I am missing something else here. I just would like to confirm that such a behavioral change would be desirable or not, before spending more time.

[dirk-thomas]

Would a patch to roscpp be acceptable ...

In general the answer is yes.

because of the significant change in behavior it can only be introduced in a future release anyway.

Agreed.

A limitation of this solution is that either all or no topics in a process must be advertised as latching. In other cases the creation of an additional publisher should fail explicitly.

I don't think that is a viable solution. It kind of "fails" in a similar but different way as currently multiple latched publishers are not doing what you would expect.

[mgrrx]

I recently started to work on allowing multiple latched publishers in Python. Due to a lack of time I stopped working on it but maybe it helps to continue the discussion here: https://github.com/magazino/ros_comm/commits/multiple-latched-publishers . This of course completely changes the behavior of existing code.
I checked the code and a similar approach seemed feasible for roscpp.

[meyerj]

I submitted a pull request with a patch following the proposal in #146 (comment) along the same line of what @mgrrx proposed for Python: #1544