On Thu, Jan 14, 2016 at 8:34 PM, Peter Hutterer <peter.hutte...@who-t.net>
wrote:
> On Wed, Jan 13, 2016 at 10:48:31AM -0800, Aaron Skomra wrote:
> > Libwacom_new_from_path() matches the vid/pid from a found device to
> > an entry in the database. Devices with entries in libwacom/data have
> > a name which identifies the physical device (e.g. “Wacom Intuos Pro
> > M”), but the generic/fallback devices have a (sysfs) name with a
> > suffix indicating the part of the device in question (e.g “Wacom
> > Intuos Pro M _Pen_”). When gnome-control-center creates panels for
> > the devices, the names of the related fallback/generic devices don’t
> > match each other, and gcc attempts to create a new panel for each of
> > the Pen, the Pad, etc. because gcc thinks they are separate devices.
> > This results in the Pad “separate” device not being connected to its
> > associated Pen device and the “Map Buttons...” button not being
> > displayed. This patch removes the suffix off of the fallback device
> > name and adds button position necessary for gnome to add the buttons.
> >
> > Signed-off-by: Aaron Skomra <aaron.sko...@wacom.com>
> > ---
> > data/generic.tablet | 6 ++++++
> > libwacom/libwacom.c | 13 +++++++++++--
> > 2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/data/generic.tablet b/data/generic.tablet
> > index 8b7f867..ca91b41 100644
> > --- a/data/generic.tablet
> > +++ b/data/generic.tablet
> > @@ -1,3 +1,6 @@
> > +# Wacom
> > +# Generic/Fallback Tablet
> > +
> > [Device]
> > Name=Generic
> > DeviceMatch=generic
> > @@ -8,3 +11,6 @@ Stylus=true
> > Ring=true
> > NumStrips=2
> > Buttons=4
> > +
> > +[Buttons]
> > +Left=A;B;C;D
> > diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c
> > index d20b66c..7d324fe 100644
> > --- a/libwacom/libwacom.c
> > +++ b/libwacom/libwacom.c
> > @@ -486,7 +486,7 @@ libwacom_new_from_path(const WacomDeviceDatabase
> *db, const char *path, WacomFal
> > const WacomDevice *device;
> > WacomDevice *ret = NULL;
> > WacomIntegrationFlags integration_flags;
> > - char *name, *match_name;
> > + char *name, *match_name, *name_copy;
> >
> > if (!db) {
> > libwacom_error_set(error, WERROR_INVALID_DB, "db is NULL");
> > @@ -521,7 +521,16 @@ libwacom_new_from_path(const WacomDeviceDatabase
> *db, const char *path, WacomFal
> >
> > if (name != NULL) {
> > g_free (ret->name);
> > - ret->name = g_strdup(name);
> > + name_copy = g_strdup(name);
> > +
> > + /* Remove the tool name suffix from the device
> name. */
> > + char * new_end = g_strrstr(name_copy, " ");
> > + new_end[0] = '\0';
>
> two style nits: space before (, though I admit htis is very much all over
> the place in the code. and we don't use ad-hoc declarations, please move
> the
> char *new_end to the top.
>
> > +
> > + ret->name = g_strdup(name_copy);
> > +
> > + new_end[0] = ' ';
> > + g_free (name_copy);
> > }
> > }
>
> I'm not sure we should have this code in libwacom, it would effectively
> become part of the API, I'd rather have this in the caller where adjusting
> to future changes is easier.
Sorry for the delay in responding, I have an excuse, really!
Just wanted to follow up on this to make sure we understand the API concerns
here. It seems like gnome could use something other than name for its
purposes.
Gnome needs to know if items are part of a the same physical device so
it can group them together on a CcWacomPage[1], and it does this with
devices that are in the libwacom database by simply by matching on their
name. (The pen, the pad, and the touch devices all have the same name
coming
from libacom.) The matching fails, though, for the default device's name
because
its name doesn't come from libwacom. You want to leave the name for
default (not
in libwacom) devices as is because the name from X could change and we
don't
want to be responsible for knowing how the name from X will look (or now
libinput I
guess). I'm thinking here about your recent blog post [2].
...Maybe in this case gnome should be matching on VID/PID instead of name?
In that case VID/PID would need to be added to GsdWacomDevicePrivate. I
could work on that, but I already have some patches on these same files
which
haven't been applied [3]. Maybe that code is going to be rewritten anyway
for
Wayland support so there is no point (at this time) in trying to make any
changes
to it?
Best,
Aaron
[1] update_current_page() in cc-wacom-panel.c
https://git.gnome.org/browse/gnome-control-center/tree/panels/wacom/cc-wacom-panel.c#n272
[2]
http://who-t.blogspot.com/2016/03/libinput-and-graphics-tablet-support.html
[3] https://bugzilla.gnome.org/show_bug.cgi?id=750745
> If we do decide to ship this in libwacom, we
> should restrict the suffix removal to known tool types only, rather than
> stripping out the last word. In theory, we can have non-wacom tablets in
> libwacom's database that may not have a Pen/Finger/... ending.
>
> Bastien, any comments?
>
> Cheers,
> Peter
>
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
