On Wed, Jun 22, 2016 at 09:20:36AM -0700, Jason Gerecke wrote:
> On 06/21/2016 06:47 PM, Peter Hutterer wrote:
> > On Tue, Jun 21, 2016 at 03:52:53PM -0700, Jason Gerecke wrote:
> >> Providing the name of a tablet to show-svg-image can be difficult at
> >> times, since the name may not be exactly what you expect or there may
> >> be more than one device which shares the same name. To make its use a
> >> little easier, allow it to take the device ID of the device to display
> >> as a command line argument.
> >>
> >> Signed-off-by: Jason Gerecke <jason.gere...@wacom.com>
> >> ---
> >> tools/show-svg-image.c | 74
> >> ++++++++++++++++++++++++++++++++++++++++++++------
> >> 1 file changed, 66 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/tools/show-svg-image.c b/tools/show-svg-image.c
> >> index 674d27e..152f71f 100644
> >> --- a/tools/show-svg-image.c
> >> +++ b/tools/show-svg-image.c
> >> @@ -355,16 +355,20 @@ main (int argc, char **argv)
> >> GdkWindow *gdk_win;
> >> GdkColor black;
> >> char *tabletname;
> >> + char *tabletid;
> >> const char *filename;
> >> GOptionEntry
> >> options[] = {
> >> - {"tablet", 't', 0, G_OPTION_ARG_STRING, &tabletname,
> >> "Name of the tablet to show", "<string>"},
> >> + {"tablet", 't', 0, G_OPTION_ARG_STRING, &tabletname,
> >> "Name of the tablet to show", "[string]"},
> >> + {"id", 'i', 0, G_OPTION_ARG_STRING, &tabletid, "ID of
> >> the tablet to show", "[string]"},
> >> {NULL}
> >> };
> >>
> >> handle = NULL;
> >> error = NULL;
> >> + device = NULL;
> >> tabletname = NULL;
> >> + tabletid = NULL;
> >>
> >> context = g_option_context_new ("- libwacom tablet viewer");
> >> g_option_context_add_main_entries (context, options, NULL);
> >> @@ -374,8 +378,12 @@ main (int argc, char **argv)
> >> g_option_context_free (context);
> >>
> >> gtk_init (&argc, &argv);
> >> - if (tabletname == NULL) {
> >> - g_warning ("No tablet name provided, exiting");
> >> + if (tabletname == NULL && tabletid == NULL) {
> >> + g_warning ("Tablet name or ID must be provided, exiting");
> >> + return 1;
> >> + }
> >> + if (tabletname != NULL && tabletid != NULL) {
> >> + g_warning ("Tablet name or ID must be provided, exiting");
> >> return 1;
> >> }
> >> db = libwacom_database_new_for_path(TOPSRCDIR"/data");
> >> @@ -383,15 +391,65 @@ main (int argc, char **argv)
> >> g_warning ("Failed to load libwacom database, exiting");
> >> return 1;
> >> }
> >> - device = libwacom_new_from_name(db, tabletname, NULL);
> >> - if (!device) {
> >> - g_warning ("Device '%s' not found in libwacom database,
> >> exiting", tabletname);
> >> - return 1;
> >> + if (tabletname != NULL) {
> >> + device = libwacom_new_from_name (db, tabletname, NULL);
> >> + if (!device) {
> >> + g_warning ("Device '%s' not found in libwacom database,
> >> exiting", tabletname);
> >> + return 1;
> >> + }
> >> + }
> >> + else if (tabletid != NULL) {
> >> + WacomDevice **devices;
> >> + WacomDevice **d;
> >> + WacomBusType bustype;
> >> + char **tabletid_token;
> >> + int vid;
> >> + int pid;
> >> +
> >> + tabletid_token = g_strsplit (tabletid, ":", -1);
> >
> > sscanf(tabletid, "%s:%x:%x") should work here too? seems easier than
> > tokenising
> >
>
> Changed locally.
>
> >> + if (g_strv_length (tabletid_token) != 3)
> >> + g_warning ("Incorrectly formatted tablet id, exiting.");
> >> +
> >> + if (g_ascii_strcasecmp ("", tabletid_token[0]) == 0)
> >> + bustype = WBUSTYPE_UNKNOWN;
> >> + else if (g_ascii_strcasecmp ("usb", tabletid_token[0]) == 0)
> >> + bustype = WBUSTYPE_USB;
> >> + else if (g_ascii_strcasecmp ("serial", tabletid_token[0]) == 0)
> >> + bustype = WBUSTYPE_SERIAL;
> >> + else if (g_ascii_strcasecmp ("bluetooth", tabletid_token[0]) ==
> >> 0)
> >> + bustype = WBUSTYPE_BLUETOOTH;
> >> + else if (g_ascii_strcasecmp ("i2c", tabletid_token[0]) == 0)
> >> + bustype = WBUSTYPE_I2C;
> >> + else {
> >> + g_warning ("Unknown bus type, exiting.");
> >> + return 1;
> >> + }
> >> +
> >> + vid = (int)g_ascii_strtoll (tabletid_token[1], NULL, 16);
> >> + pid = (int)g_ascii_strtoll (tabletid_token[2], NULL, 16);
> >> +
> >> + g_strfreev (tabletid_token);
> >> +
> >> + devices = libwacom_list_devices_from_database (db, NULL);
> >> + d = devices;
> >> + while (*d && !device) {
> >> + if (libwacom_get_vendor_id (*d) == vid &&
> >> + libwacom_get_product_id (*d) == pid &&
> >> + (libwacom_get_bustype (*d) == bustype || bustype ==
> >> WBUSTYPE_UNKNOWN))
> >> + device = *d;
> >> + d++;
> >> + }
> >> + free (devices);
> >
> > tbh, that looks like something we should have a libwacom_new_from_id() for.
> > we have from_usbid which has the obvious drawback but either way that seems
> > like something useful to have around.
> >
>
> I was a bit hesitant to update the API since a thin wrapper around
> 'libwacom_new' (which already takes vid/pid/bus arguments) seemed so
> obvious that I suspected there may be a reason it didn't exist. I could
> certainly either introduce one or corresponding
> serialid/i2cid/bluetoothid functions.
I *think* the reason it doesn't exist is because we had the new_by_usbid and
then the bluetooth/i2c tablets came later and we never needed the matching
call. I'd say rather than adding more bus-specific ones we should deprecate
the usbid one and add one that takes the bus ID as argument.
Cheers,
Peter
> Let me know which you would prefer and I'll send some patches.
>
> > and the last nit: can we please split this block into a helper function.
> > main() is already quite unwieldly
> >
>
> Changed locally.
>
> > I've pushed the other patches, thanks.
> >
> > Cheers,
> > Peter
> >
>
> Thanks.
>
> 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....
>
> >> +
> >> + if (!device) {
> >> + g_warning ("Device ID '%s' not found in libwacom
> >> database, exiting", tabletid);
> >> + return 1;
> >> + }
> >> }
> >>
> >> filename = libwacom_get_layout_filename(device);
> >> if (filename == NULL) {
> >> - g_warning ("Device '%s' has no layout available, exiting",
> >> tabletname);
> >> + g_warning ("Device '%s' has no layout available, exiting",
> >> libwacom_get_name(device));
> >> return 1;
> >> }
> >> handle = rsvg_handle_new_from_file (filename, &error);
> >> --
> >> 2.8.3
> >>
------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
