Re: [PATCH v2] HID: wacom: retrieve name from HID descriptor for generic devices

[Jason Gerecke <killertofu@gmail.com>]

On 4/20/2015 2:09 PM, Ping Cheng wrote:
> HID generic devices share the same default name, "Wacom HID". This
> causes userland programs to show same device names for different
> devices, which would confuse end users with same device names for
> different devices too.
> 
> This patch uses name retrieved from HID descriptor, if a meaningful
> name is reported. Otherwise, affix its product ID to "Wacom HID".
> 
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
> v2: updated with Jason's pid affix and extract whitespace suggestions.
> ---
>  drivers/hid/wacom_sys.c | 53 +++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 1b00d8d..a857cd2 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -1392,6 +1392,47 @@ static size_t wacom_compute_pktlen(struct hid_device *hdev)
>  	return size;
>  }
>  
> +static void wacom_update_name(struct wacom *wacom)
> +{
> +	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
> +	struct wacom_features *features = &wacom_wac->features;
> +
> +	/* Generic devices name unspecified */
> +	if ((features->type == HID_GENERIC) && !strcmp("Wacom HID", features->name)) {
> +		if (strstr(wacom->hdev->name, "Wacom") ||
> +		    strstr(wacom->hdev->name, "wacom") ||
> +		    strstr(wacom->hdev->name, "WACOM")) {
> +			/* name is in HID descriptor, use it */
> +			strlcpy(wacom_wac->name, wacom->hdev->name,
> +				sizeof(wacom_wac->name));
> +
> +			/* strip out excess whitespaces */
> +			while (1) {
> +				char *gap = strstr(wacom_wac->name, "  ");
> +				if (gap == NULL)
> +	 				break;
> +				memmove(gap, gap+1, strlen(gap));
> +			}

Alas, my snippet doesn't work as intended when placed here. It will
remove extra internal whitespace without issue, but will leave a single
trailing space at the end of the string if there were one or more spaces
there originally. Because of this, a double space can be re-introduced
once you append the e.g. " Pen" suffix below. I suggested the loop be
placed at the bottom of the function because of this.

Alternatively, you could put a call to 'strim' immediately before or
after the loop to get rid of the extra trailing space (though I don't
particularly like how it smells):

    memmove(wacom_wac->name, strim(wacom_wac->name), \
            strlen(wacom_wac->name)+1);

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....

> +		} else {
> +			/* no meaningful name retrieved. use product ID */
> +			snprintf(wacom_wac->name, sizeof(wacom_wac->name),
> +				 "%s %x", features->name, wacom->hdev->product);
> +		}
> +	} else {
> +		strlcpy(wacom_wac->name, features->name, sizeof(wacom_wac->name));
> +	}
> +	snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
> +		"%s Pad", wacom_wac->name);
> +
> +	/* Append the device type to the name */
> +	if (features->device_type != BTN_TOOL_FINGER)
> +		strlcat(wacom_wac->name, " Pen", WACOM_NAME_MAX);
> +	else if (features->touch_max)
> +		strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
> +	else
> +		strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
> +}
> +
>  static int wacom_probe(struct hid_device *hdev,
>  		const struct hid_device_id *id)
>  {
> @@ -1517,17 +1558,7 @@ static int wacom_probe(struct hid_device *hdev,
>  	}
>  	wacom_calculate_res(features);
>  
> -	strlcpy(wacom_wac->name, features->name, sizeof(wacom_wac->name));
> -	snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
> -		"%s Pad", features->name);
> -
> -	/* Append the device type to the name */
> -	if (features->device_type != BTN_TOOL_FINGER)
> -		strlcat(wacom_wac->name, " Pen", WACOM_NAME_MAX);
> -	else if (features->touch_max)
> -		strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
> -	else
> -		strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
> +	wacom_update_name(wacom);
>  
>  	error = wacom_add_shared_data(hdev);
>  	if (error)
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html