Mir

Merge lp:~andreas-pokorny/mir/input-configuration-api into lp:mir

Proposed by Andreas Pokorny
Status: Merged
Approved by: Andreas Pokorny
Approved revision: no longer in the source branch.
Merged at revision: 3106
Proposed branch: lp:~andreas-pokorny/mir/input-configuration-api
Merge into: lp:mir
Prerequisite: lp:~andreas-pokorny/mir/libinput-platform-touch-pad-settings
Diff against target: 806 lines (+476/-44)
11 files modified
include/server/mir/input/device.h (+9/-2)
include/server/mir/input/pointer_configuration.h (+71/-0)
include/server/mir/input/touchpad_configuration.h (+82/-0)
src/server/input/CMakeLists.txt (+10/-12)
src/server/input/default_device.cpp (+82/-3)
src/server/input/default_device.h (+22/-4)
src/server/input/default_input_device_hub.cpp (+5/-3)
src/server/input/default_input_device_hub.h (+2/-0)
tests/unit-tests/input/CMakeLists.txt (+1/-0)
tests/unit-tests/input/test_default_device.cpp (+135/-0)
tests/unit-tests/input/test_default_input_device_hub.cpp (+57/-20)
To merge this branch: bzr merge lp:~andreas-pokorny/mir/input-configuration-api
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alberto Aguirre (community) Approve
Chris Halse Rogers Approve
Alan Griffiths Abstain
Alexandros Frantzis (community) Abstain
Review via email: mp+273975@code.launchpad.net

Commit message

Add TouchPad- and PointerConfiguration to mir::input::Device

These structures allow configuring input device specific settings. Applicability of these configurations can be tested through optional_value<T>::is_set on the return values from mir::input::Device

Description of the change

This isnt the MP I was looking forward to...

Due to a change in behavior in libinput and a missing feature/bug in umockdev I cannot run the input configuration tests that I started to prepare.

Some details are here:
https://github.com/martinpitt/umockdev/pull/49

I am happy to keep this waiting until the above is resolved.
Apart from the lacking acceptance tests there are already some API decisions to review...

Beware there is some duplication in this MP until the other MP for
lp:~andreas-pokorny/mir/load-all-supported-input-platforms lands.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

+ virtual void apply_configuration(PointerConfiguration const&) = 0;
+ virtual void apply_configuration(TouchPadConfiguration const&) = 0;

I find it easier to read code when the functions are not overloaded in this way, but have unique names instead, e.g.:

apply_pointer_configuration()
apply_touchpad_configuration()

I think if functions are performing different operations they should be named differently. We should only use overloading for functions that perform the same operation but need different arguments.

For an extreme case of this see the super confusing (IMO) mev::make_event() family of builder functions.

+ void tap_to_click(bool enabled);
+ bool tap_to_click() const;

As above, although I feel less strongly about this particular pattern, so not blocking.

+ void scroll_mode(MirTouchPadScrollMode scroll_mode);
+ MirTouchPadScrollMode scroll_mode() const;
+ void scroll_mode(int scroll_button);

... but this is just confusing. The first overload scroll_mode() sets the scroll mode but the second overload adds to it (and changes the button scroll mode), so these are different operations and the functions should be named differently.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

+<<<<<<< TREE
 class PointerSettings;
 class TouchpadSettings;
+=======
+class PointerConfiguration;
+class TouchpadConfiguration;
+>>>>>>> MERGE-SOURCE

Looks like the "criss-cross" merge with the pre-req has confused LP.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> + virtual void apply_configuration(PointerConfiguration const&) = 0;
> + virtual void apply_configuration(TouchPadConfiguration const&) = 0;
>
> I find it easier to read code when the functions are not overloaded in this
> way, but have unique names instead, e.g.:
>
> apply_pointer_configuration()
> apply_touchpad_configuration()
>
> I think if functions are performing different operations they should be named
> differently. We should only use overloading for functions that perform the
> same operation but need different arguments.

But in both cases I only apply that configuration. I mean this is the same operation just different arguments.

> [...]
> + void scroll_mode(MirTouchPadScrollMode scroll_mode);
> + MirTouchPadScrollMode scroll_mode() const;
> + void scroll_mode(int scroll_button);
>
> ... but this is just confusing. The first overload scroll_mode() sets the
> scroll mode but the second overload adds to it (and changes the button scroll
> mode), so these are different operations and the functions should be named
> differently.

Ok I aggree this is confusing..

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

+ virtual void apply_configuration(PointerConfiguration const&) = 0;
+ virtual void apply_configuration(TouchpadConfiguration const&) = 0;

I still think that these are different operations and deserve different names for clarity, but it's not a blocker.

Looks good otherwise.

review: Abstain
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

+ virtual mir::optional_value<PointerConfiguration> pointer_configuration() const = 0;
+ virtual void apply_configuration(PointerConfiguration const&) = 0;
+
+ virtual mir::optional_value<TouchpadConfiguration> touchpad_configuration() const = 0;
+ virtual void apply_configuration(TouchpadConfiguration const&) = 0;

There's unnecessary asymmetry between XXX_configuration() and apply_configuration(xxx)

If we're calling something "XXX_configuration" then it should be "apply_XXX_configuration".

