device: get vendor/product IDs

And don't do it in MMPlugin.

3 files changed, 115 insertions, 126 deletions

diff --git a/src/mm-device.c b/src/mm-device.c
index 1ba0cd6b..6a947c3a 100644
--- a/src/mm-device.c
+++ b/src/mm-device.c
@@ -23,7 +23,7 @@
 #include <mm-errors-types.h>
 
 #include "mm-device.h"
-
+#include "mm-utils.h"
 #include "mm-log.h"
 
 G_DEFINE_TYPE (MMDevice, mm_device, G_TYPE_OBJECT);
@@ -49,6 +49,8 @@ struct _MMDevicePrivate {
     /* Parent UDev device */
     GUdevDevice *udev_device;
     gchar *udev_device_path;
+    guint16 vendor;
+    guint16 product;
 
     /* Kernel driver managing this device */
     gchar *driver;
@@ -92,6 +94,84 @@ mm_device_owns_port (MMDevice    *self,
     return !!device_find_probe_with_device (self, udev_port);
 }
 
+static gboolean
+get_device_ids (GUdevDevice *device,
+                guint16     *vendor,
+                guint16     *product)
+{
+    GUdevDevice *parent = NULL;
+    const gchar *vid = NULL, *pid = NULL, *parent_subsys;
+    gboolean success = FALSE;
+
+    parent = g_udev_device_get_parent (device);
+    if (parent) {
+        parent_subsys = g_udev_device_get_subsystem (parent);
+        if (parent_subsys) {
+            if (g_str_equal (parent_subsys, "bluetooth")) {
+                /* Bluetooth devices report the VID/PID of the BT adapter here,
+                 * which isn't really what we want.  Just return null IDs instead.
+                 */
+                success = TRUE;
+                goto out;
+            } else if (g_str_equal (parent_subsys, "pcmcia")) {
+                /* For PCMCIA devices we need to grab the PCMCIA subsystem's
+                 * manfid and cardid, since any IDs on the tty device itself
+                 * may be from PCMCIA controller or something else.
+                 */
+                vid = g_udev_device_get_sysfs_attr (parent, "manf_id");
+                pid = g_udev_device_get_sysfs_attr (parent, "card_id");
+                if (!vid || !pid)
+                    goto out;
+            } else if (g_str_equal (parent_subsys, "platform")) {
+                /* Platform devices don't usually have a VID/PID */
+                success = TRUE;
+                goto out;
+            }
+        }
+    }
+
+    if (!vid)
+        vid = g_udev_device_get_property (device, "ID_VENDOR_ID");
+    if (!vid)
+        goto out;
+
+    if (strncmp (vid, "0x", 2) == 0)
+        vid += 2;
+    if (strlen (vid) != 4)
+        goto out;
+
+    if (vendor) {
+        *vendor = (guint16) (utils_hex2byte (vid + 2) & 0xFF);
+        *vendor |= (guint16) ((utils_hex2byte (vid) & 0xFF) << 8);
+    }
+
+    if (!pid)
+        pid = g_udev_device_get_property (device, "ID_MODEL_ID");
+    if (!pid) {
+        *vendor = 0;
+        goto out;
+    }
+
+    if (strncmp (pid, "0x", 2) == 0)
+        pid += 2;
+    if (strlen (pid) != 4) {
+        *vendor = 0;
+        goto out;
+    }
+
+    if (product) {
+        *product = (guint16) (utils_hex2byte (pid + 2) & 0xFF);
+        *product |= (guint16) ((utils_hex2byte (pid) & 0xFF) << 8);
+    }
+
+    success = TRUE;
+
+out:
+    if (parent)
+        g_object_unref (parent);
+    return success;
+}
+
 static gchar *
 get_driver_name (GUdevDevice *device)
 {
@@ -132,9 +212,17 @@ mm_device_grab_port (MMDevice    *self,
     if (mm_device_owns_port (self, udev_port))
         return;
 
-    /* Get the driver name out of the first port grabbed */
-    if (!self->priv->port_probes)
+    /* Get the driver name and vendor/product IDs out of the first port
+     * grabbed */
+    if (!self->priv->port_probes) {
         self->priv->driver = get_driver_name (udev_port);
+        if (!get_device_ids (udev_port,
+                             &self->priv->vendor,
+                             &self->priv->product)) {
+            mm_dbg ("(%s) could not get vendor/product ID",
+                    self->priv->udev_device_path);
+        }
+    }
 
     /* Create and store new port probe */
     probe = mm_port_probe_new (udev_port,
@@ -322,6 +410,18 @@ mm_device_get_driver (MMDevice *self)
     return self->priv->driver;
 }
 
+guint16
+mm_device_get_vendor (MMDevice *self)
+{
+    return self->priv->vendor;
+}
+
+guint16
+mm_device_get_product (MMDevice *self)
+{
+    return self->priv->product;
+}
+
 GUdevDevice *
 mm_device_peek_udev_device (MMDevice *self)
 {
diff --git a/src/mm-device.h b/src/mm-device.h
index 97289dda..5c6acba4 100644
--- a/src/mm-device.h
+++ b/src/mm-device.h
@@ -74,6 +74,8 @@ void     mm_device_remove_modem (MMDevice  *self);
 
 const gchar *mm_device_get_path         (MMDevice *self);
 const gchar *mm_device_get_driver       (MMDevice *self);
+guint16      mm_device_get_vendor       (MMDevice *self);
+guint16      mm_device_get_product      (MMDevice *self);
 GUdevDevice *mm_device_peek_udev_device (MMDevice *self);
 GUdevDevice *mm_device_get_udev_device  (MMDevice *self);
 void         mm_device_set_plugin       (MMDevice *self,
diff --git a/src/mm-plugin.c b/src/mm-plugin.c
index 01d5e22a..a638ca9b 100644
--- a/src/mm-plugin.c
+++ b/src/mm-plugin.c
@@ -35,7 +35,6 @@
 #include "mm-qcdm-serial-port.h"
 #include "mm-serial-parsers.h"
 #include "mm-marshal.h"
-#include "mm-utils.h"
 #include "mm-private-boxed-types.h"
 #include "libqcdm/src/commands.h"
 #include "libqcdm/src/utils.h"
@@ -89,104 +88,6 @@ enum {
 
 /*****************************************************************************/
 
-static gboolean
-get_device_ids (MMPlugin *self,
-                const char *subsys,
-                const char *name,
-                guint16 *vendor,
-                guint16 *product)
-{
-    MMPluginPrivate *priv;
-    GUdevDevice *device = NULL, *parent = NULL;
-    const char *vid = NULL, *pid = NULL, *parent_subsys;
-    gboolean success = FALSE;
-
-    g_return_val_if_fail (self != NULL, FALSE);
-    g_return_val_if_fail (MM_IS_PLUGIN (self), FALSE);
-    g_return_val_if_fail (subsys != NULL, FALSE);
-    g_return_val_if_fail (name != NULL, FALSE);
-    if (vendor)
-        g_return_val_if_fail (*vendor == 0, FALSE);
-    if (product)
-        g_return_val_if_fail (*product == 0, FALSE);
-
-    priv = MM_PLUGIN_GET_PRIVATE (self);
-
-    device = g_udev_client_query_by_subsystem_and_name (priv->client, subsys, name);
-    if (!device)
-        goto out;
-
-    parent = g_udev_device_get_parent (device);
-    if (parent) {
-        parent_subsys = g_udev_device_get_subsystem (parent);
-        if (parent_subsys) {
-            if (!strcmp (parent_subsys, "bluetooth")) {
-                /* Bluetooth devices report the VID/PID of the BT adapter here,
-                 * which isn't really what we want.  Just return null IDs instead.
-                 */
-                success = TRUE;
-                goto out;
-            } else if (!strcmp (parent_subsys, "pcmcia")) {
-                /* For PCMCIA devices we need to grab the PCMCIA subsystem's
-                 * manfid and cardid, since any IDs on the tty device itself
-                 * may be from PCMCIA controller or something else.
-                 */
-                vid = g_udev_device_get_sysfs_attr (parent, "manf_id");
-                pid = g_udev_device_get_sysfs_attr (parent, "card_id");
-                if (!vid || !pid)
-                    goto out;
-            } else if (!strcmp (parent_subsys, "platform")) {
-                /* Platform devices don't usually have a VID/PID */
-                success = TRUE;
-                goto out;
-            }
-        }
-    }
-
-    if (!vid)
-        vid = g_udev_device_get_property (device, "ID_VENDOR_ID");
-    if (!vid)
-        goto out;
-
-    if (strncmp (vid, "0x", 2) == 0)
-        vid += 2;
-    if (strlen (vid) != 4)
-        goto out;
-
-    if (vendor) {
-        *vendor = (guint16) (utils_hex2byte (vid + 2) & 0xFF);
-        *vendor |= (guint16) ((utils_hex2byte (vid) & 0xFF) << 8);
-    }
-
-    if (!pid)
-        pid = g_udev_device_get_property (device, "ID_MODEL_ID");
-    if (!pid) {
-        *vendor = 0;
-        goto out;
-    }
-
-    if (strncmp (pid, "0x", 2) == 0)
-        pid += 2;
-    if (strlen (pid) != 4) {
-        *vendor = 0;
-        goto out;
-    }
-
-    if (product) {
-        *product = (guint16) (utils_hex2byte (pid + 2) & 0xFF);
-        *product |= (guint16) ((utils_hex2byte (pid) & 0xFF) << 8);
-    }
-
-    success = TRUE;
-
-out:
-    if (device)
-        g_object_unref (device);
-    if (parent)
-        g_object_unref (parent);
-    return success;
-}
-
 static char *
 get_key (const char *subsys, const char *name)
 {
@@ -249,16 +150,11 @@ apply_pre_probing_filters (MMPlugin *self,
                            gboolean *need_product_probing)
 {
     MMPluginPrivate *priv = MM_PLUGIN_GET_PRIVATE (self);
-    guint16 vendor = 0;
-    guint16 product = 0;
+    guint16 vendor;
+    guint16 product;
     gboolean product_filtered = FALSE;
     gboolean vendor_filtered = FALSE;
     guint i;
-    const gchar *subsys;
-    const gchar *name;
-
-    subsys = g_udev_device_get_subsystem (port);
-    name = g_udev_device_get_name (port);
 
     *need_vendor_probing = FALSE;
     *need_product_probing = FALSE;
@@ -266,6 +162,9 @@ apply_pre_probing_filters (MMPlugin *self,
     /* The plugin may specify that only some subsystems are supported. If that
      * is the case, filter by subsystem */
     if (priv->subsystems) {
+        const gchar *subsys;
+
+        subsys = g_udev_device_get_subsystem (port);
         for (i = 0; priv->subsystems[i]; i++) {
             if (g_str_equal (subsys, priv->subsystems[i]))
                 break;
@@ -282,7 +181,7 @@ apply_pre_probing_filters (MMPlugin *self,
         const gchar *driver;
 
         /* Detect any modems accessible through the list of virtual ports */
-        driver = (is_virtual_port (name) ?
+        driver = (is_virtual_port (g_udev_device_get_name (port)) ?
                   "virtual" :
                   mm_device_get_driver (device));
 
@@ -300,7 +199,8 @@ apply_pre_probing_filters (MMPlugin *self,
             return TRUE;
     }
 
-    get_device_ids (self, subsys, name, &vendor, &product);
+    vendor = mm_device_get_vendor (device);
+    product = mm_device_get_product (device);
 
     /* The plugin may specify that only some vendor IDs are supported. If that
      * is the case, filter by vendor ID. */
@@ -714,29 +614,16 @@ mm_plugin_create_modem (MMPlugin  *self,
     MMDevice *device = MM_DEVICE (device_o);
     MMPluginPrivate *priv = MM_PLUGIN_GET_PRIVATE (self);
     MMBaseModem *modem = NULL;
-    const gchar *name, *subsys;
-    guint16 vendor = 0, product = 0;
     GList *port_probes, *l;
 
-    /* Get info from the first probe in the list */
     port_probes = mm_device_peek_port_probe_list (device);
-    subsys = mm_port_probe_get_port_subsys (port_probes->data);
-    name = mm_port_probe_get_port_name (port_probes->data);
-
-    /* Vendor and Product IDs are really optional, we'll just warn if they
-     * cannot get loaded.
-     *
-     * TODO: load them in MMDevice once
-     **/
-    if (!get_device_ids (MM_PLUGIN (self), subsys, name, &vendor, &product))
-        mm_warn ("Could not get modem vendor/product ID");
 
     /* Let the plugin create the modem from the port probe results */
     modem = MM_PLUGIN_GET_CLASS (self)->create_modem (MM_PLUGIN (self),
                                                       mm_device_get_path (device),
                                                       mm_device_get_driver (device),
-                                                      vendor,
-                                                      product,
+                                                      mm_device_get_vendor (device),
+                                                      mm_device_get_product (device),
                                                       port_probes,
                                                       error);
     if (modem) {