Hi
2011/10/5 Peter Hutterer <peter.hutte...@who-t.net>:
> On Sun, Oct 02, 2011 at 02:14:56PM +0200, Eduard Hasenleithner wrote:
>> The LED extensions in the wacom kernel driver are scheduled for
>> inclusion into linux-3.2. This commit provides a first
>> demonstration on how the OLED part of new interface might be
>> exposed by new wacom xinput device properties.
>
> before I forget: this code needs the suitable kernel version checks so we
> can still
It is not the kernel version which indicates dupport for (O)LED, but
the presence of the "wacom_led" sysfs attribute group.
>
>>
>> The mechanisms for passing the OLED images from X11 client to
>> wacom xinput are two new device properties "Wacom OLED Pixmap"
>> and "Wacom OLED state". The first contains preallocated pixmaps,
>> one for each button with OLED display, which can be written like
>> normal drawables. The contents of the pixmaps propagate to the
>> tablet for all entries in "Wacom OLED State" which have a value
>> of '1' during call of XChangeDeviceProperty.
>>
>> Summary of changes:
>> * Added new src/wcmLED.c module which contains
>> * init function: wcmLedStart
>> called by wcmUSB.c:wcmLedStart()
>> This tries to locate the wacom_led interface in sysfs using
>> libudev. When located, the pathname ist stored in the
>> sysfs_led member of WacomDevicePtr.
>> * destroy function: wcmLedFree
>> called by wcmConfig.c:wcmFree()
>> Frees the sysfs_led string.
>> * button image set function: wcmSetOLEDProperty
>> called by wcmXCommand.c:wcmSetProperty()
>>
>> * Added set_oled_image function to xsetwacom.c.
>> Contains a very simple text renderer using XDrawString.
>>
>> * Added further fake symbols for "make check"
>>
>> Usage Example:
>> Set Button text of Button 3 to 'example'
>> > xsetwacom --set 'Wacom Intuos4 8x13 pad' Image 3 'example'
>>
>> TODO:
>> * Add ability to read images from files
>> * Add ability for more sophisticated "text layout"
>> * Add support for non TrueColor mode X Server
>> * Add function for setting luminance
>>
>> Signed-off-by: Eduard Hasenleithner <edu...@hasenleithner.at>
>> ---
>> include/wacom-properties.h | 22 +++++
>> man/xsetwacom.man | 4 +
>> src/common.mk | 1 +
>> src/wcmConfig.c | 1 +
>> src/wcmLED.c | 193
>> ++++++++++++++++++++++++++++++++++++++++++++
>> src/wcmUSB.c | 3 +
>> src/wcmXCommand.c | 15 ++++
>> src/xf86Wacom.h | 8 ++
>> src/xf86WacomDefs.h | 8 ++
>> test/fake-symbols.c | 23 +++++
>> tools/xsetwacom.c | 89 ++++++++++++++++++++-
>> 11 files changed, 366 insertions(+), 1 deletions(-)
>> create mode 100644 src/wcmLED.c
>>
>> diff --git a/include/wacom-properties.h b/include/wacom-properties.h
>> index 29d5056..c6c9852 100644
>> --- a/include/wacom-properties.h
>> +++ b/include/wacom-properties.h
>> @@ -95,6 +95,28 @@
>> */
>> #define WACOM_PROP_BUTTON_ACTIONS "Wacom Button Actions"
>>
>> +/* 8 bit, 8 values
>> + * Image state for each of the 8 OLED images.
>> + * -1: Image not yet set (i.e. Pixmap content not initialized)
>> + * 0: Image set once or more (i.e. Pixmap content is valid)
>> + * 1: Image to be updated (this value will not appear in
>> XGetDeviceProperty)
>> + * When calling XChangeDeviceProperty, all images which are to be updated
>> + * are to be marked with '1' in the state property. Within
>> + * XChangeDeviceProperty the driver will immediately, set the state to '0'
>> + * so that the images are not updated again on the next call to
>> + * XChangeDeviceProperty.
>> + */
>> +#define WACOM_PROP_OLED_STATE "Wacom OLED State"
>> +
>> +/* Pixmap, array of 8 Button Image IDs (PIXMAP IDs)
>> + * Preallocated Pixmaps, which contain the OLED images for each of
>> + * the buttons. A pixmap within this property is only valid if their
>> + * respective entry in the OLED_STATE property has a value >= 0.
>> + * This property is read-only! The Pixmaps must not be deleted!
>> + * (The Pixmaps are writable, of course)
>> + */
>> +#define WACOM_PROP_OLED_PIXMAP "Wacom OLED Pixmap"
>> +
>
> looking at this, I'm not a big fan of this API. something isn't quite right.
> Especially the disconnect between the OLED state and the actual pixmaps is a
> bit weird for clients. They'd get the property notify for one property just
> to know that another property has updated.
>
> Do we have any speed restrictions for updating the properties? Can we
> updated all of them in one go or is this prohibitively slow?
>
> I would suggest the following API:
> one property, whenever written all pixmaps update. A pixmap of None means
> that OLED is off, otherwise the OLED displays the pixmap content.
> When a client updates the pixmap directly, the client must update the
> property (with the same values) to refresh.
> A client may replace any pixmap in the list but the _client_ is responsible
> for releasing the associated memory if a pixmap is removed from the list.
> You can of course still pre-allocate empty pixmaps if you want to.
>
> If a pixmap in the list is in the wrong format, type, or invalid
> althogether, the driver will simply skip over it and continue with the
> next one (this is the bit I don't like about this approach, suggestions
> welcome).
A few points here
* In a previous email one suggestion was to split the properties, which I did
* I have not yet discovered how to erase the pixmap in-server
* In contrast to the LEDs, I don't see the OLED properties
notification not as important. While a particular "status led" has a
meaning, a OLED image has not.
* I'm hesitating to use XSetCloseDownMode, because behaves like a "memory leak"
Just for the "record": No, I don't think we have a performance problem
when always setting all the pixmaps at once. It is just that I did not
figure out yet on how to blank the pixmaps in-server.
>> /* 8 bit, 2 values, priv->debugLevel and common->debugLevel. This property
>> * is for use in the driver only and only enabled if --enable-debug is
>> * given. No client may rely on this property being present or working.
>> diff --git a/man/xsetwacom.man b/man/xsetwacom.man
>> index dc0995f..9d2c9d2 100644
>> --- a/man/xsetwacom.man
>> +++ b/man/xsetwacom.man
>> @@ -208,6 +208,10 @@ tip, eraser, or touch. The pressure levels of all
>> tablets are normalized to
>> parameter is independent of the PressureCurve parameter. Default: 27,
>> range of 0 to 2047.
>> .TP
>> +\fBImage\fR button-id text
>> +Set the button image for button-id (0-7) to the specified text.
>> +Only available on the Intuos4 M, L, or XL tablets.
>> +.TP
>> \fBToolDebugLevel\fR level
>> Set the debug level for this tool to the given level. This only affects
>> code paths that are specific to a given tool. A higher level means more
>> diff --git a/src/common.mk b/src/common.mk
>> index fae8d9f..464e4bd 100644
>> --- a/src/common.mk
>> +++ b/src/common.mk
>> @@ -10,6 +10,7 @@ DRIVER_SOURCES= \
>> $(top_srcdir)/src/wcmFilter.h \
>> $(top_srcdir)/src/xf86WacomDefs.h \
>> $(top_srcdir)/src/wcmUSB.c \
>> + $(top_srcdir)/src/wcmLED.c \
>> $(top_srcdir)/src/wcmXCommand.c \
>> $(top_srcdir)/src/wcmValidateDevice.c \
>> $(top_srcdir)/src/wcmTouchFilter.c
>> diff --git a/src/wcmConfig.c b/src/wcmConfig.c
>> index 94b188d..bd101dc 100644
>> --- a/src/wcmConfig.c
>> +++ b/src/wcmConfig.c
>> @@ -125,6 +125,7 @@ static void wcmFree(InputInfoPtr pInfo)
>> TimerFree(priv->serial_timer);
>> free(priv->tool);
>> wcmFreeCommon(&priv->common);
>> + wcmLedFree(priv);
>> free(priv);
>>
>> pInfo->private = NULL;
>> diff --git a/src/wcmLED.c b/src/wcmLED.c
>> new file mode 100644
>> index 0000000..dace711
>> --- /dev/null
>> +++ b/src/wcmLED.c
>> @@ -0,0 +1,193 @@
>> +/*
>> + * Copyright 2011 by Eduard Hasenleithner <edu...@hasenleithner.at>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307,
>> USA.
>> + */
>> +
>> +#ifdef HAVE_CONFIG_H
>> +#include <config.h>
>> +#endif
>> +#include <servermd.h>
>> +#include <libudev.h>
>> +#include <sys/stat.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +
>> +#include "xf86Wacom.h"
>> +
>> +static const char wacom_led_path[] = "/wacom_led";
>> +
>> +void wcmLedInitPixmap(WacomDevicePtr priv, int *values, int count)
>> +{
>> + int i;
>> + ScreenPtr pScreen = screenInfo.screens[0];
>> +
>> + for (i = 0; i < count; i++) {
>> + PixmapPtr pMap;
>> + XID id;
>> +
>> + pMap = pScreen->CreatePixmap(pScreen, 64, 32,
>> pScreen->rootDepth, 0);
>> + id = FakeClientID(0);
>> + pMap->drawable.id = id;
>> + AddResource(id, RT_PIXMAP, pMap);
>> +
>> + values[i] = id;
>> + }
>> +}
>> +
>> +void wcmLedStart(WacomDevicePtr priv)
>> +{
>> + struct stat s;
>> + struct udev *udev;
>> + struct udev_device *device, *parent;
>> +
>> + if (fstat(priv->pInfo->fd, &s) < 0)
>> + return;
>> +
>> + udev = udev_new();
>> + device = udev_device_new_from_devnum(udev, 'c', s.st_rdev);
>> + if (device)
>> + {
>> + parent = udev_device_get_parent_with_subsystem_devtype(
>> + device, "usb", NULL);
>
> not sure if that's just diff showing it badly but this looks like it needs
> more indentation.
Sorry for that, I still don't have "the eye" for spotting indentation errors.
>> + if (parent)
>> + {
>> + const char *sysfs_usbdev =
>> udev_device_get_syspath(parent);
>> + char *tmp;
>> +
>> + Xasprintf(&tmp, "%s%s", sysfs_usbdev, wacom_led_path);
>> + strcpy(tmp, sysfs_usbdev);
>> + strcat(tmp, wacom_led_path);
>> + if (stat(tmp, &s)>=0 && S_ISDIR(s.st_mode))
>
> space before/after >=. there may be similar ones in this patch.
Same for the spaces.
>> + priv->sysfs_led = tmp;
>> + else
>> + free(tmp);
>> + }
>> + udev_device_unref(device);
>> + }
>> + udev_unref(udev);
>> +}
>> +
>> +void wcmLedFree(WacomDevicePtr priv)
>> +{
>> + free(priv->sysfs_led);
>> + priv->sysfs_led = NULL;
>> +}
>> +
>> +static int led_surface_put(char *sysfs_path, int id,
>> + unsigned char *src, int stride)
> indentation, please align with "char * sysfs_path"
>
>> +{
>> + char *str;
>> + unsigned char img[1024];
>> + int x, y, i = 0;
>> + int fd;
>> + int r = -1;
>> +
>> + for (y = 0; y < 16; y++)
>> + {
>> + unsigned char *pixel1 = src + (1+2*y)*stride;
>> + unsigned char *pixel2 = pixel1 + stride;
>> +
>> + for (x = 0; x < 64; x++)
>> + {
>> + int lum1, lum2;
>> + pixel1 -= 4;
>> + pixel2 -= 4;
>> + lum1 = pixel1[0]+pixel1[1]+pixel1[2];
>> + lum2 = pixel2[0]+pixel2[1]+pixel2[2];
>> + lum1 /= 3*16; lum2 /= 3*16;
>> + img[i++] = (lum1&0xf) | (lum2<<4);
>> + }
>> + }
>> +
>> + Xasprintf(&str, "%s/button%d_rawimg", sysfs_path, id);
>> + fd = open(str, O_WRONLY);
>> + if (fd >= 0)
>> + {
>> + r = write(fd, img, sizeof(img));
>> + close(fd);
>> + }
>> +
>> + free(str);
>> + return r;
>> +}
>> +
>> +int wcmSetOLEDProperty(DeviceIntPtr dev, Atom property,
>> + XIPropertyValuePtr prop, BOOL checkonly,
>> + XIPropertyValuePtr pixmaps)
>> +{
>> + InputInfoPtr pInfo = (InputInfoPtr) dev->public.devicePrivate;
>> + WacomDevicePtr priv = (WacomDevicePtr) pInfo->private;
>> + DrawablePtr pDraw;
>> + void *img;
>> + int i, rc;
>> + int stride;
>> + int img_size;
>> + XID *pixmap = pixmaps->data;
>> + char *state = prop->data;
>> +
>> + if (!serverClient)
>> + return BadImplementation;
>> + if (!priv->sysfs_led)
>> + return BadAtom;
>> + if (prop->type != XA_INTEGER || prop->format != 8)
>> + return BadAtom;
>
> BadMatch
>
>> + if (prop->size != WCM_MAX_LED_IMAGES)
>> + return BadLength;
>> +
>> + /* ideally, 'pixmaps->type' would be 'XA_PIXMAP'.
>> + * The problem is that InitWcmAtom can ony create either
>> + * XA_INTEGER or XA_ATOM properties. For now, XA_ATOM
>> + * has been chosen */
>
> please fix InitWcmAtom to take XA_PIXMAP.
I didn't want to include it, because InitWcmAtom is used in quite a
lot of places, and I wanted to make the patch as small as possible. I
will make a extra patch just for that, so that it can be commited
without the OLED changes.
>> + if (pixmaps->type != XA_ATOM || pixmaps->format != 32)
>> + return BadPixmap;
>> + if (pixmaps->size != WCM_MAX_LED_IMAGES)
>> + return BadLength;
>> +
>> + for (i = 0; i < prop->size; i++)
>> + {
>> + if (!pixmap[i])
>> + return BadImplementation;
>> + rc = dixLookupDrawable(&pDraw, pixmap[i], serverClient, 0, 0);
>> + if (rc != Success)
>> + return rc;
>> + if (pDraw->width != 64 || pDraw->height != 32)
>> + return BadImplementation;
>> + if (BitsPerPixel(pDraw->depth) != 32)
>> + return BadImplementation;
>> + }
>> + if (checkonly)
>> + return Success;
>> +
>> + stride = PixmapBytePad(pDraw->width, pDraw->depth);
>> + img_size = stride * pDraw->height;
>> + img = malloc(img_size);
>> + for (i = 0; i < prop->size; i++)
>> + {
>> + /* only update images with state >= 1 */
>> + if (state[i] < 1)
>> + continue;
>> +
>> + dixLookupDrawable(&pDraw, pixmap[i], serverClient, 0, 0);
>> + pDraw->pScreen->GetImage(pDraw, 0, 0, 64, 32, ZPixmap, -1,
>> img);
>> + led_surface_put(priv->sysfs_led, i, img, stride);
>> + /* Set state to written */
>> + state[i] = 0;
>
> hmm, does this actually work? I can't remember if the driver had access to
> the property data directly.
1) I looked at the XInput source code of the X server, and
2) tested it
Result: You get back "0" on reading the property.
>> + }
>> + free(img);
>> + return Success;
>> +}
>> +
>> +/* vim: set noexpandtab tabstop=8 shiftwidth=8: */
>> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
>> index 14de990..7dd2908 100644
>> --- a/src/wcmUSB.c
>> +++ b/src/wcmUSB.c
>> @@ -133,6 +133,7 @@ static int
>> usbStart(InputInfoPtr pInfo)
>> {
>> int err;
>> + WacomDevicePtr priv = (WacomDevicePtr)pInfo->private;
>>
>> #ifdef EVIOCGRAB
>> /* Try to grab the event device so that data don't leak to
>> /dev/input/mice */
>> @@ -144,6 +145,8 @@ usbStart(InputInfoPtr pInfo)
>> xf86Msg(X_ERROR, "%s: Wacom X driver can't grab event device
>> (%s)\n",
>> pInfo->name, strerror(errno));
>> #endif
>> +
>> + wcmLedStart(priv);
>> return Success;
>> }
>>
>> diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
>> index fe71bd1..71fa2cc 100644
>> --- a/src/wcmXCommand.c
>> +++ b/src/wcmXCommand.c
>> @@ -97,6 +97,8 @@ Atom prop_gesture_param;
>> Atom prop_hover;
>> Atom prop_tooltype;
>> Atom prop_btnactions;
>> +Atom prop_oled_pixmap;
>> +Atom prop_oled_state;
>> Atom prop_product_id;
>> #ifdef DEBUG
>> Atom prop_debuglevels;
>> @@ -220,6 +222,12 @@ void InitWcmDeviceProperties(InputInfoPtr pInfo)
>> /* default to no actions */
>> memset(values, 0, sizeof(values));
>> prop_btnactions = InitWcmAtom(pInfo->dev, WACOM_PROP_BUTTON_ACTIONS,
>> -32, WCM_MAX_MOUSE_BUTTONS, values);
>> + wcmLedInitPixmap(priv, values, WCM_MAX_LED_IMAGES);
>> + prop_oled_pixmap = InitWcmAtom(pInfo->dev, WACOM_PROP_OLED_PIXMAP, -32,
>> + WCM_MAX_LED_IMAGES, values);
>> + memset(values, -1, sizeof(values));
>> + prop_oled_state = InitWcmAtom(pInfo->dev, WACOM_PROP_OLED_STATE, 8,
>> + WCM_MAX_LED_IMAGES, values);
>>
>> if (IsPad(priv)) {
>> memset(values, 0, sizeof(values));
>> @@ -828,6 +836,13 @@ int wcmSetProperty(DeviceIntPtr dev, Atom property,
>> XIPropertyValuePtr prop,
>> if (prop->size != WCM_MAX_MOUSE_BUTTONS)
>> return BadMatch;
>> wcmSetPropertyButtonActions(dev, property, prop, checkonly);
>> + } else if (property == prop_oled_pixmap) {
>> + return BadAccess;
>> + } else if (property == prop_oled_state)
>> + {
>> + XIPropertyValuePtr pixmaps;
>> + XIGetDeviceProperty(dev, prop_oled_pixmap, &pixmaps);
>> + wcmSetOLEDProperty(dev, property, prop, checkonly, pixmaps);
>> } else
>> wcmSetActionProperties(dev, property, prop, checkonly);
>>
>> diff --git a/src/xf86Wacom.h b/src/xf86Wacom.h
>> index c1b55c9..3b58572 100644
>> --- a/src/xf86Wacom.h
>> +++ b/src/xf86Wacom.h
>> @@ -184,6 +184,14 @@ enum WacomSuppressMode {
>> SUPPRESS_NON_MOTION /* Supress all events but x/y motion */
>> };
>>
>> +/* LED functions */
>> +extern void wcmLedInitPixmap(WacomDevicePtr priv, int *values, int count);
>> +extern void wcmLedStart(WacomDevicePtr priv);
>> +extern void wcmLedFree(WacomDevicePtr priv);
>> +int wcmSetOLEDProperty(DeviceIntPtr dev, Atom property,
>
> "extern" missing
>
>> + XIPropertyValuePtr prop, BOOL checkonly,
>> + XIPropertyValuePtr pixmaps);
>> +
>> /****************************************************************************/
>> #endif /* __XF86WACOM_H */
>>
>> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
>> index cf16648..4296a2b 100644
>> --- a/src/xf86WacomDefs.h
>> +++ b/src/xf86WacomDefs.h
>> @@ -189,6 +189,8 @@ struct _WacomModel
>> * tablet buttons besides the strips
>> are
>> * treated as buttons */
>>
>> +#define WCM_MAX_LED_IMAGES 8
>
> this isn't something we can get from the kernel, is it?
You are hitting a "weak" point of my patch. The problem is that
currently for reading the sysfs attribute, I need to have the input
device file handle. This is first available in wcmUsbStart. Should I
change the code to introduce the OLED attributes in wcmUsbStart?
Greetings,
Eduard
>> +
>> /* get/set/property */
>> typedef struct _PROPINFO PROPINFO;
>>
>> @@ -296,6 +298,12 @@ struct _WacomDeviceRec
>> Atom strip_actions[4];
>>
>> OsTimerPtr serial_timer; /* timer used for serial number property
>> update */
>> +
>> + /* path to the sysfs directory for (O)LED control;
>> + * NULL means that the sysfs interface is not present
>> + * (either due to missing driver,
>> + * or device has no (O)LEDs) */
>> + char *sysfs_led;
>> };
>>
>> /******************************************************************************
>> diff --git a/test/fake-symbols.c b/test/fake-symbols.c
>> index bba10e4..4e354a2 100644
>> --- a/test/fake-symbols.c
>> +++ b/test/fake-symbols.c
>> @@ -1,4 +1,5 @@
>> #include "fake-symbols.h"
>> +#include <servermd.h>
>>
>> _X_EXPORT
>> int xf86ReadSerial (int fd, void *buf, int count)
>> @@ -461,3 +462,25 @@ void
>> xf86UnblockSIGIO (int wasset)
>> {
>> }
>> +
>> +int
>> +dixLookupDrawable(DrawablePtr *pDraw, XID id, ClientPtr client,
>> + Mask type, Mask access)
>> +{
>> + return 0;
>> +}
>> +
>> +XID
>> +FakeClientID(int client)
>> +{
>> + return 0;
>> +}
>> +
>> +Bool
>> +AddResource(XID id, RESTYPE type, pointer value)
>> +{
>> + return FALSE;
>> +}
>> +
>> +PaddingInfo PixmapWidthPaddingInfo[33];
>> +ScreenInfo screenInfo;
>> diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
>> index 2dc0c2b..938eda8 100644
>> --- a/tools/xsetwacom.c
>> +++ b/tools/xsetwacom.c
>> @@ -110,6 +110,7 @@ static void set_xydefault(Display *dpy, XDevice *dev,
>> param_t *param, int argc,
>> static void get_all(Display *dpy, XDevice *dev, param_t *param, int argc,
>> char **argv);
>> static void get_param(Display *dpy, XDevice *dev, param_t *param, int argc,
>> char **argv);
>> static void set_output(Display *dpy, XDevice *dev, param_t *param, int
>> argc, char **argv);
>> +static void set_oled_image(Display *dpy, XDevice *dev, param_t *param, int
>> argc, char **argv);
>>
>> /* NOTE: When removing or changing a parameter name, add to
>> * deprecated_parameters.
>> @@ -411,6 +412,12 @@ static param_t parameters[] =
>> .prop_flags = PROP_FLAG_WRITEONLY | PROP_FLAG_OUTPUT,
>> },
>> {
>> + .name = "Image",
>> + .desc = "Set the button image. ",
>> + .set_func = set_oled_image,
>> + .prop_flags = PROP_FLAG_WRITEONLY,
>> + },
>> + {
>> .name = "all",
>> .desc = "Get value for all parameters. ",
>> .get_func = get_all,
>> @@ -2350,6 +2357,86 @@ static void set_output(Display *dpy, XDevice *dev,
>> param_t *param, int argc, cha
>> fprintf(stderr, "Unable to find an output '%s'.\n", argv[0]);
>> }
>>
>> +static void set_oled_image(Display *dpy, XDevice *dev, param_t *param, int
>> argc, char **argv)
>> +{
>> + int id, w, h, range, i;
>> + Atom oled_state = None;
>> + Atom oled_pixmap = None;
>> + Atom type;
>> + GC gc;
>> + XGCValues values;
>> + int index;
>> + int format;
>> + int r;
>> + long state_nitems;
>> + long pixmap_nitems;
>> + long after;
>> + long *pixmap_ptr;
>> + char *state_array;
>> + Pixmap pixmap;
>> + Bool success;
>> +
>> + oled_state = XInternAtom(dpy, WACOM_PROP_OLED_STATE, True);
>> + if (!oled_state)
>> + {
>> + fprintf(stderr, "WACOM_PROP_OLED_STATE not available\n");
>> + return;
>> + }
>> + oled_pixmap = XInternAtom(dpy, WACOM_PROP_OLED_PIXMAP, True);
>> + if (!oled_pixmap)
>> + {
>> + fprintf(stderr, "WACOM_PROP_OLED_PIXMAP not available\n");
>
> use fprintf(stderr, "property "%s' not available\n", WACOM_PROP_OLED_PIXMAP);
> it'll help users find out which property is missing.
>
>> + return;
>> + }
>> +
>> + if (argc < 2)
>> + {
>> + fprintf(stderr, "too few arguments\n");
>> + return;
>> + }
>> +
>> + /* Retrieve OLED state array */
>> + r = XGetDeviceProperty(dpy, dev, oled_state, 0, 8, 0,
>
> the "delete" argument should be False. Same thing in the end, but more
> obvious.
>
>> + AnyPropertyType, &type, &format,
>> + &state_nitems, &after,
>> + (unsigned char**)&state_array);
>
> We also usually also check type and format to make sure we do have the right
> property. However, this would benefit from Jason's patches to _get and
> _set, so be aware that you might have to rebase soon-ish.
>> + if (r != Success) {
>
> { on new line
>
>> + fprintf(stderr, "failed to set image\n");
>> + return;
>> + }
>> +
>> + success = convert_value_from_user(param, argv[0], &index);
>> + if (!success || index < 0 || index >= state_nitems)
>> + {
>> + fprintf(stderr, "'%s' is not a valid image index\n", argv[0]);
>> + return;
>> + }
>> +
>> + /* Retrieve Pixmap to be changed */
>> + r = XGetDeviceProperty(dpy, dev, oled_pixmap, index, 1, 0,
>
> s/0/False/ again
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
