[v3,libinput] evdev: Query mouse DPI from udev

Submitted by Peter Hutterer on Nov. 25, 2014, 10:41 p.m.

Details

Message ID 20141125224158.GA6209@jelly.redhat.com
State Not Applicable
Headers show

Not browsing as part of any series.

Commit Message

Peter Hutterer Nov. 25, 2014, 10:41 p.m.
From: Derek Foreman <derekf@osg.samsung.com>

Instead of using a hard coded mouse DPI value, we query it from udev.
If it's not present or the property is obviously broken we fall back
to default.

Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
sorry, clearly didn't have the right caffeine/blood ratio yesterday, so I
figured I fix up the remaining comments myself.

Changes to v3:
- move parsing code to be called when we set the DPI. evdev_tag_device isn't
  the right place for this.
- squashed test patch into this one, better to have the tests together with
  the code that wants the tests
- added more testcases for missing *, and @a invalid frequencies
- change ck_assert(a == b) to ck_assert_int_eq(a, b)

 src/evdev.c         | 26 ++++++++++++++++++++++++-
 src/libinput-util.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/libinput-util.h |  2 ++
 test/misc.c         | 42 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 124 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/evdev.c b/src/evdev.c
index baa1c51..015dc2f 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -1226,6 +1226,30 @@  evdev_tag_device(struct evdev_device *device)
 }
 
 static inline int
+evdev_read_dpi_prop(struct evdev_device *device)
+{
+	struct libinput *libinput = device->base.seat->libinput;
+	const char *mouse_dpi;
+	int dpi = DEFAULT_MOUSE_DPI;
+
+	mouse_dpi = udev_device_get_property_value(device->udev_device,
+						   "MOUSE_DPI");
+	if (mouse_dpi) {
+		dpi = parse_mouse_dpi_property(mouse_dpi);
+		if (!dpi) {
+			log_error(libinput, "Mouse DPI property for '%s' is "
+					    "present but invalid, using %d "
+					    "DPI instead\n",
+					    device->devname,
+					    DEFAULT_MOUSE_DPI);
+			dpi = DEFAULT_MOUSE_DPI;
+		}
+	}
+
+	return dpi;
+}
+
+static inline int
 evdev_fix_abs_resolution(struct libevdev *evdev,
 			 unsigned int code,
 			 const struct input_absinfo *absinfo)
@@ -1530,7 +1554,7 @@  evdev_device_create(struct libinput_seat *seat,
 	device->devname = libevdev_get_name(device->evdev);
 	device->scroll.threshold = 5.0; /* Default may be overridden */
 	device->scroll.direction = 0;
-	device->dpi = DEFAULT_MOUSE_DPI;
+	device->dpi = evdev_read_dpi_prop(device);
 	/* at most 5 SYN_DROPPED log-messages per 30s */
 	ratelimit_init(&device->syn_drop_limit, 30ULL * 1000, 5);
 
diff --git a/src/libinput-util.c b/src/libinput-util.c
index 34d5549..923e116 100644
--- a/src/libinput-util.c
+++ b/src/libinput-util.c
@@ -28,7 +28,9 @@ 
 
 #include "config.h"
 
+#include <ctype.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -113,3 +115,56 @@  ratelimit_test(struct ratelimit *r)
 
 	return RATELIMIT_EXCEEDED;
 }
