[PATCH v3 wayland-protocols 4/4] tablet: Add pad support to the tablet protocol

[Peter Hutterer]

On Wed, May 11, 2016 at 09:20:15AM -0500, Yong Bakos wrote:
> On May 10, 2016, at 9:51 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > 
> > From: Carlos Garnacho <carlosg at gnome.org>
> > 
> > The pad's interface is similar to the tool interface, a client is notified of
> > the pad after the tablet_added event.
> > 
> > The pad has three functionalities: buttons, rings and strips.
> > Buttons are fairly straightforward, rings and strips are separate interfaces
> > with a pointer-axis-like source/value/frame events.
> > The two interfaces are effectively identical but for the actual value they
> > send (degrees vs normalized position).
> > 
> > Buttons are sequentially indexed starting with zero, unlike other protocols
> > where a linux/input.h-style semantic event code is used. Since we expect all
> > buttons to have client-specific functionality, an additional event tells the
> > client when a given button index is not available, usually because the
> > compositor assignes some function to it (e.g. mode switching, see below).
> > 
> > Specific to the pad device is the set_feedback request which enables a client
> > to set a user-defined string to display for an OSD on the current mappings.
> > This request is available for buttons, rings and strips.
> > 
> > Finally, the pad supports "modes", effectively sets of button/ring/strip
> > configurations.
> > 
> > Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> 
> Hi Carlos and Peter,
> I have some suggestions and corrections, noted inline below.

[...]

> > +	associated with this ring, and is considered user visible; general
> 
> user-visible or perhaps just "and is visible by the user;"
 
this is one I don't quite agree with, but this may be my interpretation:
"visible by the user" to me implies that the string is visible (i.e. shown)
but that may not be true at any given time.  user-visible does not have that
"shown" connotation, since it a technical term. I might be wrong on that
though.

I did change it locally to user-visible (i.e. with the dash)
 
> > +	internationalization rules apply.
> > +      </description>
> > +      <arg name="description" type="string" summary="ring description"/>
> > +    </request>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroy the ring object">
> > +	This destroys the client's resource for this ring object.
> > +      </description>
> > +    </request>
> > +
> > +    <enum name="source">
> > +      <description summary="ring axis source">
> > +	Describes the source types for ring events. This indicates to the
> > +	client how a ring event was physically generated; a client may
> > +	adjust the user interface accordingly. For example, events
> > +	from a "finger" source may trigger kinetic scrolling.
> > +      </description>
> > +      <entry name="finger" value="1" summary="finger"/>
> > +    </enum>
> > +
> > +    <event name="source">
> > +      <description summary="ring event source">
> > +	Source information for ring events.
> > +
> > +	This event does not occur on its own. It is sent before a
> > +	wp_tablet_pad_ring.frame event and carries the source information
> > +	for all events within that frame.
> > +
> > +	The source specifies how this event was generated. If the source is
> > +	wp_tablet_pad_ring.source.finger, a wp_tablet_pad_ring.stop event
> > +	will be sent when the user lifts the finger off the device.
> > +
> > +	This event is optional. If the source is unknown for an interaction,
> > +	no event is sent.
> > +      </description>
> > +      <arg name="source" type="uint" summary="the event source"/>
> > +    </event>
> > +
> > +    <event name="angle">
> > +      <description summary="angle changed">
> > +	Sent whenever the angle on a ring changes.
> > +
> > +	The angle is provided in degrees clockwise from the logical
> > +	north of the ring in the pad's current rotation.
> > +      </description>
> > +      <arg name="angle" type="fixed" summary="the current angle"/>
> 
> Perhaps the summary should mention units, eg "the current angle in degrees,"
> similar to the wheel event's degrees arg summary.

oh, actually, this is inconsistent with the rest where we use an arg name
"degrees". I've changed this locally.

all others amended as requested, thanks heaps!

Cheers,
   Peter