Merge pull request ros#213 from cottsay/groovy-devel

rqt_reconfigure: Use a single dynamic reconfigure client

rqt_reconfigure/src/rqt_reconfigure/paramedit_widget.py

@@ -89,30 +89,14 @@ def show_reconf(self, dynreconf_widget):
         node_grn = dynreconf_widget.get_node_grn()
         rospy.logdebug('ParameditWidget.show str(node_grn)=%s', str(node_grn))
 
-        dynreconf_client = None
-
         if not node_grn in self._dynreconf_clients.keys():
-            # Add dynreconf widget if there hasn't one existed.
-
-            #TODO think about sharing dynamic_reconfigure.client instances
-            # with NodeSelecorWidget...generating 2 instances of the same node
-            # is nothing but inefficient and bad design.
-
-            try:
-                dynreconf_client = dynamic_reconfigure.client.Client(
-                                               str(node_grn), timeout=5.0)
-            except rospy.exceptions.ROSException:
-                rospy.logerr("Could not connect to %s" % node_grn)
-                #TODO(Isaac) Needs to show err msg on GUI too.
-                return
-
-            _dynreconf_client = DynreconfClientWidget(dynreconf_client,
-                                                      node_grn)
+            # Add dynreconf widget if there isn't already one.
+
             # Client gets renewed every time different node_grn was clicked.
 
-            self._dynreconf_clients.__setitem__(node_grn, _dynreconf_client)
-            self.vlayout.addWidget(_dynreconf_client)
-            _dynreconf_client.sig_node_disabled_selected.connect(
+            self._dynreconf_clients.__setitem__(node_grn, dynreconf_widget)
+            self.vlayout.addWidget(dynreconf_widget)
+            dynreconf_widget.sig_node_disabled_selected.connect(
                                                            self._node_disabled)
 
         else:  # If there has one already existed, remove it.

rqt_reconfigure/src/rqt_reconfigure/treenode_qstditem.py

@@ -112,6 +112,9 @@ def set_dynreconf_client(self, dynreconf_client):
                                                        self._dynreconf_client,
                                                        self._param_name_raw))
 
+    def clear_dynreconf_client(self):
+        self._dynreconf_client = None
+
     def get_dynreconf_widget(self):
         """
         @rtype: DynreconfClientWidget (QWidget)
@@ -146,12 +149,20 @@ def get_dynreconf_widget(self):
             self._dynreconfclient_widget = DynreconfClientWidget(
                                                        self._dynreconf_client,
                                                        self._param_name_raw)
+            # Creating the DynreconfClientWidget transfers ownership of the _dynreconf_client
+            # to it. If it is destroyed from Qt, we need to clear our reference to it and
+            # stop the param server thread we had.
+            self._dynreconfclient_widget.destroyed.connect(self.clear_dynreconfclient_widget)
+            self._dynreconfclient_widget.destroyed.connect(self.disconnect_param_server)
             rospy.logdebug('In get_dynreconf_widget 5')
 
         else:
             pass
         return self._dynreconfclient_widget
 
+    def clear_dynreconfclient_widget(self):
+        self._dynreconfclient_widget = None
+
     def connect_param_server(self):
         """
         Connect to parameter server using dynamic_reconfigure client.
@@ -172,6 +183,15 @@ def connect_param_server(self):
                                        self, self._param_name_raw)
             self._paramserver_connect_thread.start()
 
+    def disconnect_param_server(self):
+        if self._paramserver_connect_thread:
+            # Try to stop the thread
+            if self._paramserver_connect_thread.isAlive():
+              self._paramserver_connect_thread.join(1)
+            del self._paramserver_connect_thread
+            self._paramserver_connect_thread = None
+        self.clear_dynreconf_client()
+
     def enable_param_items(self):
         """
         Create QStdItem per parameter and addColumn them to myself.