+
+/* Helper function to parse the mouse DPI tag from udev.
+ * The tag is of the form:
+ * MOUSE_DPI=400 *1000 2000
+ * or
+ * MOUSE_DPI=400@125 *1000@125 2000@125
+ * Where the * indicates the default value and @number indicates device poll
+ * rate.
+ * Numbers should be in ascending order, and if rates are present they should
+ * be present for all entries.
+ *
+ * When parsing the mouse DPI property, if we find an error we just return 0
+ * since it's obviously invalid, the caller will treat that as an error and
+ * use a reasonable default instead. If the property contains multiple DPI
+ * settings but none flagged as default, we return the last because we're
+ * lazy and that's a silly way to set the property anyway.
+ */
+int
+parse_mouse_dpi_property(const char *prop)
+{
+	bool is_default = false;
+	int nread, dpi = 0, rate;
+
+	while (*prop != 0) {
+		if (*prop == ' ') {
+			prop++;
+			continue;
+		}
+		if (*prop == '*') {
+			prop++;
+			is_default = true;
+			if (!isdigit(prop[0]))
+				return 0;
+		}
+
+		/* While we don't do anything with the rate right now we
+		 * will validate that, if it's present, it is non-zero and
+		 * positive
+		 */
+		rate = 1;
+		nread = 0;
+		sscanf(prop, "%d@%d%n", &dpi, &rate, &nread);
+		if (!nread)
+			sscanf(prop, "%d%n", &dpi, &nread);
+		if (!nread || dpi <= 0 || rate <= 0 || prop[nread] == '@')
+			return 0;
+
+		if (is_default)
+			break;
+		prop += nread;
+	}
+	return dpi;
+}
diff --git a/src/libinput-util.h b/src/libinput-util.h
index 909c9db..6825841 100644
--- a/src/libinput-util.h
+++ b/src/libinput-util.h
@@ -296,4 +296,6 @@  struct ratelimit {
 void ratelimit_init(struct ratelimit *r, uint64_t ival_ms, unsigned int burst);
 enum ratelimit_state ratelimit_test(struct ratelimit *r);
 
+int parse_mouse_dpi_property(const char *prop);
+
 #endif /* LIBINPUT_UTIL_H */
diff --git a/test/misc.c b/test/misc.c
index 4ed9dce..8ad5343 100644
--- a/test/misc.c
+++ b/test/misc.c
@@ -548,6 +548,47 @@  START_TEST(ratelimit_helpers)
 }
 END_TEST
 
