Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-23 Thread Peter Hutterer
On Mon, Jan 23, 2012 at 05:26:55PM -0800, Jason Gerecke wrote:
> On Mon, Jan 23, 2012 at 3:36 PM, Peter Hutterer
>  wrote:
> > On Mon, Jan 23, 2012 at 10:30:34AM -0800, Jason Gerecke wrote:
> >> On Sun, Jan 22, 2012 at 8:49 PM, Peter Hutterer
> >>  wrote:
> >> > On Fri, Jan 20, 2012 at 10:55:59AM -0800, Jason Gerecke wrote:
> >> >> On Thu, Jan 19, 2012 at 8:12 PM, Peter Hutterer
> >> >>  wrote:
> >> >> > CC-ing Eduard, since he's been working on this as well
> >> >> >
> >> >> > On Wed, Jan 18, 2012 at 08:15:47PM -0600, Chris Bagwell wrote:
> >> >> >> Well, this showed up just in time for me to be conflicted.  I've
> >> >> >> completed support for monitoring the battery on wireless bamboo's and
> >> >> >> thinking about adding support to control powersavings timeout period
> >> >> >> like Windows driver has.  Its kinda a waste unless there is a GUI to
> >> >> >> go with it though.
> >> >> >>
> >> >> >> Since sysfs is only read/write by root, we need to overcome that.  I
> >> >> >> was going to make a python daemon that advertises wacom sysfs changes
> >> >> >> on DBUS and supports modifying timeout via DBUS requests. Then add
> >> >> >> some sort of applet GUI for end users that talks over DBUS.
> >> >> >>
> >> >> >> Well, you've provided an interesting alternative to DBUS.  It solves
> >> >> >> the root issue and if we could somehow do a select() on the sysfs 
> >> >> >> then
> >> >> >> I could monitor for battery changes as well and advertise changes as
> >> >> >> property updates.
> >> >> >>
> >> >> >> For me, its probably the same amount of total code either way.  I 
> >> >> >> need
> >> >> >> to decide on if its appropriate to put these type of features into
> >> >> >> xf86-input-wacom.
> >> >> >>
> >> >> >> I'm more comfortable working with DBUS then X properties (says the 
> >> >> >> guy
> >> >> >> working on X drivers :-) ) so probably why I chose it.
> >> >> >>
> >> >> >> I probably only have 1 code comment beyond a general concern if this
> >> >> >> is a feature we should be support (I'd like to hear others opinions).
> >> >> >> See below.
> >> >> >
> >> >> > the reason why I'd like to see this as a property is simple: graphical
> >> >> > applications already communicate with the X server for events and 
> >> >> > thus have
> >> >> > device-specific knowledge (XI/XI2 applications anyway). I consider 
> >> >> > the LED
> >> >> > just as an additional feature on the device, so controlling it 
> >> >> > through a
> >> >> > device property seems to make sense.
> >> >> >
> >> >> > Otherwise a client would have to figure out which device it is that 
> >> >> > needs
> >> >> > the LED changed, hook up to sysfs (or some custom daemon). Said 
> >> >> > daemon would
> >> >> > need to provide notification systems (if more than one client access 
> >> >> > the
> >> >> > LED), etc. All this infrastructure is already there in X, even if it 
> >> >> > may not
> >> >> > necessarily be the sanest infrastructure..
> >> >> >
> >> >> > Comments regarding the patch itself are in a separate email.
> >> >> >
> >> >> > Cheers,
> >> >> >  Peter
> >> >> >
> >> >> This is actually kind of what I thought you were getting at the first
> >> >> time you mentioned the idea of libwacom to me. An intermediary library
> >> >> that takes the ugly out of working directly with the X properties that
> >> >> we expose. Combined with a deep knowledge of the actual tablet,
> >> >> clients could do some really neat things without having to duplicate
> >> >> code. For example, it'd be awesome if clients could find out (or
> >> >> libwacom somehow handled) the Intuos4's quirk where the first button
> >> >> in the X properties is *not* the top-left button.
> >> >
> >> > refresh my memory: is this a bug or a feature?
> >> > if it's a bug, I'd argue for not worrying about it too much, otherwise 
> >> > we'll
> >> > just end up in ifdef hell.
> >> >
> >> While it'd be great if the driver made the hardware appear more
> >> consistent, I really don't see that happening. Our X properties don't
> >> carry enough semantic information to make it possible to (sanely)
> >> handle quirks like button 1 on the Intuos4, and radically changing
> >> them would just make the driver needlessly complex while
> >> simultaneously breaking compatibility. Its not the fault of the driver
> >> that button 1 is weird -- that's just the way the hardware *is*. There
> >> should be a way for interested clients to learn about and deal with
> >> these quirks, and going through the X server is not the answer.
> >> Libwacom *may* be the answer (since they share a lot of common
> >> ground), but it also might not be... I'm just mulling over the idea
> >> out loud :)
> >
> > right, it's a question of abstraction and where do we put it and there is no
> > single right answer and we have to draw the line for sanity somewhere.
> >
> > for the button issue: imo the driver should expose all buttons identically,
> > but knowing which button is where is up to the client (with the help 

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-23 Thread Jason Gerecke
On Mon, Jan 23, 2012 at 3:36 PM, Peter Hutterer
 wrote:
> On Mon, Jan 23, 2012 at 10:30:34AM -0800, Jason Gerecke wrote:
>> On Sun, Jan 22, 2012 at 8:49 PM, Peter Hutterer
>>  wrote:
>> > On Fri, Jan 20, 2012 at 10:55:59AM -0800, Jason Gerecke wrote:
>> >> On Thu, Jan 19, 2012 at 8:12 PM, Peter Hutterer
>> >>  wrote:
>> >> > CC-ing Eduard, since he's been working on this as well
>> >> >
>> >> > On Wed, Jan 18, 2012 at 08:15:47PM -0600, Chris Bagwell wrote:
>> >> >> Well, this showed up just in time for me to be conflicted.  I've
>> >> >> completed support for monitoring the battery on wireless bamboo's and
>> >> >> thinking about adding support to control powersavings timeout period
>> >> >> like Windows driver has.  Its kinda a waste unless there is a GUI to
>> >> >> go with it though.
>> >> >>
>> >> >> Since sysfs is only read/write by root, we need to overcome that.  I
>> >> >> was going to make a python daemon that advertises wacom sysfs changes
>> >> >> on DBUS and supports modifying timeout via DBUS requests. Then add
>> >> >> some sort of applet GUI for end users that talks over DBUS.
>> >> >>
>> >> >> Well, you've provided an interesting alternative to DBUS.  It solves
>> >> >> the root issue and if we could somehow do a select() on the sysfs then
>> >> >> I could monitor for battery changes as well and advertise changes as
>> >> >> property updates.
>> >> >>
>> >> >> For me, its probably the same amount of total code either way.  I need
>> >> >> to decide on if its appropriate to put these type of features into
>> >> >> xf86-input-wacom.
>> >> >>
>> >> >> I'm more comfortable working with DBUS then X properties (says the guy
>> >> >> working on X drivers :-) ) so probably why I chose it.
>> >> >>
>> >> >> I probably only have 1 code comment beyond a general concern if this
>> >> >> is a feature we should be support (I'd like to hear others opinions).
>> >> >> See below.
>> >> >
>> >> > the reason why I'd like to see this as a property is simple: graphical
>> >> > applications already communicate with the X server for events and thus 
>> >> > have
>> >> > device-specific knowledge (XI/XI2 applications anyway). I consider the 
>> >> > LED
>> >> > just as an additional feature on the device, so controlling it through a
>> >> > device property seems to make sense.
>> >> >
>> >> > Otherwise a client would have to figure out which device it is that 
>> >> > needs
>> >> > the LED changed, hook up to sysfs (or some custom daemon). Said daemon 
>> >> > would
>> >> > need to provide notification systems (if more than one client access the
>> >> > LED), etc. All this infrastructure is already there in X, even if it 
>> >> > may not
>> >> > necessarily be the sanest infrastructure..
>> >> >
>> >> > Comments regarding the patch itself are in a separate email.
>> >> >
>> >> > Cheers,
>> >> >  Peter
>> >> >
>> >> This is actually kind of what I thought you were getting at the first
>> >> time you mentioned the idea of libwacom to me. An intermediary library
>> >> that takes the ugly out of working directly with the X properties that
>> >> we expose. Combined with a deep knowledge of the actual tablet,
>> >> clients could do some really neat things without having to duplicate
>> >> code. For example, it'd be awesome if clients could find out (or
>> >> libwacom somehow handled) the Intuos4's quirk where the first button
>> >> in the X properties is *not* the top-left button.
>> >
>> > refresh my memory: is this a bug or a feature?
>> > if it's a bug, I'd argue for not worrying about it too much, otherwise 
>> > we'll
>> > just end up in ifdef hell.
>> >
>> While it'd be great if the driver made the hardware appear more
>> consistent, I really don't see that happening. Our X properties don't
>> carry enough semantic information to make it possible to (sanely)
>> handle quirks like button 1 on the Intuos4, and radically changing
>> them would just make the driver needlessly complex while
>> simultaneously breaking compatibility. Its not the fault of the driver
>> that button 1 is weird -- that's just the way the hardware *is*. There
>> should be a way for interested clients to learn about and deal with
>> these quirks, and going through the X server is not the answer.
>> Libwacom *may* be the answer (since they share a lot of common
>> ground), but it also might not be... I'm just mulling over the idea
>> out loud :)
>
> right, it's a question of abstraction and where do we put it and there is no
> single right answer and we have to draw the line for sanity somewhere.
>
> for the button issue: imo the driver should expose all buttons identically,
> but knowing which button is where is up to the client (with the help from
> libwacom or the user). anything beyond that is nasty, as you said.
>
> though even with libwacom I expect _some_ stuff will always be user- or
> client-specific knowledge that cannot be abstracted in a library.
>
>> >> Of course, the devil's in figuring out how to represent those quirks
>> >> (bot

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-23 Thread Peter Hutterer
On Mon, Jan 23, 2012 at 10:30:34AM -0800, Jason Gerecke wrote:
> On Sun, Jan 22, 2012 at 8:49 PM, Peter Hutterer
>  wrote:
> > On Fri, Jan 20, 2012 at 10:55:59AM -0800, Jason Gerecke wrote:
> >> On Thu, Jan 19, 2012 at 8:12 PM, Peter Hutterer
> >>  wrote:
> >> > CC-ing Eduard, since he's been working on this as well
> >> >
> >> > On Wed, Jan 18, 2012 at 08:15:47PM -0600, Chris Bagwell wrote:
> >> >> Well, this showed up just in time for me to be conflicted.  I've
> >> >> completed support for monitoring the battery on wireless bamboo's and
> >> >> thinking about adding support to control powersavings timeout period
> >> >> like Windows driver has.  Its kinda a waste unless there is a GUI to
> >> >> go with it though.
> >> >>
> >> >> Since sysfs is only read/write by root, we need to overcome that.  I
> >> >> was going to make a python daemon that advertises wacom sysfs changes
> >> >> on DBUS and supports modifying timeout via DBUS requests. Then add
> >> >> some sort of applet GUI for end users that talks over DBUS.
> >> >>
> >> >> Well, you've provided an interesting alternative to DBUS.  It solves
> >> >> the root issue and if we could somehow do a select() on the sysfs then
> >> >> I could monitor for battery changes as well and advertise changes as
> >> >> property updates.
> >> >>
> >> >> For me, its probably the same amount of total code either way.  I need
> >> >> to decide on if its appropriate to put these type of features into
> >> >> xf86-input-wacom.
> >> >>
> >> >> I'm more comfortable working with DBUS then X properties (says the guy
> >> >> working on X drivers :-) ) so probably why I chose it.
> >> >>
> >> >> I probably only have 1 code comment beyond a general concern if this
> >> >> is a feature we should be support (I'd like to hear others opinions).
> >> >> See below.
> >> >
> >> > the reason why I'd like to see this as a property is simple: graphical
> >> > applications already communicate with the X server for events and thus 
> >> > have
> >> > device-specific knowledge (XI/XI2 applications anyway). I consider the 
> >> > LED
> >> > just as an additional feature on the device, so controlling it through a
> >> > device property seems to make sense.
> >> >
> >> > Otherwise a client would have to figure out which device it is that needs
> >> > the LED changed, hook up to sysfs (or some custom daemon). Said daemon 
> >> > would
> >> > need to provide notification systems (if more than one client access the
> >> > LED), etc. All this infrastructure is already there in X, even if it may 
> >> > not
> >> > necessarily be the sanest infrastructure..
> >> >
> >> > Comments regarding the patch itself are in a separate email.
> >> >
> >> > Cheers,
> >> >  Peter
> >> >
> >> This is actually kind of what I thought you were getting at the first
> >> time you mentioned the idea of libwacom to me. An intermediary library
> >> that takes the ugly out of working directly with the X properties that
> >> we expose. Combined with a deep knowledge of the actual tablet,
> >> clients could do some really neat things without having to duplicate
> >> code. For example, it'd be awesome if clients could find out (or
> >> libwacom somehow handled) the Intuos4's quirk where the first button
> >> in the X properties is *not* the top-left button.
> >
> > refresh my memory: is this a bug or a feature?
> > if it's a bug, I'd argue for not worrying about it too much, otherwise we'll
> > just end up in ifdef hell.
> >
> While it'd be great if the driver made the hardware appear more
> consistent, I really don't see that happening. Our X properties don't
> carry enough semantic information to make it possible to (sanely)
> handle quirks like button 1 on the Intuos4, and radically changing
> them would just make the driver needlessly complex while
> simultaneously breaking compatibility. Its not the fault of the driver
> that button 1 is weird -- that's just the way the hardware *is*. There
> should be a way for interested clients to learn about and deal with
> these quirks, and going through the X server is not the answer.
> Libwacom *may* be the answer (since they share a lot of common
> ground), but it also might not be... I'm just mulling over the idea
> out loud :)