Looks good otherwise.

review: Abstain
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

ok followed your suggestions

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Nit:
88 + * - [-1, 0): reduced acceleration
89 + * - (0, 1]: increased acceleration
Is how that would be usually written.

=========

Strangeness:

struct TouchpadConfiguration
struct PointerConfiguration

Why do these have all the getter/setter infrastructure? We don't do any input validation, so they're just extra boilerplate?

If they *had* input validation then I'd probably make them class TouchpadConfiguration/class PointerConfiguration.

==========

774 + .WillByDefault(Return(mi::InputDeviceInfo{"touchpad", "dev-4", mi::DeviceCapability::pointer|mi::DeviceCapability::pointer}));

This looks wrong? Did you mean to OR mi::DeviceCapability::pointer with itself? This is the only thing that's actually needs-fixing; the rest are nits.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

"Why do these have all the getter/setter infrastructure? We don't do any input validation, so they're just extra boilerplate?"

+1. having getters/setters here doesn't buy you much. If ABI is a concern, you would have to go full PIMPL.

Further comments inlined.

Revision history for this message
Alberto Aguirre (albaguirre) :
review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

@AlbertA: ack inline comments should be addressed.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

My concerns are addressed. You might want to add a test that attempting to set invalid values throws, but I won't block for that.

review: Approve
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

