[gnome-color-manager] Allow adding saved devices when coldplugging and add unit tests to verify it all works as expected

[Richard Hughes <rhughes src gnome org>]

commit 37a15bf7d9d869b4aaf345984162d71f6e6daaa9
Author: Richard Hughes <richard hughsie com>
Date:   Tue May 25 18:03:25 2010 +0100

    Allow adding saved devices when coldplugging and add unit tests to verify it all works as expected

 src/gcm-apply.c     |    2 +-
 src/gcm-client.c    |  338 ++++++++++++++++++++++-----------------------------
 src/gcm-client.h    |   10 +-
 src/gcm-prefs.c     |   27 ++--
 src/gcm-self-test.c |   99 +++++++++++++++
 src/gcm-session.c   |   10 +--
 6 files changed, 266 insertions(+), 220 deletions(-)
---
diff --git a/src/gcm-apply.c b/src/gcm-apply.c
index dbf757d..f4fcf60 100644
--- a/src/gcm-apply.c
+++ b/src/gcm-apply.c
@@ -63,7 +63,7 @@ main (int argc, char **argv)
 
 	/* get devices */
 	client = gcm_client_new ();
-	ret = gcm_client_add_connected (client, GCM_CLIENT_COLDPLUG_XRANDR, &error);
+	ret = gcm_client_coldplug (client, GCM_CLIENT_COLDPLUG_XRANDR, &error);
 	if (!ret) {
 		egg_warning ("failed to get devices: %s", error->message);
 		g_error_free (error);
diff --git a/src/gcm-client.c b/src/gcm-client.c
index 1327bae..f5d2329 100644
--- a/src/gcm-client.c
+++ b/src/gcm-client.c
@@ -59,8 +59,8 @@ static void     gcm_client_finalize	(GObject     *object);
 static void gcm_client_xrandr_add (GcmClient *client, GnomeRROutput *output);
 
 #ifdef GCM_USE_SANE
-static gboolean gcm_client_add_connected_devices_sane (GcmClient *client, GError **error);
-static gpointer gcm_client_add_connected_devices_sane_thrd (GcmClient *client);
+static gboolean gcm_client_coldplug_devices_sane (GcmClient *client, GError **error);
+static gpointer gcm_client_coldplug_devices_sane_thrd (GcmClient *client);
 #endif
 
 /**
@@ -255,12 +255,51 @@ gcm_client_get_id_for_udev_device (GUdevDevice *udev_device)
 }
 
 /**
+ * gcm_client_remove_device_internal:
+ **/
+static gboolean
+gcm_client_remove_device_internal (GcmClient *client, GcmDevice *device, gboolean emit_signal, GError **error)
+{
+	gboolean ret = FALSE;
+	const gchar *device_id;
+
+	g_return_val_if_fail (GCM_IS_CLIENT (client), FALSE);
+	g_return_val_if_fail (GCM_IS_DEVICE (device), FALSE);
+
+	/* check device is not connected */
+	device_id = gcm_device_get_id (device);
+	if (gcm_device_get_connected (device)) {
+		g_set_error (error, 1, 0, "device %s is still connected", device_id);
+		goto out;
+	}
+
+	/* try to remove from array */
+	ret = g_ptr_array_remove (client->priv->array, device);
+	if (!ret) {
+		g_set_error_literal (error, 1, 0, "not found in device array");
+		goto out;
+	}
+
+	/* ensure signal handlers are disconnected */
+	g_signal_handlers_disconnect_by_func (device, G_CALLBACK (gcm_client_device_changed_cb), client);
+
+	/* emit a signal */
+	if (emit_signal) {
+		egg_debug ("emit removed: %s", device_id);
+		g_signal_emit (client, signals[SIGNAL_REMOVED], 0, device);
+	}
+out:
+	return ret;
+}
+
+/**
  * gcm_client_gudev_remove:
  **/
 static gboolean
 gcm_client_gudev_remove (GcmClient *client, GUdevDevice *udev_device)
 {
 	GcmDevice *device;
+	GError *error = NULL;
 	gchar *id;
 	gboolean ret = FALSE;
 
@@ -277,6 +316,16 @@ gcm_client_gudev_remove (GcmClient *client, GUdevDevice *udev_device)
 	/* set disconnected */
 	gcm_device_set_connected (device, FALSE);
 
+	/* remove device as it never had a profile assigned */
+	if (!gcm_device_get_saved (device)) {
+		ret = gcm_client_remove_device_internal (client, device, TRUE, &error);
+		if (!ret) {
+			egg_warning ("failed to remove: %s", error->message);
+			g_error_free (error);
+			goto out;
+		}
+	}
+
 	/* success */
 	ret = TRUE;
 out:
@@ -293,11 +342,9 @@ static gboolean
 gcm_client_gudev_add (GcmClient *client, GUdevDevice *udev_device)
 {
 	GcmDevice *device = NULL;
-	GcmDevice *device_tmp = NULL;
 	gboolean ret = FALSE;
 	const gchar *value;
 	GError *error = NULL;
-	GcmClientPrivate *priv = client->priv;
 
 	/* matches our udev rules, which we can change without recompiling */
 	value = g_udev_device_get_property (udev_device, "GCM_DEVICE");
@@ -313,37 +360,16 @@ gcm_client_gudev_add (GcmClient *client, GUdevDevice *udev_device)
 		goto out;
 	}
 
-	/* we might have a previous saved device with this ID, in which case nuke it */
-	device_tmp = gcm_client_get_device_by_id (client, gcm_device_get_id (device));
-	if (device_tmp != NULL) {
-		/* already disconnected device now connected */
-		g_object_unref (device);
-		device = g_object_ref (device_tmp);
-		gcm_device_set_connected (device, TRUE);
-	}
-
-	/* load the device */
-	ret = gcm_device_load (device, &error);
+	/* add device */
+	ret = gcm_client_add_device (client, device, &error);
 	if (!ret) {
-		egg_warning ("failed to load: %s", error->message);
+		egg_debug ("failed to set for device: %s", error->message);
 		g_error_free (error);
 		goto out;
 	}
-
-	/* add to list */
-	g_ptr_array_add (priv->array, g_object_ref (device));
-
-	/* signal the addition */
-	egg_debug ("emit: added %s to device list", gcm_device_get_id (device));
-	g_signal_emit (client, signals[SIGNAL_ADDED], 0, device);
-
-	/* connect to the changed signal */
-	g_signal_connect (device, "changed", G_CALLBACK (gcm_client_device_changed_cb), client);
 out:
 	if (device != NULL)
 		g_object_unref (device);
-	if (device_tmp != NULL)
-		g_object_unref (device_tmp);
 	return ret;
 }
 
@@ -365,14 +391,14 @@ gcm_client_sane_refresh_cb (GcmClient *client)
 	/* rescan */
 	egg_debug ("rescanning sane");
 	if (client->priv->use_threads) {
-		thread = g_thread_create ((GThreadFunc) gcm_client_add_connected_devices_sane_thrd, client, FALSE, &error);
+		thread = g_thread_create ((GThreadFunc) gcm_client_coldplug_devices_sane_thrd, client, FALSE, &error);
 		if (thread == NULL) {
 			egg_debug ("failed to rescan sane devices: %s", error->message);
 			g_error_free (error);
 			goto out;
 		}
 	} else {
-		ret = gcm_client_add_connected_devices_sane (client, &error);
+		ret = gcm_client_coldplug_devices_sane (client, &error);
 		if (!ret) {
 			egg_debug ("failed to rescan sane devices: %s", error->message);
 			g_error_free (error);
@@ -435,10 +461,10 @@ gcm_client_uevent_cb (GUdevClient *gudev_client, const gchar *action, GUdevDevic
 }
 
 /**
- * gcm_client_add_connected_devices_udev:
+ * gcm_client_coldplug_devices_udev:
  **/
 static gboolean
-gcm_client_add_connected_devices_udev (GcmClient *client, GError **error)
+gcm_client_coldplug_devices_udev (GcmClient *client, GError **error)
 {
 	GList *devices;
 	GList *l;
@@ -469,12 +495,12 @@ gcm_client_add_connected_devices_udev (GcmClient *client, GError **error)
 }
 
 /**
- * gcm_client_add_connected_devices_udev_thrd:
+ * gcm_client_coldplug_devices_udev_thrd:
  **/
 static gpointer
-gcm_client_add_connected_devices_udev_thrd (GcmClient *client)
+gcm_client_coldplug_devices_udev_thrd (GcmClient *client)
 {
-	gcm_client_add_connected_devices_udev (client, NULL);
+	gcm_client_coldplug_devices_udev (client, NULL);
 	return NULL;
 }
 
@@ -604,11 +630,8 @@ static void
 gcm_client_xrandr_add (GcmClient *client, GnomeRROutput *output)
 {
 	gboolean ret;
-	gboolean connected;
 	GError *error = NULL;
 	GcmDevice *device = NULL;
-	GcmDevice *device_tmp = NULL;
-	GcmClientPrivate *priv = client->priv;
 
 	/* if nothing connected then ignore */
 	ret = gnome_rr_output_is_connected (output);
@@ -626,51 +649,23 @@ gcm_client_xrandr_add (GcmClient *client, GnomeRROutput *output)
 		goto out;
 	}
 
-	/* we might have a previous saved device with this ID, in which use that instead */
-	device_tmp = gcm_client_get_device_by_id (client, gcm_device_get_id (device));
-	if (device_tmp != NULL) {
-
-		/* this is just a dupe */
-		connected = gcm_device_get_connected (device_tmp);
-		if (connected) {
-			egg_debug ("ignoring dupe");
-			goto out;
-		}
-
-		/* use original device */
-		g_object_unref (device);
-		device = g_object_ref (device_tmp);
-	}
-
-	/* load the device */
-	ret = gcm_device_load (device, &error);
+	/* add device */
+	ret = gcm_client_add_device (client, device, &error);
 	if (!ret) {
-		egg_warning ("failed to load: %s", error->message);
+		egg_debug ("failed to set for device: %s", error->message);
 		g_error_free (error);
 		goto out;
 	}
-
-	/* add to the array */
-	g_ptr_array_add (priv->array, g_object_ref (device));
-
-	/* signal the addition */
-	egg_debug ("emit: added %s to device list", gcm_device_get_id (device));
-	g_signal_emit (client, signals[SIGNAL_ADDED], 0, device);
-
-	/* connect to the changed signal */
-	g_signal_connect (device, "changed", G_CALLBACK (gcm_client_device_changed_cb), client);
 out:
 	if (device != NULL)
 		g_object_unref (device);
-	if (device_tmp != NULL)
-		g_object_unref (device_tmp);
 }
 
 /**
- * gcm_client_add_connected_devices_xrandr:
+ * gcm_client_coldplug_devices_xrandr:
  **/
 static gboolean
-gcm_client_add_connected_devices_xrandr (GcmClient *client, GError **error)
+gcm_client_coldplug_devices_xrandr (GcmClient *client, GError **error)
 {
 	GnomeRROutput **outputs;
 	guint i;
@@ -708,33 +703,23 @@ gcm_client_cups_add (GcmClient *client, cups_dest_t dest)
 		goto out;
 	}
 
-	/* load the device */
-	ret = gcm_device_load (device, &error);
+	/* add device */
+	ret = gcm_client_add_device (client, device, &error);
 	if (!ret) {
-		egg_warning ("failed to load: %s", error->message);
+		egg_debug ("failed to set for device: %s", error->message);
 		g_error_free (error);
 		goto out;
 	}
-
-	/* add to the array */
-	g_ptr_array_add (priv->array, g_object_ref (device));
-
-	/* signal the addition */
-	egg_debug ("emit: added %s to device list", gcm_device_get_id (device));
-	g_signal_emit (client, signals[SIGNAL_ADDED], 0, device);
-
-	/* connect to the changed signal */
-	g_signal_connect (device, "changed", G_CALLBACK (gcm_client_device_changed_cb), client);
 out:
 	if (device != NULL)
 		g_object_unref (device);
 }
 
 /**
- * gcm_client_add_connected_devices_cups:
+ * gcm_client_coldplug_devices_cups:
  **/
 static gboolean
-gcm_client_add_connected_devices_cups (GcmClient *client, GError **error)
+gcm_client_coldplug_devices_cups (GcmClient *client, GError **error)
 {
 	gint num_dests;
 	cups_dest_t *dests;
@@ -763,12 +748,12 @@ gcm_client_add_connected_devices_cups (GcmClient *client, GError **error)
 }
 
 /**
- * gcm_client_add_connected_devices_cups_thrd:
+ * gcm_client_coldplug_devices_cups_thrd:
  **/
 static gpointer
-gcm_client_add_connected_devices_cups_thrd (GcmClient *client)
+gcm_client_coldplug_devices_cups_thrd (GcmClient *client)
 {
-	gcm_client_add_connected_devices_cups (client, NULL);
+	gcm_client_coldplug_devices_cups (client, NULL);
 	return NULL;
 }
 
@@ -782,8 +767,6 @@ gcm_client_sane_add (GcmClient *client, const SANE_Device *sane_device)
 	gboolean ret;
 	GError *error = NULL;
 	GcmDevice *device = NULL;
-	GcmDevice *device_tmp = NULL;
-	GcmClientPrivate *priv = client->priv;
 
 	/* create new device */
 	device = gcm_device_sane_new ();
@@ -794,43 +777,23 @@ gcm_client_sane_add (GcmClient *client, const SANE_Device *sane_device)
 		goto out;
 	}
 