+struct parser_test {
+	char *tag;
+	int expected_dpi;
+};
+
+START_TEST(dpi_parser)
+{
+	struct parser_test tests[] = {
+		{ "450 *1800 3200", 1800 },
+		{ "*450 1800 3200", 450 },
+		{ "450 1800 *3200", 3200 },
+		{ "450 1800 3200", 3200 },
+		{ "450 1800 failboat", 0 },
+		{ "450 1800 *failboat", 0 },
+		{ "0 450 1800 *3200", 0 },
+		{ "450@37 1800@12 *3200@6", 3200 },
+		{ "450@125 1800@125   *3200@125  ", 3200 },
+		{ "450@125 *1800@125  3200@125", 1800 },
+		{ "*this @string fails", 0 },
+		{ "12@34 *45@", 0 },
+		{ "12@a *45@", 0 },
+		{ "12@a *45@25", 0 },
+		{ "                                      * 12, 450, 800", 0 },
+		{ "                                      *12, 450, 800", 12 },
+		{ "*12, *450, 800", 12 },
+		{ "*-23412, 450, 800", 0 },
+		{ "112@125, 450@125, 800@125, 900@-125", 0 },
+		{ "", 0 },
+		{ "   ", 0 },
+		{ "* ", 0 },
+		{ NULL }
+	};
+	int i, dpi;
+
+	for (i = 0; tests[i].tag != NULL; i++) {
+		dpi = parse_mouse_dpi_property(tests[i].tag);
+		ck_assert_int_eq(dpi, tests[i].expected_dpi);
+	}
+}
+END_TEST
+
 int main (int argc, char **argv) {
 	litest_add_no_device("events:conversion", event_conversion_device_notify);
 	litest_add_no_device("events:conversion", event_conversion_pointer);
@@ -560,6 +601,7 @@  int main (int argc, char **argv) {
 
 	litest_add_no_device("misc:matrix", matrix_helpers);
 	litest_add_no_device("misc:ratelimit", ratelimit_helpers);
+	litest_add_no_device("misc:dpi parser", dpi_parser);
 
 	return litest_run(argc, argv);
 }

Comments

Hi,

On 11/25/2014 11:41 PM, Peter Hutterer wrote:
> From: Derek Foreman <derekf@osg.samsung.com>
>
> Instead of using a hard coded mouse DPI value, we query it from udev.
> If it's not present or the property is obviously broken we fall back
> to default.
>
> Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

Looks good:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> ---
> sorry, clearly didn't have the right caffeine/blood ratio yesterday, so I
> figured I fix up the remaining comments myself.
>
> Changes to v3:
> - move parsing code to be called when we set the DPI. evdev_tag_device isn't
>    the right place for this.
> - squashed test patch into this one, better to have the tests together with
>    the code that wants the tests
> - added more testcases for missing *, and @a invalid frequencies
> - change ck_assert(a == b) to ck_assert_int_eq(a, b)
>
>   src/evdev.c         | 26 ++++++++++++++++++++++++-
>   src/libinput-util.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/libinput-util.h |  2 ++
>   test/misc.c         | 42 ++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 124 insertions(+), 1 deletion(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index baa1c51..015dc2f 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1226,6 +1226,30 @@ evdev_tag_device(struct evdev_device *device)
>   }
>
>   static inline int
> +evdev_read_dpi_prop(struct evdev_device *device)
> +{
> +	struct libinput *libinput = device->base.seat->libinput;
> +	const char *mouse_dpi;
> +	int dpi = DEFAULT_MOUSE_DPI;
> +
> +	mouse_dpi = udev_device_get_property_value(device->udev_device,
> +						   "MOUSE_DPI");
> +	if (mouse_dpi) {
> +		dpi = parse_mouse_dpi_property(mouse_dpi);
> +		if (!dpi) {
> +			log_error(libinput, "Mouse DPI property for '%s' is "
> +					    "present but invalid, using %d "
> +					    "DPI instead\n",
> +					    device->devname,
> +					    DEFAULT_MOUSE_DPI);
> +			dpi = DEFAULT_MOUSE_DPI;
> +		}
> +	}
> +
> +	return dpi;
> +}
> +
> +static inline int
>   evdev_fix_abs_resolution(struct libevdev *evdev,
>   			 unsigned int code,
>   			 const struct input_absinfo *absinfo)
> @@ -1530,7 +1554,7 @@ evdev_device_create(struct libinput_seat *seat,
>   	device->devname = libevdev_get_name(device->evdev);
>   	device->scroll.threshold = 5.0; /* Default may be overridden */
>   	device->scroll.direction = 0;
> -	device->dpi = DEFAULT_MOUSE_DPI;
> +	device->dpi = evdev_read_dpi_prop(device);
>   	/* at most 5 SYN_DROPPED log-messages per 30s */
>   	ratelimit_init(&device->syn_drop_limit, 30ULL * 1000, 5);
>
> diff --git a/src/libinput-util.c b/src/libinput-util.c
> index 34d5549..923e116 100644
> --- a/src/libinput-util.c
> +++ b/src/libinput-util.c
> @@ -28,7 +28,9 @@
>
>   #include "config.h"
>
> +#include <ctype.h>
>   #include <stdarg.h>
> +#include <stdbool.h>
>   #include <stdio.h>
>   #include <stdlib.h>
>
> @@ -113,3 +115,56 @@ ratelimit_test(struct ratelimit *r)
>
>   	return RATELIMIT_EXCEEDED;
>   }
> +
> +/* Helper function to parse the mouse DPI tag from udev.
> + * The tag is of the form:
> + * MOUSE_DPI=400 *1000 2000
> + * or
> + * MOUSE_DPI=400@125 *1000@125 2000@125
> + * Where the * indicates the default value and @number indicates device poll
> + * rate.
> + * Numbers should be in ascending order, and if rates are present they should
> + * be present for all entries.
> + *
> + * When parsing the mouse DPI property, if we find an error we just return 0
> + * since it's obviously invalid, the caller will treat that as an error and
> + * use a reasonable default instead. If the property contains multiple DPI
> + * settings but none flagged as default, we return the last because we're
> + * lazy and that's a silly way to set the property anyway.
> + */
> +int
> +parse_mouse_dpi_property(const char *prop)
> +{
> +	bool is_default = false;
> +	int nread, dpi = 0, rate;
> +
> +	while (*prop != 0) {
> +		if (*prop == ' ') {
> +			prop++;
> +			continue;
> +		}
> +		if (*prop == '*') {
> +			prop++;
> +			is_default = true;
> +			if (!isdigit(prop[0]))
> +				return 0;
> +		}
> +
> +		/* While we don't do anything with the rate right now we
> +		 * will validate that, if it's present, it is non-zero and
> +		 * positive
> +		 */
> +		rate = 1;
> +		nread = 0;
> +		sscanf(prop, "%d@%d%n", &dpi, &rate, &nread);
> +		if (!nread)
> +			sscanf(prop, "%d%n", &dpi, &nread);
> +		if (!nread || dpi <= 0 || rate <= 0 || prop[nread] == '@')
> +			return 0;
> +
> +		if (is_default)
> +			break;
> +		prop += nread;
> +	}
> +	return dpi;
> +}
> diff --git a/src/libinput-util.h b/src/libinput-util.h
> index 909c9db..6825841 100644
> --- a/src/libinput-util.h
> +++ b/src/libinput-util.h
> @@ -296,4 +296,6 @@ struct ratelimit {
>   void ratelimit_init(struct ratelimit *r, uint64_t ival_ms, unsigned int burst);
>   enum ratelimit_state ratelimit_test(struct ratelimit *r);
>
> +int parse_mouse_dpi_property(const char *prop);
> +
>   #endif /* LIBINPUT_UTIL_H */
> diff --git a/test/misc.c b/test/misc.c
> index 4ed9dce..8ad5343 100644
> --- a/test/misc.c
> +++ b/test/misc.c
> @@ -548,6 +548,47 @@ START_TEST(ratelimit_helpers)
>   }
>   END_TEST
>
> +struct parser_test {
> +	char *tag;
> +	int expected_dpi;
> +};
> +
> +START_TEST(dpi_parser)
> +{
> +	struct parser_test tests[] = {
> +		{ "450 *1800 3200", 1800 },
> +		{ "*450 1800 3200", 450 },
> +		{ "450 1800 *3200", 3200 },
> +		{ "450 1800 3200", 3200 },
> +		{ "450 1800 failboat", 0 },
> +		{ "450 1800 *failboat", 0 },
> +		{ "0 450 1800 *3200", 0 },
> +		{ "450@37 1800@12 *3200@6", 3200 },
> +		{ "450@125 1800@125   *3200@125  ", 3200 },
> +		{ "450@125 *1800@125  3200@125", 1800 },
> +		{ "*this @string fails", 0 },
> +		{ "12@34 *45@", 0 },
> +		{ "12@a *45@", 0 },
> +		{ "12@a *45@25", 0 },
> +		{ "                                      * 12, 450, 800", 0 },
> +		{ "                                      *12, 450, 800", 12 },
> +		{ "*12, *450, 800", 12 },
> +		{ "*-23412, 450, 800", 0 },
> +		{ "112@125, 450@125, 800@125, 900@-125", 0 },
> +		{ "", 0 },
> +		{ "   ", 0 },
> +		{ "* ", 0 },
> +		{ NULL }
> +	};
> +	int i, dpi;
> +
> +	for (i = 0; tests[i].tag != NULL; i++) {
> +		dpi = parse_mouse_dpi_property(tests[i].tag);
> +		ck_assert_int_eq(dpi, tests[i].expected_dpi);
> +	}
> +}
> +END_TEST
> +
>   int main (int argc, char **argv) {
>   	litest_add_no_device("events:conversion", event_conversion_device_notify);
>   	litest_add_no_device("events:conversion", event_conversion_pointer);
> @@ -560,6 +601,7 @@ int main (int argc, char **argv) {
>
>   	litest_add_no_device("misc:matrix", matrix_helpers);
>   	litest_add_no_device("misc:ratelimit", ratelimit_helpers);
> +	litest_add_no_device("misc:dpi parser", dpi_parser);
>
>   	return litest_run(argc, argv);
>   }
>