Optionally delay the device removal

In the case where we can trigger the replug automatically we can have to wait
for a USB re-enumeration (typically a few hundred ms) but when the user is
requred to unplug, and then replug we have to wait a bit longer.

The 'remove delay' allows us to modify per-device the removal delay. In the
case the device does not show back up in the correct time the device will be
auto-removed and the session will get a DeviceRemoved signal. In the case where
the device in bootloader mode shows up within the timeout the session just gets
a DeviceChanged event.

For the duration of the delayed removal the flags for the device are set to
zero to ensure the session does not try to interact with the device whilst
re-enumerating.

src/fu-device-list.c

@@ -36,7 +36,8 @@
  * device-id or a GUID.
  *
  * The device list will emit ::added and ::removed signals when the device list
- * has been changed.
+ * has been changed. If the #FuDevice has changed during a device replug then
+ * the ::changed signal will be emitted instead of ::added and then ::removed.
  *
  * See also: #FuDevice
  */
@@ -58,9 +59,10 @@ enum {
 
 static guint signals[SIGNAL_LAST] = { 0 };
 
-/* although this seems a waste of time, there are great plans for this... */
 typedef struct {
 	FuDevice		*device;
+	FuDeviceList		*self;		/* no ref */
+	guint			 remove_id;
 } FuDeviceItem;
 
 G_DEFINE_TYPE (FuDeviceList, fu_device_list, G_TYPE_OBJECT)
@@ -79,6 +81,13 @@ fu_device_list_emit_device_removed (FuDeviceList *self, FuDevice *device)
 	g_signal_emit (self, signals[SIGNAL_REMOVED], 0, device);
 }
 