right, it's a question of abstraction and where do we put it and there is no
single right answer and we have to draw the line for sanity somewhere.

for the button issue: imo the driver should expose all buttons identically,
but knowing which button is where is up to the client (with the help from
libwacom or the user). anything beyond that is nasty, as you said.

though even with libwacom I expect _some_ stuff will always be user- or
client-specific knowledge that cannot be abstracted in a library.

> >> Of course, the devil's in figuring out how to represent those quirks
> >> (both to libwacom in its config files, and to clients via the API)...
> >
> > some of it is true. yes, I want libwacom to be a convenience library so that
> > clients don't have 

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-23 Thread Peter Hutterer
On Mon, Jan 23, 2012 at 10:09:09AM -0800, Jason Gerecke wrote:
> On Sun, Jan 22, 2012 at 8:43 PM, Peter Hutterer
>  wrote:
> > 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
> >>  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 
> >> >> ---
> >> >> 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  LED0 2" on an Intuos4 would result in
> >> the third LED lighting up. Running "xsetwacom set  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
> A linear array with one (scaled) byte per LED is probably the easiest
> interface we can provide and has a lot going for it. The main problem
> I see is that its fairly misleading since the hardware isn't nearly as
> flexible as the interface. That's not really a bad thing, just
> annoying from the client's point of view since there's no way to know
> beforehand which of your settings will actually "stick".
> 
> > 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?
> >
> Its funny to think of LEDs

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-23 Thread Jason Gerecke
On Sun, Jan 22, 2012 at 8:49 PM, Peter Hutterer
 wrote:
