On Mon, Sep 15, 2014 at 12:27 AM, Peter Hutterer
<[email protected]> wrote:
> On Thu, Sep 11, 2014 at 03:25:45PM -0400, Benjamin Tissoires wrote:
>> PRODUCT attribute is set correctly for the input device, whether the
>> device is USB or Bluetooth. We can use a common path for those two.
>> uinput devices have their PRODUCT attribute set according to the
>> registered device, so they work too.
>>
>> This change allows libwacom to properly detect the actual PID of the
>> tablet connected through the wireless receiver when the following kernel
>> patch (or its sucessor) will be merged in 3.18:
>> https://patchwork.kernel.org/patch/4889811/
>>
>> Signed-off-by: Benjamin Tissoires <[email protected]>
>> ---
>>
>> Changes in v3:
>> - renamed "bail" to "out" as suggested by Bastien
>>
>> Changes in v2:
>> - use g_strsplit and strtol/strtoul as suggested by Bastien
>>
>> libwacom/libwacom.c | 140
>> +++++++++++++++++++++++++++-------------------------
>> 1 file changed, 72 insertions(+), 68 deletions(-)
>>
>> diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c
>> index 2a99c95..6594cf5 100644
>> --- a/libwacom/libwacom.c
>> +++ b/libwacom/libwacom.c
>> @@ -88,6 +88,67 @@ get_uinput_subsystem (GUdevDevice *device)
>> return bus_str ? g_strdup (bus_str) : NULL;
>> }
>>
>> +static gboolean
>> +get_bus_vid_pid (GUdevDevice *device,
>> + WacomBusType *bus,
>> + int *vendor_id,
>> + int *product_id,
>> + WacomError *error)
>> +{
>> + GUdevDevice *parent;
>> + const char *product_str;
>> + gchar **splitted_product = NULL;
>> + unsigned int bus_id;
>> + gboolean retval = FALSE;
>
> libwacom.c:102:11: warning: variable 'retval' set but not used
> [-Wunused-but-set-variable]
>
>
>> +
>> + /* Parse that:
>> + * E: PRODUCT=5/56a/81/100
>> + * into:
>> + * vendor 0x56a
>> + * product 0x81 */
>> + parent = g_object_ref (device);
>> + product_str = g_udev_device_get_property (device, "PRODUCT");
>> +
>> + while (!product_str && parent) {
>> + GUdevDevice *old_parent = parent;
>> + parent = g_udev_device_get_parent (old_parent);
>> + if (parent)
>> + product_str = g_udev_device_get_property (parent,
>> "PRODUCT");
>> + g_object_unref (old_parent);
>> + }
>> +
>> + if (!product_str)
>> + /* PRODUCT not found, hoping the old method will work */
>> + goto out;
>> +
>> + splitted_product = g_strsplit (product_str, "/", 4);
>> + if (g_strv_length (splitted_product) != 4) {
>> + libwacom_error_set(error, WERROR_UNKNOWN_MODEL, "Unable to
>> parse model identification");
>> + goto out;
>> + }
>> +
>> + bus_id = (int)strtoul (splitted_product[0], NULL, 10);
>> + *vendor_id = (int)strtol (splitted_product[1], NULL, 16);
>> + *product_id = (int)strtol (splitted_product[2], NULL, 16);
>> +
>> + switch (bus_id) {
>> + case 3:
>> + *bus = WBUSTYPE_USB;
>> + retval = TRUE;
>> + break;
>> + case 5:
>> + *bus = WBUSTYPE_BLUETOOTH;
>> + retval = TRUE;
>> + break;
>> + }
>> +
>> +out:
>> + if (splitted_product)
>> + g_strfreev (splitted_product);
>> + g_object_unref (parent);
>> + return TRUE;
>
> I guess you meant "return retval" here?
oops, yes.
>
>> +}
>> +
>> static char *
>> get_bus (GUdevDevice *device)
>> {
>> @@ -167,8 +228,6 @@ get_device_info (const char *path,
>> g_object_unref (parent);
>> }
>>
>> - bus_str = get_bus (device);
>> -
>> /* Is the device integrated in display? */
>> devname = g_udev_device_get_name (device);
>> if (devname != NULL) {
>> @@ -206,78 +265,23 @@ get_device_info (const char *path,
>> g_object_unref (parent);
>> }
>>
>> - *bus = bus_from_str (bus_str);
>> - if (*bus == WBUSTYPE_USB) {
>> - const char *vendor_str, *product_str;
>> - GUdevDevice *parent;
>> -
>> - vendor_str = g_udev_device_get_property (device,
>> "ID_VENDOR_ID");
>> - product_str = g_udev_device_get_property (device,
>> "ID_MODEL_ID");
>> -
>> - parent = NULL;
>> - /* uinput devices have the props set on the parent, there is
>> - * nothing a a udev rule can match on on the child */
>> - if (!vendor_str || !product_str) {
>> - parent = g_udev_device_get_parent (device);
>> - if (parent) {
>> - vendor_str = g_udev_device_get_property
>> (parent, "ID_VENDOR_ID");
>> - product_str = g_udev_device_get_property
>> (parent, "ID_MODEL_ID");
>> - }
>> - }
>> -
>> - *vendor_id = strtol (vendor_str, NULL, 16);
>> - *product_id = strtol (product_str, NULL, 16);
>> -
>> - if (parent)
>> - g_object_unref (parent);
>> - } else if (*bus == WBUSTYPE_BLUETOOTH || *bus == WBUSTYPE_SERIAL) {
>> - GUdevDevice *parent;
>> - const char *product_str;
>> - int garbage;
>> -
>> - /* Parse that:
>> - * E: PRODUCT=5/56a/81/100
>> - * into:
>> - * vendor 0x56a
>> - * product 0x81 */
>> - parent = g_object_ref (device);
>> - product_str = g_udev_device_get_property (device, "PRODUCT");
>> -
>> - while (!product_str && parent) {
>> - GUdevDevice *old_parent = parent;
>> - parent = g_udev_device_get_parent (old_parent);
>> - if (parent)
>> - product_str = g_udev_device_get_property
>> (parent, "PRODUCT");
>> - g_object_unref (old_parent);
>> - }
>> + /* Parse the PRODUCT attribute (for Bluetooth and USB) */
>> + retval = get_bus_vid_pid (device, bus, vendor_id, product_id, error);
>> + if (retval)
>> + goto bail;
>
> see Bastien's earlier comment, probably best to rename that to out: for
> consistency.
>
v4 on its way!
thanks for the review.
Cheers,
Benjamin
> Cheers,
> Peter
>
>>
>> - if (product_str) {
>> - if (sscanf(product_str, "%d/%x/%x/%d", &garbage,
>> vendor_id, product_id, &garbage) != 4) {
>> - libwacom_error_set(error,
>> WERROR_UNKNOWN_MODEL, "Unable to parse model identification");
>> - g_object_unref(parent);
>> - goto bail;
>> - }
>> - } else {
>> - g_assert(*bus == WBUSTYPE_SERIAL);
>> + bus_str = get_bus (device);
>> + *bus = bus_from_str (bus_str);
>>
>> - *vendor_id = 0;
>> - *product_id = 0;
>> - }
>> - if (parent)
>> - g_object_unref (parent);
>> + if (*bus == WBUSTYPE_SERIAL) {
>> + /* The serial bus uses 0:0 as the vid/pid */
>> + *vendor_id = 0;
>> + *product_id = 0;
>> + retval = TRUE;
>> } else {
>> libwacom_error_set(error, WERROR_UNKNOWN_MODEL, "Unsupported
>> bus '%s'", bus_str);
>> - goto bail;
>> }
>>
>> - if (*bus != WBUSTYPE_UNKNOWN &&
>> - vendor_id != 0 &&
>> - product_id != 0)
>> - retval = TRUE;
>> - /* The serial bus uses 0:0 as the vid/pid */
>> - if (*bus == WBUSTYPE_SERIAL)
>> - retval = TRUE;
>> -
>> bail:
>> if (bus_str != NULL)
>> g_free (bus_str);
>> --
>> 2.1.0
>>
>>
>> ------------------------------------------------------------------------------
>> Want excitement?
>> Manually upgrade your production database.
>> When you want reliability, choose Perforce
>> Perforce version control. Predictably reliable.
>> http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Linuxwacom-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>>
>
> ------------------------------------------------------------------------------
> Want excitement?
> Manually upgrade your production database.
> When you want reliability, choose Perforce
> Perforce version control. Predictably reliable.
> http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
> _______________________________________________
> Linuxwacom-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
