2004-10-15 Dan Williams <dcbw@redhat.com>

* panel-applet/NMWirelessApplet.c
		- Update our applet state from the GUI thread

	* panel-applet/NMWirelessAppletDbus.c
		- Greatly simplify the locking to make the GUI thread
			smoother.  Update a private copy of the device list
			and active device and only when done talking to
			NetworkManager turn it over to the GUI thread.


git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@245 4912f4e0-d625-0410-9fb7-b9a5a253dbdc

ChangeLog

@@ -1,3 +1,14 @@
+2004-10-15 Dan Williams <dcbw@redhat.com>
+
+	* panel-applet/NMWirelessApplet.c
+		- Update our applet state from the GUI thread
+
+	* panel-applet/NMWirelessAppletDbus.c
+		- Greatly simplify the locking to make the GUI thread
+			smoother.  Update a private copy of the device list
+			and active device and only when done talking to
+			NetworkManager turn it over to the GUI thread.
+
 2004-10-15 Dan Williams <dcbw@redhat.com>
 
 	* src/NetworkManagerAP.[ch]

panel-applet/NMWirelessApplet.c

@@ -98,6 +98,65 @@ static GObject *nmwa_constructor (GType type,
   return obj;
 }
 
+
+/*
+ * nmwa_update_network_state
+ *
+ * Update our state based on what NetworkManager's network state is
+ *
+ */
+static void nmwa_update_network_state (NMWirelessApplet *applet)
+{
+	g_return_if_fail (applet != NULL);
+
+	if (!applet->nm_status)
+	{
+		applet->applet_state = APPLET_STATE_NO_CONNECTION;
+		return;
+	}
+
+	if (strcmp (applet->nm_status, "scanning") == 0)
+	{
+		applet->applet_state = APPLET_STATE_WIRELESS_SCANNING;
+		return;
+	}
+	
+	if (strcmp (applet->nm_status, "disconnected") == 0)
+	{
+		applet->applet_state = APPLET_STATE_NO_CONNECTION;
+		return;
+	}
+	
+	if (!applet->active_device)
+	{
+		applet->applet_state = APPLET_STATE_NO_CONNECTION;
+		return;
+	}
+
+	/* If the device is not 802.x, we don't show state for it (yet) */
+	if (    (applet->active_device->type != DEVICE_TYPE_WIRED_ETHERNET)
+		&& (applet->active_device->type != DEVICE_TYPE_WIRELESS_ETHERNET))
+	{
+		applet->applet_state = APPLET_STATE_NO_CONNECTION;
+		return;
+	}
+	else if (applet->active_device->type == DEVICE_TYPE_WIRED_ETHERNET)
+	{
+		if (strcmp (applet->nm_status, "connecting") == 0)
+			applet->applet_state = APPLET_STATE_WIRED_CONNECTING;
+		else if (strcmp (applet->nm_status, "connected") == 0)
+			applet->applet_state = APPLET_STATE_WIRED;
+	}
+	else if (applet->active_device->type == DEVICE_TYPE_WIRELESS_ETHERNET)
+	{
+		if (strcmp (applet->nm_status, "connecting") == 0)
+			applet->applet_state = APPLET_STATE_WIRELESS_CONNECTING;
+		else if (strcmp (applet->nm_status, "connected") == 0)
+			applet->applet_state = APPLET_STATE_WIRELESS;
+	}
+}
+
+
 static gboolean
 animation_timeout (NMWirelessApplet *applet)
 {
@@ -197,15 +256,16 @@ nmwa_update_state (NMWirelessApplet *applet)
 
     }
 
