diff --git a/libfwupd/fwupd-enums.c b/libfwupd/fwupd-enums.c index 74eb39714..975e0c06b 100644 --- a/libfwupd/fwupd-enums.c +++ b/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; } diff --git a/libfwupd/fwupd-enums.h b/libfwupd/fwupd-enums.h index 6b21b0fdf..f83728f3e 100644 --- a/libfwupd/fwupd-enums.h +++ b/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; diff --git a/plugins/colorhug/fu-colorhug-device.c b/plugins/colorhug/fu-colorhug-device.c index dbef7b975..88db807cb 100644 --- a/plugins/colorhug/fu-colorhug-device.c +++ b/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 diff --git a/plugins/colorhug/fu-plugin-colorhug.c b/plugins/colorhug/fu-plugin-colorhug.c index 5c533a7d6..afe9aa27c 100644 --- a/plugins/colorhug/fu-plugin-colorhug.c +++ b/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); - 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; } diff --git a/plugins/dfu/fu-plugin-dfu.c b/plugins/dfu/fu-plugin-dfu.c index 16ef30ca6..4b2f196a1 100644 --- a/plugins/dfu/fu-plugin-dfu.c +++ b/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; } diff --git a/plugins/ebitdo/fu-plugin-ebitdo.c b/plugins/ebitdo/fu-plugin-ebitdo.c index b97ff74e0..23fed9210 100644 --- a/plugins/ebitdo/fu-plugin-ebitdo.c +++ b/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; } diff --git a/src/fu-device-list.c b/src/fu-device-list.c index 0b4c4186a..5986d8837 100644 --- a/src/fu-device-list.c +++ b/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); } diff --git a/src/fu-device-list.h b/src/fu-device-list.h index 04e635bcf..9e30b4b97 100644 --- a/src/fu-device-list.h +++ b/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 diff --git a/src/fu-engine.c b/src/fu-engine.c index 81011df50..e9bf54bbd 100644 --- a/src/fu-engine.c +++ b/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); diff --git a/src/fu-self-test.c b/src/fu-self-test.c index d2439b0ef..4ee0ab66e 100644 --- a/src/fu-self-test.c +++ b/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);