> On Fri, Jan 20, 2012 at 10:55:59AM -0800, Jason Gerecke wrote:
>> On Thu, Jan 19, 2012 at 8:12 PM, Peter Hutterer
>>  wrote:
>> > CC-ing Eduard, since he's been working on this as well
>> >
>> > On Wed, Jan 18, 2012 at 08:15:47PM -0600, Chris Bagwell wrote:
>> >> Well, this showed up just in time for me to be conflicted.  I've
>> >> completed support for monitoring the battery on wireless bamboo's and
>> >> thinking about adding support to control powersavings timeout period
>> >> like Windows driver has.  Its kinda a waste unless there is a GUI to
>> >> go with it though.
>> >>
>> >> Since sysfs is only read/write by root, we need to overcome that.  I
>> >> was going to make a python daemon that advertises wacom sysfs changes
>> >> on DBUS and supports modifying timeout via DBUS requests. Then add
>> >> some sort of applet GUI for end users that talks over DBUS.
>> >>
>> >> Well, you've provided an interesting alternative to DBUS.  It solves
>> >> the root issue and if we could somehow do a select() on the sysfs then
>> >> I could monitor for battery changes as well and advertise changes as
>> >> property updates.
>> >>
>> >> For me, its probably the same amount of total code either way.  I need
>> >> to decide on if its appropriate to put these type of features into
>> >> xf86-input-wacom.
>> >>
>> >> I'm more comfortable working with DBUS then X properties (says the guy
>> >> working on X drivers :-) ) so probably why I chose it.
>> >>
>> >> I probably only have 1 code comment beyond a general concern if this
>> >> is a feature we should be support (I'd like to hear others opinions).
>> >> See below.
>> >
>> > the reason why I'd like to see this as a property is simple: graphical
>> > applications already communicate with the X server for events and thus have
>> > device-specific knowledge (XI/XI2 applications anyway). I consider the LED
>> > just as an additional feature on the device, so controlling it through a
>> > device property seems to make sense.
>> >
>> > Otherwise a client would have to figure out which device it is that needs
>> > the LED changed, hook up to sysfs (or some custom daemon). Said daemon 
>> > would
>> > need to provide notification systems (if more than one client access the
>> > LED), etc. All this infrastructure is already there in X, even if it may 
>> > not
>> > necessarily be the sanest infrastructure..
>> >
>> > Comments regarding the patch itself are in a separate email.
>> >
>> > Cheers,
>> >  Peter
>> >
>> This is actually kind of what I thought you were getting at the first
>> time you mentioned the idea of libwacom to me. An intermediary library
>> that takes the ugly out of working directly with the X properties that
>> we expose. Combined with a deep knowledge of the actual tablet,
>> clients could do some really neat things without having to duplicate
>> code. For example, it'd be awesome if clients could find out (or
>> libwacom somehow handled) the Intuos4's quirk where the first button
>> in the X properties is *not* the top-left button.
>
> refresh my memory: is this a bug or a feature?
> if it's a bug, I'd argue for not worrying about it too much, otherwise we'll
> just end up in ifdef hell.
>
While it'd be great if the driver made the hardware appear more
consistent, I really don't see that happening. Our X properties don't
carry enough semantic information to make it possible to (sanely)
handle quirks like button 1 on the Intuos4, and radically changing
them would just make the driver needlessly complex while
simultaneously breaking compatibility. Its not the fault of the driver
that button 1 is weird -- that's just the way the hardware *is*. There
should be a way for interested clients to learn about and deal with
these quirks, and going through the X server is not the answer.
Libwacom *may* be the answer (since they share a lot of common
ground), but it also might not be... I'm just mulling over the idea
out loud :)

