diff --git a/libfwupdplugin/fu-device-private.h b/libfwupdplugin/fu-device-private.h index bf82c3502..a8e20e577 100644 --- a/libfwupdplugin/fu-device-private.h +++ b/libfwupdplugin/fu-device-private.h @@ -17,6 +17,9 @@ FuDeviceInternalFlags fu_device_internal_flag_from_string (const gchar *flag); GPtrArray *fu_device_get_parent_guids (FuDevice *self); gboolean fu_device_has_parent_guid (FuDevice *self, const gchar *guid); +GPtrArray *fu_device_get_parent_physical_ids (FuDevice *self); +gboolean fu_device_has_parent_physical_id (FuDevice *self, + const gchar *physical_id); void fu_device_set_parent (FuDevice *self, FuDevice *parent); gint fu_device_get_order (FuDevice *self); diff --git a/libfwupdplugin/fu-device.c b/libfwupdplugin/fu-device.c index 499b0ff04..5ee998032 100644 --- a/libfwupdplugin/fu-device.c +++ b/libfwupdplugin/fu-device.c @@ -51,6 +51,7 @@ typedef struct { GRWLock metadata_mutex; GPtrArray *parent_guids; GRWLock parent_guids_mutex; + GPtrArray *parent_physical_ids; /* (nullable) */ guint remove_delay; /* ms */ guint progress; guint battery_level; @@ -210,6 +211,8 @@ fu_device_internal_flag_to_string (FuDeviceInternalFlags flag) return "is-open"; if (flag == FU_DEVICE_INTERNAL_FLAG_NO_SERIAL_NUMBER) return "no-serial-number"; + if (flag == FU_DEVICE_INTERNAL_FLAG_AUTO_PARENT_CHILDREN) + return "auto-parent-children"; return NULL; } @@ -250,6 +253,8 @@ fu_device_internal_flag_from_string (const gchar *flag) return FU_DEVICE_INTERNAL_FLAG_IS_OPEN; if (g_strcmp0 (flag, "no-serial-number") == 0) return FU_DEVICE_INTERNAL_FLAG_NO_SERIAL_NUMBER; + if (g_strcmp0 (flag, "auto-parent-children") == 0) + return FU_DEVICE_INTERNAL_FLAG_AUTO_PARENT_CHILDREN; return FU_DEVICE_INTERNAL_FLAG_UNKNOWN; } @@ -883,6 +888,13 @@ fu_device_set_parent (FuDevice *self, FuDevice *parent) { g_return_if_fail (FU_IS_DEVICE (self)); + /* debug */ + if (parent != NULL) { + g_debug ("setting parent of %s [%s] to be %s [%s]", + fu_device_get_name (self), fu_device_get_id (self), + fu_device_get_name (parent), fu_device_get_id (parent)); + } + /* set the composite ID on the children and grandchildren */ if (parent != NULL) fu_device_set_composite_id (self, fu_device_get_composite_id (parent)); @@ -1120,6 +1132,86 @@ fu_device_add_parent_guid (FuDevice *self, const gchar *guid) g_ptr_array_add (priv->parent_guids, g_strdup (guid)); } +/** + * fu_device_get_parent_physical_ids: + * @self: a #FuDevice + * + * Gets any parent device IDs. If a device is added to the daemon that matches + * the physical ID added from fu_device_add_parent_physical_id() then this + * device is marked the parent of @self. + * + * Returns: (transfer none) (element-type utf8) (nullable): a list of IDs + * + * Since: 1.6.2 + **/ +GPtrArray * +fu_device_get_parent_physical_ids (FuDevice *self) +{ + FuDevicePrivate *priv = GET_PRIVATE (self); + g_return_val_if_fail (FU_IS_DEVICE (self), NULL); + return priv->parent_physical_ids; +} + +/** + * fu_device_has_parent_physical_id: + * @self: a #FuDevice + * @physical_id: a device physical ID + * + * Searches the list of parent IDs for a string match. + * + * Returns: %TRUE if the parent ID exists + * + * Since: 1.6.2 + **/ +gboolean +fu_device_has_parent_physical_id (FuDevice *self, const gchar *physical_id) +{ + FuDevicePrivate *priv = GET_PRIVATE (self); + + g_return_val_if_fail (FU_IS_DEVICE (self), FALSE); + g_return_val_if_fail (physical_id != NULL, FALSE); + + if (priv->parent_physical_ids == NULL) + return FALSE; + for (guint i = 0; i < priv->parent_physical_ids->len; i++) { + const gchar *tmp = g_ptr_array_index (priv->parent_physical_ids, i); + if (g_strcmp0 (tmp, physical_id) == 0) + return TRUE; + } + return FALSE; +} + +/** + * fu_device_add_parent_physical_id: + * @self: a #FuDevice + * @physical_id: a device physical ID + * + * Sets any parent device using the physical ID. An parent device is logically + * linked to the primary device in some way and can be added before or after @self. + * + * The IDs are searched in order, and so the order of adding IDs may be + * important if more than one parent device might match. + * + * Since: 1.6.2 + **/ +void +fu_device_add_parent_physical_id (FuDevice *self, const gchar *physical_id) +{ + FuDevicePrivate *priv = GET_PRIVATE (self); + + g_return_if_fail (FU_IS_DEVICE (self)); + g_return_if_fail (physical_id != NULL); + + /* ensure exists */ + if (priv->parent_physical_ids == NULL) + priv->parent_physical_ids = g_ptr_array_new_with_free_func (g_free); + + /* already present */ + if (fu_device_has_parent_physical_id (self, physical_id)) + return; + g_ptr_array_add (priv->parent_physical_ids, g_strdup (physical_id)); +} + static gboolean fu_device_add_child_by_type_guid (FuDevice *self, GType type, @@ -2897,6 +2989,10 @@ fu_device_add_string (FuDevice *self, guint idt, GString *str) const gchar *name = g_ptr_array_index (priv->possible_plugins, i); fu_common_string_append_kv (str, idt + 1, "PossiblePlugin", name); } + if (priv->parent_physical_ids != NULL && priv->parent_physical_ids->len > 0) { + g_autofree gchar *flags = fu_common_strjoin_array (",", priv->parent_physical_ids); + fu_common_string_append_kv (str, idt + 1, "ParentPhysicalIds", flags); + } if (priv->internal_flags != FU_DEVICE_INTERNAL_FLAG_NONE) { g_autoptr(GString) tmp2 = g_string_new (""); for (guint i = 0; i < 64; i++) { @@ -3853,6 +3949,7 @@ fu_device_incorporate (FuDevice *self, FuDevice *donor) FuDevicePrivate *priv_donor = GET_PRIVATE (donor); GPtrArray *instance_ids = fu_device_get_instance_ids (donor); GPtrArray *parent_guids = fu_device_get_parent_guids (donor); + GPtrArray *parent_physical_ids = fu_device_get_parent_physical_ids (donor); g_return_if_fail (FU_IS_DEVICE (self)); g_return_if_fail (FU_IS_DEVICE (donor)); @@ -3878,6 +3975,12 @@ fu_device_incorporate (FuDevice *self, FuDevice *donor) for (guint i = 0; i < parent_guids->len; i++) fu_device_add_parent_guid (self, g_ptr_array_index (parent_guids, i)); g_rw_lock_reader_unlock (&priv_donor->parent_guids_mutex); + if (parent_physical_ids != NULL) { + for (guint i = 0; i < parent_physical_ids->len; i++) { + const gchar *tmp = g_ptr_array_index (parent_physical_ids, i); + fu_device_add_parent_physical_id (self, tmp); + } + } g_rw_lock_reader_lock (&priv_donor->metadata_mutex); if (priv->metadata != NULL) { g_autoptr(GList) keys = g_hash_table_get_keys (priv_donor->metadata); @@ -4057,6 +4160,8 @@ fu_device_finalize (GObject *object) g_hash_table_unref (priv->metadata); if (priv->inhibits != NULL) g_hash_table_unref (priv->inhibits); + if (priv->parent_physical_ids != NULL) + g_ptr_array_unref (priv->parent_physical_ids); g_ptr_array_unref (priv->parent_guids); g_ptr_array_unref (priv->possible_plugins); g_ptr_array_unref (priv->retry_recs); diff --git a/libfwupdplugin/fu-device.h b/libfwupdplugin/fu-device.h index a5fcca2d5..3bf3e3865 100644 --- a/libfwupdplugin/fu-device.h +++ b/libfwupdplugin/fu-device.h @@ -229,6 +229,7 @@ FuDevice *fu_device_new (void); * @FU_DEVICE_INTERNAL_FLAG_INHERIT_ACTIVATION: Inherit activation status from the history database on startup * @FU_DEVICE_INTERNAL_FLAG_IS_OPEN: The device opened successfully and ready to use * @FU_DEVICE_INTERNAL_FLAG_NO_SERIAL_NUMBER: Do not attempt to read the device serial number + * @FU_DEVICE_INTERNAL_FLAG_AUTO_PARENT_CHILDREN: Automatically assign the parent for children of this device * * The device internal flags. **/ @@ -246,6 +247,7 @@ typedef enum { FU_DEVICE_INTERNAL_FLAG_INHERIT_ACTIVATION = (1llu << 9), /* Since: 1.5.9 */ FU_DEVICE_INTERNAL_FLAG_IS_OPEN = (1llu << 10), /* Since: 1.6.1 */ FU_DEVICE_INTERNAL_FLAG_NO_SERIAL_NUMBER = (1llu << 11), /* Since: 1.6.2 */ + FU_DEVICE_INTERNAL_FLAG_AUTO_PARENT_CHILDREN = (1llu << 12), /* Since: 1.6.2 */ /*< private >*/ FU_DEVICE_INTERNAL_FLAG_UNKNOWN = G_MAXUINT64, } FuDeviceInternalFlags; @@ -275,6 +277,8 @@ void fu_device_add_child (FuDevice *self, FuDevice *child); void fu_device_add_parent_guid (FuDevice *self, const gchar *guid); +void fu_device_add_parent_physical_id (FuDevice *self, + const gchar *physical_id); void fu_device_add_counterpart_guid (FuDevice *self, const gchar *guid); FuDevice *fu_device_get_proxy (FuDevice *self); diff --git a/libfwupdplugin/fu-usb-device.c b/libfwupdplugin/fu-usb-device.c index c7af1d92d..ac4a3cb06 100644 --- a/libfwupdplugin/fu-usb-device.c +++ b/libfwupdplugin/fu-usb-device.c @@ -287,6 +287,7 @@ fu_usb_device_probe (FuDevice *device, GError **error) g_autofree gchar *devid0 = NULL; g_autofree gchar *devid1 = NULL; g_autofree gchar *devid2 = NULL; + g_autofree gchar *platform_id = NULL; g_autofree gchar *vendor_id = NULL; g_autoptr(GPtrArray) intfs = NULL; @@ -344,6 +345,18 @@ fu_usb_device_probe (FuDevice *device, GError **error) fu_device_add_instance_id_full (device, intid3, FU_DEVICE_INSTANCE_FLAG_ONLY_QUIRKS); } + + /* add 2 levels of parent IDs */ + platform_id = g_strdup (g_usb_device_get_platform_id (priv->usb_device)); + for (guint i = 0; i < 2; i++) { + gchar *tok = g_strrstr (platform_id, ":"); + if (tok == NULL) + break; + *tok = '\0'; + if (g_strcmp0 (platform_id, "usb") == 0) + break; + fu_device_add_parent_physical_id (device, platform_id); + } #endif /* success */ diff --git a/libfwupdplugin/fwupdplugin.map b/libfwupdplugin/fwupdplugin.map index 527235f3c..6104e0c96 100644 --- a/libfwupdplugin/fwupdplugin.map +++ b/libfwupdplugin/fwupdplugin.map @@ -819,5 +819,8 @@ LIBFWUPDPLUGIN_1.6.1 { LIBFWUPDPLUGIN_1.6.2 { global: fu_common_check_kernel_version; + fu_device_add_parent_physical_id; + fu_device_get_parent_physical_ids; + fu_device_has_parent_physical_id; local: *; } LIBFWUPDPLUGIN_1.6.1; diff --git a/src/fu-engine.c b/src/fu-engine.c index fe50967b9..c1d579660 100644 --- a/src/fu-engine.c +++ b/src/fu-engine.c @@ -5350,27 +5350,49 @@ fu_engine_adopt_children (FuEngine *self, FuDevice *device) GPtrArray *guids; g_autoptr(GPtrArray) devices = fu_device_list_get_active (self->device_list); - /* find the parent GUID in any existing device */ - guids = fu_device_get_parent_guids (device); - for (guint j = 0; j < guids->len; j++) { - const gchar *guid = g_ptr_array_index (guids, j); + /* find the parent in any existing device */ + if (fu_device_get_parent (device) == NULL) { for (guint i = 0; i < devices->len; i++) { FuDevice *device_tmp = g_ptr_array_index (devices, i); - if (fu_device_get_parent (device) != NULL) + if (!fu_device_has_internal_flag (device_tmp, FU_DEVICE_INTERNAL_FLAG_AUTO_PARENT_CHILDREN)) continue; - if (fu_device_has_guid (device_tmp, guid)) { - g_debug ("setting parent of %s [%s] to be %s [%s]", - fu_device_get_name (device), - fu_device_get_id (device), - fu_device_get_name (device_tmp), - fu_device_get_id (device_tmp)); + if (fu_device_get_physical_id (device_tmp) == NULL) + continue; + if (fu_device_has_parent_physical_id (device, fu_device_get_physical_id (device_tmp))) { fu_device_set_parent (device, device_tmp); break; } } } + if (fu_device_get_parent (device) == NULL) { + guids = fu_device_get_parent_guids (device); + for (guint j = 0; j < guids->len; j++) { + const gchar *guid = g_ptr_array_index (guids, j); + for (guint i = 0; i < devices->len; i++) { + FuDevice *device_tmp = g_ptr_array_index (devices, i); + if (fu_device_has_guid (device_tmp, guid)) { + fu_device_set_parent (device, device_tmp); + break; + } + } + } + } /* the new device is the parent to an existing child */ + for (guint j = 0; j < devices->len; j++) { + GPtrArray *parent_physical_ids = NULL; + FuDevice *device_tmp = g_ptr_array_index (devices, j); + if (fu_device_get_parent (device_tmp) != NULL) + continue; + parent_physical_ids = fu_device_get_parent_physical_ids (device_tmp); + if (parent_physical_ids == NULL) + continue; + for (guint i = 0; i < parent_physical_ids->len; i++) { + const gchar *parent_physical_id = g_ptr_array_index (parent_physical_ids, i); + if (g_strcmp0 (parent_physical_id, fu_device_get_physical_id (device)) == 0) + fu_device_set_parent (device_tmp, device); + } + } guids = fu_device_get_guids (device); for (guint j = 0; j < guids->len; j++) { const gchar *guid = g_ptr_array_index (guids, j); @@ -5378,14 +5400,8 @@ fu_engine_adopt_children (FuEngine *self, FuDevice *device) FuDevice *device_tmp = g_ptr_array_index (devices, i); if (fu_device_get_parent (device_tmp) != NULL) continue; - if (fu_device_has_parent_guid (device_tmp, guid)) { - g_debug ("setting parent of %s [%s] to be %s [%s]", - fu_device_get_name (device_tmp), - fu_device_get_id (device_tmp), - fu_device_get_name (device), - fu_device_get_id (device)); + if (fu_device_has_parent_guid (device_tmp, guid)) fu_device_set_parent (device_tmp, device); - } } } } diff --git a/src/fu-self-test.c b/src/fu-self-test.c index bf4be61f2..fd7361e7a 100644 --- a/src/fu-self-test.c +++ b/src/fu-self-test.c @@ -1046,7 +1046,7 @@ fu_engine_requirements_parent_device_func (gconstpointer user_data) } static void -fu_engine_device_parent_func (gconstpointer user_data) +fu_engine_device_parent_guid_func (gconstpointer user_data) { g_autoptr(FuDevice) device1 = fu_device_new (); g_autoptr(FuDevice) device2 = fu_device_new (); @@ -1098,6 +1098,75 @@ fu_engine_device_parent_func (gconstpointer user_data) g_assert_cmpint (fu_device_get_order (device3), ==, -1); } +static void +fu_engine_device_parent_id_func (gconstpointer user_data) +{ + g_autoptr(FuDevice) device1 = fu_device_new (); + g_autoptr(FuDevice) device2 = fu_device_new (); + g_autoptr(FuDevice) device3 = fu_device_new (); + g_autoptr(FuDevice) device4 = fu_device_new (); + g_autoptr(FuEngine) engine = fu_engine_new (FU_APP_FLAGS_NONE); + g_autoptr(XbSilo) silo_empty = xb_silo_new (); + + /* no metadata in daemon */ + fu_engine_set_silo (engine, silo_empty); + + /* add child */ + fu_device_set_id (device1, "child1"); + fu_device_set_name (device1, "Child1"); + fu_device_set_physical_id (device1, "child-ID1"); + fu_device_add_vendor_id (device1, "USB:FFFF"); + fu_device_add_protocol (device1, "com.acme"); + fu_device_add_instance_id (device1, "child-GUID-1"); + fu_device_add_parent_physical_id (device1, "parent-ID-notfound"); + fu_device_add_parent_physical_id (device1, "parent-ID"); + fu_device_convert_instance_ids (device1); + fu_engine_add_device (engine, device1); + + /* parent */ + fu_device_set_id (device2, "parent"); + fu_device_set_name (device2, "Parent"); + fu_device_set_physical_id (device2, "parent-ID"); + fu_device_add_vendor_id (device2, "USB:FFFF"); + fu_device_add_protocol (device2, "com.acme"); + fu_device_add_instance_id (device2, "parent-GUID"); + fu_device_set_vendor (device2, "oem"); + fu_device_add_internal_flag (device2, FU_DEVICE_INTERNAL_FLAG_AUTO_PARENT_CHILDREN); + fu_device_convert_instance_ids (device2); + + /* add another child */ + fu_device_set_id (device3, "child2"); + fu_device_set_name (device3, "Child2"); + fu_device_set_physical_id (device3, "child-ID2"); + fu_device_add_instance_id (device3, "child-GUID-2"); + fu_device_add_parent_physical_id (device3, "parent-ID"); + fu_device_convert_instance_ids (device3); + fu_device_add_child (device2, device3); + + /* add two together */ + fu_engine_add_device (engine, device2); + + /* add non-child */ + fu_device_set_id (device4, "child4"); + fu_device_set_name (device4, "Child4"); + fu_device_set_physical_id (device4, "child-ID4"); + fu_device_add_vendor_id (device4, "USB:FFFF"); + fu_device_add_protocol (device4, "com.acme"); + fu_device_add_instance_id (device4, "child-GUID-4"); + fu_device_add_parent_physical_id (device4, "parent-ID"); + fu_device_convert_instance_ids (device4); + fu_engine_add_device (engine, device4); + + /* this is normally done by fu_plugin_device_add() */ + fu_engine_add_device (engine, device4); + + /* verify both children were adopted */ + g_assert (fu_device_get_parent (device3) == device2); + g_assert (fu_device_get_parent (device4) == device2); + g_assert (fu_device_get_parent (device1) == device2); + g_assert_cmpstr (fu_device_get_vendor (device3), ==, "oem"); +} + static void fu_engine_partial_hash_func (gconstpointer user_data) { @@ -3246,8 +3315,10 @@ main (int argc, char **argv) fu_engine_requirements_device_plain_func); g_test_add_data_func ("/fwupd/engine{requirements-version-format}", self, fu_engine_requirements_version_format_func); - g_test_add_data_func ("/fwupd/engine{device-auto-parent}", self, - fu_engine_device_parent_func); + g_test_add_data_func ("/fwupd/engine{device-auto-parent-id}", self, + fu_engine_device_parent_id_func); + g_test_add_data_func ("/fwupd/engine{device-auto-parent-guid}", self, + fu_engine_device_parent_guid_func); g_test_add_data_func ("/fwupd/engine{install-duration}", self, fu_engine_install_duration_func); g_test_add_data_func ("/fwupd/engine{generate-md}", self,