Skip to content
This repository has been archived by the owner on Feb 3, 2025. It is now read-only.

Destroying a publisher stops all publishers to that topic #1782

Closed
osrf-migration opened this issue Nov 12, 2015 · 10 comments
Closed

Destroying a publisher stops all publishers to that topic #1782

osrf-migration opened this issue Nov 12, 2015 · 10 comments
Labels
all bug Something isn't working major transport

Comments

@osrf-migration
Copy link

Original report (archived issue) by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


  1. Create publisher aPub to topic A

  2. Create publisher aPub2 to topic A

  3. Destroy aPub and do gz topic -i A, no publishers will show. Try to publish with aPub2 and it doesn't publish.

  4. Create publisher aPub3 to topic A and do gz topic -i A, now a publisher will show. Try to publish with aPub2 and now it publishes, as well as aPub3.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


We could extend the INTEGRATION_transport test to include this case. There are several tests involving multiple publishers.

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


Argh, found the problem. We're unadvertizing a topic whenever we unadvertize one of its publishers at TopicManager::Unadvertise(PublisherPtr _pub).

I'll see if I can write a simple test for this. I wonder if it's worth it at this point, I don't fully understand how the integration with ign-transport will be made.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


I wonder if we're noticing the problem now because of pull request #1951 (backported to gazebo5/6 in pull request #1964)?

I haven't debugged it fully, so I'm not sure if it's related, but I'm experiencing a problem with loading gzclient with robonaut on the current gazebo6 branch. If I revert to the 6.5.1 tag, it's ok, so I think it might be related to those pull requests.

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


I think the problem I described is not related to the pull requests you mentioned. (I discovered this while implementing undo)

My problem is fixed it with this patch:

diff -r 642bc5ebbca12e4a9ad67d16ca99e3be48a45476 gazebo/transport/Publisher.cc
--- a/gazebo/transport/Publisher.cc	Wed Nov 11 15:43:41 2015 -0800
+++ b/gazebo/transport/Publisher.cc	Thu Nov 12 15:21:49 2015 -0800
@@ -254,8 +254,7 @@
   if (!this->messages.empty())
     this->SendMessage();
 
-  if (!this->topic.empty())
-    TopicManager::Instance()->Unadvertise(this->topic);
+  TopicManager::Instance()->Unadvertise(shared_from_this());
 
   common::Time slept;
 
diff -r 642bc5ebbca12e4a9ad67d16ca99e3be48a45476 gazebo/transport/Publisher.hh
--- a/gazebo/transport/Publisher.hh	Wed Nov 11 15:43:41 2015 -0800
+++ b/gazebo/transport/Publisher.hh	Thu Nov 12 15:21:49 2015 -0800
@@ -41,6 +41,7 @@
     /// \class Publisher Publisher.hh transport/transport.hh
     /// \brief A publisher of messages on a topic
     class GZ_TRANSPORT_VISIBLE Publisher
+      : public boost::enable_shared_from_this<Publisher>
     {
       /// \brief Constructor
       /// \param[in] _topic Name of topic to be published
diff -r 642bc5ebbca12e4a9ad67d16ca99e3be48a45476 gazebo/transport/TopicManager.cc
--- a/gazebo/transport/TopicManager.cc	Wed Nov 11 15:43:41 2015 -0800
+++ b/gazebo/transport/TopicManager.cc	Thu Nov 12 15:21:49 2015 -0800
@@ -412,7 +412,7 @@
     if (publication)
       publication->RemovePublisher(_pub);
 
-    this->Unadvertise(_pub->GetTopic());
+//    this->Unadvertise(_pub->GetTopic());
   }
 }

@osrf-migration
Copy link
Author

Original comment by Ian Chen (Bitbucket: Ian Chen, GitHub: iche033).


@scpeters does the robonaut take forever to load? it could be related to the refactoring done on ArrowVisual class in pull request #1967.

@osrf-migration
Copy link
Author

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


@iche033 you are right, my problem is fixed by pull request #1967. sorry for the noise @chapulina

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


Pull request #2005

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


  • changed state from "new" to "resolved"

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • set version to "all"

@osrf-migration
Copy link
Author

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


  • changed state from "resolved" to "closed"

@osrf-migration osrf-migration added major transport bug Something isn't working all labels Apr 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
all bug Something isn't working major transport
Projects
None yet
Development

No branches or pull requests

1 participant