On Fri, Jan 20, 2012 at 10:56:23AM -0800, Jason Gerecke wrote:
> Ping's out for a little while, so I'll be taking over the patch. I've
> got other stuff on my plate though, so it may take some time to fold
> in the changes.
> 
> On Thu, Jan 19, 2012 at 8:13 PM, Peter Hutterer
> <[email protected]> wrote:
> > Thank you, I CC'd Eduard here to, he may have some comments related to his
> > code.
> >
> > On Wed, Jan 18, 2012 at 04:25:42PM -0800, Ping Cheng wrote:
> >> Signed-off-by: Ping Cheng <[email protected]>
> >> ---
> >> Changes to v1:
> >> Updated test_parameter_number;
> >> Added DBG to report sysfs node.
> >> ---
> >>  include/wacom-properties.h |    3 ++
> >>  man/xsetwacom.man          |    7 ++++
> >>  src/wcmCommon.c            |    4 ++-
> >>  src/wcmConfig.c            |   67 
> >> +++++++++++++++++++++++++++++++++++++++++++-
> >>  src/wcmXCommand.c          |   39 +++++++++++++++++++++++++-
> >>  src/xf86Wacom.c            |   14 ++++++++-
> >>  src/xf86WacomDefs.h        |    6 +++-
> >>  tools/xsetwacom.c          |   18 +++++++++++-
> >>  8 files changed, 152 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/include/wacom-properties.h b/include/wacom-properties.h
> >> index 0bb84b1..b2a3523 100644
> >> --- a/include/wacom-properties.h
> >> +++ b/include/wacom-properties.h
> >> @@ -46,6 +46,9 @@
> >>    */
> >>  #define WACOM_PROP_STRIPBUTTONS "Wacom Strip Buttons"
> >>
> >> +/* 8 bit, 2 values, LED0 (right side) and LED1 (left side) */
> >> +#define WACOM_PROP_LED "Wacom LEDs"
> >
> > I'd prefer "Wacom LED Luminosity" or something that hints at _what_ the
> > property affects on the LEDs. In the future we may need a "Wacom LED
> > colors", so we should differentiate from the start.
> >
> That sounds like a reasonable name, especially with the possibility of
> multiple LED brightness levels (which I also think I remember seeing
> something about)

comment below

> >> +
> >>  /* 8 bit, 6 values, rel wheel up, rel wheel down, abs wheel up, abs wheel 
> >> down, abs wheel 2 up, abs wheel 2 down
> >>     OR
> >>     Atom, 6 values , rel wheel up, rel wheel down, abs wheel up, abs wheel 
> >> down, abs wheel 2 up, abs wheel 2 down
> >> diff --git a/man/xsetwacom.man b/man/xsetwacom.man
> >> index dc0995f..97da416 100644
> >> --- a/man/xsetwacom.man
> >> +++ b/man/xsetwacom.man
> >> @@ -201,6 +201,13 @@ sets the max distance from tablet to stop reporting 
> >> movement for cursor in
> >>  relative mode. Default for Intuos series is 10, for Graphire series 
> >> (including
> >>  Volitos) is 42. Only available for the cursor/puck device.
> >>  .TP
> >> +\fBLED0\fR status
> >> +Set the right LED status of an Intuos4/Cintiq 21UX/Cintiq 24HD. Default:  
> >> 0,
> >> +range of 0 to 3.
> >
> > what do those numbers refer to?
> These numbers refer to the LED number that should be lit. At most one
> LED in a bank may be lit at a time (both a hardware and sysfs
> interface limitation). I haven't run the code myself, but it looks
> like running "xsetwacom set <id> LED0 2" on an Intuos4 would result in
> the third LED lighting up. Running "xsetwacom set <id> LED1 2" should
> have no effect since the Intuos4 has only one bank of LEDs. It should
> be noted that the 24HD is an odd case. All other tablets have 4 LEDs
> per bank and one of them will always be lit. The 24HD has three LEDs,
> and trying to set the 4th LED actually results in all the LEDs in that
> bank turning off since the "lit" LED is non-existent.

whoah, imo that exposes too much of the implementation. fwiw, I read the
code as "the tablet has 2 LEDs with 4 brightness levels from 0..3".
hence my "Luminosity" suggestion. 

how about a nice interface of one byte per LED, especially if we have 128
levels? Plus, I'd even argue to map the 0..128 range into 0.255, just to
provide less-random scaling. But this exposes the main issue with this -
it's again a driver-specific property that all clients need to know the
exact details about. Because when range-mapped, a binary LED won't actually
switch on for values 128+. What we really need here is a proper device class
that also provides ranges and the ability to change the value. XI 2.3
anyone?

> > We'll likely have more LEDs in the future, so an interface of
> > xsetwacom set ... LED 1 <status> seems better, similar to the Button
> > interface.
> >
> I'd actually argue for "xsetwacom set <id> LED <bank> <led number>"
> rather than a linear mapping.

will the LEDs always be in banks? i've got a wireles I4 here and presumably
the two LEDs that signal connection could one day be used for driver control
too (hypothetical case). 

Also note that the property itself doesn't need to be the same format as the
xsetwacom interface, e.g. property linearly, xsetwacom supports banks if
given.

Cheers,
  Peter

> > also, IIRC at some point we talked about 255 levels for LEDs?
> >
> The kernel appears to support 128 levels through sysfs (see
> "status0_luminance" and "status1_luminance"), so that could be added
> as well. This would have to be done in a patch of its own though since
> I don't have enough time to do the development and testing...


------------------------------------------------------------------------------
Try before you buy = See our experts in action!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-dev2
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to