Allow different plugins to add the same device

In this instance, we define the 'same device' to be a FuDevice that has at
least one matching GUID. We allow the plugins to define which one is 'better'
than other plugins, and use this to only have one FuDevice for the physical
device.

Alternative to https://github.com/hughsie/fwupd/pull/604

src/fu-device-list.c

@@ -436,6 +436,33 @@ fu_device_list_add (FuDeviceList *self, FuDevice *device)
 		return;
 	}
 
+	/* added the same device from a different plugin */
+	if (item != NULL && g_strcmp0 (fu_device_get_plugin (item->device),
+				       fu_device_get_plugin (device)) != 0) {
+		if (fu_device_get_priority (device) < fu_device_get_priority (item->device)) {
+			g_debug ("ignoring device %s [%s] as better device %s [%s] already exists",
+				 fu_device_get_id (device),
+				 fu_device_get_plugin (device),
+				 fu_device_get_id (item->device),
+				 fu_device_get_plugin (item->device));
+			return;
+		}
+		if (fu_device_get_priority (device) == fu_device_get_priority (item->device)) {
+			g_warning ("ignoring device %s [%s] existing device %s [%s] already exists",
+				   fu_device_get_id (device),
+				   fu_device_get_plugin (device),
+				   fu_device_get_id (item->device),
+				   fu_device_get_plugin (item->device));
+			return;
+		}
+		g_debug ("removing device %s [%s] as better device %s [%s] added",
+			 fu_device_get_id (item->device),
+			 fu_device_get_plugin (item->device),
+			 fu_device_get_id (device),
+			 fu_device_get_plugin (device));
+		fu_device_list_remove (self, item->device);
+	}
+
 	/* add helper */
 	item = g_new0 (FuDeviceItem, 1);
 	item->self = self; /* no ref */

src/fu-device-private.h

@@ -18,6 +18,9 @@ gboolean	 fu_device_has_parent_guid		(FuDevice	*device,
 guint		 fu_device_get_order			(FuDevice	*device);
 void		 fu_device_set_order			(FuDevice	*device,
 							 guint		 order);
+guint		 fu_device_get_priority			(FuDevice	*device);
+void		 fu_device_set_priority			(FuDevice	*device,
+							 guint		 priority);
 void		 fu_device_set_alternate		(FuDevice	*device,
 							 FuDevice	*alternate);
 

src/fu-device.c

@@ -39,6 +39,7 @@ typedef struct {
 	FwupdStatus			 status;
 	guint				 progress;
 	guint				 order;
+	guint				 priority;
 } FuDevicePrivate;
 
 enum {
@@ -137,6 +138,39 @@ fu_device_set_order (FuDevice *device, guint order)
 	priv->order = order;
 }
 
+/**
+ * fu_device_get_priority:
+ * @device: a #FuPlugin
+ *
+ * Gets the device priority, where higher numbers are better.
+ *
+ * Returns: the integer value
+ *
+ * Since: 1.1.1
+ **/
+guint
+fu_device_get_priority (FuDevice *device)
+{
+	FuDevicePrivate *priv = fu_device_get_instance_private (device);
+	return priv->priority;
+}
+
+/**
+ * fu_device_set_priority:
+ * @device: a #FuDevice
+ * @priority: a integer value
+ *
+ * Sets the device priority, where higher numbers are better.
+ *
+ * Since: 1.1.1
+ **/
+void
+fu_device_set_priority (FuDevice *device, guint priority)
+{
+	FuDevicePrivate *priv = fu_device_get_instance_private (device);
+	priv->priority = priority;
+}
+
 const gchar *
 fu_device_get_equivalent_id (FuDevice *device)
 {

src/fu-engine.c

@@ -2813,6 +2813,7 @@ fu_engine_plugin_device_added_cb (FuPlugin *plugin,
 				  gpointer user_data)
 {
 	FuEngine *self = (FuEngine *) user_data;
+	fu_device_set_priority (device, fu_plugin_get_priority (plugin));
 	fu_engine_add_device (self, device);
 }
 

src/fu-plugin-list.c

@@ -198,6 +198,36 @@ fu_plugin_list_depsolve (FuPluginList *self, GError **error)
 			}
 		}
 
