Allow multiple devices to set WAIT_FOR_REPLUG

This also means we no longer pass in just one device to the
fu_device_list_wait_for_replug() function, but we rather wait for *all*
of the active devices.

This also cleans up the confusion on whether the engine should wait for
the 'root' device or the child device as either (or both) can be set as
required.

This makes the replug far more predictable for devices that have
children even in bootloader mode.

src/fu-device-list.c

@@ -761,14 +761,24 @@ fu_device_list_devices_wait_removed (FuDeviceList *self)
 	return cnt;
 }
 
+static GPtrArray *
+fu_device_list_get_wait_for_replug (FuDeviceList *self)
+{
+	GPtrArray *devices = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
+	for (guint i = 0; i < self->devices->len; i++) {
+		FuDeviceItem *item_tmp = g_ptr_array_index (self->devices, i);
+		if (fu_device_has_flag (item_tmp->device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG))
+			g_ptr_array_add (devices, g_object_ref (item_tmp->device));
+	}
+	return devices;
+}
+
 /**
  * fu_device_list_wait_for_replug:
  * @self: a device list
- * @device: a device
  * @error: (nullable): optional return location for an error
  *
- * Waits for a specific device to replug if %FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG
+ * Waits for all the devices with %FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG to replug.
- * is set.
  *
  * If the device does not exist this function returns without an error.
  *
@@ -777,49 +787,37 @@ fu_device_list_devices_wait_removed (FuDeviceList *self)
  * Since: 1.1.2
  **/
 gboolean