LGTM.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/server/mir/input/device.h'
2--- include/server/mir/input/device.h 2015-10-26 03:33:22 +0000
3+++ include/server/mir/input/device.h 2015-11-04 19:48:42 +0000
4@@ -22,6 +22,7 @@
5
6 #include "mir/input/device_capability.h"
7 #include "mir_toolkit/event.h"
8+#include "mir/optional_value.h"
9
10 #include <memory>
11
12@@ -30,8 +31,8 @@
13 namespace input
14 {
15
16-class PointerSettings;
17-class TouchpadSettings;
18+class PointerConfiguration;
19+class TouchpadConfiguration;
20
21 class Device
22 {
23@@ -43,6 +44,12 @@
24 virtual std::string name() const = 0;
25 virtual std::string unique_id() const = 0;
26
27+ virtual mir::optional_value<PointerConfiguration> pointer_configuration() const = 0;
28+ virtual void apply_pointer_configuration(PointerConfiguration const&) = 0;
29+
30+ virtual mir::optional_value<TouchpadConfiguration> touchpad_configuration() const = 0;
31+ virtual void apply_touchpad_configuration(TouchpadConfiguration const&) = 0;
32+
33 private:
34 Device(Device const&) = delete;
35 Device& operator=(Device const&) = delete;
36
37=== added file 'include/server/mir/input/pointer_configuration.h'
38--- include/server/mir/input/pointer_configuration.h 1970-01-01 00:00:00 +0000
39+++ include/server/mir/input/pointer_configuration.h 2015-11-04 19:48:42 +0000
40@@ -0,0 +1,71 @@
41+/*
42+ * Copyright © 2015 Canonical Ltd.
43+ *
44+ * This program is free software: you can redistribute it and/or modify it
45+ * under the terms of the GNU General Public License version 3,
46+ * as published by the Free Software Foundation.
47+ *
48+ * This program is distributed in the hope that it will be useful,
49+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
50+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
51+ * GNU General Public License for more details.
52+ *
53+ * You should have received a copy of the GNU General Public License
54+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
55+ *
56+ * Authored by:
57+ * Andreas Pokorny <andreas.pokorny@canonical.com>
58+ */
59+
60+#ifndef MIR_INPUT_POINTER_CONFIGURATION_H_
61+#define MIR_INPUT_POINTER_CONFIGURATION_H_
62+
63+#include "mir_toolkit/common.h"
64+#include "mir_toolkit/client_types.h"
65+#include "mir_toolkit/mir_input_device.h"
66+
67+namespace mir
68+{
69+namespace input
70+{
71+
72+struct PointerConfiguration
73+{
74+ PointerConfiguration() {}
75+
76+ PointerConfiguration(MirPointerHandedness handedness, double acceleration_bias, double horizontal_scroll_scale,
77+ double vertical_scroll_scale)
78+ : handedness{handedness}, cursor_acceleration_bias{acceleration_bias},
79+ horizontal_scroll_scale{horizontal_scroll_scale}, vertical_scroll_scale{vertical_scroll_scale}
80+ {
81+ }
82+
83+ /*!
84+ * Configure which button shall be used as primary button. That way the input device is configured to be either
85+ * right or left handed.
86+ */
87+ MirPointerHandedness handedness{mir_pointer_handedness_right};
88+
89+ /*!
90+ * Configures the intensity of the cursor acceleration. Values within the range of [-1, 1] are allowed.
91+ * - 0: default acceleration
92+ * - [-1, 0): reduced acceleration
93+ * - (0, 1]: increased acceleration
94+ */
95+ double cursor_acceleration_bias{0.0};
96+
97+ /*!
98+ * Configures a signed scale of the horizontal scrolling. Use negative values to configure 'natural scrolling'
99+ */
100+ double horizontal_scroll_scale{1.0};
101+
102+ /*!
103+ * Configures a signed scale of the vertical scrolling. Use negative values to configure 'natural scrolling'
104+ */
105+ double vertical_scroll_scale{1.0};
106+};
107+
108+}
109+}
110+
111+#endif
112
113=== added file 'include/server/mir/input/touchpad_configuration.h'
114--- include/server/mir/input/touchpad_configuration.h 1970-01-01 00:00:00 +0000
115+++ include/server/mir/input/touchpad_configuration.h 2015-11-04 19:48:42 +0000
116@@ -0,0 +1,82 @@
117+/*
118+ * Copyright © 2015 Canonical Ltd.
119+ *
120+ * This program is free software: you can redistribute it and/or modify it
121+ * under the terms of the GNU General Public License version 3,
122+ * as published by the Free Software Foundation.
123+ *
124+ * This program is distributed in the hope that it will be useful,
125+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
126+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
127+ * GNU General Public License for more details.
128+ *
129+ * You should have received a copy of the GNU General Public License
130+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
131+ *
132+ * Authored by:
133+ * Andreas Pokorny <andreas.pokorny@canonical.com>
134+ */
135+
136+#ifndef MIR_INPUT_TOUCH_PAD_CONFIGURATION_H_
137+#define MIR_INPUT_TOUCH_PAD_CONFIGURATION_H_
138+
139+#include "mir_toolkit/common.h"
140+#include "mir_toolkit/mir_input_device.h"
141+
142+namespace mir
143+{
144+namespace input
145+{
146+
147+struct TouchpadConfiguration
148+{
149+ TouchpadConfiguration() {}
150+ TouchpadConfiguration(MirTouchpadClickModes click_mode,
151+ MirTouchpadScrollModes scroll_mode,
152+ int button_down_scroll_button,
153+ bool tap_to_click,
154+ bool disable_while_typing,
155+ bool disable_with_mouse,
156+ bool middle_mouse_button_emulation)
157+ : click_mode{click_mode}, scroll_mode{scroll_mode}, button_down_scroll_button{button_down_scroll_button},
158+ tap_to_click{tap_to_click}, middle_mouse_button_emulation{middle_mouse_button_emulation},
159+ disable_with_mouse{disable_with_mouse}, disable_while_typing{disable_while_typing}
160+ {
161+ }
162+
163+ /*!
164+ * The click mode defines when the touchpad generates software emulated button events.
165+ */
166+ MirTouchpadClickModes click_mode{mir_touchpad_click_mode_finger_count};
167+/*!
168+ * The scroll mode defines when the touchpad generates scroll events instead of pointer motion events.
169+ */
170+ MirTouchpadScrollModes scroll_mode{mir_touchpad_scroll_mode_two_finger_scroll};
171+
172+ /*!
173+ * Configures the button used for the on-button-down scroll mode
174+ */
175+ int button_down_scroll_button{0};
176+
177+ /*!
178+ * When tap to click is enabled the system will interpret short finger touch down/up sequences as button clicks.
179+ */
180+ bool tap_to_click{true};
181+ /*!
182+ * Emulates a middle mouse button press when the left and right buttons on a touchpad are pressed.
183+ */
184+ bool middle_mouse_button_emulation{true};
185+ /*!
186+ * When disable-with-mouse is enabled the touchpad will stop to emit user input events when another pointing device is plugged in.
187+ */
188+ bool disable_with_mouse{false};
189+ /*!
190+ * When disable-with-mouse is enabled the touchpad will stop to emit user input events when the user starts to use a keyboard and a short period after.
191+ */
192+ bool disable_while_typing{false};
193+};
194+
195+}
196+}
197+
198+#endif
199
200=== modified file 'src/server/input/CMakeLists.txt'
201--- src/server/input/CMakeLists.txt 2015-11-04 07:43:28 +0000
202+++ src/server/input/CMakeLists.txt 2015-11-04 19:48:42 +0000
203@@ -4,26 +4,24 @@
204 set(
205 INPUT_SOURCES
206
207+ builtin_cursor_images.cpp
208+ cursor_controller.cpp
209+ default_configuration.cpp
210 default_device.cpp
211 default_event_builder.cpp
212+ default_input_device_hub.cpp
213 default_input_manager.cpp
214- default_input_device_hub.cpp
215+ display_input_region.cpp
216+ event_filter_chain_dispatcher.cpp
217 input_modifier_utils.cpp
218+ key_repeat_dispatcher.cpp
219+ null_input_channel_factory.cpp
220+ null_input_dispatcher.cpp
221 seat.cpp
222-
223 surface_input_dispatcher.cpp
224- event_filter_chain_dispatcher.cpp
225-
226- null_input_channel_factory.cpp
227- null_input_dispatcher.cpp
228- display_input_region.cpp
229- vt_filter.cpp
230- cursor_controller.cpp
231- default_configuration.cpp
232- builtin_cursor_images.cpp
233 touchspot_controller.cpp
234- key_repeat_dispatcher.cpp
235 validator.cpp
236+ vt_filter.cpp
237 )
238
239 add_subdirectory(android)
240
241=== modified file 'src/server/input/default_device.cpp'
242--- src/server/input/default_device.cpp 2015-09-30 20:33:59 +0000
243+++ src/server/input/default_device.cpp 2015-11-04 19:48:42 +0000
244@@ -17,12 +17,21 @@
245 * Andreas Pokorny <andreas.pokorny@canonical.com>
246 */
247
248-#include "device_handle.h"
249+#include "default_device.h"
250+#include "mir/dispatch/action_queue.h"
251+#include "mir/input/device_capability.h"
252+#include "mir/input/input_device.h"
253+#include "mir/input/touchpad_configuration.h"
254+#include "mir/input/pointer_configuration.h"
255+
256+#include <boost/throw_exception.hpp>
257+#include <stdexcept>
258
259 namespace mi = mir::input;
260
261-mi::DefaultDevice::DefaultDevice(MirInputDeviceId id, mi::InputDeviceInfo const& info) :
262- device_id{id}, info(info)
263+mi::DefaultDevice::DefaultDevice(MirInputDeviceId id, std::shared_ptr<dispatch::ActionQueue> const& actions,
264+ std::shared_ptr<InputDevice> const& device) :
265+ device_id{id}, device{device}, info(device->get_device_info()), pointer{device->get_pointer_settings()}, touchpad{device->get_touchpad_settings()}, actions{actions}
266 {
267 }
268
269@@ -45,3 +54,73 @@
270 {
271 return device_id;
272 }
273+
274+mir::optional_value<mi::PointerConfiguration> mi::DefaultDevice::pointer_configuration() const
275+{
276+ if (!pointer.is_set())
277+ return {};
278+
279+ auto const& settings = pointer.value();
280+
281+ return PointerConfiguration(settings.handedness, settings.cursor_acceleration_bias,
282+ settings.horizontal_scroll_scale, settings.vertical_scroll_scale);
283+}
284+
285+mir::optional_value<mi::TouchpadConfiguration> mi::DefaultDevice::touchpad_configuration() const
286+{
287+ if (!touchpad.is_set())
288+ return {};
289+
290+ auto const& settings = touchpad.value();
291+
292+ return TouchpadConfiguration(settings.click_mode, settings.scroll_mode, settings.button_down_scroll_button,
293+ settings.tap_to_click, settings.disable_while_typing, settings.disable_with_mouse,
294+ settings.middle_mouse_button_emulation);
295+}
296+
297+void mi::DefaultDevice::apply_pointer_configuration(mi::PointerConfiguration const& conf)
298+{
299+ if (!pointer.is_set())
300+ BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot apply a pointer configuration"));
301+
302+ if (conf.cursor_acceleration_bias < -1.0 || conf.cursor_acceleration_bias > 1.0)
303+ BOOST_THROW_EXCEPTION(std::invalid_argument("Cursor acceleration bias out of range"));
304+
305+ PointerSettings settings;
306+ settings.handedness = conf.handedness;
307+ settings.cursor_acceleration_bias = conf.cursor_acceleration_bias;
308+ settings.vertical_scroll_scale = conf.vertical_scroll_scale;
309+ settings.horizontal_scroll_scale = conf.horizontal_scroll_scale;
310+
311+ pointer = settings;
312+
313+ actions->enqueue([settings = std::move(settings), dev=device]
314+ {
315+ dev->apply_settings(settings);
316+ });
317+}
318+
319+void mi::DefaultDevice::apply_touchpad_configuration(mi::TouchpadConfiguration const& conf)
320+{
321+ if (!touchpad.is_set())
322+ BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot apply a touchpad configuration"));
323+
324+ if (conf.scroll_mode & mir_touchpad_scroll_mode_button_down_scroll && conf.button_down_scroll_button == mi::no_scroll_button)
325+ BOOST_THROW_EXCEPTION(std::invalid_argument("No scroll button configured"));
326+
327+ TouchpadSettings settings;
328+ settings.click_mode = conf.click_mode;
329+ settings.scroll_mode = conf.scroll_mode;
330+ settings.button_down_scroll_button = conf.button_down_scroll_button;
331+ settings.disable_with_mouse = conf.disable_with_mouse;
332+ settings.disable_while_typing = conf.disable_while_typing;
333+ settings.tap_to_click = conf.tap_to_click;
334+ settings.middle_mouse_button_emulation = conf.middle_mouse_button_emulation;
335+
336+ touchpad = settings;
337+
338+ actions->enqueue([settings = std::move(settings), dev=device]
339+ {
340+ dev->apply_settings(settings);
341+ });
342+}
343
344=== renamed file 'src/server/input/device_handle.h' => 'src/server/input/default_device.h'
345--- src/server/input/device_handle.h 2015-10-01 09:52:28 +0000
346+++ src/server/input/default_device.h 2015-11-04 19:48:42 +0000
347@@ -23,26 +23,44 @@
348 #include "mir_toolkit/event.h"
349 #include "mir/input/device.h"
350 #include "mir/input/input_device_info.h"
351-#include "mir/module_deleter.h"
352+#include "mir/input/pointer_settings.h"
353+#include "mir/input/touchpad_settings.h"
354+#include "mir/optional_value.h"
355
356 #include <memory>
357
358 namespace mir
359 {
360+namespace dispatch
361+{
362+class ActionQueue;
363+}
364 namespace input
365 {
366
367+class InputDevice;
368+
369 class DefaultDevice : public Device
370 {
371 public:
372- DefaultDevice(MirInputDeviceId id, InputDeviceInfo const& info);
373+ DefaultDevice(MirInputDeviceId id, std::shared_ptr<dispatch::ActionQueue> const& actions,
374+ std::shared_ptr<InputDevice> const& device);
375 MirInputDeviceId id() const override;
376 DeviceCapabilities capabilities() const override;
377 std::string name() const override;
378 std::string unique_id() const override;
379+
380+ optional_value<PointerConfiguration> pointer_configuration() const override;
381+ void apply_pointer_configuration(PointerConfiguration const&) override;
382+ optional_value<TouchpadConfiguration> touchpad_configuration() const override;
383+ void apply_touchpad_configuration(TouchpadConfiguration const&) override;
384 private:
385- MirInputDeviceId device_id;
386- InputDeviceInfo info;
387+ MirInputDeviceId const device_id;
388+ std::shared_ptr<InputDevice> const device;
389+ InputDeviceInfo const info;
390+ optional_value<PointerSettings> pointer;
391+ optional_value<TouchpadSettings> touchpad;
392+ std::shared_ptr<dispatch::ActionQueue> const actions;
393 };
394
395 }
396
397=== modified file 'src/server/input/default_input_device_hub.cpp'
398--- src/server/input/default_input_device_hub.cpp 2015-11-04 07:43:28 +0000
399+++ src/server/input/default_input_device_hub.cpp 2015-11-04 19:48:42 +0000
400@@ -17,7 +17,7 @@
401 */
402
403 #include "default_input_device_hub.h"
404-#include "device_handle.h"
405+#include "default_device.h"
406
407 #include "seat.h"
408 #include "mir/input/input_dispatcher.h"
409@@ -27,6 +27,7 @@
410 #include "mir/geometry/point.h"
411 #include "mir/events/event_builders.h"
412 #include "mir/dispatch/multiplexing_dispatchable.h"
413+#include "mir/dispatch/action_queue.h"
414 #include "mir/server_action_queue.h"
415 #include "mir/cookie_factory.h"
416 #define MIR_LOG_COMPONENT "Input"
417@@ -52,11 +53,13 @@
418 : input_dispatcher(input_dispatcher),
419 input_dispatchable{input_multiplexer},
420 observer_queue(observer_queue),
421+ device_queue(std::make_shared<dispatch::ActionQueue>()),
422 input_region(input_region),
423 cookie_factory(cookie_factory),
424 seat(touch_visualizer, cursor_listener),
425 device_id_generator{0}
426 {
427+ input_dispatchable->add_watch(device_queue);
428 }
429
430 void mi::DefaultInputDeviceHub::add_device(std::shared_ptr<InputDevice> const& device)
431@@ -80,8 +83,7 @@
432 auto const& dev = devices.back();
433 seat.add_device(dev->id());
434
435- auto handle = std::make_shared<DefaultDevice>(
436- dev->id(), device->get_device_info());
437+ auto handle = std::make_shared<DefaultDevice>(dev->id(), device_queue, device);
438
439 // pass input device handle to observer loop..
440 observer_queue->enqueue(this,
441
442=== modified file 'src/server/input/default_input_device_hub.h'
443--- src/server/input/default_input_device_hub.h 2015-11-04 07:43:28 +0000
444+++ src/server/input/default_input_device_hub.h 2015-11-04 19:48:42 +0000
445@@ -44,6 +44,7 @@
446 {
447 class Dispatchable;
448 class MultiplexingDispatchable;
449+class ActionQueue;
450 }
451 namespace input
452 {
453@@ -82,6 +83,7 @@
454 std::shared_ptr<InputDispatcher> const input_dispatcher;
455 std::shared_ptr<dispatch::MultiplexingDispatchable> const input_dispatchable;
456 std::shared_ptr<ServerActionQueue> const observer_queue;
457+ std::shared_ptr<dispatch::ActionQueue> const device_queue;
458 std::shared_ptr<InputRegion> const input_region;
459 std::shared_ptr<cookie::CookieFactory> const cookie_factory;
460 Seat seat;
461
462=== modified file 'tests/unit-tests/input/CMakeLists.txt'
463--- tests/unit-tests/input/CMakeLists.txt 2015-10-29 03:39:10 +0000
464+++ tests/unit-tests/input/CMakeLists.txt 2015-11-04 19:48:42 +0000
465@@ -9,6 +9,7 @@
466 ${CMAKE_CURRENT_SOURCE_DIR}/test_touchspot_controller.cpp
467 ${CMAKE_CURRENT_SOURCE_DIR}/test_input_event.cpp
468 ${CMAKE_CURRENT_SOURCE_DIR}/test_event_builders.cpp
469+ ${CMAKE_CURRENT_SOURCE_DIR}/test_default_device.cpp
470 ${CMAKE_CURRENT_SOURCE_DIR}/test_default_input_device_hub.cpp
471 ${CMAKE_CURRENT_SOURCE_DIR}/test_default_input_manager.cpp
472 ${CMAKE_CURRENT_SOURCE_DIR}/test_surface_input_dispatcher.cpp
473
474=== added file 'tests/unit-tests/input/test_default_device.cpp'
475--- tests/unit-tests/input/test_default_device.cpp 1970-01-01 00:00:00 +0000
476+++ tests/unit-tests/input/test_default_device.cpp 2015-11-04 19:48:42 +0000
477@@ -0,0 +1,135 @@
478+/*
479+ * Copyright © 2015 Canonical Ltd.
480+ *
481+ * This program is free software: you can redistribute it and/or modify
482+ * it under the terms of the GNU General Public License version 3 as
483+ * published by the Free Software Foundation.
484+ *
485+ * This program is distributed in the hope that it will be useful,
486+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
487+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
488+ * GNU General Public License for more details.
489+ *
490+ * You should have received a copy of the GNU General Public License
491+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
492+ *
493+ * Authored by: Andreas Pokorny <andreas.pokorny@canonical.com>
494+ */
495+
496+#include "src/server/input/default_device.h"
497+#include "mir/input/input_device.h"
498+#include "mir/input/touchpad_configuration.h"
499+#include "mir/input/pointer_configuration.h"
500+#include "mir/dispatch/action_queue.h"
501+#include "mir/test/fake_shared.h"
502+#include <gtest/gtest.h>
503+#include <gmock/gmock.h>
504+#include <stdexcept>
505+
506+namespace mi = mir::input;
507+namespace mt = mir::test;
508+namespace md = mir::dispatch;
509+using namespace ::testing;
510+namespace
511+{
512+struct MockInputDevice : public mi::InputDevice
513+{
514+ MOCK_METHOD2(start, void(mi::InputSink* destination, mi::EventBuilder* builder));
515+ MOCK_METHOD0(stop, void());
516+ MOCK_METHOD0(get_device_info, mi::InputDeviceInfo());
517+ MOCK_CONST_METHOD0(get_pointer_settings, mir::optional_value<mi::PointerSettings>());
518+ MOCK_METHOD1(apply_settings, void(mi::PointerSettings const&));
519+ MOCK_CONST_METHOD0(get_touchpad_settings, mir::optional_value<mi::TouchpadSettings>());
520+ MOCK_METHOD1(apply_settings, void(mi::TouchpadSettings const&));
521+};
522+
523+struct DefaultDevice : Test
524+{
525+ NiceMock<MockInputDevice> touchpad;
526+ NiceMock<MockInputDevice> mouse;
527+ NiceMock<MockInputDevice> keyboard;
528+ std::shared_ptr<md::ActionQueue> queue{std::make_shared<md::ActionQueue>()};
529+
530+ DefaultDevice()
531+ {
532+ using optional_pointer_settings = mir::optional_value<mi::PointerSettings>;
533+ using optional_touchpad_settings = mir::optional_value<mi::TouchpadSettings>;
534+ ON_CALL(touchpad, get_device_info())
535+ .WillByDefault(Return(mi::InputDeviceInfo{"name", "unique", mi::DeviceCapability::touchpad|mi::DeviceCapability::pointer}));
536+ ON_CALL(touchpad, get_pointer_settings())
537+ .WillByDefault(Return(optional_pointer_settings{mi::PointerSettings{}}));
538+ ON_CALL(touchpad, get_touchpad_settings())
539+ .WillByDefault(Return(optional_touchpad_settings{mi::TouchpadSettings{}}));
540+
541+ ON_CALL(mouse, get_device_info())
542+ .WillByDefault(Return(mi::InputDeviceInfo{"name", "unique", mi::DeviceCapability::pointer}));
543+ ON_CALL(mouse, get_pointer_settings()).WillByDefault(Return(optional_pointer_settings{mi::PointerSettings{}}));
544+ ON_CALL(mouse, get_touchpad_settings()).WillByDefault(Return(optional_touchpad_settings{}));
545+
546+ ON_CALL(keyboard, get_device_info())
547+ .WillByDefault(Return(mi::InputDeviceInfo{"name", "unique", mi::DeviceCapability::keyboard}));
548+ ON_CALL(keyboard, get_pointer_settings()).WillByDefault(Return(optional_pointer_settings{}));
549+ ON_CALL(keyboard, get_touchpad_settings()).WillByDefault(Return(optional_touchpad_settings{}));
550+ }
551+};
552+
553+}
554+
555+TEST_F(DefaultDevice, refuses_touchpad_config_on_mice)
556+{
557+ mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(mouse));
558+ mi::TouchpadConfiguration touch_conf;
559+
560+ EXPECT_THROW({dev.apply_touchpad_configuration(touch_conf);}, std::invalid_argument);
561+}
562+
563+TEST_F(DefaultDevice, refuses_touchpad_and_pointer_settings_on_keyboards)
564+{
565+ mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(keyboard));
566+ mi::TouchpadConfiguration touch_conf;
567+ mi::PointerConfiguration pointer_conf;
568+
569+ EXPECT_THROW({dev.apply_touchpad_configuration(touch_conf);}, std::invalid_argument);
570+ EXPECT_THROW({dev.apply_pointer_configuration(pointer_conf);}, std::invalid_argument);
571+}
572+
573+
574+TEST_F(DefaultDevice, accepts_pointer_config_on_mice)
575+{
576+ mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(mouse));
577+ mi::PointerConfiguration pointer_conf;
578+
579+ EXPECT_CALL(mouse, apply_settings(Matcher<mi::PointerSettings const&>(_)));
580+
581+ dev.apply_pointer_configuration(pointer_conf);
582+ queue->dispatch(md::FdEvent::readable);
583+}
584+
585+TEST_F(DefaultDevice, accepts_touchpad_and_pointer_config_on_touchpads)
586+{
587+ mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(touchpad));
588+ mi::TouchpadConfiguration touch_conf;
589+ mi::PointerConfiguration pointer_conf;
590+
591+ EXPECT_CALL(touchpad, apply_settings(Matcher<mi::PointerSettings const&>(_)));
592+ EXPECT_CALL(touchpad, apply_settings(Matcher<mi::TouchpadSettings const&>(_)));
593+
594+ dev.apply_touchpad_configuration(touch_conf);
595+ dev.apply_pointer_configuration(pointer_conf);
596+ queue->dispatch(md::FdEvent::readable);
597+ queue->dispatch(md::FdEvent::readable);
598+}
599+
600+TEST_F(DefaultDevice, ensures_cursor_accleration_bias_is_in_range)
601+{
602+ mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(touchpad));
603+
604+ mi::PointerConfiguration pointer_conf;
605+ pointer_conf.cursor_acceleration_bias = 3.0;
606+ EXPECT_THROW({dev.apply_pointer_configuration(pointer_conf);}, std::invalid_argument);
607+ pointer_conf.cursor_acceleration_bias = -2.0;
608+ EXPECT_THROW({dev.apply_pointer_configuration(pointer_conf);}, std::invalid_argument);
609+
610+ pointer_conf.cursor_acceleration_bias = 1.0;
611+ EXPECT_NO_THROW(dev.apply_pointer_configuration(pointer_conf));
612+}
613
614=== modified file 'tests/unit-tests/input/test_default_input_device_hub.cpp'
615--- tests/unit-tests/input/test_default_input_device_hub.cpp 2015-11-04 07:43:28 +0000
616+++ tests/unit-tests/input/test_default_input_device_hub.cpp 2015-11-04 19:48:42 +0000
617@@ -29,6 +29,8 @@
618 #include "mir/input/touchpad_settings.h"
619 #include "mir/input/device.h"
620 #include "mir/input/touch_visualizer.h"
621+#include "mir/input/pointer_configuration.h"
622+#include "mir/input/touchpad_configuration.h"
623 #include "mir/input/input_device_observer.h"
624 #include "mir/dispatch/multiplexing_dispatchable.h"
625 #include "mir/dispatch/action_queue.h"
626@@ -48,6 +50,7 @@
627 namespace mtd = mt::doubles;
628 namespace geom = mir::geometry;
629 using namespace std::literals::chrono_literals;
630+using namespace ::testing;
631
632 namespace mir
633 {
634@@ -112,25 +115,43 @@
635 Nice<MockInputDevice> device;
636 Nice<MockInputDevice> another_device;
637 Nice<MockInputDevice> third_device;
638+ Nice<MockInputDevice> touchpad;
639
640 std::chrono::nanoseconds arbitrary_timestamp;
641
642 InputDeviceHubTest()
643 {
644- using namespace testing;
645- ON_CALL(device,get_device_info())
646+ ON_CALL(device, get_device_info())
647 .WillByDefault(Return(mi::InputDeviceInfo{"device","dev-1", mi::DeviceCapability::unknown}));
648+ ON_CALL(device, get_pointer_settings())
649+ .WillByDefault(Return(mir::optional_value<mi::PointerSettings>()));
650+ ON_CALL(device, get_touchpad_settings())
651+ .WillByDefault(Return(mir::optional_value<mi::TouchpadSettings>()));
652
653- ON_CALL(another_device,get_device_info())
654+ ON_CALL(another_device, get_device_info())
655 .WillByDefault(Return(mi::InputDeviceInfo{"another_device","dev-2", mi::DeviceCapability::keyboard}));
656+ ON_CALL(another_device, get_pointer_settings())
657+ .WillByDefault(Return(mir::optional_value<mi::PointerSettings>()));
658+ ON_CALL(another_device, get_touchpad_settings())
659+ .WillByDefault(Return(mir::optional_value<mi::TouchpadSettings>()));
660
661 ON_CALL(third_device,get_device_info())
662 .WillByDefault(Return(mi::InputDeviceInfo{"third_device","dev-3", mi::DeviceCapability::keyboard}));
663+ ON_CALL(third_device, get_pointer_settings())
664+ .WillByDefault(Return(mir::optional_value<mi::PointerSettings>()));
665+ ON_CALL(third_device, get_touchpad_settings())
666+ .WillByDefault(Return(mir::optional_value<mi::TouchpadSettings>()));
667+
668+ ON_CALL(touchpad, get_device_info())
669+ .WillByDefault(Return(mi::InputDeviceInfo{"touchpad", "dev-4", mi::DeviceCapability::touchpad|mi::DeviceCapability::pointer}));
670+ ON_CALL(touchpad, get_pointer_settings())
671+ .WillByDefault(Return(mi::PointerSettings()));
672+ ON_CALL(touchpad, get_touchpad_settings())
673+ .WillByDefault(Return(mi::TouchpadSettings()));
674 }
675
676 void capture_input_sink(Nice<MockInputDevice>& dev, mi::InputSink*& sink, mi::EventBuilder*& builder)
677 {
678- using namespace ::testing;
679 ON_CALL(dev,start(_,_))
680 .WillByDefault(Invoke([&sink,&builder](mi::InputSink* input_sink, mi::EventBuilder* event_builder)
681 {
682@@ -143,8 +164,6 @@
683
684 TEST_F(InputDeviceHubTest, input_device_hub_starts_device)
685 {
686- using namespace ::testing;
687-
688 EXPECT_CALL(device,start(_,_));
689
690 hub.add_device(mt::fake_shared(device));
691@@ -152,8 +171,6 @@
692
693 TEST_F(InputDeviceHubTest, input_device_hub_stops_device_on_removal)
694 {
695- using namespace ::testing;
696-
697 EXPECT_CALL(device,stop());
698
699 hub.add_device(mt::fake_shared(device));
700@@ -162,7 +179,6 @@
701
702 TEST_F(InputDeviceHubTest, input_device_hub_ignores_removal_of_unknown_devices)
703 {
704- using namespace ::testing;
705
706 EXPECT_CALL(device,start(_,_)).Times(0);
707 EXPECT_CALL(device,stop()).Times(0);
708@@ -172,7 +188,6 @@
709
710 TEST_F(InputDeviceHubTest, input_device_hub_start_stop_happens_in_order)
711 {
712- using namespace ::testing;
713
714 InSequence seq;
715 EXPECT_CALL(device, start(_,_));
716@@ -199,8 +214,6 @@
717
718 TEST_F(InputDeviceHubTest, observers_receive_devices_on_add)
719 {
720- using namespace ::testing;
721-
722 std::shared_ptr<mi::Device> handle_1, handle_2;
723
724 InSequence seq;
725@@ -239,8 +252,6 @@
726
727 TEST_F(InputDeviceHubTest, observers_receive_device_changes)
728 {
729- using namespace ::testing;
730-
731 InSequence seq;
732 EXPECT_CALL(mock_observer, changes_complete());
733 EXPECT_CALL(mock_observer, device_added(WithName("device")));
734@@ -257,8 +268,6 @@
735
736 TEST_F(InputDeviceHubTest, input_sink_posts_events_to_input_dispatcher)
737 {
738- using namespace ::testing;
739-
740 mi::InputSink* sink;
741 mi::EventBuilder* builder;
742 std::shared_ptr<mi::Device> handle;
743@@ -283,7 +292,6 @@
744
745 TEST_F(InputDeviceHubTest, forwards_touch_spots_to_visualizer)
746 {
747- using namespace ::testing;
748 mi::InputSink* sink;
749 mi::EventBuilder* builder;
750
751@@ -352,7 +360,6 @@
752
753 TEST_F(InputDeviceHubTest, confines_pointer_movement)
754 {
755- using namespace ::testing;
756 geom::Point confined_pos{10, 18};
757
758 ON_CALL(mock_region,confine(_))
759@@ -375,8 +382,6 @@
760
761 TEST_F(InputDeviceHubTest, forwards_pointer_updates_to_cursor_listener)
762 {
763- using namespace ::testing;
764-
765 auto x = 12.2f, y = 14.3f;
766
767 mi::InputSink* sink;
768@@ -391,6 +396,38 @@
769 sink->handle_input(*event);
770 }
771
772+TEST_F(InputDeviceHubTest, forwards_pointer_settings_to_input_device)
773+{
774+ std::shared_ptr<mi::Device> dev;
775+ ON_CALL(mock_observer, device_added(_)).WillByDefault(SaveArg<0>(&dev));
776+
777+ hub.add_observer(mt::fake_shared(mock_observer));
778+ hub.add_device(mt::fake_shared(touchpad));
779+ observer_loop.trigger_server_actions();
780+
781+ EXPECT_CALL(touchpad, apply_settings(Matcher<mi::PointerSettings const&>(_)));
782+
783+ auto conf = dev->pointer_configuration();
784+ dev->apply_pointer_configuration(conf.value());
785+ multiplexer.dispatch(mir::dispatch::FdEvent::readable);
786+}
787+
788+TEST_F(InputDeviceHubTest, forwards_touchpad_settings_to_input_device)
789+{
790+ std::shared_ptr<mi::Device> dev;
791+ ON_CALL(mock_observer, device_added(_)).WillByDefault(SaveArg<0>(&dev));
792+
793+ hub.add_observer(mt::fake_shared(mock_observer));
794+ hub.add_device(mt::fake_shared(touchpad));
795+ observer_loop.trigger_server_actions();
796+
797+ EXPECT_CALL(touchpad, apply_settings(Matcher<mi::TouchpadSettings const&>(_)));
798+
799+ auto conf = dev->touchpad_configuration();
800+ dev->apply_touchpad_configuration(conf.value());
801+ multiplexer.dispatch(mir::dispatch::FdEvent::readable);
802+}
803+
804 TEST_F(InputDeviceHubTest, input_sink_tracks_modifier)
805 {
806 using namespace ::testing;

Subscribers

People subscribed via source and target branches