From 3124faf28e23009bd304dbd66cd32227c4142a9b Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Thu, 2 Feb 2023 16:43:16 +0000 Subject: Do not emit spurious ::device-removed events when emulating If we load new emulation data we have to do horrible tricks in the caller to avoid handling the remove event when loading new event data. Make the caller (fwupd) much simpler by emitting the correct signals. Also add a ::device-changed signal that we can use to reload the device events. --- gusb/gusb-context.c | 113 +++++++++++++++++++++++++++++++++++++-------- gusb/gusb-context.h | 3 +- gusb/gusb-device-private.h | 3 ++ gusb/gusb-device.c | 10 ++++ gusb/gusb-self-test.c | 98 +++++++++++++++++++++++++++++++++++---- 5 files changed, 200 insertions(+), 27 deletions(-) diff --git a/gusb/gusb-context.c b/gusb/gusb-context.c index f8ec3cf..eeac93e 100644 --- a/gusb/gusb-context.c +++ b/gusb/gusb-context.c @@ -23,7 +23,7 @@ enum { PROP_0, PROP_LIBUSB_CONTEXT, PROP_DEBUG_LEVEL, N_PROPERTIES }; -enum { DEVICE_ADDED_SIGNAL, DEVICE_REMOVED_SIGNAL, LAST_SIGNAL }; +enum { DEVICE_ADDED_SIGNAL, DEVICE_REMOVED_SIGNAL, DEVICE_CHANGED_SIGNAL, LAST_SIGNAL }; #define G_USB_CONTEXT_HOTPLUG_POLL_INTERVAL_DEFAULT 1000 /* ms */ @@ -244,6 +244,25 @@ g_usb_context_class_init(GUsbContextClass *klass) G_TYPE_NONE, 1, G_USB_TYPE_DEVICE); + + /** + * GUsbContext::device-changed: + * @self: the #GUsbContext instance that emitted the signal + * @device: A #GUsbDevice + * + * This signal is emitted when a USB device is changed. + **/ + signals[DEVICE_CHANGED_SIGNAL] = + g_signal_new("device-changed", + G_TYPE_FROM_CLASS(klass), + G_SIGNAL_RUN_LAST, + G_STRUCT_OFFSET(GUsbContextClass, device_changed), + NULL, + NULL, + g_cclosure_marshal_VOID__OBJECT, + G_TYPE_NONE, + 1, + G_USB_TYPE_DEVICE); } static void @@ -255,6 +274,8 @@ g_usb_context_emit_device_add(GUsbContext *self, GUsbDevice *device) if (!priv->done_enumerate) return; + if (_g_usb_context_has_flag(self, G_USB_CONTEXT_FLAGS_DEBUG)) + g_debug("emitting ::device-added(%s)", g_usb_device_get_platform_id(device)); g_signal_emit(self, signals[DEVICE_ADDED_SIGNAL], 0, device); } @@ -267,9 +288,25 @@ g_usb_context_emit_device_remove(GUsbContext *self, GUsbDevice *device) if (!priv->done_enumerate) return; + if (_g_usb_context_has_flag(self, G_USB_CONTEXT_FLAGS_DEBUG)) + g_debug("emitting ::device-removed(%s)", g_usb_device_get_platform_id(device)); g_signal_emit(self, signals[DEVICE_REMOVED_SIGNAL], 0, device); } +static void +g_usb_context_emit_device_changed(GUsbContext *self, GUsbDevice *device) +{ + GUsbContextPrivate *priv = GET_PRIVATE(self); + + /* should not happen, if it does we would not fire any signal */ + if (!priv->done_enumerate) + return; + + if (_g_usb_context_has_flag(self, G_USB_CONTEXT_FLAGS_DEBUG)) + g_debug("emitting ::device-changed(%s)", g_usb_device_get_platform_id(device)); + g_signal_emit(self, signals[DEVICE_CHANGED_SIGNAL], 0, device); +} + static void g_usb_context_add_device(GUsbContext *self, struct libusb_device *dev) { @@ -401,25 +438,19 @@ g_usb_context_load_with_tag(GUsbContext *self, { GUsbContextPrivate *priv = GET_PRIVATE(self); JsonArray *json_array; - g_autoptr(GPtrArray) devices_to_remove = + g_autoptr(GPtrArray) devices_added = + g_ptr_array_new_with_free_func((GDestroyNotify)g_object_unref); + g_autoptr(GPtrArray) devices_remove = g_ptr_array_new_with_free_func((GDestroyNotify)g_object_unref); g_return_val_if_fail(G_USB_IS_CONTEXT(self), FALSE); g_return_val_if_fail(json_object != NULL, FALSE); g_return_val_if_fail(error == NULL || *error == NULL, FALSE); - /* remove existing devices with this tag: two step */ - for (guint i = 0; i < priv->devices->len; i++) { - GUsbDevice *device = g_ptr_array_index(priv->devices, i); - if (tag == NULL || g_usb_device_has_tag(device, tag)) - g_ptr_array_add(devices_to_remove, g_object_ref(device)); - } - for (guint i = 0; i < devices_to_remove->len; i++) { - GUsbDevice *device = g_ptr_array_index(devices_to_remove, i); - g_usb_context_emit_device_remove(self, device); - g_ptr_array_remove(priv->devices, device); - } + /* if not already set */ + priv->done_enumerate = TRUE; + /* sanity check */ if (!json_object_has_member(json_object, "UsbDevices")) { g_set_error_literal(error, G_IO_ERROR, @@ -427,22 +458,68 @@ g_usb_context_load_with_tag(GUsbContext *self, "no UsbDevices array"); return FALSE; } + + /* four steps: + * + * 1. store all the existing devices matching the tag in devices_remove + * 2. read the devices in the array: + * - if the platform-id exists: replace the event data & remove from devices_remove + * - otherwise add to devices_added + * 3. emit devices in devices_remove + * 4. emit devices in devices_added + */ + for (guint i = 0; i < priv->devices->len; i++) { + GUsbDevice *device = g_ptr_array_index(priv->devices, i); + if (tag == NULL || g_usb_device_has_tag(device, tag)) + g_ptr_array_add(devices_remove, g_object_ref(device)); + } json_array = json_object_get_array_member(json_object, "UsbDevices"); for (guint i = 0; i < json_array_get_length(json_array); i++) { JsonNode *node_tmp = json_array_get_element(json_array, i); JsonObject *obj_tmp = json_node_get_object(node_tmp); - g_autoptr(GUsbDevice) device = + g_autoptr(GUsbDevice) device_old = NULL; + g_autoptr(GUsbDevice) device_tmp = g_object_new(G_USB_TYPE_DEVICE, "context", self, NULL); - if (!_g_usb_device_load(device, obj_tmp, error)) + if (!_g_usb_device_load(device_tmp, obj_tmp, error)) return FALSE; - if (tag != NULL && !g_usb_device_has_tag(device, tag)) + if (tag != NULL && !g_usb_device_has_tag(device_tmp, tag)) continue; + + /* does a device with this platform ID [and the same created date] already exist */ + device_old = + g_usb_context_find_by_platform_id(self, + g_usb_device_get_platform_id(device_tmp), + NULL); + if (device_old != NULL && g_date_time_equal(g_usb_device_get_created(device_old), + g_usb_device_get_created(device_tmp))) { + g_autoptr(GPtrArray) events = g_usb_device_get_events(device_tmp); + g_usb_device_clear_events(device_old); + for (guint j = 0; j < events->len; j++) { + GUsbDeviceEvent *event = g_ptr_array_index(events, j); + _g_usb_device_add_event(device_old, event); + } + g_usb_context_emit_device_changed(self, device_old); + g_ptr_array_remove(devices_remove, device_old); + continue; + } + + /* new to us! */ + g_ptr_array_add(devices_added, g_object_ref(device_tmp)); + } + + /* emit removes then adds */ + for (guint i = 0; i < devices_remove->len; i++) { + GUsbDevice *device = g_ptr_array_index(devices_remove, i); + g_usb_context_emit_device_remove(self, device); + g_ptr_array_remove(priv->devices, device); + } + for (guint i = 0; i < devices_added->len; i++) { + GUsbDevice *device = g_ptr_array_index(devices_added, i); g_ptr_array_add(priv->devices, g_object_ref(device)); - g_signal_emit(self, signals[DEVICE_ADDED_SIGNAL], 0, device); + g_usb_context_emit_device_add(self, device); } /* success */ - priv->done_enumerate = TRUE; return TRUE; } diff --git a/gusb/gusb-context.h b/gusb/gusb-context.h index 3c47b5e..b1d4d54 100644 --- a/gusb/gusb-context.h +++ b/gusb/gusb-context.h @@ -22,12 +22,13 @@ struct _GUsbContextClass { GObjectClass parent_class; void (*device_added)(GUsbContext *self, GUsbDevice *device); void (*device_removed)(GUsbContext *self, GUsbDevice *device); + void (*device_changed)(GUsbContext *self, GUsbDevice *device); /*< private >*/ /* * If adding fields to this struct, remove corresponding * amount of padding to avoid changing overall struct size */ - gchar _gusb_reserved[62]; + gchar _gusb_reserved[61]; }; typedef enum { G_USB_CONTEXT_ERROR_INTERNAL } GUsbContextError; diff --git a/gusb/gusb-device-private.h b/gusb/gusb-device-private.h index 70218b2..d1cd376 100644 --- a/gusb/gusb-device-private.h +++ b/gusb/gusb-device-private.h @@ -7,6 +7,7 @@ #pragma once +#include #include G_BEGIN_DECLS @@ -17,6 +18,8 @@ gboolean _g_usb_device_load(GUsbDevice *self, JsonObject *json_object, GError **error); gboolean _g_usb_device_save(GUsbDevice *self, JsonBuilder *json_builder, GError **error); +void +_g_usb_device_add_event(GUsbDevice *self, GUsbDeviceEvent *event); libusb_device * _g_usb_device_get_device(GUsbDevice *self); diff --git a/gusb/gusb-device.c b/gusb/gusb-device.c index 60c70c1..bbda96c 100644 --- a/gusb/gusb-device.c +++ b/gusb/gusb-device.c @@ -222,6 +222,16 @@ g_usb_device_init(GUsbDevice *self) priv->tags = g_ptr_array_new_with_free_func(g_free); } +/* private */ +void +_g_usb_device_add_event(GUsbDevice *self, GUsbDeviceEvent *event) +{ + GUsbDevicePrivate *priv = GET_PRIVATE(self); + g_return_if_fail(G_USB_IS_DEVICE(self)); + g_return_if_fail(G_USB_IS_DEVICE_EVENT(event)); + g_ptr_array_add(priv->events, g_object_ref(event)); +} + gboolean _g_usb_device_load(GUsbDevice *self, JsonObject *json_object, GError **error) { diff --git a/gusb/gusb-self-test.c b/gusb/gusb-self-test.c index 970ae21..26dd4f5 100644 --- a/gusb/gusb-self-test.c +++ b/gusb/gusb-self-test.c @@ -408,21 +408,47 @@ gusb_device_munki_func(void) } static void -gusb_device_json_func(void) +_context_signal_count_cb(GUsbContext *ctx, GUsbDevice *usb_device, gpointer user_data) +{ + guint *cnt = (guint *)user_data; + (*cnt)++; +} + +static gboolean +_g_usb_context_load_json(GUsbContext *ctx, const gchar *json, GError **error) { JsonObject *json_obj; + g_autoptr(JsonParser) parser = json_parser_new(); + + if (!json_parser_load_from_data(parser, json, -1, error)) + return FALSE; + json_obj = json_node_get_object(json_parser_get_root(parser)); + return g_usb_context_load_with_tag(ctx, json_obj, "emulation", error); +} + +static void +gusb_device_json_func(void) +{ gboolean ret; guint8 idx; + guint added_cnt = 0; + guint changed_cnt = 0; + guint removed_cnt = 0; g_autoptr(GUsbDevice) device = NULL; + g_autoptr(GUsbDevice) device2 = NULL; + g_autoptr(GUsbDevice) device3 = NULL; g_autofree gchar *tmp = NULL; g_autoptr(GUsbContext) ctx = NULL; g_autoptr(GError) error = NULL; - g_autoptr(JsonParser) parser = json_parser_new(); const gchar *json = "{" " \"UsbDevices\" : [" " {" - " \"PlatformId\" : \"usb:01:00:06\"," + " \"PlatformId\" : \"usb:AA:AA:06\"," + " \"Created\" : \"2023-02-01T16:35:03.302027Z\"," + " \"Tags\" : [" + " \"emulation\"" + " ]," " \"IdVendor\" : 10047," " \"IdProduct\" : 4100," " \"Device\" : 2," @@ -490,23 +516,55 @@ gusb_device_json_func(void) " }" " ]" "}"; + const gchar *json_new = "{" + " \"UsbDevices\" : [" + " {" + " \"PlatformId\" : \"usb:FF:FF:06\"," + " \"Created\" : \"2099-02-01T16:35:03.302027Z\"," + " \"Tags\" : [" + " \"emulation\"" + " ]," + " \"IdVendor\" : 10047," + " \"IdProduct\" : 4101," + " \"Device\" : 2," + " \"USB\" : 512," + " \"Manufacturer\" : 1" + " }" + " ]" + "}"; ctx = g_usb_context_new(&error); + g_usb_context_set_flags(ctx, G_USB_CONTEXT_FLAGS_DEBUG); g_assert_no_error(error); g_assert(ctx != NULL); + /* watch events */ + g_usb_context_enumerate(ctx); + g_signal_connect(G_USB_CONTEXT(ctx), + "device-added", + G_CALLBACK(_context_signal_count_cb), + &added_cnt); + g_signal_connect(G_USB_CONTEXT(ctx), + "device-removed", + G_CALLBACK(_context_signal_count_cb), + &removed_cnt); + g_signal_connect(G_USB_CONTEXT(ctx), + "device-changed", + G_CALLBACK(_context_signal_count_cb), + &changed_cnt); + /* parse */ - ret = json_parser_load_from_data(parser, json, -1, &error); - g_assert_no_error(error); - g_assert(ret); - json_obj = json_node_get_object(json_parser_get_root(parser)); - ret = g_usb_context_load(ctx, json_obj, &error); + ret = _g_usb_context_load_json(ctx, json, &error); g_assert_no_error(error); g_assert(ret); + g_assert_cmpint(added_cnt, ==, 1); + g_assert_cmpint(removed_cnt, ==, 0); + g_assert_cmpint(changed_cnt, ==, 0); /* get vendor data */ device = g_usb_context_find_by_vid_pid(ctx, 0x273f, 0x1004, &error); g_assert_no_error(error); g_assert(device != NULL); + g_assert_true(g_usb_device_has_tag(device, "emulation")); idx = g_usb_device_get_custom_index(device, G_USB_DEVICE_CLASS_VENDOR_SPECIFIC, 'F', @@ -537,6 +595,30 @@ gusb_device_json_func(void) tmp = g_usb_device_get_string_descriptor(device, idx, &error); g_assert_no_error(error); g_assert_cmpstr(tmp, ==, "2.0.7"); + + /* load the same data */ + ret = _g_usb_context_load_json(ctx, json, &error); + g_assert_no_error(error); + g_assert(ret); + g_assert_cmpint(added_cnt, ==, 1); + g_assert_cmpint(removed_cnt, ==, 0); + g_assert_cmpint(changed_cnt, ==, 1); + device2 = g_usb_context_find_by_vid_pid(ctx, 0x273f, 0x1004, &error); + g_assert_no_error(error); + g_assert(device2 != NULL); + g_assert_true(g_usb_device_has_tag(device2, "emulation")); + + /* load a different device */ + ret = _g_usb_context_load_json(ctx, json_new, &error); + g_assert_no_error(error); + g_assert(ret); + g_assert_cmpint(added_cnt, ==, 2); + g_assert_cmpint(changed_cnt, ==, 1); + g_assert_cmpint(removed_cnt, ==, 1); + device3 = g_usb_context_find_by_vid_pid(ctx, 0x273f, 0x1005, &error); + g_assert_no_error(error); + g_assert(device3 != NULL); + g_assert_true(g_usb_device_has_tag(device3, "emulation")); } static void -- cgit v1.2.1