Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
Hi Peter,
Peter Hutterer said the following on 11/28/2012 01:43 AM:
[...]
I have to admit, this confused me quite a bit during testing (reason was
that I forgot to install the data files at first, so list-local-devices gave
me wrong values).
turns out integration_flags is the one read from the kernel and can only be
DISPLAY or NONE, so we need this weird construct to keep the SYSTEM alive
(if it is there in the database).
Now, that brings up the question: do we really want to override
the device database entries with kernel-queried ones?
It should be the other way around. The kernel flags are used only if
the integration flags are not set in the database.
get_device_info() which is the routines that reads the kernel flags is
called from libwacom_new_from_path();
libwacom_new_from_path() passes an integration_flags local variable
to get_device_info() which is set based on the kernel flags.
Then only the display part of the flag is used to update the device flags:
if (device) {
if (integration_flags == WACOM_DEVICE_INTEGRATED_DISPLAY)
ret-integration_flags |= WACOM_DEVICE_INTEGRATED_DISPLAY;
else if (integration_flags == WACOM_DEVICE_INTEGRATED_NONE)
ret-integration_flags = ~WACOM_DEVICE_INTEGRATED_DISPLAY;
return ret;
}
(was like that with is_builtin, so nothing new here). The system
part of the integration flags is left untouched and remains to
whatever is set in the database (the kernel does not allow to
automatically determine ISDs)
[...]
+WacomIntegrationFlags libwacom_get_integration_flags (WacomDevice *device)
+{
sorry, I didn't spot this earlier, this should be const WacomDevice *device.
(only noticed that when doing some printf testing on my device). Given that
is_builtin, is_reversible, etc aren't const we should update all of them in
a follow-up patch.
Yes, I simply copy/pasted from one of the previous functions... I'll
send a separate global constification patch.
Cheers,
Olivier.
--
Keep yourself connected to Go Parallel:
VERIFY Test and improve your parallel project with help from experts
and peers. http://goparallel.sourceforge.net
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
Olivier Fourdan said the following on 11/29/2012 05:49 PM: [...] It should be the other way around. The kernel flags are used only if the integration flags are not set in the database. Sorry please ignore that sentence in my previous reply, it was left over from a first draft... meh :( Cheers, Olivier. -- Keep yourself connected to Go Parallel: VERIFY Test and improve your parallel project with help from experts and peers. http://goparallel.sourceforge.net ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
Peter Hutterer said the following on 11/27/2012 12:27 AM:
[...]
+ device-integration_flags = WACOM_DEVICE_INTEGRATED_NONE;
+ for (i = 0; string_list[i]; i++) {
+ for (n = 0; n G_N_ELEMENTS (integration_flags); n++) {
+ if (!strcmp(string_list[i],
integration_flags[n].key)) {
+ device-integration_flags |=
integration_flags[n].value;
+ break;
+ }
+ }
+ }
+ g_strfreev (string_list);
+ }
+
a follow-up patch could check for invalid IntegratedIn values and warn about
it.
No need to wait for a follow-up patch really, it's very useful and
trivial to do.
Added :)
static gboolean
-get_device_info (const char *path,
-int *vendor_id,
-int *product_id,
-char**name,
-WacomBusType *bus,
-IsBuiltin*builtin,
-WacomError *error)
+get_device_info (const char*path,
+int *vendor_id,
+int *product_id,
+char **name,
+WacomBusType *bus,
+WacomIntegrationFlags *integration_flags,
+WacomError*error)
please fix up the alignment here, path is misplaced (may look correct in code,
in which case ignore this comment)
Alignment is wrong in the patch because of the tab cars in the
following lines, yet it's just fine in the code.
I left that out.
[...]
/**
+ * Tablet integration.
+ */
+typedef enum {
+ WACOM_DEVICE_INTEGRATED_UNSET = -1,
mark this as internal with
WACOM_DEVICE_INTEGRATED_UNSET = -1, /** For internal use only */
tough tbh we should not really expose this in the public header if we don't
want clients to use it.
we should be able to
#define WACOM_DEVICE_INTEGRATED_UNSET (WACOM_DEVICE_INTEGRATED_NONE - 1)
in libwacomint.h to avoid having this exposed.
Done.
It works, yet I'm not sure what effect that may have on the compiler
optimization, the enul would be treated as an unsigned yet we use a
neg value. Eanyway, gcc does not seem to complain so I guess it's OK.
[...]
+#ifndef LIBWACOM_DISABLE_DEPRECATED
/**
* @param device The tablet to query
- * @return non-zero if the device is built-in or zero if the device is an
- * external tablet
+ * @return non-zero if the device is built into the screen (ie a screen tablet)
+ * or zero if the device is an external tablet
+ * Deprecated: 0.7: Use libwacom_get_integration_flags() instead.
*/
int libwacom_is_builtin(WacomDevice *device);
+#endif
my previous comments about __attribute__((deprecated)) and @deprecated still
stand :)
Oh yeah, I completely missed that in the original review, my bad :(
Done, added a LIBWACOM_DEPRECATED macro. Tested it works as expected.
That allowed me to spot the use of the deprecated function in
test/load.c so that's now fixed as well.
The load tests for both integration fields for the integrated in
device + screen Wacom serial now.
Updated patch attached.
Do you want all 3 patches at once so we don't miss anything?
Cheers,
Olivier.
--
əɔıʌəp əɯos ɥʇıʍ əɹəɥʍəɯos ɯoɹɟ ʇuəs
From 06748d1e713cf98872bbaabc9b93e7ca68c64ccd Mon Sep 17 00:00:00 2001
From: Olivier Fourdan ofour...@redhat.com
Date: Thu, 11 Oct 2012 12:04:45 +0200
Subject: [PATCH 3/3] lib: add IntegratedIn to device group
to describe integrated system devices and screen tablets.
A bit field allows to identify the level of integration of
a device:
WACOM_DEVICE_INTEGRATED_NONEdevice is a standalone tablet
WACOM_DEVICE_INTEGRATED_DISPLAY device is a screen tablet
WACOM_DEVICE_INTEGRATED_SYSTEM device is an ISD such as
a tablet PC.
These definitions supersede the previous libwacom_is_builtin()
API which is now deprecated.
Signed-off-by: Olivier Fourdan ofour...@redhat.com
---
libwacom/libwacom-database.c | 64 --
libwacom/libwacom.c | 70 ++
libwacom/libwacom.h | 29 +++--
libwacom/libwacomint.h | 11 ++-
test/load.c |6 ++--
5 files changed, 129 insertions(+), 51 deletions(-)
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index bbea114..e0ee52e 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -256,6 +256,14 @@ struct {
{ Touchstrip2, WACOM_STATUS_LED_TOUCHSTRIP2 }
};
+struct {
+ const char *key;
+ WacomIntegrationFlags value;
+} integration_flags[] = {
+ { Display, WACOM_DEVICE_INTEGRATED_DISPLAY },
+ { System, WACOM_DEVICE_INTEGRATED_SYSTEM }
+};
+
static void
libwacom_parse_buttons_key(WacomDevice
Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
On Tue, Nov 27, 2012 at 11:02:03AM +0100, Olivier Fourdan wrote:
Peter Hutterer said the following on 11/27/2012 12:27 AM:
[...]
+device-integration_flags = WACOM_DEVICE_INTEGRATED_NONE;
+for (i = 0; string_list[i]; i++) {
+for (n = 0; n G_N_ELEMENTS (integration_flags); n++) {
+if (!strcmp(string_list[i],
integration_flags[n].key)) {
+device-integration_flags |=
integration_flags[n].value;
+break;
+}
+}
+}
+g_strfreev (string_list);
+}
+
a follow-up patch could check for invalid IntegratedIn values and warn about
it.
No need to wait for a follow-up patch really, it's very useful and
trivial to do.
Added :)
static gboolean
-get_device_info (const char *path,
- int *vendor_id,
- int *product_id,
- char**name,
- WacomBusType *bus,
- IsBuiltin*builtin,
- WacomError *error)
+get_device_info (const char*path,
+ int *vendor_id,
+ int *product_id,
+ char **name,
+ WacomBusType *bus,
+ WacomIntegrationFlags *integration_flags,
+ WacomError*error)
please fix up the alignment here, path is misplaced (may look correct in
code,
in which case ignore this comment)
Alignment is wrong in the patch because of the tab cars in the
following lines, yet it's just fine in the code.
I left that out.
[...]
/**
+ * Tablet integration.
+ */
+typedef enum {
+WACOM_DEVICE_INTEGRATED_UNSET = -1,
mark this as internal with
WACOM_DEVICE_INTEGRATED_UNSET = -1, /** For internal use only */
tough tbh we should not really expose this in the public header if we don't
want clients to use it.
we should be able to
#define WACOM_DEVICE_INTEGRATED_UNSET (WACOM_DEVICE_INTEGRATED_NONE - 1)
in libwacomint.h to avoid having this exposed.
Done.
It works, yet I'm not sure what effect that may have on the compiler
optimization, the enul would be treated as an unsigned yet we use a
neg value. Eanyway, gcc does not seem to complain so I guess it's
OK.
if it's a signed enum, (0 - 1) == -1
if it's unsigned, (0 - 1) will rollover and give us the unsigned equiv of -1
[...]
+#ifndef LIBWACOM_DISABLE_DEPRECATED
/**
* @param device The tablet to query
- * @return non-zero if the device is built-in or zero if the device is an
- * external tablet
+ * @return non-zero if the device is built into the screen (ie a screen
tablet)
+ * or zero if the device is an external tablet
+ * Deprecated: 0.7: Use libwacom_get_integration_flags() instead.
*/
int libwacom_is_builtin(WacomDevice *device);
+#endif
my previous comments about __attribute__((deprecated)) and @deprecated still
stand :)
Oh yeah, I completely missed that in the original review, my bad :(
Done, added a LIBWACOM_DEPRECATED macro. Tested it works as expected.
That allowed me to spot the use of the deprecated function in
test/load.c so that's now fixed as well.
The load tests for both integration fields for the integrated in
device + screen Wacom serial now.
Updated patch attached.
Do you want all 3 patches at once so we don't miss anything?
I've pushed 1/3 out already, 2/3 is in my tree ready to go and I've merged
this one for now (see one comment below though).
so no need to re-send.
From 06748d1e713cf98872bbaabc9b93e7ca68c64ccd Mon Sep 17 00:00:00 2001
From: Olivier Fourdan ofour...@redhat.com
Date: Thu, 11 Oct 2012 12:04:45 +0200
Subject: [PATCH 3/3] lib: add IntegratedIn to device group
to describe integrated system devices and screen tablets.
A bit field allows to identify the level of integration of
a device:
WACOM_DEVICE_INTEGRATED_NONEdevice is a standalone tablet
WACOM_DEVICE_INTEGRATED_DISPLAY device is a screen tablet
WACOM_DEVICE_INTEGRATED_SYSTEM device is an ISD such as
a tablet PC.
These definitions supersede the previous libwacom_is_builtin()
API which is now deprecated.
Signed-off-by: Olivier Fourdan ofour...@redhat.com
---
libwacom/libwacom-database.c | 64 --
libwacom/libwacom.c | 70
++
libwacom/libwacom.h | 29 +++--
libwacom/libwacomint.h | 11 ++-
test/load.c |6 ++--
5 files changed, 129 insertions(+), 51 deletions(-)
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index bbea114..e0ee52e 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -256,6 +256,14 @@ struct {
Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
Olivier Fourdan said the following on 11/26/2012 08:44 AM: So how about making UNSET internal-only (in which case you can leave it as -1) but filter it to NONE before returning it to the client? Yeap, agreed, updated patch attached. Not USER is filtered out and NONE is returned to the client instead. This sentence may have been encrypted dynamically between brain and fingers... That should read Yeap, agreed, updated patch attached. _Now_ _UNSET_ is filtered out and NONE is returned to the client instead. Monday morning fog is more likely to blame. Sorry for the noise. Cheers, Olivier. -- Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications! http://p.sf.net/sfu/zoho_dev2dev_nov ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
On Mon, Nov 26, 2012 at 08:44:56AM +0100, Olivier Fourdan wrote:
Hi Peter,
Peter Hutterer said the following on 11/26/2012 07:47 AM:
[...]
I'd like to get this API right before we make it public and have clients
depend on it, with all the lag that entails (I know you already have
something rely on it, but... :)
Oh no no, I don't. not yet... all I have is plans :)
Is UNSET actually necessary outside of libwacom itself? I can see why you
need it for the auto-detection internally, but why does a client care if
something is UNSET or NONE? assume that for the client UNSET == NONE in any
case (how else could you treat it?). if that not correct it's a bug that
needs to be fixed with a database entry.
So how about making UNSET internal-only (in which case you can leave it as
-1) but filter it to NONE before returning it to the client?
Yeap, agreed, updated patch attached. Not USER is filtered out and
NONE is returned to the client instead.
Thanks again
Olivier
From 6cd18976ef4b58a791a50218ef0e5e73dcd07f19 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan ofour...@redhat.com
Date: Thu, 11 Oct 2012 12:04:45 +0200
Subject: [PATCH 3/3] lib: add IntegratedIn to device group
to describe integrated system devices and screen tablets.
A bit field allows to identify the level of integration of
a device:
WACOM_DEVICE_INTEGRATED_NONEdevice is a standalone tablet
WACOM_DEVICE_INTEGRATED_DISPLAY device is a screen tablet
WACOM_DEVICE_INTEGRATED_SYSTEM device is an ISD such as
a tablet PC.
These definitions supersede the previous libwacom_is_builtin()
API which is now deprecated.
Signed-off-by: Olivier Fourdan ofour...@redhat.com
---
libwacom/libwacom-database.c | 59 +--
libwacom/libwacom.c | 70
++
libwacom/libwacom.h | 23 -
libwacom/libwacomint.h | 10 +-
4 files changed, 115 insertions(+), 47 deletions(-)
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index bbea114..ae73239 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -256,6 +256,14 @@ struct {
{ Touchstrip2,WACOM_STATUS_LED_TOUCHSTRIP2 }
};
+struct {
+ const char *key;
+ WacomIntegrationFlags value;
+} integration_flags[] = {
+ { Display,WACOM_DEVICE_INTEGRATED_DISPLAY },
+ { System, WACOM_DEVICE_INTEGRATED_SYSTEM }
+};
+
static void
libwacom_parse_buttons_key(WacomDevice *device,
GKeyFile *keyfile,
@@ -329,8 +337,7 @@ libwacom_parse_tablet_keyfile(const char *datadir, const
char *filename)
char *layout;
char *class;
char *match;
- char **styli_list;
- char **leds_list;
+ char **string_list;
keyfile = g_key_file_new();
@@ -361,31 +368,50 @@ libwacom_parse_tablet_keyfile(const char *datadir,
const char *filename)
device-name = g_key_file_get_string(keyfile, DEVICE_GROUP, Name,
NULL);
device-width = g_key_file_get_integer(keyfile, DEVICE_GROUP, Width,
NULL);
device-height = g_key_file_get_integer(keyfile, DEVICE_GROUP,
Height, NULL);
+
+ device-integration_flags = WACOM_DEVICE_INTEGRATED_UNSET;
+ string_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP,
IntegratedIn, NULL, NULL);
+ if (string_list) {
+ guint i, n;
+
+ device-integration_flags = WACOM_DEVICE_INTEGRATED_NONE;
+ for (i = 0; string_list[i]; i++) {
+ for (n = 0; n G_N_ELEMENTS (integration_flags); n++) {
+ if (!strcmp(string_list[i],
integration_flags[n].key)) {
+ device-integration_flags |=
integration_flags[n].value;
+ break;
+ }
+ }
+ }
+ g_strfreev (string_list);
+ }
+
a follow-up patch could check for invalid IntegratedIn values and warn about
it.
layout = g_key_file_get_string(keyfile, DEVICE_GROUP, Layout, NULL);
if (layout) {
/* For the layout, we store the full path to the SVG layout */
device-layout = g_build_filename (datadir, layouts, layout,
NULL);
g_free (layout);
}
+
class = g_key_file_get_string(keyfile, DEVICE_GROUP, Class, NULL);
device-cls = libwacom_class_string_to_enum(class);
g_free(class);
- styli_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP, Styli,
NULL, NULL);
- if (styli_list) {
+ string_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP,
Styli, NULL, NULL);
+ if (string_list) {
GArray *array;
guint i;
array = g_array_new (FALSE,
Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
On Fri, Nov 23, 2012 at 09:28:26AM +0100, Olivier Fourdan wrote: Hey Peter, Peter Hutterer said the following on 11/23/2012 12:21 AM: [...] two things: I'd like to see a test that screams bloody murder when the flag is in fact unset. presumably if we add new devices we known whether they're built-in and the test should complain if we forget to add this. No, it's what is meant here. A database entry can miss the IntegratedIn filed, that's OK, many do actually. By Not specifying IntegratedIn in the device database, you let libwacom decide by using the kernel flags (see previous patch). If you specify IntegratedIn empty, then it means you actually force the value and ignore the kernel flags. That's how it was before with BuiltIn actually, if BuiltIn is set it takes precedence over the kernel flags. OK, that makes sense. though given the limited set of integrated tablets in our comparatively small database it would still make sense to add the field to all descriptions in any case. the other thing: having UNSET as -1 for a bitmask is not a good choice, it requires any user to check for UNSET _and_ check for the mask set, since -1 will always set all bits. Oh I completely agree to this. My first submitted patch was using WACOM_DEVICE_INTEGRATED_UNSET = (1 31) so that all lower (meaningful) bits where guaranteed to be zero, while we could still tell if the flags were set in the database definition or not (becuase we also use that to determine if we use the kernel flags if not set in the database definition, just like it was with BuiltIn). The reason I changed that to -1 as a flag is because I've been told to in the previous review, see [1], [2] and [3]. So I'd rather not revert the change even though I agree. Unless we all reach some sort of a consensus (contradictory reviews somehow make it harder to adjust the patches). I'd like to get this API right before we make it public and have clients depend on it, with all the lag that entails (I know you already have something rely on it, but... :) Is UNSET actually necessary outside of libwacom itself? I can see why you need it for the auto-detection internally, but why does a client care if something is UNSET or NONE? assume that for the client UNSET == NONE in any case (how else could you treat it?). if that not correct it's a bug that needs to be fixed with a database entry. So how about making UNSET internal-only (in which case you can leave it as -1) but filter it to NONE before returning it to the client? Cheers, Peter so either we bump all up by one so UNSET is 0 and NONE is 1, or we leave NONE at 0 and guarantee that we always define this and drop UNSET. I don't think we want to drop unset (at least if we want to keep consistent with current behavior). As to why 0 is not a good idea, well, that's because we want to tell if the flag is set in the database, see [2] Cheers, Olivier. [1] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5302 [2] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5305 [3] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5315 -- Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications! http://p.sf.net/sfu/zoho_dev2dev_nov ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
Hey Peter, Peter Hutterer said the following on 11/23/2012 12:21 AM: [...] two things: I'd like to see a test that screams bloody murder when the flag is in fact unset. presumably if we add new devices we known whether they're built-in and the test should complain if we forget to add this. No, it's what is meant here. A database entry can miss the IntegratedIn filed, that's OK, many do actually. By Not specifying IntegratedIn in the device database, you let libwacom decide by using the kernel flags (see previous patch). If you specify IntegratedIn empty, then it means you actually force the value and ignore the kernel flags. That's how it was before with BuiltIn actually, if BuiltIn is set it takes precedence over the kernel flags. the other thing: having UNSET as -1 for a bitmask is not a good choice, it requires any user to check for UNSET _and_ check for the mask set, since -1 will always set all bits. Oh I completely agree to this. My first submitted patch was using WACOM_DEVICE_INTEGRATED_UNSET = (1 31) so that all lower (meaningful) bits where guaranteed to be zero, while we could still tell if the flags were set in the database definition or not (becuase we also use that to determine if we use the kernel flags if not set in the database definition, just like it was with BuiltIn). The reason I changed that to -1 as a flag is because I've been told to in the previous review, see [1], [2] and [3]. So I'd rather not revert the change even though I agree. Unless we all reach some sort of a consensus (contradictory reviews somehow make it harder to adjust the patches). so either we bump all up by one so UNSET is 0 and NONE is 1, or we leave NONE at 0 and guarantee that we always define this and drop UNSET. I don't think we want to drop unset (at least if we want to keep consistent with current behavior). As to why 0 is not a good idea, well, that's because we want to tell if the flag is set in the database, see [2] Cheers, Olivier. [1] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5302 [2] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5305 [3] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5315 -- Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications! http://p.sf.net/sfu/zoho_dev2dev_nov ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
On Wed, Nov 07, 2012 at 01:22:11PM +0100, Olivier Fourdan wrote:
From 15f93f5d3bd9f2aefd59a5087b2ce44a3c53403c Mon Sep 17 00:00:00 2001
From: Olivier Fourdan ofour...@redhat.com
Date: Thu, 11 Oct 2012 12:04:45 +0200
Subject: [PATCH 3/3] lib: add IntegratedIn to device group
to describe integrated system devices and screen tablets.
A bit field allows to identify the level of integration of
a device:
WACOM_DEVICE_INTEGRATED_NONEdevice is a standalone tablet
WACOM_DEVICE_INTEGRATED_DISPLAY device is a screen tablet
WACOM_DEVICE_INTEGRATED_SYSTEM device is an ISD such as
a tablet PC.
Or -1 (WACOM_DEVICE_INTEGRATED_UNSET) if the information is not
available
two things:
I'd like to see a test that screams bloody murder when the flag is in fact
unset. presumably if we add new devices we known whether they're built-in
and the test should complain if we forget to add this.
the other thing: having UNSET as -1 for a bitmask is not a good choice, it
requires any user to check for UNSET _and_ check for the mask set, since -1
will always set all bits.
so either we bump all up by one so UNSET is 0 and NONE is 1, or we leave
NONE at 0 and guarantee that we always define this and drop UNSET.
These definitions supersede the previous libwacom_is_builtin()
API which is now deprecated.
Signed-off-by: Olivier Fourdan ofour...@redhat.com
---
libwacom/libwacom-database.c | 59 ++--
libwacom/libwacom.c | 67 +
libwacom/libwacom.h | 24 ++-
libwacom/libwacomint.h | 10 +-
4 files changed, 113 insertions(+), 47 deletions(-)
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index bbea114..ae73239 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -256,6 +256,14 @@ struct {
{ Touchstrip2,WACOM_STATUS_LED_TOUCHSTRIP2 }
};
+struct {
+ const char *key;
+ WacomIntegrationFlags value;
+} integration_flags[] = {
+ { Display,WACOM_DEVICE_INTEGRATED_DISPLAY },
+ { System, WACOM_DEVICE_INTEGRATED_SYSTEM }
+};
+
static void
libwacom_parse_buttons_key(WacomDevice *device,
GKeyFile *keyfile,
@@ -329,8 +337,7 @@ libwacom_parse_tablet_keyfile(const char *datadir, const
char *filename)
char *layout;
char *class;
char *match;
- char **styli_list;
- char **leds_list;
+ char **string_list;
keyfile = g_key_file_new();
@@ -361,31 +368,50 @@ libwacom_parse_tablet_keyfile(const char *datadir,
const char *filename)
device-name = g_key_file_get_string(keyfile, DEVICE_GROUP, Name,
NULL);
device-width = g_key_file_get_integer(keyfile, DEVICE_GROUP, Width,
NULL);
device-height = g_key_file_get_integer(keyfile, DEVICE_GROUP,
Height, NULL);
+
+ device-integration_flags = WACOM_DEVICE_INTEGRATED_UNSET;
+ string_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP,
IntegratedIn, NULL, NULL);
+ if (string_list) {
+ guint i, n;
+
+ device-integration_flags = WACOM_DEVICE_INTEGRATED_NONE;
+ for (i = 0; string_list[i]; i++) {
+ for (n = 0; n G_N_ELEMENTS (integration_flags); n++) {
+ if (!strcmp(string_list[i],
integration_flags[n].key)) {
+ device-integration_flags |=
integration_flags[n].value;
+ break;
+ }
+ }
+ }
+ g_strfreev (string_list);
+ }
+
layout = g_key_file_get_string(keyfile, DEVICE_GROUP, Layout, NULL);
if (layout) {
/* For the layout, we store the full path to the SVG layout */
device-layout = g_build_filename (datadir, layouts, layout,
NULL);
g_free (layout);
}
+
class = g_key_file_get_string(keyfile, DEVICE_GROUP, Class, NULL);
device-cls = libwacom_class_string_to_enum(class);
g_free(class);
- styli_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP, Styli,
NULL, NULL);
- if (styli_list) {
+ string_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP,
Styli, NULL, NULL);
+ if (string_list) {
GArray *array;
guint i;
array = g_array_new (FALSE, FALSE, sizeof(int));
device-num_styli = 0;
- for (i = 0; styli_list[i]; i++) {
- glong long_value = strtol (styli_list[i], NULL, 0);
+ for (i = 0; string_list[i]; i++) {
+ glong long_value = strtol (string_list[i], NULL, 0);
int int_value = long_value;
g_array_append_val
[Linuxwacom-devel] libwacom: [PATCH 3/3] lib: add IntegratedIn to device group
From 15f93f5d3bd9f2aefd59a5087b2ce44a3c53403c Mon Sep 17 00:00:00 2001
From: Olivier Fourdan ofour...@redhat.com
Date: Thu, 11 Oct 2012 12:04:45 +0200
Subject: [PATCH 3/3] lib: add IntegratedIn to device group
to describe integrated system devices and screen tablets.
A bit field allows to identify the level of integration of
a device:
WACOM_DEVICE_INTEGRATED_NONEdevice is a standalone tablet
WACOM_DEVICE_INTEGRATED_DISPLAY device is a screen tablet
WACOM_DEVICE_INTEGRATED_SYSTEM device is an ISD such as
a tablet PC.
Or -1 (WACOM_DEVICE_INTEGRATED_UNSET) if the information is not
available
These definitions supersede the previous libwacom_is_builtin()
API which is now deprecated.
Signed-off-by: Olivier Fourdan ofour...@redhat.com
---
libwacom/libwacom-database.c | 59 ++--
libwacom/libwacom.c | 67 +
libwacom/libwacom.h | 24 ++-
libwacom/libwacomint.h | 10 +-
4 files changed, 113 insertions(+), 47 deletions(-)
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index bbea114..ae73239 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -256,6 +256,14 @@ struct {
{ Touchstrip2, WACOM_STATUS_LED_TOUCHSTRIP2 }
};
+struct {
+ const char *key;
+ WacomIntegrationFlags value;
+} integration_flags[] = {
+ { Display, WACOM_DEVICE_INTEGRATED_DISPLAY },
+ { System, WACOM_DEVICE_INTEGRATED_SYSTEM }
+};
+
static void
libwacom_parse_buttons_key(WacomDevice *device,
GKeyFile *keyfile,
@@ -329,8 +337,7 @@ libwacom_parse_tablet_keyfile(const char *datadir, const char *filename)
char *layout;
char *class;
char *match;
- char **styli_list;
- char **leds_list;
+ char **string_list;
keyfile = g_key_file_new();
@@ -361,31 +368,50 @@ libwacom_parse_tablet_keyfile(const char *datadir, const char *filename)
device-name = g_key_file_get_string(keyfile, DEVICE_GROUP, Name, NULL);
device-width = g_key_file_get_integer(keyfile, DEVICE_GROUP, Width, NULL);
device-height = g_key_file_get_integer(keyfile, DEVICE_GROUP, Height, NULL);
+
+ device-integration_flags = WACOM_DEVICE_INTEGRATED_UNSET;
+ string_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP, IntegratedIn, NULL, NULL);
+ if (string_list) {
+ guint i, n;
+
+ device-integration_flags = WACOM_DEVICE_INTEGRATED_NONE;
+ for (i = 0; string_list[i]; i++) {
+ for (n = 0; n G_N_ELEMENTS (integration_flags); n++) {
+if (!strcmp(string_list[i], integration_flags[n].key)) {
+ device-integration_flags |= integration_flags[n].value;
+ break;
+}
+ }
+ }
+ g_strfreev (string_list);
+ }
+
layout = g_key_file_get_string(keyfile, DEVICE_GROUP, Layout, NULL);
if (layout) {
/* For the layout, we store the full path to the SVG layout */
device-layout = g_build_filename (datadir, layouts, layout, NULL);
g_free (layout);
}
+
class = g_key_file_get_string(keyfile, DEVICE_GROUP, Class, NULL);
device-cls = libwacom_class_string_to_enum(class);
g_free(class);
- styli_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP, Styli, NULL, NULL);
- if (styli_list) {
+ string_list = g_key_file_get_string_list(keyfile, DEVICE_GROUP, Styli, NULL, NULL);
+ if (string_list) {
GArray *array;
guint i;
array = g_array_new (FALSE, FALSE, sizeof(int));
device-num_styli = 0;
- for (i = 0; styli_list[i]; i++) {
- glong long_value = strtol (styli_list[i], NULL, 0);
+ for (i = 0; string_list[i]; i++) {
+ glong long_value = strtol (string_list[i], NULL, 0);
int int_value = long_value;
g_array_append_val (array, int_value);
device-num_styli++;
}
- g_strfreev (styli_list);
+ g_strfreev (string_list);
device-supported_styli = (int *) g_array_free (array, FALSE);
} else {
device-supported_styli = g_new (int, 2);
@@ -407,15 +433,13 @@ libwacom_parse_tablet_keyfile(const char *datadir, const char *filename)
if (g_key_file_get_boolean(keyfile, FEATURES_GROUP, Ring2, NULL))
device-features |= FEATURE_RING2;
- if (g_key_file_get_boolean(keyfile, FEATURES_GROUP, BuiltIn, NULL))
- device-features |= FEATURE_BUILTIN;
-
if (g_key_file_get_boolean(keyfile, FEATURES_GROUP, Reversible, NULL))
device-features |= FEATURE_REVERSIBLE;
- if (device-features FEATURE_BUILTIN
+ if (device-integration_flags != WACOM_DEVICE_INTEGRATED_UNSET
+ device-integration_flags WACOM_DEVICE_INTEGRATED_DISPLAY
device-features FEATURE_REVERSIBLE)
- g_warning (Tablet '%s' is both reversible and builtin. This is impossible, libwacom_get_match(device));
+ g_warning (Tablet '%s' is both reversible and integrated in screen. This is impossible, libwacom_get_match(device));
if (!(device-features FEATURE_RING)
(device-features FEATURE_RING2))
@@ -433,22 +457,23 @@ libwacom_parse_tablet_keyfile(const char *datadir, const char *filename)