-	/* we might have a previous saved device with this ID, in which use that instead */
-	device_tmp = gcm_client_get_device_by_id (client, gcm_device_get_id (device));
-	if (device_tmp != NULL) {
-		gcm_device_set_connected (device_tmp, TRUE);
-		g_object_unref (device);
-		device = g_object_ref (device_tmp);
-	}
-
-	/* load the device */
-	ret = gcm_device_load (device, &error);
+	/* add device */
+	ret = gcm_client_add_device (client, device, &error);
 	if (!ret) {
-		egg_warning ("failed to load: %s", error->message);
+		egg_debug ("failed to set for device: %s", error->message);
 		g_error_free (error);
 		goto out;
 	}
-
-	/* add to the array */
-	g_ptr_array_add (priv->array, g_object_ref (device));
-
-	/* signal the addition */
-	egg_debug ("emit: added %s to device list", gcm_device_get_id (device));
-	g_signal_emit (client, signals[SIGNAL_ADDED], 0, device);
-
-	/* connect to the changed signal */
-	g_signal_connect (device, "changed", G_CALLBACK (gcm_client_device_changed_cb), client);
 out:
 	if (device != NULL)
 		g_object_unref (device);
-	if (device_tmp != NULL)
-		g_object_unref (device_tmp);
 }
 
 /**
- * gcm_client_add_connected_devices_sane:
+ * gcm_client_coldplug_devices_sane:
  **/
 static gboolean