-  if (g_slist_length (applet->devices) == 1 &&
+  if (g_slist_length (applet->device_list) == 1 &&
       applet->applet_state != APPLET_STATE_NO_NM)
     {
-      if (((NetworkDevice *)applet->devices->data)->type == DEVICE_TYPE_WIRED_ETHERNET)
+      if (((NetworkDevice *)applet->device_list->data)->type == DEVICE_TYPE_WIRED_ETHERNET)
 	show_applet = FALSE;
     }
-  g_mutex_unlock (applet->data_mutex);
 
-/*  print_state (applet->applet_state); */
+  nmwa_update_network_state (applet);
+
+  /* print_state (applet->applet_state); */
   switch (applet->applet_state)
     {
     case (APPLET_STATE_NO_CONNECTION):
@@ -247,6 +307,7 @@ nmwa_update_state (NMWirelessApplet *applet)
     default:
       break;
     }
+  g_mutex_unlock (applet->data_mutex);
 
   /*determine if we should hide the notification icon*/
   gtk_image_set_from_pixbuf (GTK_IMAGE (applet->pixmap), pixbuf);
@@ -411,7 +472,7 @@ NetworkDevice *nmwa_get_device_for_nm_device (NMWirelessApplet *applet, const ch
 	g_return_val_if_fail (strlen (nm_dev), NULL);
 
 	g_mutex_lock (applet->data_mutex);
-	element = applet->devices;
+	element = applet->device_list;
 	while (element)
 	{
 		NetworkDevice	*dev = (NetworkDevice *)(element->data);
@@ -560,7 +621,7 @@ create_wireless_adaptor_model (NMWirelessApplet *applet)
   GSList *element;
 
   retval = gtk_list_store_new (2, G_TYPE_STRING, G_TYPE_POINTER);
-  for (element = applet->devices; element; element = element->next)
+  for (element = applet->device_list; element; element = element->next)
     {
       NetworkDevice *network = (NetworkDevice *)(element->data);
 
@@ -637,7 +698,7 @@ custom_essid_item_selected (GtkWidget *menu_item, NMWirelessApplet *applet)
 
   /* Do we have multiple Network cards? */
   g_mutex_lock (applet->data_mutex);
-  for (element = applet->devices; element; element = element->next)
+  for (element = applet->device_list; element; element = element->next)
     {
       NetworkDevice *dev = (NetworkDevice *)(element->data);
 
@@ -822,16 +883,16 @@ static void nmwa_menu_add_devices (GtkWidget *menu, NMWirelessApplet *applet)
 	g_return_if_fail (applet != NULL);
 
 	g_mutex_lock (applet->data_mutex);
-	if (! applet->devices)
+	if (! applet->device_list)
 	{
 		nmwa_menu_add_text_item (menu, _("No network devices have been found"));
 		g_mutex_unlock (applet->data_mutex);
 		return;
 	}
 
-	applet->devices = g_slist_sort (applet->devices, sort_networks_function);
+	applet->device_list = g_slist_sort (applet->device_list, sort_networks_function);
 
-	for (element = applet->devices; element; element = element->next)
+	for (element = applet->device_list; element; element = element->next)
 	{
 		NetworkDevice *dev = (NetworkDevice *)(element->data);
 
@@ -851,7 +912,7 @@ static void nmwa_menu_add_devices (GtkWidget *menu, NMWirelessApplet *applet)
 	}
 
 	/* Add all devices in our device list to the menu */
-	for (element = applet->devices; element; element = element->next)
+	for (element = applet->device_list; element; element = element->next)
 	{
 		NetworkDevice *dev = (NetworkDevice *)(element->data);
 
@@ -1017,8 +1078,9 @@ static GtkWidget * nmwa_get_instance (NMWirelessApplet *applet)
 		return (NULL);
 
 	applet->applet_state = APPLET_STATE_NO_NM;
-	applet->devices = NULL;
+	applet->device_list = NULL;
 	applet->active_device = NULL;
+	applet->nm_status = NULL;
 
 	/* Start our dbus thread */
 	if (!(applet->data_mutex = g_mutex_new ()))

panel-applet/NMWirelessApplet.h

@@ -111,9 +111,11 @@ typedef struct
 
         /* Data model elements */
 	GMutex			*data_mutex;
-	GSList			*devices;
-	NetworkDevice		*active_device;
 	AppletState		 applet_state;
+	GSList			*device_list;
+	NetworkDevice		*active_device;
+	char				*nm_status;
+	NetworkDevice		*dbus_active_device;
 
         GdkPixbuf *no_nm_icon;
         GdkPixbuf *wired_icon;
@@ -137,7 +139,7 @@ typedef struct
 	GtkWidget			*pixmap;
 	GtkWidget			*menu;
 	GtkWidget			*toplevel_menu;
-	GtkSizeGroup                    *encryption_size_group;
+	GtkSizeGroup        *encryption_size_group;
 } NMWirelessApplet;
 
 

panel-applet/NMWirelessAppletDbus.c

@@ -152,7 +152,7 @@ static int nmwa_dbus_get_int (DBusConnection *connection, const char *path, cons
 
 	if (reply == NULL)
 	{
-		fprintf( stderr, "nmwa_dbus_get_int(): dbus reply message was NULL\n" );
+		fprintf (stderr, "nmwa_dbus_get_int(): dbus reply message was NULL\n" );
 		return (RETURN_FAILURE);
 	}
 
@@ -213,7 +213,7 @@ static int nmwa_dbus_get_bool (DBusConnection *connection, const char *path, con
 
 	if (reply == NULL)
 	{
-		fprintf( stderr, "nmwa_dbus_get_bool(): dbus reply message was NULL\n" );
+		fprintf (stderr, "nmwa_dbus_get_bool(): dbus reply message was NULL\n" );
 		return (RETURN_FAILURE);
 	}
 
@@ -287,7 +287,7 @@ static int nmwa_dbus_get_string_array (DBusConnection *connection, const char *p
 
 	if (reply == NULL)
 	{
-		fprintf( stderr, "nmwa_dbus_get_string_array(): dbus reply message was NULL\n" );
+		fprintf (stderr, "nmwa_dbus_get_string_array(): dbus reply message was NULL\n" );
 		return (RETURN_FAILURE);
 	}
 
@@ -606,7 +606,7 @@ static char *nmwa_dbus_get_hal_device_string_property (DBusConnection *connectio
 
 	if (reply == NULL)
 	{
-		fprintf( stderr, "nmwa_dbus_get_hal_device_string_property(): dbus reply message was NULL\n" );
+		fprintf (stderr, "nmwa_dbus_get_hal_device_string_property(): dbus reply message was NULL\n" );
 		return (NULL);
 	}
 
@@ -657,7 +657,7 @@ static char *nmwa_dbus_get_hal_device_info (DBusConnection *connection, const ch
 
 	if (reply == NULL)
 	{
-		fprintf( stderr, "nmwa_dbus_get_hal_device_info(): dbus reply message was NULL\n" );
+		fprintf (stderr, "nmwa_dbus_get_hal_device_info(): dbus reply message was NULL\n" );
 		return (NULL);
 	}
 
@@ -693,15 +693,15 @@ void nmwa_dbus_set_device (DBusConnection *connection, const NetworkDevice *dev,
 	{
 		if ((dev->type == DEVICE_TYPE_WIRELESS_ETHERNET) && network && network->essid)
 		{
-fprintf( stderr, "Forcing device '%s' and network '%s'\n", dev->nm_device, network->essid);
+			fprintf (stderr, "Forcing device '%s' and network '%s'\n", dev->nm_device, network->essid);
 			dbus_message_append_args (message, DBUS_TYPE_STRING, dev->nm_device,
 									DBUS_TYPE_STRING, network->essid, DBUS_TYPE_INVALID);
 		}
 		else
-{
+		{
-fprintf( stderr, "Forcing device '%s'\n", dev->nm_device);
+			fprintf (stderr, "Forcing device '%s'\n", dev->nm_device);
 			dbus_message_append_args (message, DBUS_TYPE_STRING, dev->nm_device, DBUS_TYPE_INVALID);
-}
+		}
 		dbus_connection_send (connection, message, NULL);
 	}
 	else
@@ -926,7 +926,7 @@ NetworkDevice *network_device_copy (NetworkDevice *src)
  * NOTE: caller must lock device list if necessary
  *
  */
-static void nmwa_dbus_update_device_wireless_networks (NetworkDevice *dev, NMWirelessApplet *applet)
+static void nmwa_dbus_update_device_wireless_networks (NetworkDevice *dev, gboolean active_dev, NMWirelessApplet *applet)
 {
 	char		 *active_network = NULL;
 	char		**networks = NULL;
@@ -941,8 +941,7 @@ static void nmwa_dbus_update_device_wireless_networks (NetworkDevice *dev, NMWir
 	if (dev->type != DEVICE_TYPE_WIRELESS_ETHERNET)
 		goto out;
 
-	
+	if (active_dev)
-	if (dev == applet->active_device)
 		active_network = nmwa_dbus_get_active_network (applet, dev->nm_device, APPLET_STATE_IGNORE);
 
 	if (applet->applet_state == APPLET_STATE_NO_NM)
@@ -995,132 +994,20 @@ out:
 }
 
 
-/*
- * nmwa_dbus_update_network_state
- *
- * Update our state based on what NetworkManager's network state is
- *
- */
-static void nmwa_dbus_update_network_state (NMWirelessApplet *applet)
-{
-	char		*nm_status = NULL;
-
-	g_return_if_fail (applet != NULL);
-
-	/* Grab NetworkManager's status */
-	if (!(nm_status = nmwa_dbus_get_nm_status (applet, APPLET_STATE_NO_CONNECTION)))
-		return;
-
-	if (strcmp (nm_status, "scanning") == 0)
-	{
-		applet->applet_state = APPLET_STATE_WIRELESS_SCANNING;
-		goto out;
-	}
-	
-	if (strcmp (nm_status, "disconnected") == 0)
-	{
-		applet->applet_state = APPLET_STATE_NO_CONNECTION;
-		goto out;
-	}
-	
-	if (!applet->active_device)
-	{
-		applet->applet_state = APPLET_STATE_NO_CONNECTION;
-		goto out;
-	}
-
-	/* If the device is not 802.x, we don't show state for it (yet) */
-	if (    (applet->active_device->type != DEVICE_TYPE_WIRED_ETHERNET)
-		&& (applet->active_device->type != DEVICE_TYPE_WIRELESS_ETHERNET))
-	{
-		applet->applet_state = APPLET_STATE_NO_CONNECTION;
-		goto out;
-	}
-	else if (applet->active_device->type == DEVICE_TYPE_WIRED_ETHERNET)
-	{
-		if (strcmp (nm_status, "connecting") == 0)
-			applet->applet_state = APPLET_STATE_WIRED_CONNECTING;
-		else if (strcmp (nm_status, "connected") == 0)
-			applet->applet_state = APPLET_STATE_WIRED;
-	}
-	else if (applet->active_device->type == DEVICE_TYPE_WIRELESS_ETHERNET)
-	{
-		if (strcmp (nm_status, "connecting") == 0)
-			applet->applet_state = APPLET_STATE_WIRELESS_CONNECTING;
-		else if (strcmp (nm_status, "connected") == 0)
-			applet->applet_state = APPLET_STATE_WIRELESS;
-	}
-
-out:
-	dbus_free (nm_status);
-}
-
-
-/*
- * nmwa_dbus_update_active_device
- *
- * Get the active device from NetworkManager
- *
- */
-static void nmwa_dbus_update_active_device (NMWirelessApplet *applet)
-{
-	GSList	*element;
-	char		*nm_act_dev;
-
-	g_return_if_fail (applet != NULL);
-
-	nm_act_dev = nmwa_dbus_get_active_device (applet, APPLET_STATE_IGNORE);
-
-	g_mutex_lock (applet->data_mutex);
-	if (applet->active_device)
-		network_device_unref (applet->active_device);
-	applet->active_device = NULL;
-
-	if (nm_act_dev)
-	{
-		element = applet->devices;
-		while (element)
-		{
-			NetworkDevice	*dev = (NetworkDevice *)(element->data);
-			if (dev)
-			{
-				if (strcmp (dev->nm_device, nm_act_dev) == 0)
-				{
-					applet->active_device = dev;
-					network_device_ref (applet->active_device);
-					break;
-				}
-			}
-			element = g_slist_next (element);
-		}
-	}
-
-	g_mutex_unlock (applet->data_mutex);
-	dbus_free (nm_act_dev);
-}
-
-
 /*
  * nmwa_dbus_update_active_device_strength
  *
  * Update the active device's current wireless network strength
  *
  */
-void nmwa_dbus_update_active_device_strength (NMWirelessApplet *applet)
+static int nmwa_dbus_update_active_device_strength (NMWirelessApplet *applet)
 {
 	g_return_if_fail (applet != NULL);
 
-	g_mutex_lock (applet->data_mutex);
+	if (applet->dbus_active_device && (applet->active_device->type == DEVICE_TYPE_WIRELESS_ETHERNET))
-	if (!applet->active_device)
+		applet->dbus_active_device->strength = nmwa_dbus_get_object_strength (applet, applet->dbus_active_device->nm_device);
-	{
-		g_mutex_unlock (applet->data_mutex);
-		return;
-	}
 
-	if (applet->active_device->type == DEVICE_TYPE_WIRELESS_ETHERNET)
+	return (TRUE);
-		applet->active_device->strength = nmwa_dbus_get_object_strength (applet, applet->active_device->nm_device);
-
-	g_mutex_unlock (applet->data_mutex);
 }
 
 
@@ -1132,12 +1019,19 @@ void nmwa_dbus_update_active_device_strength (NMWirelessApplet *applet)
  */
 static void nmwa_dbus_update_devices (NMWirelessApplet *applet)
 {
-	char	**devices = NULL;
+	char			**devices = NULL;
-	int	  num_items = 0;
+	int			  num_items = 0;
-	int	  i;
+	int			  i;
+	char			 *nm_act_dev = NULL;
+	GSList		 *device_list = NULL;
+	NetworkDevice	 *active_device = NULL;
+	char			 *nm_status = NULL;
 
 	g_return_if_fail (applet->data_mutex != NULL);
 
+	if (!(nm_status = nmwa_dbus_get_nm_status (applet, APPLET_STATE_NO_CONNECTION)))
+		return;
+
 	switch (nmwa_dbus_get_string_array (applet->connection, NM_DBUS_PATH, "getDevices", &num_items, &devices))
 	{
 		case (RETURN_NO_NM):
@@ -1151,14 +1045,11 @@ static void nmwa_dbus_update_devices (NMWirelessApplet *applet)
 	if (!devices)
 		return;
 
-	/* Clear out existing device list */
+	if (applet->dbus_active_device)
-	g_mutex_lock (applet->data_mutex);
+		network_device_unref (applet->dbus_active_device);
-	g_slist_foreach (applet->devices, (GFunc) network_device_unref, NULL);
+	applet->dbus_active_device = NULL;
-	g_slist_free (applet->devices);
+
-	if (applet->active_device)
+	nm_act_dev = nmwa_dbus_get_active_device (applet, APPLET_STATE_IGNORE);
-		network_device_unref (applet->active_device);
-	applet->active_device = NULL;
-	applet->devices = NULL;
 
 	for (i = 0; i < num_items; i++)
 	{
@@ -1181,16 +1072,42 @@ static void nmwa_dbus_update_devices (NMWirelessApplet *applet)
 					network_device_unref (dev);
 				else
 				{
-					applet->devices = g_slist_append (applet->devices, dev);
+					device_list = g_slist_append (device_list, dev);
-					nmwa_dbus_update_device_wireless_networks (dev, applet);
+					if (nm_act_dev && !strcmp (nm_act_dev, devices[i]))
+					{
+						active_device = dev;
+						network_device_ref (dev);
+						applet->dbus_active_device = dev;
+						network_device_ref (dev);
+						nmwa_dbus_update_device_wireless_networks (dev, TRUE, applet);
+					}
+					else
+						nmwa_dbus_update_device_wireless_networks (dev, FALSE, applet);
 				}
 			}
 		}
 		dbus_free (name);
 	}
+	dbus_free (nm_act_dev);
+	dbus_free_string_array (devices);
+
+	/* Now move the data over to the GUI side */
+	g_mutex_lock (applet->data_mutex);
+	if (applet->device_list)
+	{
+		g_slist_foreach (applet->device_list, (GFunc) network_device_unref, NULL);
+		g_slist_free (applet->device_list);
+	}
+	if (applet->active_device)
+		network_device_unref (applet->active_device);
+	if (applet->nm_status)
+		g_free (applet->nm_status);
+
+	applet->device_list = device_list;
+	applet->active_device = active_device;
+	applet->nm_status = nm_status;
 
 	g_mutex_unlock (applet->data_mutex);
-	dbus_free_string_array (devices);
 }
 
 
@@ -1238,42 +1155,17 @@ static DBusHandlerResult nmwa_dbus_filter (DBusConnection *connection, DBusMessa
 	else if (    dbus_message_is_signal (message, NM_DBUS_INTERFACE, "WirelessNetworkAppeared")
 			|| dbus_message_is_signal (message, NM_DBUS_INTERFACE, "WirelessNetworkDisappeared"))
 	{
-		char		*nm_device = NULL;
+		nmwa_dbus_update_devices (applet);
-		char		*network = NULL;
-		DBusError	 error;
-
-		dbus_error_init (&error);
-		if (dbus_message_get_args (message, &error, DBUS_TYPE_STRING, &nm_device,
-				DBUS_TYPE_STRING, &network, DBUS_TYPE_INVALID))
-		{
-			NetworkDevice	*dev;
-
-			if ((dev = nmwa_get_device_for_nm_device (applet, nm_device)))
-			{
-				g_mutex_lock (applet->data_mutex);
-				nmwa_dbus_update_device_wireless_networks (dev, applet);
-				g_mutex_unlock (applet->data_mutex);
-			}
-		}
-
-		if (dbus_error_is_set (&error))
-			dbus_error_free (&error);
-
-		dbus_free (nm_device);
-		dbus_free (network);
 	}
 	else if (    dbus_message_is_signal (message, NM_DBUS_INTERFACE, "DeviceNowActive")
 			|| dbus_message_is_signal (message, NM_DBUS_INTERFACE, "DeviceNoLongerActive")
 			|| dbus_message_is_signal (message, NM_DBUS_INTERFACE, "DeviceActivating"))
 	{
 		nmwa_dbus_update_devices (applet);
-		nmwa_dbus_update_active_device (applet);
-		nmwa_dbus_update_network_state (applet);
 	}
 	else if (dbus_message_is_signal (message, NM_DBUS_INTERFACE, "DevicesChanged"))
 	{
 		nmwa_dbus_update_devices (applet);
-		nmwa_dbus_update_active_device (applet);
 	}
 	else
 		handled = FALSE;
@@ -1374,15 +1266,8 @@ static gboolean nmwa_dbus_timeout_worker (gpointer user_data)
 		{
 			applet->applet_state = APPLET_STATE_NO_CONNECTION;
 			nmwa_dbus_update_devices (applet);
-			nmwa_dbus_update_active_device (applet);
-			nmwa_dbus_update_network_state (applet);
 		}
 	}
-	else
-	{
-		nmwa_dbus_update_active_device_strength (applet);
-		/* FIXME: update wireless networks strength in real-time */
-	}
 
 	return (TRUE);
 }
@@ -1401,6 +1286,8 @@ gpointer nmwa_dbus_worker (gpointer user_data)
 	GMainLoop			*thread_loop;
 	guint			 timeout_id;
 	GSource			*timeout_source;
+	guint			 strength_id;
+	GSource			*strength_source;
 
 	g_return_val_if_fail (applet != NULL, NULL);
 
@@ -1415,12 +1302,14 @@ gpointer nmwa_dbus_worker (gpointer user_data)
 	g_source_set_callback (timeout_source, nmwa_dbus_timeout_worker, applet, NULL);
 	timeout_id = g_source_attach (timeout_source, applet->thread_context);
 
+	strength_source = g_timeout_source_new (1000);
+	g_source_set_callback (strength_source, nmwa_dbus_update_active_device_strength, applet, NULL);
+	strength_id = g_source_attach (strength_source, applet->thread_context);
+
 	if (applet->connection && nmwa_dbus_nm_is_running (applet->connection))
 	{
 		applet->applet_state = APPLET_STATE_NO_CONNECTION;
 		nmwa_dbus_update_devices (applet);
-		nmwa_dbus_update_active_device (applet);
-		nmwa_dbus_update_network_state (applet);
 	}
 	else
 		applet->applet_state = APPLET_STATE_NO_NM;
@@ -1428,6 +1317,7 @@ gpointer nmwa_dbus_worker (gpointer user_data)
 	g_main_loop_run (thread_loop);
 
 	g_source_destroy (timeout_source);
+	g_source_destroy (strength_source);
 
 	return NULL;
 }