re-reviewing this with fresher eyes this time:
On Fri, Aug 24, 2012 at 09:51:46AM -0700, Jason Gerecke wrote:
> On Thu, Aug 23, 2012 at 11:27 PM, Peter Hutterer
> <peter.hutte...@who-t.net> wrote:
> > On Thu, Aug 02, 2012 at 05:53:06PM -0700, Jason Gerecke wrote:
> >> Tablets like the Graphire4 have mouse buttons present on the tablet
> >> rather than ExpressKeys. Because mouse buttons are expected to work
> >> in a certain way by default, it is insufficient for some clients to
> >> know only that e.g. button 'A' has been pressed; they furthermore
> >> need to know that it is expected to by default trigger e.g.
> >> 'navigate-forward'.
why do they need to know that?
if the button is left as-is, it'll do what it does by default. otherwise,
it'll do what it is mapped to. Why do we need the special casing here?
is it more useful to add some mapping that describes the default for any
button (where applicable), regardless of what that button's scancode
actually is?
> >> Five mouse button types have been added: Left, Middle, Right,
> >> Forward, and Back. To associate these types with buttons, data files
> >> require a new "[MouseButtons]" section that accepts the previously-
> >> mentioned types as keywords and button lists as values. See the
> >> Graphire4 data file for example.
doesn't need a new section, tbh, this could be added to the current buttons
section. but if you plan to keep it separate, "DefaultMappings" (or
something similar) would be a more explanatory name.
> >>
> >> Signed-off-by: Jason Gerecke <killert...@gmail.com>
> >> ---
> >> This is just one of several possible ways to augment libwacom with
> >> mouse button data. If you think something would be better done
> >> differently, please let me know -- I don't work with libwacom much
> >> myself :)
> >>
> >> data/graphire4-4x5.tablet | 4 +++
> >> libwacom/libwacom-database.c | 76
> >> +++++++++++++++++++++++++++++++++++++++-----
> >> libwacom/libwacom.h | 9 ++++++
> >> test/load.c | 9 ++++++
> >> 4 files changed, 90 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/data/graphire4-4x5.tablet b/data/graphire4-4x5.tablet
> >> index b93c65b..84aa7cd 100644
> >> --- a/data/graphire4-4x5.tablet
> >> +++ b/data/graphire4-4x5.tablet
> >> @@ -29,3 +29,7 @@ Buttons=2
> >>
> >> [Buttons]
> >> Top=A;B
> >> +
> >> +[MouseButtons]
> >> +Back=A
> >> +Forward=B
> >> diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
> >> index 5468975..e24f967 100644
> >> --- a/libwacom/libwacom-database.c
> >> +++ b/libwacom/libwacom-database.c
> >> @@ -41,6 +41,7 @@
> >> #define FEATURES_GROUP "Features"
> >> #define DEVICE_GROUP "Device"
> >> #define BUTTONS_GROUP "Buttons"
> >> +#define MOUSEBUTTONS_GROUP "MouseButtons"
> >>
> >> static WacomClass
> >> libwacom_class_string_to_enum(const char *class)
> >> @@ -246,6 +247,26 @@ struct {
> >> { "OLEDs", WACOM_BUTTON_OLED }
> >> };
> >>
> >> +static int
> >> +libwacom_button_to_index (const char *button)
> >> +{
> >> + char val;
> >> +
> >> + if (button == NULL)
> >> + return -1;
> >> +
> >> + val = *button;
> >> + if (strlen (button) > 1 ||
> >> + val < 'A' ||
> >> + val > 'Z') {
> >> + return -2;
> >> + }
> >> + val -= 'A';
> >> +
> >> + g_assert (val >= 0);
> >> + return val;
> >> +}
> >> +
> >> static void
> >> libwacom_parse_buttons_key(WacomDevice *device,
> >> GKeyFile *keyfile,
> >> @@ -259,17 +280,12 @@ libwacom_parse_buttons_key(WacomDevice *device,
> >> if (vals == NULL)
> >> return;
> >> for (i = 0; vals[i] != NULL; i++) {
> >> - char val;
> >> -
> >> - val = *vals[i];
> >> - if (strlen (vals[i]) > 1 ||
> >> - val < 'A' ||
> >> - val > 'Z') {
> >> + int index = libwacom_button_to_index(vals[i]);
> >> + if (index < 0) {
> >> g_warning ("Ignoring value '%s' in key '%s'",
> >> vals[i], key);
> >> continue;
> >> }
> >> - val -= 'A';
> >> - device->buttons[(int) val] |= flag;
> >> + device->buttons[index] |= flag;
> >> }
> >>
> >> g_strfreev (vals);
> >> @@ -308,6 +324,49 @@ libwacom_parse_buttons(WacomDevice *device,
> >> device->strips_num_modes = libwacom_parse_num_modes(device, keyfile,
> >> "StripsNumModes", WACOM_BUTTON_TOUCHSTRIP_MODESWITCH);
> >> }
> >>
> >> +struct {
> >> + const char *key;
> >> + WacomMouseButton button;
> >> +} mouseoptions[] = {
> >> + { "Left", WMOUSE_LEFT },
> >> + { "Middle", WMOUSE_MIDDLE },
> >> + { "Right", WMOUSE_RIGHT },
> >> + { "Forward", WMOUSE_FORWARD },
> >> + { "Back", WMOUSE_BACK },
> >> +};
> >> +
> >> +static void
> >> +libwacom_parse_mousebuttons_key (WacomDevice *device,
> >> + GKeyFile *keyfile,
> >> + const char *key,
> >> + WacomMouseButton button)
> >> +{
> >> + guint i;
> >> + char **vals;
> >> +
> >> + vals = g_key_file_get_string_list (keyfile, MOUSEBUTTONS_GROUP, key,
> >> NULL, NULL);
> >> + if (vals == NULL)
> >> + return;
> >> + for (i = 0; vals[i] != NULL; i++) {
> >> + int index = libwacom_button_to_index (vals[i]);
> >> + if (index < 0) {
> >> + g_warning ("Ignoring value '%s' in key '%s'",
> >> vals[i], key);
> >> + continue;
> >> + }
> >> + device->buttons[index] |= button;
> >> + }
> >> +}
> >> +
> >> +static void
> >> +libwacom_parse_mousebuttons (WacomDevice *device,
> >> + GKeyFile *keyfile)
> >> +{
> >> + guint i;
> >> +
> >> + for (i = 0; i < G_N_ELEMENTS (mouseoptions); i++)
> >> + libwacom_parse_mousebuttons_key (device, keyfile,
> >> mouseoptions[i].key, mouseoptions[i].button);
> >> +}
> >> +
> >> static WacomDevice*
> >> libwacom_parse_tablet_keyfile(const char *path)
> >> {
> >> @@ -412,6 +471,7 @@ libwacom_parse_tablet_keyfile(const char *path)
> >> if (device->num_buttons > 0) {
> >> device->buttons = g_new0 (WacomButtonFlags,
> >> device->num_buttons);
> >> libwacom_parse_buttons(device, keyfile);
> >> + libwacom_parse_mousebuttons(device, keyfile);
> >> }
> >>
> >> out:
> >> diff --git a/libwacom/libwacom.h b/libwacom/libwacom.h
> >> index f7e6cf9..463d93b 100644
> >> --- a/libwacom/libwacom.h
> >> +++ b/libwacom/libwacom.h
> >> @@ -145,6 +145,14 @@ typedef enum {
> >> WSTYLUS_PUCK
> >> } WacomStylusType;
> >>
> >> +typedef enum {
> >> + WMOUSE_LEFT = (1 & 0xf) << 10,
> >> + WMOUSE_MIDDLE = (2 & 0xf) << 10,
> >> + WMOUSE_RIGHT = (3 & 0xf) << 10,
> >> + WMOUSE_BACK = (8 & 0xf) << 10,
> >> + WMOUSE_FORWARD = (9 & 0xf) << 10
> >> +} WacomMouseButton;
btw, the 0xf is superfluous, it won't do anything. and I'm not sure what
you're planing to do here, these values won't work as bitmasks but you
appear to be setting them as bitmasks in libwacom_parse_mousebuttons.
> >> +
> >> /**
> >> * Capabilities of the various tablet buttons
> >> */
> >> @@ -159,6 +167,7 @@ typedef enum {
> >> WACOM_BUTTON_TOUCHSTRIP_MODESWITCH = (1 << 7),
> >> WACOM_BUTTON_TOUCHSTRIP2_MODESWITCH = (1 << 8),
> >> WACOM_BUTTON_OLED = (1 << 9),
> >> + WACOM_BUTTON_MOUSEBUTTON = (0xf << 10), /* 10, 11, 12, 13
> >> */
btw, just noticing this: if you have to explain your bitmasks, you might as
well use the numbers directly
> >
> > Do we need to double-abstract this? i.e. one bit for "mouse button
> > capability" and then an interface for getting the list of actual buttons
> > present? bit worried here about ending up too many bits for the buttons if
> > we start adding more meanings in the future.
> >
> I was worried about this as well. We could store the mousebutton info
> elsewhere and provide a function "getMouseButton(char physicalButton)"
> (ignore naming style for the moment -- I haven't bothered to check
> that its consistent) that will return the mouse button that the kernel
> will send for a given physical button identifier.
>
> > Then again, the Graphire's are an older series, so I suspect this is
> > outdated now by the expresskeys?
> >
> The Graphires used to report BTN_n "expresskey" buttons like any other
> tablet, but were then changed to report mouse buttons to bring them in
> line with the original Bamboo Touch (which apparently was first
> written to work with xf86-input-synaptics).
>
> Actually, I'm surprised at how recently this changeover occurred:
> Linux 3.1. This shouldn't be a problem for the faster-releasing
> distros like Fedora and Ubuntu (both of which only use newer kernels
> in currently-supported releases), but may cause hiccoughs in some
> other environments.
I'm still unsure on how you actually want to use this. can you knock up some
client pseudo-code on how you expect this to be used?
Cheers,
Peter
> >
> >> WACOM_BUTTON_MODESWITCH = (WACOM_BUTTON_RING_MODESWITCH
> >> | WACOM_BUTTON_RING2_MODESWITCH | WACOM_BUTTON_TOUCHSTRIP_MODESWITCH |
> >> WACOM_BUTTON_TOUCHSTRIP2_MODESWITCH),
> >> WACOM_BUTTON_DIRECTION = (WACOM_BUTTON_POSITION_LEFT |
> >> WACOM_BUTTON_POSITION_RIGHT | WACOM_BUTTON_POSITION_TOP |
> >> WACOM_BUTTON_POSITION_BOTTOM),
> >> WACOM_BUTTON_RINGS_MODESWITCH = (WACOM_BUTTON_RING_MODESWITCH
> >> | WACOM_BUTTON_RING2_MODESWITCH),
> >> diff --git a/test/load.c b/test/load.c
> >> index b449fa6..48dcd15 100644
> >> --- a/test/load.c
> >> +++ b/test/load.c
> >> @@ -121,6 +121,15 @@ int main(int argc, char **argv)
> >> assert(device);
> >> assert(libwacom_is_builtin(device));
> >>
> >> + libwacom_destroy(device);
> >> +
> >> + device = libwacom_new_from_name(db, "Wacom Graphire4 4x5", NULL);
> >> + assert(device);
> >> + assert(libwacom_get_button_flag(device, 'A') & WMOUSE_BACK);
> >> + assert(libwacom_get_button_flag(device, 'B') & WMOUSE_FORWARD);
> >> +
> >> + libwacom_destroy(device);
> >> +
> >> libwacom_database_destroy (db);
> >>
> >> return 0;
> >> --
> >> 1.7.11.4
> >>
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