>> Of course, the devil's in figuring out how to represent those quirks
>> (both to libwacom in its config files, and to clients via the API)...
>
> some of it is true. yes, I want libwacom to be a convenience library so that
> clients don't have to poke at properties around. Nonetheless, the above is
> still true - the client that has to do all the work would just be libwacom
> (plus any client not using it).
>
>> Or the quirk that
>> the 24HD can actually turn off all the LEDs in a bank since it only
>> has 3 of the expected 4 LEDs.
>
> other tablets can't turn off all LEDs?
>
That's correct. I'm not sure if future tablets will be like the 24HD
and let you turn them off, but the I4 and 21UX2 have no "off" option
I'm aware of. You might be able to fake it by setting the active and
inactive luminosity to zero, but I'm not sure if zero is really "off"
or just "dim".

> Cheers,
>  Peter
>
>> >> On Wed, Jan 18, 2012 at 6:25 PM, Ping Cheng  wrote:
>> >> > Signed-off-by: Ping Cheng 
>> >> >

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-23 Thread Jason Gerecke
On Sun, Jan 22, 2012 at 8:43 PM, Peter Hutterer
 wrote:
> 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
>>  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 
>> >> ---
>> >> 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  LED0 2" on an Intuos4 would result in
>> the third LED lighting up. Running "xsetwacom set  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
A linear array with one (scaled) byte per LED is probably the easiest
interface we can provide and has a lot going for it. The main problem
I see is that its fairly misleading since the hardware isn't nearly as
flexible as the interface. That's not really a bad thing, just
annoying from the client's point of view since there's no way to know
beforehand which of your settings will actually "stick".