-fu_device_list_wait_for_replug (FuDeviceList *self, FuDevice *device, GError **error)
+fu_device_list_wait_for_replug (FuDeviceList *self, GError **error)
 {
-	FuDeviceItem *item;
+	guint remove_delay = 0;
-	guint remove_delay;
 	guint wait_removed;
 	guint wait_removed_old = 0;
 	g_autoptr(GTimer) timer = g_timer_new ();
+	g_autoptr(GPtrArray) devices_wfr1 = NULL;
+	g_autoptr(GPtrArray) devices_wfr2 = NULL;
 
 	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)) {
+	devices_wfr1 = fu_device_list_get_wait_for_replug (self);
+	if (devices_wfr1->len == 0) {
 		g_debug ("no replug or re-enumerate required");
 		return TRUE;
 	}
 
-	/* check that no other devices are waiting for replug too */
+	/* use the maximum of all the devices */
-	for (guint i = 0; i < self->devices->len; i++) {
+	for (guint i = 0; i < devices_wfr1->len; i++) {
-		FuDeviceItem *item_tmp = g_ptr_array_index (self->devices, i);
+		FuDevice *device_tmp = g_ptr_array_index (devices_wfr1, i);
-		if (item_tmp->device != device &&
+		if (fu_device_get_remove_delay (device_tmp) > remove_delay)
-		    fu_device_has_flag (item_tmp->device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)) {
+			remove_delay = fu_device_get_remove_delay (device_tmp);
-			g_warning ("%s is wait-for-replug when %s scheduled, unsetting",
-				   fu_device_get_id (item_tmp->device),
-				   fu_device_get_id (device));
-			fu_device_remove_flag (item_tmp->device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
-		}
 	}
 
 	/* 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, "
+		g_warning ("plugin did not specify a remove delay, "
 			   "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);
@@ -827,6 +825,8 @@ fu_device_list_wait_for_replug (FuDeviceList *self, FuDevice *device, GError **e
 
 	/* time to unplug and then re-plug */
 	do {
+		g_autoptr(GPtrArray) devices_wfr_tmp = NULL;
+
 		/* count how many devices are in the remove waiting state */
 		wait_removed = fu_device_list_devices_wait_removed (self);
 		if (wait_removed != wait_removed_old) {
@@ -836,32 +836,33 @@ fu_device_list_wait_for_replug (FuDeviceList *self, FuDevice *device, GError **e
 		}
 		g_usleep (1000);
 		g_main_context_iteration (NULL, FALSE);
-		if (!fu_device_has_flag (item->device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG) &&
+		devices_wfr_tmp = fu_device_list_get_wait_for_replug (self);
-		    wait_removed == 0)
+		if (devices_wfr_tmp->len == 0 && wait_removed == 0)
 			break;
 	} while (g_timer_elapsed (timer, NULL) * 1000.f < remove_delay);
 
-	/* device was not added back to the device list */
+	/* check that no other devices are still waiting for replug */
-	if (fu_device_has_flag (item->device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)) {
+	devices_wfr2 = fu_device_list_get_wait_for_replug (self);
+	if (devices_wfr2->len > 0) {
+		g_autoptr(GPtrArray) device_ids = g_ptr_array_new_with_free_func (g_free);
+		g_autofree gchar *device_ids_str = NULL;
+
+		/* unset and build error string */
+		for (guint i = 0; i < devices_wfr2->len; i++) {
+			FuDevice *device_tmp = g_ptr_array_index (devices_wfr2, i);
+			fu_device_remove_flag (device_tmp, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
+			g_ptr_array_add (device_ids,
+					 g_strdup (fu_device_get_id (device_tmp)));
+		}
+		device_ids_str = fu_common_strjoin_array (",", device_ids);
 		g_set_error (error,
 			     FWUPD_ERROR,
 			     FWUPD_ERROR_NOT_FOUND,
 			     "device %s did not come back",
-			     fu_device_get_id (device));
+			     device_ids_str);
-		fu_device_remove_flag (item->device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
 		return FALSE;
 	}
 
-	/* check that no other devices are waiting for replug instead */
-	for (guint i = 0; i < self->devices->len; i++) {
-		FuDeviceItem *item_tmp = g_ptr_array_index (self->devices, i);
-		if (fu_device_has_flag (item_tmp->device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)) {
-			g_warning ("%s is wait-for-replug when %s performed",
-				   fu_device_get_id (item_tmp->device),
-				   fu_device_get_id (device));
-		}
-	}
-
 	/* the loop was quit without the timer */
 	g_debug ("waited for replug");
 	return TRUE;

src/fu-device-list.h

@@ -29,7 +29,6 @@ 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);
 void		 fu_device_list_depsolve_order		(FuDeviceList	*self,
 							 FuDevice	*device);

src/fu-engine.c

@@ -2680,41 +2680,23 @@ fu_engine_get_plugins (FuEngine *self)
 FuDevice *
 fu_engine_get_device (FuEngine *self, const gchar *device_id, GError **error)
 {
-	g_autoptr(FuDevice) device1 = NULL;
+	g_autoptr(FuDevice) device = NULL;
-	g_autoptr(FuDevice) device2 = NULL;
-	g_autoptr(FuDevice) root = NULL;
 
-	/* find device */
+	/* wait for any device to disconnect and reconnect */
-	device1 = fu_device_list_get_by_id (self->device_list, device_id, error);
+	if (!fu_device_list_wait_for_replug (self->device_list, error)) {
-	if (device1 == NULL)
-		return NULL;
-
-	/* wait for device to disconnect and reconnect */
-	root = fu_device_get_root (device1);
-	if (fu_device_has_flag (device1, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)) {
-		if (!fu_device_list_wait_for_replug (self->device_list, device1, error)) {
 		g_prefix_error (error, "failed to wait for detach replug: ");
 		return NULL;
 	}
-	} else if (fu_device_has_flag (root, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)) {
-		if (!fu_device_list_wait_for_replug (self->device_list, root, error)) {
-			g_prefix_error (error, "failed to wait for detach replug: ");
-			return NULL;
-		}
-	} else {
-		/* no replug required */
-		return g_steal_pointer (&device1);
-	}
 
 	/* get the new device */
-	device2 = fu_device_list_get_by_id (self->device_list, device_id, error);
+	device = fu_device_list_get_by_id (self->device_list, device_id, error);
-	if (device2 == NULL) {
+	if (device == NULL) {
 		g_prefix_error (error, "failed to get device after replug: ");
 		return NULL;
 	}
 
 	/* success */
-	return g_steal_pointer (&device2);
+	return g_steal_pointer (&device);
 }
 
 /* same as FuDevice->prepare, but with the device open */
@@ -2795,12 +2777,10 @@ fu_engine_update_prepare (FuEngine *self,
 	}
 
 	/* wait for device to disconnect and reconnect */
-	if (fu_device_has_flag (device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)) {
+	if (!fu_device_list_wait_for_replug (self->device_list, error)) {
-		if (!fu_device_list_wait_for_replug (self->device_list, device, error)) {
 		g_prefix_error (error, "failed to wait for prepare replug: ");
 		return FALSE;
 	}
-	}
 	return TRUE;
 }
 
@@ -2829,12 +2809,10 @@ fu_engine_update_cleanup (FuEngine *self,
 	}
 
 	/* wait for device to disconnect and reconnect */
-	if (fu_device_has_flag (device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG)) {
+	if (!fu_device_list_wait_for_replug (self->device_list, error)) {
-		if (!fu_device_list_wait_for_replug (self->device_list, device, error)) {
 		g_prefix_error (error, "failed to wait for cleanup replug: ");
 		return FALSE;
 	}
-	}
 	return TRUE;
 }
 

src/fu-self-test.c

@@ -2246,7 +2246,7 @@ fu_device_list_replug_auto_func (gconstpointer user_data)
 	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);
+	ret = fu_device_list_wait_for_replug (device_list, &error);
 	g_assert_no_error (error);
 	g_assert (ret);
 
@@ -2254,7 +2254,7 @@ fu_device_list_replug_auto_func (gconstpointer user_data)
 	fu_device_list_add (device_list, device1);
 
 	/* not waiting */
-	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	ret = fu_device_list_wait_for_replug (device_list, &error);
 	g_assert_no_error (error);
 	g_assert (ret);
 
@@ -2265,7 +2265,7 @@ fu_device_list_replug_auto_func (gconstpointer user_data)
 	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);
+	ret = fu_device_list_wait_for_replug (device_list, &error);
 	g_assert_no_error (error);
 	g_assert (ret);
 	g_assert_false (fu_device_has_flag (device1, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG));
@@ -2275,10 +2275,10 @@ fu_device_list_replug_auto_func (gconstpointer user_data)
 
 	/* waiting, failed */
 	fu_device_add_flag (device2, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
-	ret = fu_device_list_wait_for_replug (device_list, device2, &error);
+	ret = fu_device_list_wait_for_replug (device_list, &error);
 	g_assert_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND);
 	g_assert (!ret);
-	g_assert_true (fu_device_has_flag (device1, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG));
+	g_assert_false (fu_device_has_flag (device1, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG));
 }
 
 static void
@@ -2309,7 +2309,7 @@ fu_device_list_replug_user_func (gconstpointer user_data)
 	fu_device_convert_instance_ids (device2);
 
 	/* not yet added */
-	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	ret = fu_device_list_wait_for_replug (device_list, &error);
 	g_assert_no_error (error);
 	g_assert (ret);
 
@@ -2317,7 +2317,7 @@ fu_device_list_replug_user_func (gconstpointer user_data)
 	fu_device_list_add (device_list, device1);
 
 	/* not waiting */
-	ret = fu_device_list_wait_for_replug (device_list, device1, &error);
+	ret = fu_device_list_wait_for_replug (device_list, &error);
 	g_assert_no_error (error);
 	g_assert (ret);
 
@@ -2328,7 +2328,7 @@ fu_device_list_replug_user_func (gconstpointer user_data)
 	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);
+	ret = fu_device_list_wait_for_replug (device_list, &error);
 	g_assert_no_error (error);
 	g_assert (ret);
 	g_assert_false (fu_device_has_flag (device1, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG));