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().
This commit is contained in:
Richard Hughes 2018-08-30 11:51:10 +01:00
parent 457888e418
commit 3a8d532855
10 changed files with 281 additions and 49 deletions

View File

@ -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;
}

View File

@ -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;

View File

@ -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

View File

@ -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);
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 */
fu_device_add_flag (device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
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);
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 */
fu_device_add_flag (device, FWUPD_DEVICE_FLAG_WAIT_FOR_REPLUG);
return TRUE;
}

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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);
}

View File

@ -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

View File

@ -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);

View File

@ -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);