+		/* set priority as well */
+		for (guint i = 0; i < self->plugins->len; i++) {
+			FuPlugin *plugin = g_ptr_array_index (self->plugins, i);
+			deps = fu_plugin_get_rules (plugin, FU_PLUGIN_RULE_BETTER_THAN);
+			for (guint j = 0; j < deps->len && !changes; j++) {
+				const gchar *plugin_name = g_ptr_array_index (deps, j);
+				dep = fu_plugin_list_find_by_name (self, plugin_name, NULL);
+				if (dep == NULL) {
+					g_debug ("cannot find plugin '%s' "
+						 "referenced by '%s'",
+						 plugin_name,
+						 fu_plugin_get_name (plugin));
+					continue;
+				}
+				if (!fu_plugin_get_enabled (dep))
+					continue;
+				if (fu_plugin_get_priority (plugin) <= fu_plugin_get_priority (dep)) {
+					g_debug ("%s [%u] better than %s [%u] "
+						 "so bumping to [%u]",
+						 fu_plugin_get_name (plugin),
+						 fu_plugin_get_priority (plugin),
+						 fu_plugin_get_name (dep),
+						 fu_plugin_get_priority (dep),
+						 fu_plugin_get_priority (dep) + 1);
+					fu_plugin_set_priority (plugin, fu_plugin_get_priority (dep) + 1);
+					changes = TRUE;
+				}
+			}
+		}
+
 		/* check we're not stuck */
 		if (dep_loop_check++ > 100) {
 			g_set_error_literal (error,

src/fu-plugin-private.h

@@ -35,6 +35,9 @@ void		 fu_plugin_set_smbios			(FuPlugin	*plugin,
 guint		 fu_plugin_get_order			(FuPlugin	*plugin);
 void		 fu_plugin_set_order			(FuPlugin	*plugin,
 							 guint		 order);
+guint		 fu_plugin_get_priority			(FuPlugin	*plugin);
+void		 fu_plugin_set_priority			(FuPlugin	*plugin,
+							 guint		 priority);
 void		 fu_plugin_set_name			(FuPlugin	*plugin,
 							 const gchar 	*name);
 GPtrArray	*fu_plugin_get_rules			(FuPlugin	*plugin,

src/fu-plugin.c

@@ -39,6 +39,7 @@ typedef struct {
 	GUsbContext		*usb_ctx;
 	gboolean		 enabled;
 	guint			 order;
+	guint			 priority;
 	GPtrArray		*rules[FU_PLUGIN_RULE_LAST];
 	gchar			*name;
 	FuHwids			*hwids;
@@ -1475,6 +1476,35 @@ fu_plugin_set_order (FuPlugin *plugin, guint order)
 	priv->order = order;
 }
 
+/**
+ * fu_plugin_get_priority:
+ * @plugin: a #FuPlugin
+ *
+ * Gets the plugin priority, where higher numbers are better.
+ *
+ * Returns: the integer value
+ **/
+guint
+fu_plugin_get_priority (FuPlugin *plugin)
+{
+	FuPluginPrivate *priv = fu_plugin_get_instance_private (plugin);
+	return priv->priority;
+}
+
+/**
+ * fu_plugin_set_priority:
+ * @plugin: a #FuPlugin
+ * @priority: a integer value
+ *
+ * Sets the plugin priority, where higher numbers are better.
+ **/
+void
+fu_plugin_set_priority (FuPlugin *plugin, guint priority)
+{
+	FuPluginPrivate *priv = fu_plugin_get_instance_private (plugin);
+	priv->priority = priority;
+}
+
 /**
  * fu_plugin_add_rule:
  * @plugin: a #FuPlugin

src/fu-plugin.h

@@ -64,6 +64,7 @@ typedef enum {
  * @FU_PLUGIN_RULE_RUN_AFTER:		Order the plugin after another
  * @FU_PLUGIN_RULE_RUN_BEFORE:		Order the plugin before another
  * @FU_PLUGIN_RULE_REQUIRES_QUIRK:	Requires a specific quirk
+ * @FU_PLUGIN_RULE_BETTER_THAN:		Is better than another plugin
  *
  * The rules used for ordering plugins.
  * Plugins are expected to add rules in fu_plugin_initialize().
@@ -73,6 +74,7 @@ typedef enum {
 	FU_PLUGIN_RULE_RUN_AFTER,
 	FU_PLUGIN_RULE_RUN_BEFORE,
 	FU_PLUGIN_RULE_REQUIRES_QUIRK,
+	FU_PLUGIN_RULE_BETTER_THAN,
 	/*< private >*/
 	FU_PLUGIN_RULE_LAST
 } FuPluginRule;

src/fu-self-test.c

@@ -186,6 +186,48 @@ fu_engine_requirements_device_func (void)
 	g_assert (ret);
 }
 
+static void
+fu_engine_device_priority_func (void)
+{
+	g_autoptr(FuDevice) device1 = fu_device_new ();
+	g_autoptr(FuDevice) device2 = fu_device_new ();
+	g_autoptr(FuDevice) device3 = fu_device_new ();
+	g_autoptr(FuDevice) device = NULL;
+	g_autoptr(FuEngine) engine = fu_engine_new (FU_APP_FLAGS_NONE);
+	g_autoptr(GError) error = NULL;
+
+	/* add low prio then high then low */
+	fu_device_set_id (device1, "id1");
+	fu_device_set_priority (device1, 0);
+	fu_device_set_plugin (device1, "udev");
+	fu_device_add_guid (device1, "GUID1");
+	fu_engine_add_device (engine, device1);
+	fu_device_set_id (device2, "id2");
+	fu_device_set_priority (device2, 1);
+	fu_device_set_plugin (device2, "redfish");
+	fu_device_add_guid (device2, "GUID1");
+	fu_engine_add_device (engine, device2);
+	fu_device_set_id (device3, "id3");
+	fu_device_set_priority (device3, 0);
+	fu_device_set_plugin (device3, "uefi");
+	fu_device_add_guid (device3, "GUID1");
+	fu_engine_add_device (engine, device3);
+
+	/* get the high prio device */
+	device = fu_engine_get_device (engine, "867d5f8110f8aa79dd63d7440f21724264f10430", &error);
+	g_assert_no_error (error);
+	g_assert_cmpint (fu_device_get_priority (device), ==, 1);
+
+	/* the now-removed low-prio device */
+	device = fu_engine_get_device (engine, "4e89d81a2e6fb4be2578d245fd8511c1f4ad0b58", &error);
+	g_assert_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND);
+	g_clear_error (&error);
+
+	/* the never-added 2nd low-prio device */
+	device = fu_engine_get_device (engine, "c48feddbbcfee514f530ce8f7f2dccd98b6cc150", &error);
+	g_assert_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND);
+}
+
 static void
 fu_engine_device_parent_func (void)
 {
@@ -2271,6 +2313,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/fwupd/engine{requirements-unsupported}", fu_engine_requirements_unsupported_func);
 	g_test_add_func ("/fwupd/engine{requirements-device}", fu_engine_requirements_device_func);
 	g_test_add_func ("/fwupd/engine{device-auto-parent}", fu_engine_device_parent_func);
+	g_test_add_func ("/fwupd/engine{device-priority}", fu_engine_device_priority_func);
 	g_test_add_func ("/fwupd/hwids", fu_hwids_func);
 	g_test_add_func ("/fwupd/smbios", fu_smbios_func);
 	g_test_add_func ("/fwupd/smbios3", fu_smbios3_func);