> 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?
>
Its funny to think of LEDs being an "input" device, but I don't
suppose X really has a notion of "output" devices that act in remotely
the same manner. :(

>> > We'll likely have more LEDs in the future, so an interface of
>> > xsetwacom set ... LED 1  seems better, similar to the Button
>> > interface.
>>

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-22 Thread Peter Hutterer
On Fri, Jan 20, 2012 at 10:55:59AM -0800, Jason Gerecke wrote:
> On Thu, Jan 19, 2012 at 8:12 PM, Peter Hutterer
>  wrote:
> > CC-ing Eduard, since he's been working on this as well
> >
> > On Wed, Jan 18, 2012 at 08:15:47PM -0600, Chris Bagwell wrote:
> >> Well, this showed up just in time for me to be conflicted.  I've
> >> completed support for monitoring the battery on wireless bamboo's and
> >> thinking about adding support to control powersavings timeout period
> >> like Windows driver has.  Its kinda a waste unless there is a GUI to
> >> go with it though.
> >>
> >> Since sysfs is only read/write by root, we need to overcome that.  I
> >> was going to make a python daemon that advertises wacom sysfs changes
> >> on DBUS and supports modifying timeout via DBUS requests. Then add
> >> some sort of applet GUI for end users that talks over DBUS.
> >>
> >> Well, you've provided an interesting alternative to DBUS.  It solves
> >> the root issue and if we could somehow do a select() on the sysfs then
> >> I could monitor for battery changes as well and advertise changes as
> >> property updates.
> >>
> >> For me, its probably the same amount of total code either way.  I need
> >> to decide on if its appropriate to put these type of features into
> >> xf86-input-wacom.
> >>
> >> I'm more comfortable working with DBUS then X properties (says the guy
> >> working on X drivers :-) ) so probably why I chose it.
> >>
> >> I probably only have 1 code comment beyond a general concern if this
> >> is a feature we should be support (I'd like to hear others opinions).
> >> See below.
> >
> > the reason why I'd like to see this as a property is simple: graphical
> > applications already communicate with the X server for events and thus have
> > device-specific knowledge (XI/XI2 applications anyway). I consider the LED
> > just as an additional feature on the device, so controlling it through a
> > device property seems to make sense.
> >
> > Otherwise a client would have to figure out which device it is that needs
> > the LED changed, hook up to sysfs (or some custom daemon). Said daemon would
> > need to provide notification systems (if more than one client access the
> > LED), etc. All this infrastructure is already there in X, even if it may not
> > necessarily be the sanest infrastructure..
> >
> > Comments regarding the patch itself are in a separate email.
> >
> > Cheers,
> >  Peter
> >
> This is actually kind of what I thought you were getting at the first
> time you mentioned the idea of libwacom to me. An intermediary library
> that takes the ugly out of working directly with the X properties that
> we expose. Combined with a deep knowledge of the actual tablet,
> clients could do some really neat things without having to duplicate
> code. For example, it'd be awesome if clients could find out (or
> libwacom somehow handled) the Intuos4's quirk where the first button
> in the X properties is *not* the top-left button. 

refresh my memory: is this a bug or a feature?
if it's a bug, I'd argue for not worrying about it too much, otherwise we'll
just end up in ifdef hell.

> Of course, the devil's in figuring out how to represent those quirks
> (both to libwacom in its config files, and to clients via the API)...

some of it is true. yes, I want libwacom to be a convenience library so that
clients don't have to poke at properties around. Nonetheless, the above is
still true - the client that has to do all the work would just be libwacom
(plus any client not using it).

> Or the quirk that
> the 24HD can actually turn off all the LEDs in a bank since it only
> has 3 of the expected 4 LEDs.

other tablets can't turn off all LEDs?

Cheers,
  Peter
 
> >> On Wed, Jan 18, 2012 at 6:25 PM, Ping Cheng  wrote:
> >> > Signed-off-by: Ping Cheng 
> >> > ---
> >> > 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"
> >> > +
> >> >  /* 8 bit, 6 values, rel wheel up, rel wheel down, abs wheel up, abs 
> >> > wheel down, abs wheel 2 up, abs wheel 2 down
> >> >

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-22 Thread Peter Hutterer
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
>  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 
> >> ---
> >> 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  LED0 2" on an Intuos4 would result in
> the third LED lighting up. Running "xsetwacom set  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  seems better, similar to the Button
> > interface.
> >
> I'd actually argue for "xsetwacom set  LED  "
> 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

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-20 Thread Jason Gerecke
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
 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 
>> ---
>> 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)

>> +
>>  /* 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  LED0 2" on an Intuos4 would result in
the third LED lighting up. Running "xsetwacom set  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.

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

> 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...

>> +.TP
>> +\fBLED1\fR status
>> +Set the left LED status of a Cintiq 21UX/24HD. Default:  0, range of 0 to 3.
>> +.TP
>>  \fBThreshold\fR level
>>  Set the minimum pressure necessary to generate a Button event for the stylus
>>  tip, eraser, or touch.  The pressure levels of all tablets are normalized to
>> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
>> index 0f041e3..e4a8630 100644
>> --- a/src/wcmCommon.c
>> +++ b/src/wcmCommon.c
>> @@ -1,6 +1,6 @@
>>  /*
>>   * Copyright 1995-2002 by Frederic Lepied, France. 
>> - * Copyright 2002-2010 by Ping Cheng, Wacom. 
>> + * Copyright 2002-2012 by Ping Cheng, Wacom. 
>>   *
>>   * This program is free software; you can redistribute it and/or
>>   * modify it under the terms of the GNU General Public License
>> @@ -1472,6 +1472,8 @@ WacomCommonPtr wcmNewCommon(void)
>>       common->wcmMaxStripY = 4096;       /* Max fingerstrip Y */
>>       common->wcmMaxtiltX = 128;         /* Max tilt in X directory */
>>       common->wcmMaxtiltY = 128;         /* Max tilt in Y directory */
>> +     common->fd_sysfs0 = -1;            /* file descriptor to sysfs led0 */
>> +     common->fd_sysfs1 = -1;            /* file descriptor to sysfs led1 */
>
> array please
>
>>       common->wcmCursorProxoutDistDefa

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-20 Thread Jason Gerecke
On Thu, Jan 19, 2012 at 8:12 PM, Peter Hutterer
 wrote:
