diff --git a/src/fu-device-list.c b/src/fu-device-list.c index 7730e2336..860980519 100644 --- a/src/fu-device-list.c +++ b/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); } diff --git a/src/fu-device.c b/src/fu-device.c index 798bff54f..a183d9d1f 100644 --- a/src/fu-device.c +++ b/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 diff --git a/src/fu-device.h b/src/fu-device.h index 8512bc618..829c0ffeb 100644 --- a/src/fu-device.h +++ b/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 diff --git a/src/fu-self-test.c b/src/fu-self-test.c index e4a531b28..a068b6a25 100644 --- a/src/fu-self-test.c +++ b/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);