Hi Peter,
Peter Hutterer said the following on 10/17/2012 10:39 AM:
On 17/10/12 18:00 , Olivier Fourdan wrote:
Hey Peter,
Peter Hutterer said the following on 10/17/2012 09:48 AM:
I shall send a fix to get to the same level as that one then.
if that's the only difference, just revert the current patch and
apply
the right one on top, no need to diff the two up.
But it's been already pushed and a revert would look ugly, wouldn't
it?
we're not in for a beauty contest :)
revert vs diff is always a personal decision, I admit. IMO ideally
each bug fix is a single patch only* (for the purpose of git blame
or backporting). So when you notice that the wrong patch got pushed,
or that there was a bit missing, a revert can sometimes be better
than a new patch - it explicitly shows what went wrong, why, and
then has the fix for it in one unit. That can be very useful.
Never be afraid of reverting. A nice looking history isn't
necessarily the same thing as a useful history.
I've attached the 2 patches here, eventually it's the same as "[PATCH]
lib: Fix printing StatusLEDs data" (so if this gets pushed, please
disregard "[PATCH] lib: Fix printing StatusLEDs data")
Whatever gets pushed is fine with me, as long as the issue is fixed :-)
Cheers,
Olivier.
>From 9250f7c492e02f6f9424f739e58be968221b1a78 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofour...@redhat.com>
Date: Wed, 17 Oct 2012 10:49:13 +0200
Subject: [PATCH 1/2] Revert "lib: add helper functions to list available status LED"
This reverts commit ad98ad915f6cd7d3ef90112b1449d964b71fd6c4.
Previously the code would read "StatusLEDs=" and
print "LEDs=" so dbverify would fail while trying
to reload the printed data.
Signed-off-by: Olivier Fourdan <ofour...@redhat.com>
---
libwacom/libwacom-database.c | 30 ----------------
libwacom/libwacom.c | 77 ------------------------------------------
libwacom/libwacom.h | 24 -------------
libwacom/libwacomint.h | 3 --
4 files changed, 0 insertions(+), 134 deletions(-)
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index d39305d..5468975 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -246,16 +246,6 @@ struct {
{ "OLEDs", WACOM_BUTTON_OLED }
};
-struct {
- const char *key;
- WacomStatusLEDs value;
-} supported_leds[] = {
- { "Ring", WACOM_STATUS_LED_RING },
- { "Ring2", WACOM_STATUS_LED_RING2 },
- { "Touchstrip", WACOM_STATUS_LED_TOUCHSTRIP },
- { "Touchstrip2", WACOM_STATUS_LED_TOUCHSTRIP2 }
-};
-
static void
libwacom_parse_buttons_key(WacomDevice *device,
GKeyFile *keyfile,
@@ -328,7 +318,6 @@ libwacom_parse_tablet_keyfile(const char *path)
char *class;
char *match;
char **styli_list;
- char **leds_list;
keyfile = g_key_file_new();
@@ -425,25 +414,6 @@ libwacom_parse_tablet_keyfile(const char *path)
libwacom_parse_buttons(device, keyfile);
}
- leds_list = g_key_file_get_string_list(keyfile, FEATURES_GROUP, "StatusLEDs", NULL, NULL);
- if (leds_list) {
- GArray *array;
- guint i, n;
- array = g_array_new (FALSE, FALSE, sizeof(WacomStatusLEDs));
- device->num_leds = 0;
- for (i = 0; leds_list[i]; i++) {
- for (n = 0; n < G_N_ELEMENTS (supported_leds); n++) {
- if (!strcmp(leds_list[i], supported_leds[n].key)) {
- g_array_append_val (array, supported_leds[n].value);
- device->num_leds++;
- break;
- }
- }
- }
- g_strfreev (leds_list);
- device->status_leds = (WacomStatusLEDs *) g_array_free (array, FALSE);
- }
-
out:
if (keyfile)
g_key_file_free(keyfile);
diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c
index 986fe10..fe850a2 100644
--- a/libwacom/libwacom.c
+++ b/libwacom/libwacom.c
@@ -304,8 +304,6 @@ libwacom_copy(const WacomDevice *device)
d->ring2_num_modes = device->ring2_num_modes;
d->num_styli = device->num_styli;
d->supported_styli = g_memdup (device->supported_styli, sizeof(int) * device->num_styli);
- d->num_leds = device->num_leds;
- d->status_leds = g_memdup (device->status_leds, sizeof(WacomStatusLEDs) * device->num_leds);
d->num_buttons = device->num_buttons;
d->buttons = g_memdup (device->buttons, sizeof(WacomButtonFlags) * device->num_buttons);
return d;
@@ -374,12 +372,6 @@ libwacom_compare(WacomDevice *a, WacomDevice *b, WacomCompareFlags flags)
if (memcmp(a->supported_styli, b->supported_styli, sizeof(int) * a->num_styli) != 0)
return 1;
- if (a->num_leds != b->num_leds)
- return 1;
-
- if (memcmp(a->status_leds, b->status_leds, sizeof(WacomStatusLEDs) * a->num_leds) != 0)
- return 1;
-
if (memcmp(a->buttons, b->buttons, sizeof(WacomButtonFlags) * a->num_buttons) != 0)
return 1;
@@ -537,26 +529,6 @@ static void print_styli_for_device (int fd, WacomDevice *device)
dprintf(fd, "\n");
}
-static void print_supported_leds (int fd, WacomDevice *device)
-{
- char *leds_name[] = {
- "Ring",
- "Ring2",
- "Touchstrip",
- "Touchstrip2"
- };
- int num_leds;
- const WacomStatusLEDs *status_leds;
- int i;
-
- status_leds = libwacom_get_status_leds(device, &num_leds);
-
- dprintf(fd, "LEDs=");
- for (i = 0; i < num_leds; i++)
- dprintf(fd, "%s;", leds_name [status_leds[i]]);
- dprintf(fd, "\n");
-}
-
static void print_button_flag_if(int fd, WacomDevice *device, const char *label, int flag)
{
int nbuttons = libwacom_get_num_buttons(device);
@@ -648,7 +620,6 @@ libwacom_print_device_description(int fd, WacomDevice *device)
dprintf(fd, "Ring2=%s\n", libwacom_has_ring2(device) ? "true" : "false");
dprintf(fd, "BuiltIn=%s\n", libwacom_is_builtin(device) ? "true" : "false");
dprintf(fd, "Touch=%s\n", libwacom_has_touch(device) ? "true" : "false");
- print_supported_leds(fd, device);
dprintf(fd, "NumStrips=%d\n", libwacom_get_num_strips(device));
dprintf(fd, "Buttons=%d\n", libwacom_get_num_buttons(device));
@@ -673,7 +644,6 @@ libwacom_destroy(WacomDevice *device)
}
g_free (device->matches);
g_free (device->supported_styli);
- g_free (device->status_leds);
g_free (device->buttons);
g_free (device);
}
@@ -809,53 +779,6 @@ int libwacom_get_strips_num_modes(WacomDevice *device)
return device->strips_num_modes;
}
-
-const WacomStatusLEDs *libwacom_get_status_leds(WacomDevice *device, int *num_leds)
-{
- *num_leds = device->num_leds;
- return device->status_leds;
-}
-
-struct {
- WacomButtonFlags button_flags;
- WacomStatusLEDs status_leds;
-} button_status_leds[] = {
- { WACOM_BUTTON_RING_MODESWITCH, WACOM_STATUS_LED_RING },
- { WACOM_BUTTON_RING2_MODESWITCH, WACOM_STATUS_LED_RING2 },
- { WACOM_BUTTON_TOUCHSTRIP_MODESWITCH, WACOM_STATUS_LED_TOUCHSTRIP },
- { WACOM_BUTTON_TOUCHSTRIP2_MODESWITCH, WACOM_STATUS_LED_TOUCHSTRIP2 }
-};
-
-int libwacom_get_button_led_group (WacomDevice *device,
- char button)
-{
- int button_index, led_index;
- WacomButtonFlags button_flags;
-
- g_return_val_if_fail (device->num_buttons > 0, -1);
- g_return_val_if_fail (button >= 'A', -1);
- g_return_val_if_fail (button < 'A' + device->num_buttons, -1);
-
- button_index = button - 'A';
- button_flags = device->buttons[button_index];
-
- if (!(button_flags & WACOM_BUTTON_MODESWITCH))
- return -1;
-
- for (led_index = 0; led_index < device->num_leds; led_index++) {
- guint n;
-
- for (n = 0; n < G_N_ELEMENTS (button_status_leds); n++) {
- if ((button_flags & button_status_leds[n].button_flags) &&
- (device->status_leds[led_index] == button_status_leds[n].status_leds)) {
- return led_index;
- }
- }
- }
-
- return WACOM_STATUS_LED_UNAVAILABLE;
-}
-
int libwacom_is_builtin(WacomDevice *device)
{
return !!(device->features & FEATURE_BUILTIN);
diff --git a/libwacom/libwacom.h b/libwacom/libwacom.h
index 8830db8..f7e6cf9 100644
--- a/libwacom/libwacom.h
+++ b/libwacom/libwacom.h
@@ -175,14 +175,6 @@ typedef enum {
WCOMPARE_MATCHES = (1 << 1), /**< compare all possible matches too */
} WacomCompareFlags;
-typedef enum {
- WACOM_STATUS_LED_UNAVAILABLE = -1,
- WACOM_STATUS_LED_RING = 0,
- WACOM_STATUS_LED_RING2 = 1,
- WACOM_STATUS_LED_TOUCHSTRIP = 2,
- WACOM_STATUS_LED_TOUCHSTRIP2 = 3
-} WacomStatusLEDs;
-
/**
* Allocate a new structure for error reporting.
*
@@ -440,22 +432,6 @@ int libwacom_get_strips_num_modes(WacomDevice *device);
/**
* @param device The tablet to query
- * @param num_leds Return location for the number of supported status LEDs
- * @return an array of status LEDs supported by the device
- */
-const WacomStatusLEDs *libwacom_get_status_leds(WacomDevice *device, int *num_leds);
-
-/**
- * @param device The tablet to query
- * @param button The ID of the button to check for, between 'A' and 'Z'
- * @return the status LED group id to use
- * or -1 if no LED is available for the given tablet / button
- */
-int libwacom_get_button_led_group (WacomDevice *device,
- char button);
-
-/**
- * @param device The tablet to query
* @return non-zero if the device is built-in or zero if the device is an
* external tablet
*/
diff --git a/libwacom/libwacomint.h b/libwacom/libwacomint.h
index 55436e9..6d07e16 100644
--- a/libwacom/libwacomint.h
+++ b/libwacom/libwacomint.h
@@ -90,9 +90,6 @@ struct _WacomDevice {
int num_buttons;
WacomButtonFlags *buttons;
- int num_leds;
- WacomStatusLEDs *status_leds;
-
gint refcnt; /* for the db hashtable */
};
--
1.7.1
>From ce3f30950b5ad658fc16fd9a81d7ae6b4df7aff1 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofour...@redhat.com>
Date: Wed, 3 Oct 2012 15:31:24 +0200
Subject: [PATCH 2/2] lib: add helper functions to list available status LED
Parses the newly added "StatusLEDs" field from the
database entries,
Adds a WacomStatusLEDs enumeration type to identify
known status LED types,
Adds libwacom_get_status_leds() function to retrieve the
number and list of status LED availble on the device in
the same order as their controlling entry in SysFS,
Adds libwacom_get_button_led_group() function to get the
LED group correspondoing to a given mode switch button,
or -1 if no LED is avaiable for that mode switch.
Signed-off-by: Olivier Fourdan <ofour...@redhat.com>
---
libwacom/libwacom-database.c | 30 ++++++++++++++++
libwacom/libwacom.c | 77 ++++++++++++++++++++++++++++++++++++++++++
libwacom/libwacom.h | 24 +++++++++++++
libwacom/libwacomint.h | 3 ++
4 files changed, 134 insertions(+), 0 deletions(-)
diff --git a/libwacom/libwacom-database.c b/libwacom/libwacom-database.c
index 5468975..d39305d 100644
--- a/libwacom/libwacom-database.c
+++ b/libwacom/libwacom-database.c
@@ -246,6 +246,16 @@ struct {
{ "OLEDs", WACOM_BUTTON_OLED }
};
+struct {
+ const char *key;
+ WacomStatusLEDs value;
+} supported_leds[] = {
+ { "Ring", WACOM_STATUS_LED_RING },
+ { "Ring2", WACOM_STATUS_LED_RING2 },
+ { "Touchstrip", WACOM_STATUS_LED_TOUCHSTRIP },
+ { "Touchstrip2", WACOM_STATUS_LED_TOUCHSTRIP2 }
+};
+
static void
libwacom_parse_buttons_key(WacomDevice *device,
GKeyFile *keyfile,
@@ -318,6 +328,7 @@ libwacom_parse_tablet_keyfile(const char *path)
char *class;
char *match;
char **styli_list;
+ char **leds_list;
keyfile = g_key_file_new();
@@ -414,6 +425,25 @@ libwacom_parse_tablet_keyfile(const char *path)
libwacom_parse_buttons(device, keyfile);
}
+ leds_list = g_key_file_get_string_list(keyfile, FEATURES_GROUP, "StatusLEDs", NULL, NULL);
+ if (leds_list) {
+ GArray *array;
+ guint i, n;
+ array = g_array_new (FALSE, FALSE, sizeof(WacomStatusLEDs));
+ device->num_leds = 0;
+ for (i = 0; leds_list[i]; i++) {
+ for (n = 0; n < G_N_ELEMENTS (supported_leds); n++) {
+ if (!strcmp(leds_list[i], supported_leds[n].key)) {
+ g_array_append_val (array, supported_leds[n].value);
+ device->num_leds++;
+ break;
+ }
+ }
+ }
+ g_strfreev (leds_list);
+ device->status_leds = (WacomStatusLEDs *) g_array_free (array, FALSE);
+ }
+
out:
if (keyfile)
g_key_file_free(keyfile);
diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c
index fe850a2..4f8c5f9 100644
--- a/libwacom/libwacom.c
+++ b/libwacom/libwacom.c
@@ -304,6 +304,8 @@ libwacom_copy(const WacomDevice *device)
d->ring2_num_modes = device->ring2_num_modes;
d->num_styli = device->num_styli;
d->supported_styli = g_memdup (device->supported_styli, sizeof(int) * device->num_styli);
+ d->num_leds = device->num_leds;
+ d->status_leds = g_memdup (device->status_leds, sizeof(WacomStatusLEDs) * device->num_leds);
d->num_buttons = device->num_buttons;
d->buttons = g_memdup (device->buttons, sizeof(WacomButtonFlags) * device->num_buttons);
return d;
@@ -372,6 +374,12 @@ libwacom_compare(WacomDevice *a, WacomDevice *b, WacomCompareFlags flags)
if (memcmp(a->supported_styli, b->supported_styli, sizeof(int) * a->num_styli) != 0)
return 1;
+ if (a->num_leds != b->num_leds)
+ return 1;
+
+ if (memcmp(a->status_leds, b->status_leds, sizeof(WacomStatusLEDs) * a->num_leds) != 0)
+ return 1;
+
if (memcmp(a->buttons, b->buttons, sizeof(WacomButtonFlags) * a->num_buttons) != 0)
return 1;
@@ -529,6 +537,26 @@ static void print_styli_for_device (int fd, WacomDevice *device)
dprintf(fd, "\n");
}
+static void print_supported_leds (int fd, WacomDevice *device)
+{
+ char *leds_name[] = {
+ "Ring",
+ "Ring2",
+ "Touchstrip",
+ "Touchstrip2"
+ };
+ int num_leds;
+ const WacomStatusLEDs *status_leds;
+ int i;
+
+ status_leds = libwacom_get_status_leds(device, &num_leds);
+
+ dprintf(fd, "StatusLEDs=");
+ for (i = 0; i < num_leds; i++)
+ dprintf(fd, "%s;", leds_name [status_leds[i]]);
+ dprintf(fd, "\n");
+}
+
static void print_button_flag_if(int fd, WacomDevice *device, const char *label, int flag)
{
int nbuttons = libwacom_get_num_buttons(device);
@@ -620,6 +648,7 @@ libwacom_print_device_description(int fd, WacomDevice *device)
dprintf(fd, "Ring2=%s\n", libwacom_has_ring2(device) ? "true" : "false");
dprintf(fd, "BuiltIn=%s\n", libwacom_is_builtin(device) ? "true" : "false");
dprintf(fd, "Touch=%s\n", libwacom_has_touch(device) ? "true" : "false");
+ print_supported_leds(fd, device);
dprintf(fd, "NumStrips=%d\n", libwacom_get_num_strips(device));
dprintf(fd, "Buttons=%d\n", libwacom_get_num_buttons(device));
@@ -644,6 +673,7 @@ libwacom_destroy(WacomDevice *device)
}
g_free (device->matches);
g_free (device->supported_styli);
+ g_free (device->status_leds);
g_free (device->buttons);
g_free (device);
}
@@ -779,6 +809,53 @@ int libwacom_get_strips_num_modes(WacomDevice *device)
return device->strips_num_modes;
}
+
+const WacomStatusLEDs *libwacom_get_status_leds(WacomDevice *device, int *num_leds)
+{
+ *num_leds = device->num_leds;
+ return device->status_leds;
+}
+
+struct {
+ WacomButtonFlags button_flags;
+ WacomStatusLEDs status_leds;
+} button_status_leds[] = {
+ { WACOM_BUTTON_RING_MODESWITCH, WACOM_STATUS_LED_RING },
+ { WACOM_BUTTON_RING2_MODESWITCH, WACOM_STATUS_LED_RING2 },
+ { WACOM_BUTTON_TOUCHSTRIP_MODESWITCH, WACOM_STATUS_LED_TOUCHSTRIP },
+ { WACOM_BUTTON_TOUCHSTRIP2_MODESWITCH, WACOM_STATUS_LED_TOUCHSTRIP2 }
+};
+
+int libwacom_get_button_led_group (WacomDevice *device,
+ char button)
+{
+ int button_index, led_index;
+ WacomButtonFlags button_flags;
+
+ g_return_val_if_fail (device->num_buttons > 0, -1);
+ g_return_val_if_fail (button >= 'A', -1);
+ g_return_val_if_fail (button < 'A' + device->num_buttons, -1);
+
+ button_index = button - 'A';
+ button_flags = device->buttons[button_index];
+
+ if (!(button_flags & WACOM_BUTTON_MODESWITCH))
+ return -1;
+
+ for (led_index = 0; led_index < device->num_leds; led_index++) {
+ guint n;
+
+ for (n = 0; n < G_N_ELEMENTS (button_status_leds); n++) {
+ if ((button_flags & button_status_leds[n].button_flags) &&
+ (device->status_leds[led_index] == button_status_leds[n].status_leds)) {
+ return led_index;
+ }
+ }
+ }
+
+ return WACOM_STATUS_LED_UNAVAILABLE;
+}
+
int libwacom_is_builtin(WacomDevice *device)
{
return !!(device->features & FEATURE_BUILTIN);
diff --git a/libwacom/libwacom.h b/libwacom/libwacom.h
index f7e6cf9..8830db8 100644
--- a/libwacom/libwacom.h
+++ b/libwacom/libwacom.h
@@ -175,6 +175,14 @@ typedef enum {
WCOMPARE_MATCHES = (1 << 1), /**< compare all possible matches too */
} WacomCompareFlags;
+typedef enum {
+ WACOM_STATUS_LED_UNAVAILABLE = -1,
+ WACOM_STATUS_LED_RING = 0,
+ WACOM_STATUS_LED_RING2 = 1,
+ WACOM_STATUS_LED_TOUCHSTRIP = 2,
+ WACOM_STATUS_LED_TOUCHSTRIP2 = 3
+} WacomStatusLEDs;
+
/**
* Allocate a new structure for error reporting.
*
@@ -432,6 +440,22 @@ int libwacom_get_strips_num_modes(WacomDevice *device);
/**
* @param device The tablet to query
+ * @param num_leds Return location for the number of supported status LEDs
+ * @return an array of status LEDs supported by the device
+ */
+const WacomStatusLEDs *libwacom_get_status_leds(WacomDevice *device, int *num_leds);
+
+/**
+ * @param device The tablet to query
+ * @param button The ID of the button to check for, between 'A' and 'Z'
+ * @return the status LED group id to use
+ * or -1 if no LED is available for the given tablet / button
+ */
+int libwacom_get_button_led_group (WacomDevice *device,
+ char button);
+
+/**
+ * @param device The tablet to query
* @return non-zero if the device is built-in or zero if the device is an
* external tablet
*/
diff --git a/libwacom/libwacomint.h b/libwacom/libwacomint.h
index 6d07e16..55436e9 100644
--- a/libwacom/libwacomint.h
+++ b/libwacom/libwacomint.h
@@ -90,6 +90,9 @@ struct _WacomDevice {
int num_buttons;
WacomButtonFlags *buttons;
+ int num_leds;
+ WacomStatusLEDs *status_leds;
+
gint refcnt; /* for the db hashtable */
};
--
1.7.1
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