> CC-ing Eduard, since he's been working on this as well
>
> On Wed, Jan 18, 2012 at 08:15:47PM -0600, Chris Bagwell wrote:
>> Well, this showed up just in time for me to be conflicted.  I've
>> completed support for monitoring the battery on wireless bamboo's and
>> thinking about adding support to control powersavings timeout period
>> like Windows driver has.  Its kinda a waste unless there is a GUI to
>> go with it though.
>>
>> Since sysfs is only read/write by root, we need to overcome that.  I
>> was going to make a python daemon that advertises wacom sysfs changes
>> on DBUS and supports modifying timeout via DBUS requests. Then add
>> some sort of applet GUI for end users that talks over DBUS.
>>
>> Well, you've provided an interesting alternative to DBUS.  It solves
>> the root issue and if we could somehow do a select() on the sysfs then
>> I could monitor for battery changes as well and advertise changes as
>> property updates.
>>
>> For me, its probably the same amount of total code either way.  I need
>> to decide on if its appropriate to put these type of features into
>> xf86-input-wacom.
>>
>> I'm more comfortable working with DBUS then X properties (says the guy
>> working on X drivers :-) ) so probably why I chose it.
>>
>> I probably only have 1 code comment beyond a general concern if this
>> is a feature we should be support (I'd like to hear others opinions).
>> See below.
>
> the reason why I'd like to see this as a property is simple: graphical
> applications already communicate with the X server for events and thus have
> device-specific knowledge (XI/XI2 applications anyway). I consider the LED
> just as an additional feature on the device, so controlling it through a
> device property seems to make sense.
>
> Otherwise a client would have to figure out which device it is that needs
> the LED changed, hook up to sysfs (or some custom daemon). Said daemon would
> need to provide notification systems (if more than one client access the
> LED), etc. All this infrastructure is already there in X, even if it may not
> necessarily be the sanest infrastructure..
>
> Comments regarding the patch itself are in a separate email.
>
> Cheers,
>  Peter
>
This is actually kind of what I thought you were getting at the first
time you mentioned the idea of libwacom to me. An intermediary library
that takes the ugly out of working directly with the X properties that
we expose. Combined with a deep knowledge of the actual tablet,
clients could do some really neat things without having to duplicate
code. For example, it'd be awesome if clients could find out (or
libwacom somehow handled) the Intuos4's quirk where the first button
in the X properties is *not* the top-left button. Or the quirk that
the 24HD can actually turn off all the LEDs in a bank since it only
has 3 of the expected 4 LEDs.

Of course, the devil's in figuring out how to represent those quirks
(both to libwacom in its config files, and to clients via the API)...

Jason

---
Day xee-nee-svsh duu-'ushtlh-ts'it;
nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it.
Huu-chan xuu naa~-gha.


>> On Wed, Jan 18, 2012 at 6:25 PM, Ping Cheng  wrote:
>> > Signed-off-by: Ping Cheng 
>> > ---
>> > 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"
>> > +
>> >  /* 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 

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-19 Thread Peter Hutterer
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 
> ---
> 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.

> +
>  /* 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?
We'll likely have more LEDs in the future, so an interface of 
xsetwacom set ... LED 1  seems better, similar to the Button
interface.

also, IIRC at some point we talked about 255 levels for LEDs?

> +.TP
> +\fBLED1\fR status
> +Set the left LED status of a Cintiq 21UX/24HD. Default:  0, range of 0 to 3.
> +.TP
>  \fBThreshold\fR level
>  Set the minimum pressure necessary to generate a Button event for the stylus
>  tip, eraser, or touch.  The pressure levels of all tablets are normalized to
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index 0f041e3..e4a8630 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright 1995-2002 by Frederic Lepied, France. 
> - * Copyright 2002-2010 by Ping Cheng, Wacom. 
> + * Copyright 2002-2012 by Ping Cheng, Wacom. 
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -1472,6 +1472,8 @@ WacomCommonPtr wcmNewCommon(void)
>   common->wcmMaxStripY = 4096;   /* Max fingerstrip Y */
>   common->wcmMaxtiltX = 128; /* Max tilt in X directory */
>   common->wcmMaxtiltY = 128; /* Max tilt in Y directory */
> + common->fd_sysfs0 = -1;/* file descriptor to sysfs led0 */
> + common->fd_sysfs1 = -1;/* file descriptor to sysfs led1 */

array please