-gcm_client_add_connected_devices_sane (GcmClient *client, GError **error)
+gcm_client_coldplug_devices_sane (GcmClient *client, GError **error)
 {
 	gint i;
 	gboolean ret = TRUE;
@@ -874,12 +837,12 @@ out:
 }
 
 /**
- * gcm_client_add_connected_devices_sane_thrd:
+ * gcm_client_coldplug_devices_sane_thrd:
  **/
 static gpointer
-gcm_client_add_connected_devices_sane_thrd (GcmClient *client)
+gcm_client_coldplug_devices_sane_thrd (GcmClient *client)
 {
-	gcm_client_add_connected_devices_sane (client, NULL);
+	gcm_client_coldplug_devices_sane (client, NULL);
 	return NULL;
 }
 #endif
@@ -899,7 +862,6 @@ gcm_client_add_unconnected_device (GcmClient *client, GKeyFile *keyfile, const g
 	gboolean ret;
 	gboolean virtual;
 	GError *error = NULL;
-	GcmClientPrivate *priv = client->priv;
 
 	/* add new device */
 	title = g_key_file_get_string (keyfile, id, "title", NULL);
@@ -950,23 +912,13 @@ gcm_client_add_unconnected_device (GcmClient *client, GKeyFile *keyfile, const g
 		      "colorspace", colorspace,
 		      NULL);
 
-	/* load the device */
-	ret = gcm_device_load (device, &error);
+	/* add device */
+	ret = gcm_client_add_device (client, device, &error);
 	if (!ret) {
-		egg_warning ("failed to load: %s", error->message);
+		egg_debug ("failed to set for device: %s", error->message);
 		g_error_free (error);
 		goto out;
 	}
-
-	/* add to list */
-	g_ptr_array_add (priv->array, g_object_ref (device));
-
-	/* signal the addition */
-	egg_debug ("emit: added %s to device list", id);
-	g_signal_emit (client, signals[SIGNAL_ADDED], 0, device);
-
-	/* connect to the changed signal */
-	g_signal_connect (device, "changed", G_CALLBACK (gcm_client_device_changed_cb), client);
 out:
 	if (device != NULL)
 		g_object_unref (device);
@@ -1040,7 +992,7 @@ out:
 /**
  * gcm_client_add_saved:
  **/
-gboolean
+static gboolean
 gcm_client_add_saved (GcmClient *client, GError **error)
 {
 	gboolean ret;
@@ -1060,8 +1012,14 @@ gcm_client_add_saved (GcmClient *client, GError **error)
 	/* load the config file */
 	keyfile = g_key_file_new ();
 	ret = g_key_file_load_from_file (keyfile, filename, G_KEY_FILE_NONE, error);
-	if (!ret)
+	if (!ret) {
+		/* if the file is missing then ignore the error */
+		if ((*error)->domain == G_FILE_ERROR && (*error)->code == G_FILE_ERROR_NOENT) {
+			g_clear_error (error);
+			ret = TRUE;
+		}
 		goto out;
+	}
 
 	/* get groups */
 	groups = g_key_file_get_groups (keyfile, NULL);
@@ -1090,12 +1048,12 @@ out:
 }
 
 /**
- * gcm_client_add_connected:
+ * gcm_client_coldplug:
  **/
 gboolean
-gcm_client_add_connected (GcmClient *client, GcmClientColdplug coldplug, GError **error)
+gcm_client_coldplug (GcmClient *client, GcmClientColdplug coldplug, GError **error)
 {
-	gboolean ret;
+	gboolean ret = TRUE;
 	GThread *thread;
 
 	g_return_val_if_fail (GCM_IS_CLIENT (client), FALSE);
@@ -1106,11 +1064,18 @@ gcm_client_add_connected (GcmClient *client, GcmClientColdplug coldplug, GError
 	/* reset */
 	client->priv->loading_refcount = 0;
 
+	/* saved devices */
+	if (!coldplug || coldplug & GCM_CLIENT_COLDPLUG_SAVED) {
+		ret = gcm_client_add_saved (client, error);
+		if (!ret)
+			goto out;
+	}
+
 	/* XRandR */
 	if (!coldplug || coldplug & GCM_CLIENT_COLDPLUG_XRANDR) {
 		gcm_client_add_loading (client);
 		egg_debug ("adding devices of type XRandR");
-		ret = gcm_client_add_connected_devices_xrandr (client, error);
+		ret = gcm_client_coldplug_devices_xrandr (client, error);
 		if (!ret)
 			goto out;
 	}
@@ -1120,11 +1085,11 @@ gcm_client_add_connected (GcmClient *client, GcmClientColdplug coldplug, GError
 		gcm_client_add_loading (client);
 		egg_debug ("adding devices of type UDEV");
 		if (client->priv->use_threads) {
-			thread = g_thread_create ((GThreadFunc) gcm_client_add_connected_devices_udev_thrd, client, FALSE, error);
+			thread = g_thread_create ((GThreadFunc) gcm_client_coldplug_devices_udev_thrd, client, FALSE, error);
 			if (thread == NULL)
 				goto out;
 		} else {
-			ret = gcm_client_add_connected_devices_udev (client, error);
+			ret = gcm_client_coldplug_devices_udev (client, error);
 			if (!ret)
 				goto out;
 		}
@@ -1135,11 +1100,11 @@ gcm_client_add_connected (GcmClient *client, GcmClientColdplug coldplug, GError
 		gcm_client_add_loading (client);
 		egg_debug ("adding devices of type CUPS");
 		if (client->priv->use_threads) {
-			thread = g_thread_create ((GThreadFunc) gcm_client_add_connected_devices_cups_thrd, client, FALSE, error);
+			thread = g_thread_create ((GThreadFunc) gcm_client_coldplug_devices_cups_thrd, client, FALSE, error);
 			if (thread == NULL)
 				goto out;
 		} else {
-			ret = gcm_client_add_connected_devices_cups (client, error);
+			ret = gcm_client_coldplug_devices_cups (client, error);
 			if (!ret)
 				goto out;
 		}
@@ -1151,11 +1116,11 @@ gcm_client_add_connected (GcmClient *client, GcmClientColdplug coldplug, GError
 		gcm_client_add_loading (client);
 		egg_debug ("adding devices of type SANE");
 		if (client->priv->use_threads) {
-			thread = g_thread_create ((GThreadFunc) gcm_client_add_connected_devices_sane_thrd, client, FALSE, error);
+			thread = g_thread_create ((GThreadFunc) gcm_client_coldplug_devices_sane_thrd, client, FALSE, error);
 			if (thread == NULL)
 				goto out;
 		} else {
-			ret = gcm_client_add_connected_devices_sane (client, error);
+			ret = gcm_client_coldplug_devices_sane (client, error);
 			if (!ret)
 				goto out;
 		}
@@ -1166,43 +1131,40 @@ out:
 }
 
 /**
- * gcm_client_add_virtual_device:
+ * gcm_client_add_device:
  **/
 gboolean
-gcm_client_add_virtual_device (GcmClient *client, GcmDevice *device, GError **error)
+gcm_client_add_device (GcmClient *client, GcmDevice *device, GError **error)
 {
 	gboolean ret = FALSE;
-	const gchar *id;
-	gboolean virtual;
+	const gchar *device_id;
 	GcmDevice *device_tmp = NULL;
 
 	g_return_val_if_fail (GCM_IS_CLIENT (client), FALSE);
 	g_return_val_if_fail (GCM_IS_DEVICE (device), FALSE);
 
-	/* check removable */
-	virtual = gcm_device_get_virtual (device);
-	if (!virtual) {
-		g_set_error_literal (error, 1, 0, "cannot add non-virtual devices");
-		goto out;
-	}
-
 	/* look to see if device already exists */
-	id = gcm_device_get_id (device);
-	device_tmp = gcm_client_get_device_by_id (client, id);
+	device_id = gcm_device_get_id (device);
+	device_tmp = gcm_client_get_device_by_id (client, device_id);
 	if (device_tmp != NULL) {
-		/* TRANSLATORS: error message */
-		g_set_error_literal (error, 1, 0, _("This device already exists"));
-		goto out;
+		egg_debug ("already exists, copy settings and remove old device: %s", device_id);
+		gcm_device_set_profile_filename (device, gcm_device_get_profile_filename (device_tmp));
+		gcm_device_set_saved (device, gcm_device_get_saved (device_tmp));
+		ret = gcm_client_remove_device_internal (client, device_tmp, FALSE, error);
+		if (!ret)
+			goto out;
 	}
 
+	/* load the device */
+	ret = gcm_device_load (device, error);
+	if (!ret)
+		goto out;
+
 	/* add to the array */
 	g_ptr_array_add (client->priv->array, g_object_ref (device));
 
-	/* update status */
-	gcm_device_set_saved (device, FALSE);
-
 	/* emit a signal */
-	egg_debug ("emit added: %s", id);
+	egg_debug ("emit added: %s", device_id);
 	g_signal_emit (client, signals[SIGNAL_ADDED], 0, device);
 
 	/* connect to the changed signal */
@@ -1217,45 +1179,37 @@ out:
 }
 
 /**
+ * gcm_client_remove_device:
+ **/
+gboolean
+gcm_client_remove_device (GcmClient *client, GcmDevice *device, GError **error)
+{
+	return gcm_client_remove_device_internal (client, device, TRUE, error);
+}
+
+/**
  * gcm_client_delete_device:
  **/
 gboolean
 gcm_client_delete_device (GcmClient *client, GcmDevice *device, GError **error)
 {
 	gboolean ret = FALSE;
-	const gchar *id;
+	const gchar *device_id;
 	gchar *data = NULL;
 	gchar *filename = NULL;
 	GKeyFile *keyfile = NULL;
-	gboolean saved;
-	gboolean connected;
 
 	g_return_val_if_fail (GCM_IS_CLIENT (client), FALSE);
 	g_return_val_if_fail (GCM_IS_DEVICE (device), FALSE);
 
-	/* check device is not connected */
-	connected = gcm_device_get_connected (device);
-	if (connected) {
-		g_set_error_literal (error, 1, 0, "device is still connected");
-		goto out;
-	}
-
-	/* try to remove from array */
-	ret = g_ptr_array_remove (client->priv->array, device);
-	if (!ret) {
-		g_set_error_literal (error, 1, 0, "not found in device array");
-		goto out;
-	}
-
 	/* check device is saved */
-	id = gcm_device_get_id (device);
-	saved = gcm_device_get_saved (device);
-	if (!saved)
-		goto not_saved;
+	device_id = gcm_device_get_id (device);
+	if (!gcm_device_get_saved (device))
+		goto out;
 
 	/* get the config file */
 	filename = gcm_utils_get_default_config_location ();
-	egg_debug ("removing %s from %s", id, filename);
+	egg_debug ("removing %s from %s", device_id, filename);
 
 	/* load the config file */
 	keyfile = g_key_file_new ();
@@ -1264,7 +1218,7 @@ gcm_client_delete_device (GcmClient *client, GcmDevice *device, GError **error)
 		goto out;
 
 	/* remove from the config file */
-	ret = g_key_file_remove_group (keyfile, id, error);
+	ret = g_key_file_remove_group (keyfile, device_id, error);
 	if (!ret)
 		goto out;
 
@@ -1283,10 +1237,10 @@ gcm_client_delete_device (GcmClient *client, GcmDevice *device, GError **error)
 	/* update status */
 	gcm_device_set_saved (device, FALSE);
 
-not_saved:
-	/* emit a signal */
-	egg_debug ("emit removed: %s", id);
-	g_signal_emit (client, signals[SIGNAL_REMOVED], 0, device);
+	/* remove device */
+	ret = gcm_client_remove_device_internal (client, device, TRUE, error);
+	if (!ret)
+		goto out;
 out:
 	g_free (data);
 	g_free (filename);
diff --git a/src/gcm-client.h b/src/gcm-client.h
index 083cb5d..079e57a 100644
--- a/src/gcm-client.h
+++ b/src/gcm-client.h
@@ -66,6 +66,7 @@ typedef enum {
 	GCM_CLIENT_COLDPLUG_CUPS	= 0x02,
 	GCM_CLIENT_COLDPLUG_SANE	= 0x04,
 	GCM_CLIENT_COLDPLUG_UDEV	= 0x08,
+	GCM_CLIENT_COLDPLUG_SAVED	= 0x10,
 	GCM_CLIENT_COLDPLUG_LAST,
 } GcmClientColdplug;
 
@@ -76,17 +77,18 @@ GcmDevice	*gcm_client_get_device_by_id			(GcmClient		*client,
 								 const gchar		*id);
 GcmDevice	*gcm_client_get_device_by_window		(GcmClient		*client,
 								 GdkWindow		*window);
-gboolean	 gcm_client_add_virtual_device			(GcmClient		*client,
+gboolean	 gcm_client_add_device				(GcmClient		*client,
+								 GcmDevice		*device,
+								 GError			**error);
+gboolean	 gcm_client_remove_device			(GcmClient		*client,
 								 GcmDevice		*device,
 								 GError			**error);
 gboolean	 gcm_client_delete_device			(GcmClient		*client,
 								 GcmDevice		*device,
 								 GError			**error);
-gboolean	 gcm_client_add_connected			(GcmClient		*client,
+gboolean	 gcm_client_coldplug				(GcmClient		*client,
 								 GcmClientColdplug	 coldplug,
 								 GError			**error);
-gboolean	 gcm_client_add_saved				(GcmClient		*client,
-								 GError			**error);
 GPtrArray	*gcm_client_get_devices				(GcmClient		*client);
 void		 gcm_client_set_use_threads			(GcmClient		*client,
 								 gboolean		 use_threads);
diff --git a/src/gcm-prefs.c b/src/gcm-prefs.c
index 1c8ff9b..8c9b9a0 100644
--- a/src/gcm-prefs.c
+++ b/src/gcm-prefs.c
@@ -638,7 +638,7 @@ gcm_prefs_profile_add_virtual_file (GFile *file)
 	}
 
 	/* add to the device list */
-	ret = gcm_client_add_virtual_device (gcm_client, device, &error);
+	ret = gcm_client_add_device (gcm_client, device, &error);
 	if (!ret) {
 		/* TRANSLATORS: could not add virtual device */
 		gcm_prefs_error_dialog (_("Failed to add virtual device"), error->message);
@@ -1049,7 +1049,7 @@ gcm_prefs_button_virtual_add_cb (GtkWidget *widget, gpointer data)
 	}
 
 	/* add to the device list */
-	ret = gcm_client_add_virtual_device (gcm_client, device, &error);
+	ret = gcm_client_add_device (gcm_client, device, &error);
 	if (!ret) {
 		/* TRANSLATORS: could not add virtual device */
 		gcm_prefs_error_dialog (_("Failed to add virtual device"), error->message);
@@ -2280,9 +2280,10 @@ static gboolean
 gcm_prefs_added_idle_cb (GcmDevice *device)
 {
 	GcmDeviceKind kind;
-	egg_debug ("added: %s (connected: %i)",
+	egg_debug ("added: %s (connected: %i, saved: %i)",
 		   gcm_device_get_id (device),
-		   gcm_device_get_connected (device));
+		   gcm_device_get_connected (device),
+		   gcm_device_get_saved (device));
 
 	/* remove the saved device if it's already there */
 	gcm_prefs_remove_device (device);
@@ -2314,6 +2315,12 @@ gcm_prefs_added_cb (GcmClient *gcm_client_, GcmDevice *gcm_device, gpointer user
 static void
 gcm_prefs_changed_cb (GcmClient *gcm_client_, GcmDevice *gcm_device, gpointer user_data)
 {
+	/* no not re-add to the ui if we just deleted this */
+	if (!gcm_device_get_connected (gcm_device) &&
+	    !gcm_device_get_saved (gcm_device)) {
+		egg_warning ("ignoring uninteresting device: %s", gcm_device_get_id (gcm_device));
+		return;
+	}
 	g_idle_add ((GSourceFunc) gcm_prefs_added_idle_cb, g_object_ref (gcm_device));
 }
 
@@ -2335,7 +2342,7 @@ gcm_prefs_removed_cb (GcmClient *gcm_client_, GcmDevice *gcm_device, gpointer us
 	/* ensure this device is re-added if it's been saved */
 	connected = gcm_device_get_connected (gcm_device);
 	if (connected)
-		gcm_client_add_saved (gcm_client, NULL);
+		gcm_client_coldplug (gcm_client, GCM_CLIENT_COLDPLUG_SAVED, NULL);
 
 	/* select the first device */
 	ret = gtk_tree_model_get_iter_first (GTK_TREE_MODEL (list_store_devices), &iter);
@@ -2564,21 +2571,13 @@ gcm_prefs_startup_phase1_idle_cb (gpointer user_data)
 			  G_CALLBACK (gcm_prefs_renderer_combo_changed_cb), (gpointer) "softproof");
 
 	/* coldplug plugged in devices */
-	ret = gcm_client_add_connected (gcm_client, GCM_CLIENT_COLDPLUG_ALL, &error);
+	ret = gcm_client_coldplug (gcm_client, GCM_CLIENT_COLDPLUG_ALL, &error);
 	if (!ret) {
 		egg_warning ("failed to add connected devices: %s", error->message);
 		g_error_free (error);
 		goto out;
 	}
 
-	/* coldplug saved devices */
-	ret = gcm_client_add_saved (gcm_client, &error);
-	if (!ret) {
-		egg_warning ("failed to add saved devices: %s", error->message);
-		g_clear_error (&error);
-		/* do not fail */
-	}
-
 	/* set calibrate button sensitivity */
 	gcm_prefs_set_calibrate_button_sensitivity ();
 
diff --git a/src/gcm-self-test.c b/src/gcm-self-test.c
index 587be3e..0fb874b 100644
--- a/src/gcm-self-test.c
+++ b/src/gcm-self-test.c
@@ -28,9 +28,11 @@
 #include "gcm-calibrate-dialog.h"
 #include "gcm-calibrate-manual.h"
 #include "gcm-cie-widget.h"
+#include "gcm-client.h"
 #include "gcm-clut.h"
 #include "gcm-device.h"
 #include "gcm-device-udev.h"
+#include "gcm-device-xrandr.h"
 #include "gcm-dmi.h"
 #include "gcm-edid.h"
 #include "gcm-exif.h"
@@ -1047,6 +1049,102 @@ gcm_test_xyz_func (void)
 	g_object_unref (xyz);
 }
 
+static void
+gcm_test_client_func (void)
+{
+	GcmClient *client;
+	GError *error = NULL;
+	gboolean ret;
+	GPtrArray *array;
+	GcmDevice *device;
+	gchar *filename;
+	gchar *data = NULL;
+
+	client = gcm_client_new ();
+	g_assert (client != NULL);
+
+	/* ensure file is gone */
+	g_setenv ("GCM_TEST", "1", TRUE);
+	filename = gcm_utils_get_default_config_location ();
+	g_unlink (filename);
+
+	/* ensure we don't fail with no config file */
+	ret = gcm_client_coldplug (client, GCM_CLIENT_COLDPLUG_SAVED, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	array = gcm_client_get_devices (client);
+	g_assert_cmpint (array->len, ==, 0);
+	g_ptr_array_unref (array);
+
+	/* ensure we get one device */
+	ret = g_file_set_contents (filename,
+				   "[xrandr_goldstar]\n"
+				   "profile=dave.icc\n"
+				   "title=Goldstar\n"
+				   "type=display\n"
+				   "colorspace=rgb\n", -1, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	ret = gcm_client_coldplug (client, GCM_CLIENT_COLDPLUG_SAVED, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	array = gcm_client_get_devices (client);
+	g_assert_cmpint (array->len, ==, 1);
+	device = g_ptr_array_index (array, 0);
+	g_assert_cmpstr (gcm_device_get_id (device), ==, "xrandr_goldstar");
+	g_assert_cmpstr (gcm_device_get_title (device), ==, "Goldstar");
+	g_assert_cmpstr (gcm_device_get_profile_filename (device), ==, "dave.icc");
+	g_assert (gcm_device_get_saved (device));
+	g_assert (!gcm_device_get_connected (device));
+	g_assert (GCM_IS_DEVICE_XRANDR (device));
+	g_ptr_array_unref (array);
+
+	device = gcm_device_udev_new ();
+	gcm_device_set_id (device, "xrandr_goldstar");
+	gcm_device_set_title (device, "Slightly different");
+	gcm_device_set_connected (device, TRUE);
+	ret = gcm_client_add_device (client, device, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	/* ensure we merge saved properties into current devices */
+	array = gcm_client_get_devices (client);
+	g_assert_cmpint (array->len, ==, 1);
+	device = g_ptr_array_index (array, 0);
+	g_assert_cmpstr (gcm_device_get_id (device), ==, "xrandr_goldstar");
+	g_assert_cmpstr (gcm_device_get_title (device), ==, "Slightly different");
+	g_assert_cmpstr (gcm_device_get_profile_filename (device), ==, "dave.icc");
+	g_assert (gcm_device_get_saved (device));
+	g_assert (gcm_device_get_connected (device));
+	g_assert (GCM_IS_DEVICE_UDEV (device));
+	g_ptr_array_unref (array);
+
+	/* delete */
+	gcm_device_set_connected (device, FALSE);
+	ret = gcm_client_delete_device (client, device, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	array = gcm_client_get_devices (client);
+	g_assert_cmpint (array->len, ==, 0);
+	g_ptr_array_unref (array);
+
+	ret = g_file_get_contents (filename, &data, NULL, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	g_assert_cmpstr (data, ==, "");
+
+	g_object_unref (client);
+	g_object_unref (device);
+	g_unlink (filename);
+	g_free (filename);
+	g_free (data);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -1055,6 +1153,7 @@ main (int argc, char **argv)
 	gtk_init (&argc, &argv);
 	g_test_init (&argc, &argv, NULL);
 
+	g_test_add_func ("/color/client", gcm_test_client_func);
 	g_test_add_func ("/color/dmi", gcm_test_dmi_func);
 	g_test_add_func ("/color/calibrate", gcm_test_calibrate_func);
 	g_test_add_func ("/color/edid", gcm_test_edid_func);
diff --git a/src/gcm-session.c b/src/gcm-session.c
index 2fb3056..7ec94ac 100644
--- a/src/gcm-session.c
+++ b/src/gcm-session.c
@@ -776,16 +776,8 @@ main (int argc, char *argv[])
 	profile_store = gcm_profile_store_new ();
 	timer = g_timer_new ();
 
-	/* get all saved devices */
-	ret = gcm_client_add_saved (client, &error);
-	if (!ret) {
-		egg_warning ("failed to add saved devices: %s", error->message);
-		g_clear_error (&error);
-		/* non-fatal */
-	}
-
 	/* get all connected devices */
-	ret = gcm_client_add_connected (client, GCM_CLIENT_COLDPLUG_ALL, &error);
+	ret = gcm_client_coldplug (client, GCM_CLIENT_COLDPLUG_ALL, &error);
 	if (!ret) {
 		egg_warning ("failed to coldplug: %s", error->message);
 		g_error_free (error);