+static void
+fu_device_list_emit_device_changed (FuDeviceList *self, FuDevice *device)
+{
+	g_debug ("::changed %s", fu_device_get_id (device));
+	g_signal_emit (self, signals[SIGNAL_CHANGED], 0, device);
+}
+
 /**
  * fu_device_list_get_all:
  * @self: A #FuDeviceList
@@ -102,6 +111,33 @@ fu_device_list_get_all (FuDeviceList *self)
 	return devices;
 }
 
+static FuDeviceItem *
+fu_device_list_find_by_device (FuDeviceList *self, FuDevice *device)
+{
+	for (guint i = 0; i < self->devices->len; i++) {
+		FuDeviceItem *item = g_ptr_array_index (self->devices, i);
+		if (item->device == device)
+			return item;
+	}
+	return NULL;
+}
+
+static gboolean
+fu_device_list_device_delayed_remove_cb (gpointer user_data)
+{
+	FuDeviceItem *item = (FuDeviceItem *) user_data;
+	FuDeviceList *self = FU_DEVICE_LIST (item->self);
+
+	/* no longer valid */
+	item->remove_id = 0;
+
+	/* just remove now */
+	g_debug ("doing delayed removal");
+	fu_device_list_emit_device_removed (self, item->device);
+	g_ptr_array_remove (self->devices, item);
+	return G_SOURCE_REMOVE;
+}
+
 /**
  * fu_device_list_remove:
  * @self: A #FuDeviceList
@@ -109,24 +145,55 @@ fu_device_list_get_all (FuDeviceList *self)
  *
  * Removes a specific device from the list if it exists.
  *
- * The ::removed signal will also be emitted if @device is found in the list.
+ * If the @device has a remove-delay set then a timeout will be started. If
+ * the exact same #FuDevice is added to the list with fu_device_list_add()
+ * within the timeout then only a ::changed signal will be emitted.
+ *
+ * If there is no remove-delay set, the ::removed signal will be emitted
+ * straight away.
  *
  * Since: 1.0.2
  **/
 void
 fu_device_list_remove (FuDeviceList *self, FuDevice *device)
 {
+	FuDeviceItem *item;
+
 	g_return_if_fail (FU_IS_DEVICE_LIST (self));
 	g_return_if_fail (FU_IS_DEVICE (device));
 
-	for (guint i = 0; i < self->devices->len; i++) {
-		FuDeviceItem *item = g_ptr_array_index (self->devices, i);
-		if (item->device == device) {
-			g_ptr_array_remove (self->devices, item);
-			fu_device_list_emit_device_removed (self, device);
-			return;
-		}
+	/* check the device already exists */
+	item = fu_device_list_find_by_device (self, device);
+	if (item == NULL) {
+		g_debug ("device %s not found", fu_device_get_id (device));
+		return;
 	}
+
+	/* ensure never fired if the remove delay is changed */
+	if (item->remove_id > 0) {
+		g_source_remove (item->remove_id);
+		item->remove_id = 0;
+	}
+
+	/* delay the removal and check for replug */
+	if (fu_device_get_remove_delay (item->device) > 0) {
+
+		/* we can't do anything with an unconnected device */
+		fu_device_set_flags (item->device, FWUPD_DEVICE_FLAG_NONE);
+
+		/* give the hardware time to re-enumerate or the user time to
+		 * re-insert the device with a magic button pressed */
+		g_debug ("waiting %ums for device removal",
+			 fu_device_get_remove_delay (item->device));
+		item->remove_id = g_timeout_add (fu_device_get_remove_delay (item->device),
+						 fu_device_list_device_delayed_remove_cb,
+						 item);
+		return;
+	}
+
+	/* remove right now */
+	fu_device_list_emit_device_removed (self, item->device);
+	g_ptr_array_remove (self->devices, item);
 }
 
 /**
@@ -135,10 +202,11 @@ fu_device_list_remove (FuDeviceList *self, FuDevice *device)
  * @device: A #FuDevice
  * @error: A #GError, or %NULL
  *
- * Adds a specific device to the device list.
+ * Adds a specific device to the device list if not already present.
  *
- * The ::added signal will also be emitted if @device is not already found in
- * the list.
+ * If the @device has been previously removed within the remove-timeout then
+ * only the ::changed signal will be emitted on calling this function.
+ * Otherwise the ::added signal will be emitted straight away.
  *
  * Returns: (transfer none): a device, or %NULL if not found
  *
@@ -152,10 +220,22 @@ fu_device_list_add (FuDeviceList *self, FuDevice *device)
 	g_return_if_fail (FU_IS_DEVICE_LIST (self));
 	g_return_if_fail (FU_IS_DEVICE (device));
 
-	/* FIXME: verify the device does not already exist */
+	/* verify the device does not already exist */
+	item = fu_device_list_find_by_device (self, device);
+	if (item != NULL) {
+		g_debug ("found existing device %s, reusing item",
+			 fu_device_get_id (item->device));
+		if (item->remove_id != 0) {
+			g_source_remove (item->remove_id);
+			item->remove_id = 0;
+		}
+		fu_device_list_emit_device_changed (self, device);
+		return;
+	}
 
 	/* add helper */
 	item = g_new0 (FuDeviceItem, 1);
+	item->self = self; /* no ref */
 	item->device = g_object_ref (device);
 	g_ptr_array_add (self->devices, item);
 	fu_device_list_emit_device_added (self, device);
@@ -260,6 +340,8 @@ fu_device_list_find_by_id (FuDeviceList *self, const gchar *device_id, GError **
 static void
 fu_device_list_item_free (FuDeviceItem *item)
 {
+	if (item->remove_id != 0)
+		g_source_remove (item->remove_id);
 	g_object_unref (item->device);
 	g_free (item);
 }

src/fu-device.c

@@ -45,6 +45,7 @@ typedef struct {
 	gchar				*filename_pending;
 	FuDevice			*alternate;
 	GHashTable			*metadata;
+	guint				 remove_delay;	/* ms */
 } FuDevicePrivate;
 
 G_DEFINE_TYPE_WITH_PRIVATE (FuDevice, fu_device, FWUPD_TYPE_DEVICE)
@@ -391,6 +392,47 @@ fwupd_pad_kv_str (GString *str, const gchar *key, const gchar *value)
 	g_string_append_printf (str, "%s\n", value);
 }
 
+/**
+ * fu_device_get_remove_delay:
+ * @device: A #FuDevice
+ *
+ * Returns the maximum delay expected when replugging the device going into
+ * bootloader mode.
+ *
+ * Returns: time in milliseconds
+ *
+ * Since: 1.0.2
+ **/
+guint
+fu_device_get_remove_delay (FuDevice *device)
+{
+	FuDevicePrivate *priv = GET_PRIVATE (device);
+	g_return_val_if_fail (FU_IS_DEVICE (device), 0);
+	return priv->remove_delay;
+}
+
+/**
+ * fu_device_set_remove_delay:
+ * @device: A #FuDevice
+ * @remove_delay: the remove_delay value
+ *
+ * Sets the amount of time a device is allowed to return in bootloader mode.
+ *
+ * NOTE: this should be less than 3000ms for devices that just have to reset
+ * and automatically re-enumerate, but significantly longer if it involves a
+ * user removing a cable, pressing several buttons and removing a cable.
+ * A suggested value for this would be 10,000ms.
+ *
+ * Since: 1.0.2
+ **/
+void
+fu_device_set_remove_delay (FuDevice *device, guint remove_delay)
+{
+	FuDevicePrivate *priv = GET_PRIVATE (device);
+	g_return_if_fail (FU_IS_DEVICE (device));
+	priv->remove_delay = remove_delay;
+}
+
 /**
  * fu_device_to_string:
  * @device: A #FuDevice

src/fu-device.h

@@ -35,6 +35,24 @@ struct _FuDeviceClass
 	FwupdDeviceClass	 parent_class;
 };
 
+/**
+ * FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE:
+ *
+ * The default removal delay for device re-enumeration taking into account a
+ * chain of slow USB hubs. This should be used when the device is able to
+ * reset itself between bootloader->runtime->bootloader.
+ */
+#define FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE		5000
+
+/**
+ * FU_DEVICE_REMOVE_DELAY_USER_REPLUG:
+ *
+ * The default removal delay for device re-plug taking into account humans
+ * being slow and clumsy. This should be used when the user has to do something,
+ * e.g. unplug, press a magic button and then replug.
+ */
+#define FU_DEVICE_REMOVE_DELAY_USER_REPLUG		20000
+
 FuDevice	*fu_device_new				(void);
 
 /* helpful casting macros */
@@ -105,6 +123,9 @@ void		 fu_device_set_platform_id		(FuDevice	*device,
 							 const gchar	*platform_id);
 void		 fu_device_set_name			(FuDevice	*device,
 							 const gchar	*value);
+guint		 fu_device_get_remove_delay		(FuDevice	*device);
+void		 fu_device_set_remove_delay		(FuDevice	*device,
+							 guint		 remove_delay);
 
 G_END_DECLS
 

src/fu-self-test.c

@@ -314,6 +314,47 @@ _device_list_count_cb (FuDeviceList *device_list, FuDevice *device, gpointer use
 	(*cnt)++;
 }
 
+static void
+fu_device_list_delay_func (void)
+{
+	g_autoptr(FuDevice) device1 = fu_device_new ();
+	g_autoptr(FuDeviceList) device_list = fu_device_list_new ();
+	guint added_cnt = 0;
+	guint changed_cnt = 0;
+	guint removed_cnt = 0;
+
+	g_signal_connect (device_list, "added",
+			  G_CALLBACK (_device_list_count_cb),
+			  &added_cnt);
+	g_signal_connect (device_list, "removed",
+			  G_CALLBACK (_device_list_count_cb),
+			  &removed_cnt);
+	g_signal_connect (device_list, "changed",
+			  G_CALLBACK (_device_list_count_cb),
+			  &changed_cnt);
+
+	/* add one device */
+	fu_device_set_id (device1, "device1");
+	fu_device_add_guid (device1, "foobar");
+	fu_device_set_remove_delay (device1, 100);
+	fu_device_list_add (device_list, device1);
+	g_assert_cmpint (added_cnt, ==, 1);
+	g_assert_cmpint (removed_cnt, ==, 0);
+	g_assert_cmpint (changed_cnt, ==, 0);
+
+	/* spin a bit */
+	fu_test_loop_run_with_timeout (10);
+	fu_test_loop_quit ();
+
+	/* verify only a changed event was generated */
+	added_cnt = removed_cnt = changed_cnt = 0;
+	fu_device_list_remove (device_list, device1);
+	fu_device_list_add (device_list, device1);
+	g_assert_cmpint (added_cnt, ==, 0);
+	g_assert_cmpint (removed_cnt, ==, 0);
+	g_assert_cmpint (changed_cnt, ==, 1);
+}
+
 static void
 fu_device_list_func (void)
 {
@@ -1244,6 +1285,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/fwupd/device-locker{fail}", fu_device_locker_fail_func);
 	g_test_add_func ("/fwupd/device{metadata}", fu_device_metadata_func);
 	g_test_add_func ("/fwupd/device-list", fu_device_list_func);
+	g_test_add_func ("/fwupd/device-list{delay}", fu_device_list_delay_func);
 	g_test_add_func ("/fwupd/engine", fu_engine_func);
 	g_test_add_func ("/fwupd/engine{require-hwid}", fu_engine_require_hwid_func);
 	g_test_add_func ("/fwupd/engine{partial-hash}", fu_engine_partial_hash_func);