>   common->wcmCursorProxoutDistDefault = PROXOUT_INTUOS_DISTANCE;
>   /* default to Intuos */
>   common->wcmSuppress = DEFAULT_SUPPRESS;
> diff --git a/src/wcmConfig.c b/src/wcmConfig.c
> index 5920e11..aff8b0f 100644
> --- a/src/wcmConfig.c
> +++ b/src/wcmConfig.c
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright 1995-2002 by Frederic Lepied, France. 
> - * Copyright 2002-2010 by Ping Cheng, Wacom. 
> + * Copyright 2002-2012 by Ping Cheng, Wacom. 
>   *   
>  
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
> @@ -21,6 +21,7 @@
>  #include 
>  #endif
>  
> +#include 
>  #include "xf86Wacom.h"
>  #include "wcmFilter.h"
>  #include 
> @@ -441,6 +442,65 @@ static int wcmIsHotpluggedDevice(InputInfoPtr pInfo)
>   return !strcmp(source, "_driver/wacom");
>  }
>  
> +static void wcmStoreLEDsysfsInfo(InputInfoPtr pInfo)
> +{
> + WacomDevicePtr  priv = (WacomDevicePtr)pInfo->private;
> + WacomCommonPtr  common = priv->common;
> + struct stat st;
> + struct udev *udev = udev_new();
> + struct udev_device *parent;
> + struct udev_device *device;
> + char fs_path[128], buf[10];
> + int err = -1;
> +
> + stat(common->device_path, &st);
> + device = udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> + if (!device)
> + return;
> +
> + parent = udev_device_get_parent

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-19 Thread Peter Hutterer
CC-ing Eduard, since he's been working on this as well

On Wed, Jan 18, 2012 at 08:15:47PM -0600, Chris Bagwell wrote:
> Well, this showed up just in time for me to be conflicted.  I've
> completed support for monitoring the battery on wireless bamboo's and
> thinking about adding support to control powersavings timeout period
> like Windows driver has.  Its kinda a waste unless there is a GUI to
> go with it though.
> 
> Since sysfs is only read/write by root, we need to overcome that.  I
> was going to make a python daemon that advertises wacom sysfs changes
> on DBUS and supports modifying timeout via DBUS requests. Then add
> some sort of applet GUI for end users that talks over DBUS.
> 
> Well, you've provided an interesting alternative to DBUS.  It solves
> the root issue and if we could somehow do a select() on the sysfs then
> I could monitor for battery changes as well and advertise changes as
> property updates.
> 
> For me, its probably the same amount of total code either way.  I need
> to decide on if its appropriate to put these type of features into
> xf86-input-wacom.
> 
> I'm more comfortable working with DBUS then X properties (says the guy
> working on X drivers :-) ) so probably why I chose it.
> 
> I probably only have 1 code comment beyond a general concern if this
> is a feature we should be support (I'd like to hear others opinions).
> See below.

the reason why I'd like to see this as a property is simple: graphical
applications already communicate with the X server for events and thus have
device-specific knowledge (XI/XI2 applications anyway). I consider the LED
just as an additional feature on the device, so controlling it through a
device property seems to make sense.

Otherwise a client would have to figure out which device it is that needs
the LED changed, hook up to sysfs (or some custom daemon). Said daemon would
need to provide notification systems (if more than one client access the
LED), etc. All this infrastructure is already there in X, even if it may not
necessarily be the sanest infrastructure..

Comments regarding the patch itself are in a separate email.

Cheers,
  Peter

> On Wed, Jan 18, 2012 at 6:25 PM, Ping Cheng  wrote:
> > Signed-off-by: Ping Cheng 
> > ---
> > 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"
> > +
> >  /* 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.
> > +.TP
> > +\fBLED1\fR status
> > +Set the left LED status of a Cintiq 21UX/24HD. Default:  0, range of 0 to 
> > 3.
> > +.TP
> >  \fBThreshold\fR level
> >  Set the minimum pressure necessary to generate a Button event for the 
> > stylus
> >  tip, eraser, or touch.  The pressure levels of all tablets are normalized 
> > to
> > diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> > index 0f041e3..e4a8630 100644
> > --- a/src/wcmCommon.c
> > +++ b/src/wcmCommon.c
> > @@ -1,6 +1,6 @@
> >  /*
> >  * Copyright 1995-2002 by Frederic Lepied, France. 
> > - * Copyright 2002-2010 by Ping Cheng, Wacom. 
> > + * Copyright 2002-2012 by Ping Cheng, Wacom. 
> >  *
> >  * This program is free software; you can redistribute it and/or
> >  * modify it under the terms of the GNU General Public License
> > @@ -1472,6 +1472,8 @@ WacomCommonPtr wcmNewCommon(void)
> >        common->wcmMaxStripY = 4096;       /* Max fingerstrip Y */
> >        common->wcmMaxtiltX = 128;         /* Max ti

Re: [Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-18 Thread Chris Bagwell
Well, this showed up just in time for me to be conflicted.  I've
completed support for monitoring the battery on wireless bamboo's and
thinking about adding support to control powersavings timeout period
like Windows driver has.  Its kinda a waste unless there is a GUI to
go with it though.

Since sysfs is only read/write by root, we need to overcome that.  I
was going to make a python daemon that advertises wacom sysfs changes
on DBUS and supports modifying timeout via DBUS requests. Then add
some sort of applet GUI for end users that talks over DBUS.

Well, you've provided an interesting alternative to DBUS.  It solves
the root issue and if we could somehow do a select() on the sysfs then
I could monitor for battery changes as well and advertise changes as
property updates.

For me, its probably the same amount of total code either way.  I need
to decide on if its appropriate to put these type of features into
xf86-input-wacom.

I'm more comfortable working with DBUS then X properties (says the guy
working on X drivers :-) ) so probably why I chose it.

I probably only have 1 code comment beyond a general concern if this
is a feature we should be support (I'd like to hear others opinions).
See below.

On Wed, Jan 18, 2012 at 6:25 PM, Ping Cheng  wrote:
> Signed-off-by: Ping Cheng 
> ---
> 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"
> +
>  /* 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.
> +.TP
> +\fBLED1\fR status
> +Set the left LED status of a Cintiq 21UX/24HD. Default:  0, range of 0 to 3.
> +.TP
>  \fBThreshold\fR level
>  Set the minimum pressure necessary to generate a Button event for the stylus
>  tip, eraser, or touch.  The pressure levels of all tablets are normalized to
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index 0f041e3..e4a8630 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -1,6 +1,6 @@
>  /*
>  * Copyright 1995-2002 by Frederic Lepied, France. 
> - * Copyright 2002-2010 by Ping Cheng, Wacom. 
> + * Copyright 2002-2012 by Ping Cheng, Wacom. 
>  *
>  * This program is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU General Public License
> @@ -1472,6 +1472,8 @@ WacomCommonPtr wcmNewCommon(void)
>        common->wcmMaxStripY = 4096;       /* Max fingerstrip Y */
>        common->wcmMaxtiltX = 128;         /* Max tilt in X directory */
>        common->wcmMaxtiltY = 128;         /* Max tilt in Y directory */
> +       common->fd_sysfs0 = -1;            /* file descriptor to sysfs led0 */
> +       common->fd_sysfs1 = -1;            /* file descriptor to sysfs led1 */
>        common->wcmCursorProxoutDistDefault = PROXOUT_INTUOS_DISTANCE;
>                        /* default to Intuos */
>        common->wcmSuppress = DEFAULT_SUPPRESS;
> diff --git a/src/wcmConfig.c b/src/wcmConfig.c
> index 5920e11..aff8b0f 100644
> --- a/src/wcmConfig.c
> +++ b/src/wcmConfig.c
> @@ -1,6 +1,6 @@
>  /*
>  * Copyright 1995-2002 by Frederic Lepied, France. 
> - * Copyright 2002-2010 by Ping Cheng, Wacom. 
> + * Copyright 2002-2012 by Ping Cheng, Wacom. 
>  *
>  * This program is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU General Public License
> @@ -21,6 +21,7 @@
>  #include 
>  #endif
>
> +#include 
>  #include "xf86Wacom.h"
>  #include "wcmFilter.h"
>  #include 
> @@ -441,6 +442,65 @@ static int wcmIsHotpluggedDevice(InputInfoPtr pInfo)
>        return !s

[Linuxwacom-devel] [PATCH v2] Enable LED status change through xsetwacom

2012-01-18 Thread Ping Cheng
Signed-off-by: Ping Cheng 
---
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"
+
 /* 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.
+.TP
+\fBLED1\fR status
+Set the left LED status of a Cintiq 21UX/24HD. Default:  0, range of 0 to 3.
+.TP
 \fBThreshold\fR level
 Set the minimum pressure necessary to generate a Button event for the stylus
 tip, eraser, or touch.  The pressure levels of all tablets are normalized to
diff --git a/src/wcmCommon.c b/src/wcmCommon.c
index 0f041e3..e4a8630 100644
--- a/src/wcmCommon.c
+++ b/src/wcmCommon.c
@@ -1,6 +1,6 @@
 /*
  * Copyright 1995-2002 by Frederic Lepied, France. 
- * Copyright 2002-2010 by Ping Cheng, Wacom. 
+ * Copyright 2002-2012 by Ping Cheng, Wacom. 
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -1472,6 +1472,8 @@ WacomCommonPtr wcmNewCommon(void)
common->wcmMaxStripY = 4096;   /* Max fingerstrip Y */
common->wcmMaxtiltX = 128; /* Max tilt in X directory */
common->wcmMaxtiltY = 128; /* Max tilt in Y directory */
+   common->fd_sysfs0 = -1;/* file descriptor to sysfs led0 */
+   common->fd_sysfs1 = -1;/* file descriptor to sysfs led1 */
common->wcmCursorProxoutDistDefault = PROXOUT_INTUOS_DISTANCE;
/* default to Intuos */
common->wcmSuppress = DEFAULT_SUPPRESS;
diff --git a/src/wcmConfig.c b/src/wcmConfig.c
index 5920e11..aff8b0f 100644
--- a/src/wcmConfig.c
+++ b/src/wcmConfig.c
@@ -1,6 +1,6 @@
 /*
  * Copyright 1995-2002 by Frederic Lepied, France. 
- * Copyright 2002-2010 by Ping Cheng, Wacom. 
+ * Copyright 2002-2012 by Ping Cheng, Wacom. 
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -21,6 +21,7 @@
 #include 
 #endif
 
+#include 
 #include "xf86Wacom.h"
 #include "wcmFilter.h"
 #include 
@@ -441,6 +442,65 @@ static int wcmIsHotpluggedDevice(InputInfoPtr pInfo)
return !strcmp(source, "_driver/wacom");
 }
 
+static void wcmStoreLEDsysfsInfo(InputInfoPtr pInfo)
+{
+   WacomDevicePtr  priv = (WacomDevicePtr)pInfo->private;
+   WacomCommonPtr  common = priv->common;
+   struct stat st;
+   struct udev *udev = udev_new();
+   struct udev_device *parent;
+   struct udev_device *device;
+   char fs_path[128], buf[10];
+   int err = -1;
+
+   stat(common->device_path, &st);
+   device = udev_device_new_from_devnum(udev, 'c', st.st_rdev);
+   if (!device)
+   return;
+
+   parent = udev_device_get_parent_with_subsystem_devtype(device,
+"usb", NULL);
+
+   if (!parent)
+   return;
+
+   sprintf(fs_path, "%s/wacom_led/status_led0_select",
+   udev_device_get_syspath(parent));
+   common->fd_sysfs0 = open(fs_path, O_RDWR);
+   if (common->fd_sysfs0 >= 0)
+   {
+   SYSCALL(err = read(common->fd_sysfs0, buf, 1));
+   if (err < -1)
+   {
+   xf86Msg(X_WARNING, "%s: failed to get led0 status in "
+   "wcmStoreLEDsysfsInfo.\n", pInfo->name);
+   }
+   else
+   common->led0_status = buf[0] - '0';
+   }
+   else
+   DBG(2, common, "No LED0 sysfs on %s for %s\n",
+