Allow the device list to take care of waiting for the device replug

This means that individual plugins do not have to manage thier own GUsbDevice
lifecycle and no longer have to call g_usb_context_wait_for_replug().

libfwupd/fwupd-enums.c

@@ -145,6 +145,8 @@ fwupd_device_flag_to_string (FwupdDeviceFlags device_flag)
 		return "install-parent-first";
 	if (device_flag == FWUPD_DEVICE_FLAG_IS_BOOTLOADER)
 		return "is-bootloader";
+	if (device_flag == FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)
+		return "wait-for-replug";
 	if (device_flag == FWUPD_DEVICE_FLAG_UNKNOWN)
 		return "unknown";
 	return NULL;
@@ -195,6 +197,8 @@ fwupd_device_flag_from_string (const gchar *device_flag)
 		return FWUPD_DEVICE_FLAG_INSTALL_PARENT_FIRST;
 	if (g_strcmp0 (device_flag, "is-bootloader") == 0)
 		return FWUPD_DEVICE_FLAG_IS_BOOTLOADER;
+	if (g_strcmp0 (device_flag, "wait-for-replug") == 0)
+		return FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG;
 	return FWUPD_DEVICE_FLAG_UNKNOWN;
 }
 

libfwupd/fwupd-enums.h

@@ -78,6 +78,7 @@ typedef enum {
  * @FWUPD_DEVICE_FLAG_USE_RUNTIME_VERSION:	Always use the runtime version rather than the bootloader
  * @FWUPD_DEVICE_FLAG_INSTALL_PARENT_FIRST:	Install composite firmware on the parent before the child
  * @FWUPD_DEVICE_FLAG_IS_BOOTLOADER:		Is currently in bootloader mode
+ * @FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG:		The hardware is waiting to be replugged
  *
  * The device flags.
  **/
@@ -96,6 +97,7 @@ typedef enum {
 #define FWUPD_DEVICE_FLAG_USE_RUNTIME_VERSION	(1u << 11)	/* Since: 1.0.6 */
 #define FWUPD_DEVICE_FLAG_INSTALL_PARENT_FIRST	(1u << 12)	/* Since: 1.0.8 */
 #define FWUPD_DEVICE_FLAG_IS_BOOTLOADER		(1u << 13)	/* Since: 1.0.8 */
+#define FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG	(1u << 14)	/* Since: 1.1.2 */
 #define FWUPD_DEVICE_FLAG_UNKNOWN		G_MAXUINT64	/* Since: 0.7.3 */
 typedef guint64 FwupdDeviceFlags;
 

plugins/colorhug/fu-colorhug-device.c

@@ -413,6 +413,8 @@ fu_colorhug_device_init (FuColorhugDevice *self)
 {
 	/* this is the application code */
 	self->start_addr = CH_EEPROM_ADDR_RUNCODE;
+	fu_device_set_remove_delay (FU_DEVICE (self),
+				    FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE);
 }
 
 static void

plugins/colorhug/fu-plugin-colorhug.c

@@ -20,10 +20,7 @@ fu_plugin_init (FuPlugin *plugin)
 gboolean
 fu_plugin_update_detach (FuPlugin *plugin, FuDevice *device, GError **error)
 {
-	FuColorhugDevice *self = FU_COLORHUG_DEVICE (device);
-	GUsbDevice *usb_device = fu_usb_device_get_dev (FU_USB_DEVICE (device));
 	g_autoptr(FuDeviceLocker) locker = NULL;
-	g_autoptr(GUsbDevice) usb_device2 = NULL;
 
 	/* open device */
 	locker = fu_device_locker_new (device, error);
@@ -41,29 +38,14 @@ fu_plugin_update_detach (FuPlugin *plugin, FuDevice *device, GError **error)
 		return FALSE;
 
 	/* wait for replug */
-	g_clear_object (&locker);
+	fu_device_add_flag (device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
-	usb_device2 = g_usb_context_wait_for_replug (fu_plugin_get_usb_context (plugin),
-						     usb_device,
-						     10000, error);
-	if (usb_device2 == NULL) {
-		g_prefix_error (error, "device did not come back: ");
-		return FALSE;
-	}
-
-	/* set the new device until we can use a new FuDevice */
-	fu_usb_device_set_dev (FU_USB_DEVICE (self), usb_device2);
-
-	/* success */
 	return TRUE;
 }
 
 gboolean
 fu_plugin_update_attach (FuPlugin *plugin, FuDevice *device, GError **error)
 {
-	FuColorhugDevice *self = FU_COLORHUG_DEVICE (device);
-	GUsbDevice *usb_device = fu_usb_device_get_dev (FU_USB_DEVICE (device));
 	g_autoptr(FuDeviceLocker) locker = NULL;
-	g_autoptr(GUsbDevice) usb_device2 = NULL;
 
 	/* open device */
 	locker = fu_device_locker_new (device, error);
@@ -81,19 +63,7 @@ fu_plugin_update_attach (FuPlugin *plugin, FuDevice *device, GError **error)
 		return FALSE;
 
 	/* wait for replug */
-	g_clear_object (&locker);
+	fu_device_add_flag (device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
-	usb_device2 = g_usb_context_wait_for_replug (fu_plugin_get_usb_context (plugin),
-						     usb_device,
-						     10000, error);
-	if (usb_device2 == NULL) {
-		g_prefix_error (error, "device did not come back: ");
-		return FALSE;
-	}
-
-	/* set the new device until we can use a new FuDevice */
-	fu_usb_device_set_dev (FU_USB_DEVICE (self), usb_device2);
-
-	/* success */
 	return TRUE;
 }
 

plugins/dfu/fu-plugin-dfu.c

@@ -84,10 +84,9 @@ fu_plugin_update_detach (FuPlugin *plugin, FuDevice *dev, GError **error)
 	/* detach and USB reset */
 	if (!dfu_device_detach (device, error))
 		return FALSE;
-	if (!dfu_device_wait_for_replug (device, FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE, error))
-		return FALSE;
 
-	/* success */
+	/* wait for replug */
+	fu_device_add_flag (dev, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
 	return TRUE;
 }
 
@@ -112,10 +111,9 @@ fu_plugin_update_attach (FuPlugin *plugin, FuDevice *dev, GError **error)
 	/* attach it */
 	if (!dfu_device_attach (device, error))
 		return FALSE;
-	if (!dfu_device_wait_for_replug (device, FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE, error))
-		return FALSE;
 
-	/* success */
+	/* wait for replug */
+	fu_device_add_flag (dev, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
 	return TRUE;
 }
 

plugins/ebitdo/fu-plugin-ebitdo.c

@@ -45,7 +45,6 @@ fu_plugin_update (FuPlugin *plugin,
 	GUsbDevice *usb_device = fu_usb_device_get_dev (FU_USB_DEVICE (dev));
 	FuEbitdoDevice *ebitdo_dev = FU_EBITDO_DEVICE (dev);
 	g_autoptr(FuDeviceLocker) locker = NULL;
-	g_autoptr(GUsbDevice) usb_device2 = NULL;
 
 	/* get version */
 	if (!fu_device_has_flag (dev, FWUPD_DEVICE_FLAG_IS_BOOTLOADER)) {
@@ -70,16 +69,9 @@ fu_plugin_update (FuPlugin *plugin,
 		g_prefix_error (error, "failed to force-reset device: ");
 		return FALSE;
 	}
-	g_clear_object (&locker);
-	usb_device2 = g_usb_context_wait_for_replug (fu_plugin_get_usb_context (plugin),
-						     usb_device, 10000, error);
-	if (usb_device2 == NULL) {
-		g_prefix_error (error, "device did not come back: ");
-		return FALSE;
-	}
-	fu_usb_device_set_dev (FU_USB_DEVICE (ebitdo_dev), usb_device2);
 
-	/* success */
+	/* wait for replug */
+	fu_device_add_flag (dev, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
 	return TRUE;
 }
 

src/fu-device-list.c

@@ -49,6 +49,8 @@ typedef struct {
 	FuDevice		*device;
 	FuDevice		*device_old;
 	FuDeviceList		*self;		/* no ref */
+	GMainLoop		*replug_loop;	/* block waiting for replug */
+	guint			 replug_id;	/* timeout the loop */
 	guint			 remove_id;
 } FuDeviceItem;
 
@@ -191,6 +193,26 @@ fu_device_list_find_by_guid (FuDeviceList *self, const gchar *guid)
 	return NULL;
 }
 
+static FuDeviceItem *
+fu_device_list_find_by_platform_id (FuDeviceList *self, const gchar *platform_id)
+{
+	for (guint i = 0; i < self->devices->len; i++) {
+		FuDeviceItem *item_tmp = g_ptr_array_index (self->devices, i);
+		FuDevice *device = item_tmp->device;
+		if (device != NULL &&
+		    g_strcmp0 (fu_device_get_platform_id (device), platform_id) == 0)
+			return item_tmp;
+	}
+	for (guint i = 0; i < self->devices->len; i++) {
+		FuDeviceItem *item_tmp = g_ptr_array_index (self->devices, i);
+		FuDevice *device = item_tmp->device_old;
+		if (device != NULL &&
+		    g_strcmp0 (fu_device_get_platform_id (device), platform_id) == 0)
+			return item_tmp;
+	}
+	return NULL;
+}
+
 static FuDeviceItem *
 fu_device_list_find_by_id (FuDeviceList *self,
 			   const gchar *device_id,
@@ -215,6 +237,10 @@ fu_device_list_find_by_id (FuDeviceList *self,
 			}
 		}
 	}
+	if (item != NULL)
+		return item;
+
+	/* only search old devices if we didn't find the active device */
 	for (guint i = 0; i < self->devices->len; i++) {
 		FuDeviceItem *item_tmp = g_ptr_array_index (self->devices, i);
 		const gchar *ids[3] = { NULL };
@@ -404,6 +430,12 @@ fu_device_list_replace (FuDeviceList *self, FuDeviceItem *item, FuDevice *device
 	g_set_object (&item->device_old, item->device);
 	g_set_object (&item->device, device);
 	fu_device_list_emit_device_changed (self, device);
+
+	/* we were waiting for this... */
+	if (g_main_loop_is_running (item->replug_loop)) {
+		g_debug ("quitting replug loop");
+		g_main_loop_quit (item->replug_loop);
+	}
 }
 
 /**
@@ -445,7 +477,7 @@ fu_device_list_add (FuDeviceList *self, FuDevice *device)
 			g_source_remove (item->remove_id);
 			item->remove_id = 0;
 		}
-		fu_device_list_emit_device_changed (self, device);
+		fu_device_list_replace (self, item, device);
 		return;
 	}
 
@@ -458,6 +490,10 @@ fu_device_list_add (FuDeviceList *self, FuDevice *device)
 
 	/* verify a compatible device does not already exist */
 	item = fu_device_list_get_by_guids (self, fu_device_get_guids (device));
+	if (item == NULL) {
+		item = fu_device_list_find_by_platform_id (self,
+							   fu_device_get_platform_id (device));
+	}
 	if (item != NULL && item->remove_id != 0) {
 		g_debug ("found compatible device %s recently removed, reusing "
 			 "item from plugin %s for plugin %s",
@@ -499,6 +535,7 @@ fu_device_list_add (FuDeviceList *self, FuDevice *device)
 	item = g_new0 (FuDeviceItem, 1);
 	item->self = self; /* no ref */
 	item->device = g_object_ref (device);
+	item->replug_loop = g_main_loop_new (NULL, FALSE);
 	g_ptr_array_add (self->devices, item);
 	fu_device_list_emit_device_added (self, device);
 }
@@ -533,6 +570,90 @@ fu_device_list_get_by_guid (FuDeviceList *self, const gchar *guid, GError **erro
 	return NULL;
 }
 
+static gboolean
+fu_device_list_replug_cb (gpointer user_data)
+{
+	FuDeviceItem *item = (FuDeviceItem *) user_data;
+
+	/* no longer valid */
+	item->replug_id = 0;
+
+	/* quit loop */
+	g_debug ("device did not replug");
+	g_main_loop_quit (item->replug_loop);
+	return FALSE;
+}
+
+/**
+ * fu_device_list_wait_for_replug:
+ * @self: A #FuDeviceList
+ * @guid: A device GUID
+ * @error: A #GError, or %NULL
+ *
+ * Waits for a specific devic to replug if %FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG
+ * is set.
+ *
+ * If the device does not exist this function returns without an error.
+ *
+ * Returns: %TRUE for success
+ *
+ * Since: 1.1.2
+ **/
+gboolean
+fu_device_list_wait_for_replug (FuDeviceList *self, FuDevice *device, GError **error)
+{
+	FuDeviceItem *item;
+	guint remove_delay;
+
+	g_return_val_if_fail (FU_IS_DEVICE_LIST (self), FALSE);
+	g_return_val_if_fail (FU_IS_DEVICE (device), FALSE);
+	g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
+
+	/* not found */
+	item = fu_device_list_find_by_device (self, device);
+	if (item == NULL)
+		return TRUE;
+
+	/* not required, or possibly literally just happened */
+	if (!fu_device_has_flag (item->device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)) {
+		g_debug ("no replug or re-enumerate required");
+		return TRUE;
+	}
+
+	/* plugin did not specify */
+	remove_delay = fu_device_get_remove_delay (device);
+	if (remove_delay == 0) {
+		remove_delay = FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE;
+		g_warning ("plugin %s did not specify a remove delay for %s, "
+			   "so guessing we should wait %ums for replug",
+			   fu_device_get_plugin (device),
+			   fu_device_get_id (device),
+			   remove_delay);
+	} else {
+		g_debug ("waiting %ums for replug", remove_delay);
+	}
+
+	/* time to unplug and then re-plug */
+	item->replug_id = g_timeout_add (remove_delay, fu_device_list_replug_cb, item);
+	g_main_loop_run (item->replug_loop);
+
+	/* the loop was quit without the timer */
+	if (item->replug_id != 0) {
+		g_debug ("waited for replug");
+		g_source_remove (item->replug_id);
+		item->replug_id = 0;
+		return TRUE;
+	}
+
+	/* device was not added back to the device list */
+	g_set_error (error,
+		     FWUPD_ERROR,
+		     FWUPD_ERROR_NOT_FOUND,
+		     "device %s did not come back",
+		     fu_device_get_id (device));
+	return FALSE;
+}
+
 /**
  * fu_device_list_get_by_id:
  * @self: A #FuDeviceList
@@ -586,8 +707,11 @@ fu_device_list_item_free (FuDeviceItem *item)
 {
 	if (item->remove_id != 0)
 		g_source_remove (item->remove_id);
+	if (item->replug_id != 0)
+		g_source_remove (item->replug_id);
 	if (item->device_old != NULL)
 		g_object_unref (item->device_old);
+	g_main_loop_unref (item->replug_loop);
 	g_object_unref (item->device);
 	g_free (item);
 }

src/fu-device-list.h

@@ -33,6 +33,9 @@ FuDevice	*fu_device_list_get_by_id		(FuDeviceList	*self,
 FuDevice	*fu_device_list_get_by_guid		(FuDeviceList	*self,
 							 const gchar	*guid,
 							 GError		**error);
+gboolean	 fu_device_list_wait_for_replug		(FuDeviceList	*self,
+							 FuDevice	*device,
+							 GError		**error);
 
 G_END_DECLS
 

src/fu-engine.c

@@ -1434,6 +1434,11 @@ fu_engine_install_blob (FuEngine *self,
 		return FALSE;
 	}
 	device = fu_device_list_get_by_id (self->device_list, device_id_orig, error);
+	if (device == NULL)
+		return FALSE;
+	if (!fu_device_list_wait_for_replug (self->device_list, device, error))
+		return FALSE;
+	device = fu_device_list_get_by_id (self->device_list, device_id_orig, error);
 	if (device == NULL)
 		return FALSE;
 	if (!fu_plugin_runner_update (plugin,
@@ -1477,6 +1482,11 @@ fu_engine_install_blob (FuEngine *self,
 		return FALSE;
 	}
 	device = fu_device_list_get_by_id (self->device_list, device_id_orig, error);
+	if (device == NULL)
+		return FALSE;
+	if (!fu_device_list_wait_for_replug (self->device_list, device, error))
+		return FALSE;
+	device = fu_device_list_get_by_id (self->device_list, device_id_orig, error);
 	if (device == NULL)
 		return FALSE;
 	if (!fu_plugin_runner_update_attach (plugin, device, &error_local)) {
@@ -1491,6 +1501,11 @@ fu_engine_install_blob (FuEngine *self,
 		g_propagate_error (error, g_steal_pointer (&error_local));
 		return FALSE;
 	}
+	device = fu_device_list_get_by_id (self->device_list, device_id_orig, error);
+	if (device == NULL)
+		return FALSE;
+	if (!fu_device_list_wait_for_replug (self->device_list, device, error))
+		return FALSE;
 
 	/* get the new version number */
 	device = fu_device_list_get_by_id (self->device_list, device_id_orig, error);

src/fu-self-test.c

@@ -854,6 +854,126 @@ fu_device_list_delay_func (void)
 	g_assert_cmpint (changed_cnt, ==, 1);
 }
 
+typedef struct {
+	FuDevice	*device_new;
+	FuDevice	*device_old;
+	FuDeviceList	*device_list;
+} FuDeviceListReplugHelper;
+
+static gboolean
+fu_device_list_remove_cb (gpointer user_data)
+{
+	FuDeviceListReplugHelper *helper = (FuDeviceListReplugHelper *) user_data;
+	fu_device_list_remove (helper->device_list, helper->device_old);
+	return FALSE;
+}
+
+static gboolean
+fu_device_list_add_cb (gpointer user_data)
+{
+	FuDeviceListReplugHelper *helper = (FuDeviceListReplugHelper *) user_data;
+	fu_device_list_add (helper->device_list, helper->device_new);
+	return FALSE;
+}
+
+static void
+fu_device_list_replug_auto_func (void)
+{
+	gboolean ret;
+	g_autoptr(FuDevice) device1 = fu_device_new ();
+	g_autoptr(FuDevice) device2 = fu_device_new ();
+	g_autoptr(FuDeviceList) device_list = fu_device_list_new ();
+	g_autoptr(GError) error = NULL;
+	FuDeviceListReplugHelper helper;
+
+	/* fake devices */
+	fu_device_set_id (device1, "device1");
+	fu_device_set_platform_id (device1, "ID");
+	fu_device_set_plugin (device1, "self-test");
+	fu_device_set_remove_delay (device1, FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE);
+	fu_device_set_id (device2, "device2");
+	fu_device_set_platform_id (device2, "ID"); /* matches */
+	fu_device_set_plugin (device2, "self-test");
+	fu_device_set_remove_delay (device2, FU_DEVICE_REMOVE_DELAY_RE_ENUMERATE);
+
+	/* not yet added */
+	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	/* add device */
+	fu_device_list_add (device_list, device1);
+
+	/* not waiting */
+	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	/* waiting */
+	helper.device_old = device1;
+	helper.device_new = device2;
+	helper.device_list = device_list;
+	g_timeout_add (100, fu_device_list_remove_cb, &helper);
+	g_timeout_add (200, fu_device_list_add_cb, &helper);
+	fu_device_add_flag (device1, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
+	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	/* waiting, failed */
+	fu_device_add_flag (device2, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
+	ret = fu_device_list_wait_for_replug (device_list, device2, &error);
+	g_assert_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND);
+	g_assert (!ret);
+}
+
+static void
+fu_device_list_replug_user_func (void)
+{
+	gboolean ret;
+	g_autoptr(FuDevice) device1 = fu_device_new ();
+	g_autoptr(FuDevice) device2 = fu_device_new ();
+	g_autoptr(FuDeviceList) device_list = fu_device_list_new ();
+	g_autoptr(GError) error = NULL;
+	FuDeviceListReplugHelper helper;
+
+	/* fake devices */
+	fu_device_set_id (device1, "device1");
+	fu_device_add_guid (device1, "foo");
+	fu_device_add_guid (device1, "bar");
+	fu_device_set_plugin (device1, "self-test");
+	fu_device_set_remove_delay (device1, FU_DEVICE_REMOVE_DELAY_USER_REPLUG);
+	fu_device_set_id (device2, "device2");
+	fu_device_add_guid (device2, "baz");
+	fu_device_add_guid (device2, "bar"); /* matches */
+	fu_device_set_plugin (device2, "self-test");
+	fu_device_set_remove_delay (device2, FU_DEVICE_REMOVE_DELAY_USER_REPLUG);
+
+	/* not yet added */
+	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	/* add device */
+	fu_device_list_add (device_list, device1);
+
+	/* not waiting */
+	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+
+	/* waiting */
+	helper.device_old = device1;
+	helper.device_new = device2;
+	helper.device_list = device_list;
+	g_timeout_add (100, fu_device_list_remove_cb, &helper);
+	g_timeout_add (200, fu_device_list_add_cb, &helper);
+	fu_device_add_flag (device1, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
+	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+}
+
 static void
 fu_device_list_compatible_func (void)
 {
@@ -2456,6 +2576,8 @@ main (int argc, char **argv)
 	g_test_add_func ("/fwupd/engine{device-unlock}", fu_engine_device_unlock_func);
 	g_test_add_func ("/fwupd/engine{history-success}", fu_engine_history_func);
 	g_test_add_func ("/fwupd/engine{history-error}", fu_engine_history_error_func);
+	g_test_add_func ("/fwupd/device-list{replug-auto}", fu_device_list_replug_auto_func);
+	g_test_add_func ("/fwupd/device-list{replug-user}", fu_device_list_replug_user_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);
 	g_test_add_func ("/fwupd/engine{downgrade}", fu_engine_downgrade_func);