Mir

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

input-configuration-api
Merge into development-branch

Proposed by Andreas Pokorny on 2015-10-09

Status:	Merged
Approved by:	Andreas Pokorny on 2015-11-18
Approved revision:	2998
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
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2015-11-19
Alberto Aguirre			Approve on 2015-11-12
Chris Halse Rogers			Approve on 2015-11-06
Alan Griffiths			Abstain on 2015-11-02
Alexandros Frantzis (community)		2015-10-09	Abstain on 2015-10-30
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.

PS Jenkins bot (ps-jenkins) wrote on 2015-10-09:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5221/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-10-09:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5228/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-10-20:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5346/rebuild

review: Approve (continuous-integration)

Alexandros Frantzis (afrantzis) wrote on 2015-10-23:

+ 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

PS Jenkins bot (ps-jenkins) wrote on 2015-10-23:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5386/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-10-23:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5390/rebuild

review: Approve (continuous-integration)

Alan Griffiths (alan-griffiths) wrote on 2015-10-26:

+<<<<<<< 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

PS Jenkins bot (ps-jenkins) wrote on 2015-10-26:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5401/rebuild

review: Approve (continuous-integration)

Andreas Pokorny (andreas-pokorny) wrote on 2015-10-27:

> + 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..

PS Jenkins bot (ps-jenkins) wrote on 2015-10-27:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5417/rebuild

review: Needs Fixing (continuous-integration)

Alexandros Frantzis (afrantzis) wrote on 2015-10-30:

+ 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

Alan Griffiths (alan-griffiths) wrote on 2015-11-02:

+ 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

Andreas Pokorny (andreas-pokorny) wrote on 2015-11-03:

ok followed your suggestions

PS Jenkins bot (ps-jenkins) wrote on 2015-11-03:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5494/rebuild

review: Needs Fixing (continuous-integration)

Chris Halse Rogers (raof) wrote on 2015-11-04:

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

PS Jenkins bot (ps-jenkins) wrote on 2015-11-04:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5501/rebuild

review: Approve (continuous-integration)

Alberto Aguirre (albaguirre) wrote on 2015-11-04:

"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.

Alberto Aguirre (albaguirre) on 2015-11-04:

review: Needs Fixing

PS Jenkins bot (ps-jenkins) wrote on 2015-11-04:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5505/rebuild

review: Needs Fixing (continuous-integration)

Andreas Pokorny (andreas-pokorny) wrote on 2015-11-04:

@AlbertA: ack inline comments should be addressed.

PS Jenkins bot (ps-jenkins) wrote on 2015-11-04:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5507/rebuild

review: Approve (continuous-integration)

Chris Halse Rogers (raof) wrote on 2015-11-06:

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

Alberto Aguirre (albaguirre) wrote on 2015-11-12:

LGTM.

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-11-13:

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-11-13:

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-11-16:

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) on 2015-11-19:

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Andreas Pokorny

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'include/server/mir/input/device.h'
 --- include/server/mir/input/device.h	2015-10-26 03:33:22 +0000
 +++ include/server/mir/input/device.h	2015-11-04 19:48:42 +0000
@@ -22,6 +22,7 @@
  #include "mir/input/device_capability.h"
  #include "mir_toolkit/event.h"
++#include "mir/optional_value.h"
  #include <memory>
@@ -30,8 +31,8 @@
  namespace input
+ {
--class PointerSettings;
--class TouchpadSettings;
++class PointerConfiguration;
++class TouchpadConfiguration;
  class Device
+ {
@@ -43,6 +44,12 @@
      virtual std::string name() const = 0;
      virtual std::string unique_id() const = 0;
++    virtual mir::optional_value<PointerConfiguration> pointer_configuration() const = 0;
++    virtual void apply_pointer_configuration(PointerConfiguration const&) = 0;
++
++    virtual mir::optional_value<TouchpadConfiguration> touchpad_configuration() const = 0;
++    virtual void apply_touchpad_configuration(TouchpadConfiguration const&) = 0;
++
  private:
      Device(Device const&) = delete;
      Device& operator=(Device const&) = delete;
 === added file 'include/server/mir/input/pointer_configuration.h'
 --- include/server/mir/input/pointer_configuration.h	1970-01-01 00:00:00 +0000
 +++ include/server/mir/input/pointer_configuration.h	2015-11-04 19:48:42 +0000
@@ -0,0 +1,71 @@
++/*
++ * Copyright © 2015 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by:
++ *   Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_INPUT_POINTER_CONFIGURATION_H_
++#define MIR_INPUT_POINTER_CONFIGURATION_H_
++
++#include "mir_toolkit/common.h"
++#include "mir_toolkit/client_types.h"
++#include "mir_toolkit/mir_input_device.h"
++
++namespace mir
++{
++namespace input
++{
++
++struct PointerConfiguration
++{
++    PointerConfiguration() {}
++
++    PointerConfiguration(MirPointerHandedness handedness, double acceleration_bias, double horizontal_scroll_scale,
++                         double vertical_scroll_scale)
++        : handedness{handedness}, cursor_acceleration_bias{acceleration_bias},
++          horizontal_scroll_scale{horizontal_scroll_scale}, vertical_scroll_scale{vertical_scroll_scale}
++    {
++    }
++
++    /*!
++     * Configure which button shall be used as primary button. That way the input device is configured to be either
++     * right or left handed.
++     */
++    MirPointerHandedness handedness{mir_pointer_handedness_right};
++
++    /*!
++     * Configures the intensity of the cursor acceleration. Values within the range of [-1, 1] are allowed.
++     *   - 0: default acceleration
++     *   - [-1, 0): reduced acceleration
++     *   - (0, 1]: increased acceleration
++     */
++    double cursor_acceleration_bias{0.0};
++
++    /*!
++     * Configures a signed scale of the horizontal scrolling. Use negative values to configure 'natural scrolling'
++     */
++    double horizontal_scroll_scale{1.0};
++
++    /*!
++     * Configures a signed scale of the vertical scrolling. Use negative values to configure 'natural scrolling'
++     */
++    double vertical_scroll_scale{1.0};
++};
++
++}
++}
++
++#endif
 === added file 'include/server/mir/input/touchpad_configuration.h'
 --- include/server/mir/input/touchpad_configuration.h	1970-01-01 00:00:00 +0000
 +++ include/server/mir/input/touchpad_configuration.h	2015-11-04 19:48:42 +0000
@@ -0,0 +1,82 @@
++/*
++ * Copyright © 2015 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by:
++ *   Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#ifndef MIR_INPUT_TOUCH_PAD_CONFIGURATION_H_
++#define MIR_INPUT_TOUCH_PAD_CONFIGURATION_H_
++
++#include "mir_toolkit/common.h"
++#include "mir_toolkit/mir_input_device.h"
++
++namespace mir
++{
++namespace input
++{
++
++struct TouchpadConfiguration
++{
++    TouchpadConfiguration() {}
++    TouchpadConfiguration(MirTouchpadClickModes click_mode,
++                          MirTouchpadScrollModes scroll_mode,
++                          int button_down_scroll_button,
++                          bool tap_to_click,
++                          bool disable_while_typing,
++                          bool disable_with_mouse,
++                          bool middle_mouse_button_emulation)
++        : click_mode{click_mode}, scroll_mode{scroll_mode}, button_down_scroll_button{button_down_scroll_button},
++          tap_to_click{tap_to_click}, middle_mouse_button_emulation{middle_mouse_button_emulation},
++          disable_with_mouse{disable_with_mouse}, disable_while_typing{disable_while_typing}
++    {
++    }
++
++    /*!
++     * The click mode defines when the touchpad generates software emulated button events.
++     */
++    MirTouchpadClickModes click_mode{mir_touchpad_click_mode_finger_count};
++/*!
++     * The scroll mode defines when the touchpad generates scroll events instead of pointer motion events.
++     */
++    MirTouchpadScrollModes scroll_mode{mir_touchpad_scroll_mode_two_finger_scroll};
++
++    /*!
++     * Configures the button used for the on-button-down scroll mode
++     */
++    int button_down_scroll_button{0};
++
++    /*!
++     * When tap to click is enabled the system will interpret short finger touch down/up sequences as button clicks.
++     */
++    bool tap_to_click{true};
++    /*!
++     * Emulates a middle mouse button press when the left and right buttons on a touchpad are pressed.
++     */
++    bool middle_mouse_button_emulation{true};
++    /*!
++     * When disable-with-mouse is enabled the touchpad will stop to emit user input events when another pointing device is plugged in.
++     */
++    bool disable_with_mouse{false};
++    /*!
++     * 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.
++     */
++    bool disable_while_typing{false};
++};
++
++}
++}
++
++#endif
 === modified file 'src/server/input/CMakeLists.txt'
 --- src/server/input/CMakeLists.txt	2015-11-04 07:43:28 +0000
 +++ src/server/input/CMakeLists.txt	2015-11-04 19:48:42 +0000
@@ -4,26 +4,24 @@
  set(
    INPUT_SOURCES
++  builtin_cursor_images.cpp
++  cursor_controller.cpp
++  default_configuration.cpp
    default_device.cpp
    default_event_builder.cpp
++  default_input_device_hub.cpp
    default_input_manager.cpp
--  default_input_device_hub.cpp
++  display_input_region.cpp
++  event_filter_chain_dispatcher.cpp
    input_modifier_utils.cpp
++  key_repeat_dispatcher.cpp
++  null_input_channel_factory.cpp
++  null_input_dispatcher.cpp
    seat.cpp
--
    surface_input_dispatcher.cpp
--  event_filter_chain_dispatcher.cpp
--
--  null_input_channel_factory.cpp
--  null_input_dispatcher.cpp
--  display_input_region.cpp
--  vt_filter.cpp
--  cursor_controller.cpp
--  default_configuration.cpp
--  builtin_cursor_images.cpp
    touchspot_controller.cpp
--  key_repeat_dispatcher.cpp
    validator.cpp
++  vt_filter.cpp
+ )
  add_subdirectory(android)
 === modified file 'src/server/input/default_device.cpp'
 --- src/server/input/default_device.cpp	2015-09-30 20:33:59 +0000
 +++ src/server/input/default_device.cpp	2015-11-04 19:48:42 +0000
@@ -17,12 +17,21 @@
   *   Andreas Pokorny <andreas.pokorny@canonical.com>
   */
--#include "device_handle.h"
++#include "default_device.h"
++#include "mir/dispatch/action_queue.h"
++#include "mir/input/device_capability.h"
++#include "mir/input/input_device.h"
++#include "mir/input/touchpad_configuration.h"
++#include "mir/input/pointer_configuration.h"
++
++#include <boost/throw_exception.hpp>
++#include <stdexcept>
  namespace mi = mir::input;
--mi::DefaultDevice::DefaultDevice(MirInputDeviceId id, mi::InputDeviceInfo const& info) :
--    device_id{id}, info(info)
++mi::DefaultDevice::DefaultDevice(MirInputDeviceId id, std::shared_ptr<dispatch::ActionQueue> const& actions,
++                                 std::shared_ptr<InputDevice> const& device) :
++    device_id{id}, device{device}, info(device->get_device_info()), pointer{device->get_pointer_settings()}, touchpad{device->get_touchpad_settings()}, actions{actions}
+ {
+ }
@@ -45,3 +54,73 @@
+ {
      return device_id;
+ }
++
++mir::optional_value<mi::PointerConfiguration> mi::DefaultDevice::pointer_configuration() const
++{
++    if (!pointer.is_set())
++        return {};
++
++    auto const& settings = pointer.value();
++
++    return PointerConfiguration(settings.handedness, settings.cursor_acceleration_bias,
++                                settings.horizontal_scroll_scale, settings.vertical_scroll_scale);
++}
++
++mir::optional_value<mi::TouchpadConfiguration> mi::DefaultDevice::touchpad_configuration() const
++{
++    if (!touchpad.is_set())
++        return {};
++
++    auto const& settings = touchpad.value();
++
++    return TouchpadConfiguration(settings.click_mode, settings.scroll_mode, settings.button_down_scroll_button,
++                                 settings.tap_to_click, settings.disable_while_typing, settings.disable_with_mouse,
++                                 settings.middle_mouse_button_emulation);
++}
++
++void mi::DefaultDevice::apply_pointer_configuration(mi::PointerConfiguration const& conf)
++{
++    if (!pointer.is_set())
++        BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot apply a pointer configuration"));
++
++    if (conf.cursor_acceleration_bias < -1.0 || conf.cursor_acceleration_bias > 1.0)
++        BOOST_THROW_EXCEPTION(std::invalid_argument("Cursor acceleration bias out of range"));
++
++    PointerSettings settings;
++    settings.handedness = conf.handedness;
++    settings.cursor_acceleration_bias = conf.cursor_acceleration_bias;
++    settings.vertical_scroll_scale = conf.vertical_scroll_scale;
++    settings.horizontal_scroll_scale = conf.horizontal_scroll_scale;
++
++    pointer = settings;
++
++    actions->enqueue([settings = std::move(settings), dev=device]
++                     {
++                         dev->apply_settings(settings);
++                     });
++}
++
++void mi::DefaultDevice::apply_touchpad_configuration(mi::TouchpadConfiguration const& conf)
++{
++    if (!touchpad.is_set())
++        BOOST_THROW_EXCEPTION(std::invalid_argument("Cannot apply a touchpad configuration"));
++
++    if (conf.scroll_mode & mir_touchpad_scroll_mode_button_down_scroll && conf.button_down_scroll_button == mi::no_scroll_button)
++        BOOST_THROW_EXCEPTION(std::invalid_argument("No scroll button configured"));
++
++    TouchpadSettings settings;
++    settings.click_mode = conf.click_mode;
++    settings.scroll_mode = conf.scroll_mode;
++    settings.button_down_scroll_button = conf.button_down_scroll_button;
++    settings.disable_with_mouse = conf.disable_with_mouse;
++    settings.disable_while_typing = conf.disable_while_typing;
++    settings.tap_to_click = conf.tap_to_click;
++    settings.middle_mouse_button_emulation = conf.middle_mouse_button_emulation;
++
++    touchpad = settings;
++
++    actions->enqueue([settings = std::move(settings), dev=device]
++                     {
++                         dev->apply_settings(settings);
++                     });
++}
 === renamed file 'src/server/input/device_handle.h' => 'src/server/input/default_device.h'
 --- src/server/input/device_handle.h	2015-10-01 09:52:28 +0000
 +++ src/server/input/default_device.h	2015-11-04 19:48:42 +0000
@@ -23,26 +23,44 @@
  #include "mir_toolkit/event.h"
  #include "mir/input/device.h"
  #include "mir/input/input_device_info.h"
--#include "mir/module_deleter.h"
++#include "mir/input/pointer_settings.h"
++#include "mir/input/touchpad_settings.h"
++#include "mir/optional_value.h"
  #include <memory>
  namespace mir
+ {
++namespace dispatch
++{
++class ActionQueue;
++}
  namespace input
+ {
++class InputDevice;
++
  class DefaultDevice : public Device
+ {
  public:
--    DefaultDevice(MirInputDeviceId id, InputDeviceInfo const& info);
++    DefaultDevice(MirInputDeviceId id, std::shared_ptr<dispatch::ActionQueue> const& actions,
++                  std::shared_ptr<InputDevice> const& device);
      MirInputDeviceId id() const override;
      DeviceCapabilities capabilities() const override;
      std::string name() const override;
      std::string unique_id() const override;
++
++    optional_value<PointerConfiguration> pointer_configuration() const override;
++    void apply_pointer_configuration(PointerConfiguration const&) override;
++    optional_value<TouchpadConfiguration> touchpad_configuration() const override;
++    void apply_touchpad_configuration(TouchpadConfiguration const&) override;
  private:
--    MirInputDeviceId device_id;
--    InputDeviceInfo info;
++    MirInputDeviceId const device_id;
++    std::shared_ptr<InputDevice> const device;
++    InputDeviceInfo const info;
++    optional_value<PointerSettings> pointer;
++    optional_value<TouchpadSettings> touchpad;
++    std::shared_ptr<dispatch::ActionQueue> const actions;
  };
+ }
 === modified file 'src/server/input/default_input_device_hub.cpp'
 --- src/server/input/default_input_device_hub.cpp	2015-11-04 07:43:28 +0000
 +++ src/server/input/default_input_device_hub.cpp	2015-11-04 19:48:42 +0000
@@ -17,7 +17,7 @@
   */
  #include "default_input_device_hub.h"
--#include "device_handle.h"
++#include "default_device.h"
  #include "seat.h"
  #include "mir/input/input_dispatcher.h"
@@ -27,6 +27,7 @@
  #include "mir/geometry/point.h"
  #include "mir/events/event_builders.h"
  #include "mir/dispatch/multiplexing_dispatchable.h"
++#include "mir/dispatch/action_queue.h"
  #include "mir/server_action_queue.h"
  #include "mir/cookie_factory.h"
  #define MIR_LOG_COMPONENT "Input"
@@ -52,11 +53,13 @@
      : input_dispatcher(input_dispatcher),
        input_dispatchable{input_multiplexer},
        observer_queue(observer_queue),
++      device_queue(std::make_shared<dispatch::ActionQueue>()),
        input_region(input_region),
        cookie_factory(cookie_factory),
        seat(touch_visualizer, cursor_listener),
        device_id_generator{0}
+ {
++    input_dispatchable->add_watch(device_queue);
+ }
  void mi::DefaultInputDeviceHub::add_device(std::shared_ptr<InputDevice> const& device)
@@ -80,8 +83,7 @@
          auto const& dev = devices.back();
          seat.add_device(dev->id());
--        auto handle = std::make_shared<DefaultDevice>(
--            dev->id(), device->get_device_info());
++        auto handle = std::make_shared<DefaultDevice>(dev->id(), device_queue, device);
          // pass input device handle to observer loop..
          observer_queue->enqueue(this,
 === modified file 'src/server/input/default_input_device_hub.h'
 --- src/server/input/default_input_device_hub.h	2015-11-04 07:43:28 +0000
 +++ src/server/input/default_input_device_hub.h	2015-11-04 19:48:42 +0000
@@ -44,6 +44,7 @@
+ {
  class Dispatchable;
  class MultiplexingDispatchable;
++class ActionQueue;
+ }
  namespace input
+ {
@@ -82,6 +83,7 @@
      std::shared_ptr<InputDispatcher> const input_dispatcher;
      std::shared_ptr<dispatch::MultiplexingDispatchable> const input_dispatchable;
      std::shared_ptr<ServerActionQueue> const observer_queue;
++    std::shared_ptr<dispatch::ActionQueue> const device_queue;
      std::shared_ptr<InputRegion> const input_region;
      std::shared_ptr<cookie::CookieFactory> const cookie_factory;
      Seat seat;
 === modified file 'tests/unit-tests/input/CMakeLists.txt'
 --- tests/unit-tests/input/CMakeLists.txt	2015-10-29 03:39:10 +0000
 +++ tests/unit-tests/input/CMakeLists.txt	2015-11-04 19:48:42 +0000
@@ -9,6 +9,7 @@
    ${CMAKE_CURRENT_SOURCE_DIR}/test_touchspot_controller.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_input_event.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_event_builders.cpp
++  ${CMAKE_CURRENT_SOURCE_DIR}/test_default_device.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_default_input_device_hub.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_default_input_manager.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_surface_input_dispatcher.cpp
 === added file 'tests/unit-tests/input/test_default_device.cpp'
 --- tests/unit-tests/input/test_default_device.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/input/test_default_device.cpp	2015-11-04 19:48:42 +0000
@@ -0,0 +1,135 @@
++/*
++ * Copyright © 2015 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License version 3 as
++ * published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Andreas Pokorny <andreas.pokorny@canonical.com>
++ */
++
++#include "src/server/input/default_device.h"
++#include "mir/input/input_device.h"
++#include "mir/input/touchpad_configuration.h"
++#include "mir/input/pointer_configuration.h"
++#include "mir/dispatch/action_queue.h"
++#include "mir/test/fake_shared.h"
++#include <gtest/gtest.h>
++#include <gmock/gmock.h>
++#include <stdexcept>
++
++namespace mi = mir::input;
++namespace mt = mir::test;
++namespace md = mir::dispatch;
++using namespace ::testing;
++namespace
++{
++struct MockInputDevice : public mi::InputDevice
++{
++    MOCK_METHOD2(start, void(mi::InputSink* destination, mi::EventBuilder* builder));
++    MOCK_METHOD0(stop, void());
++    MOCK_METHOD0(get_device_info, mi::InputDeviceInfo());
++    MOCK_CONST_METHOD0(get_pointer_settings, mir::optional_value<mi::PointerSettings>());
++    MOCK_METHOD1(apply_settings, void(mi::PointerSettings const&));
++    MOCK_CONST_METHOD0(get_touchpad_settings, mir::optional_value<mi::TouchpadSettings>());
++    MOCK_METHOD1(apply_settings, void(mi::TouchpadSettings const&));
++};
++
++struct DefaultDevice : Test
++{
++    NiceMock<MockInputDevice> touchpad;
++    NiceMock<MockInputDevice> mouse;
++    NiceMock<MockInputDevice> keyboard;
++    std::shared_ptr<md::ActionQueue> queue{std::make_shared<md::ActionQueue>()};
++
++    DefaultDevice()
++    {
++        using optional_pointer_settings = mir::optional_value<mi::PointerSettings>;
++        using optional_touchpad_settings = mir::optional_value<mi::TouchpadSettings>;
++        ON_CALL(touchpad, get_device_info())
++            .WillByDefault(Return(mi::InputDeviceInfo{"name", "unique", mi::DeviceCapability::touchpad|mi::DeviceCapability::pointer}));
++        ON_CALL(touchpad, get_pointer_settings())
++            .WillByDefault(Return(optional_pointer_settings{mi::PointerSettings{}}));
++        ON_CALL(touchpad, get_touchpad_settings())
++            .WillByDefault(Return(optional_touchpad_settings{mi::TouchpadSettings{}}));
++
++        ON_CALL(mouse, get_device_info())
++            .WillByDefault(Return(mi::InputDeviceInfo{"name", "unique", mi::DeviceCapability::pointer}));
++        ON_CALL(mouse, get_pointer_settings()).WillByDefault(Return(optional_pointer_settings{mi::PointerSettings{}}));
++        ON_CALL(mouse, get_touchpad_settings()).WillByDefault(Return(optional_touchpad_settings{}));
++
++        ON_CALL(keyboard, get_device_info())
++            .WillByDefault(Return(mi::InputDeviceInfo{"name", "unique", mi::DeviceCapability::keyboard}));
++        ON_CALL(keyboard, get_pointer_settings()).WillByDefault(Return(optional_pointer_settings{}));
++        ON_CALL(keyboard, get_touchpad_settings()).WillByDefault(Return(optional_touchpad_settings{}));
++    }
++};
++
++}
++
++TEST_F(DefaultDevice, refuses_touchpad_config_on_mice)
++{
++    mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(mouse));
++    mi::TouchpadConfiguration touch_conf;
++
++    EXPECT_THROW({dev.apply_touchpad_configuration(touch_conf);}, std::invalid_argument);
++}
++
++TEST_F(DefaultDevice, refuses_touchpad_and_pointer_settings_on_keyboards)
++{
++    mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(keyboard));
++    mi::TouchpadConfiguration touch_conf;
++    mi::PointerConfiguration pointer_conf;
++
++    EXPECT_THROW({dev.apply_touchpad_configuration(touch_conf);}, std::invalid_argument);
++    EXPECT_THROW({dev.apply_pointer_configuration(pointer_conf);}, std::invalid_argument);
++}
++
++
++TEST_F(DefaultDevice, accepts_pointer_config_on_mice)
++{
++    mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(mouse));
++    mi::PointerConfiguration pointer_conf;
++
++    EXPECT_CALL(mouse, apply_settings(Matcher<mi::PointerSettings const&>(_)));
++
++    dev.apply_pointer_configuration(pointer_conf);
++    queue->dispatch(md::FdEvent::readable);
++}
++
++TEST_F(DefaultDevice, accepts_touchpad_and_pointer_config_on_touchpads)
++{
++    mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(touchpad));
++    mi::TouchpadConfiguration touch_conf;
++    mi::PointerConfiguration pointer_conf;
++
++    EXPECT_CALL(touchpad, apply_settings(Matcher<mi::PointerSettings const&>(_)));
++    EXPECT_CALL(touchpad, apply_settings(Matcher<mi::TouchpadSettings const&>(_)));
++
++    dev.apply_touchpad_configuration(touch_conf);
++    dev.apply_pointer_configuration(pointer_conf);
++    queue->dispatch(md::FdEvent::readable);
++    queue->dispatch(md::FdEvent::readable);
++}
++
++TEST_F(DefaultDevice, ensures_cursor_accleration_bias_is_in_range)
++{
++    mi::DefaultDevice dev(MirInputDeviceId{17}, queue, mt::fake_shared(touchpad));
++
++    mi::PointerConfiguration pointer_conf;
++    pointer_conf.cursor_acceleration_bias = 3.0;
++    EXPECT_THROW({dev.apply_pointer_configuration(pointer_conf);}, std::invalid_argument);
++    pointer_conf.cursor_acceleration_bias = -2.0;
++    EXPECT_THROW({dev.apply_pointer_configuration(pointer_conf);}, std::invalid_argument);
++
++    pointer_conf.cursor_acceleration_bias = 1.0;
++    EXPECT_NO_THROW(dev.apply_pointer_configuration(pointer_conf));
++}
 === modified file 'tests/unit-tests/input/test_default_input_device_hub.cpp'
 --- tests/unit-tests/input/test_default_input_device_hub.cpp	2015-11-04 07:43:28 +0000
 +++ tests/unit-tests/input/test_default_input_device_hub.cpp	2015-11-04 19:48:42 +0000
@@ -29,6 +29,8 @@
  #include "mir/input/touchpad_settings.h"
  #include "mir/input/device.h"
  #include "mir/input/touch_visualizer.h"
++#include "mir/input/pointer_configuration.h"
++#include "mir/input/touchpad_configuration.h"
  #include "mir/input/input_device_observer.h"
  #include "mir/dispatch/multiplexing_dispatchable.h"
  #include "mir/dispatch/action_queue.h"
@@ -48,6 +50,7 @@
  namespace mtd = mt::doubles;
  namespace geom = mir::geometry;
  using namespace std::literals::chrono_literals;
++using namespace ::testing;
  namespace mir
+ {
@@ -112,25 +115,43 @@
      Nice<MockInputDevice> device;
      Nice<MockInputDevice> another_device;
      Nice<MockInputDevice> third_device;
++    Nice<MockInputDevice> touchpad;
      std::chrono::nanoseconds arbitrary_timestamp;
      InputDeviceHubTest()
+     {
--        using namespace testing;
--        ON_CALL(device,get_device_info())
++        ON_CALL(device, get_device_info())
              .WillByDefault(Return(mi::InputDeviceInfo{"device","dev-1", mi::DeviceCapability::unknown}));
++        ON_CALL(device, get_pointer_settings())
++            .WillByDefault(Return(mir::optional_value<mi::PointerSettings>()));
++        ON_CALL(device, get_touchpad_settings())
++            .WillByDefault(Return(mir::optional_value<mi::TouchpadSettings>()));
--        ON_CALL(another_device,get_device_info())
++        ON_CALL(another_device, get_device_info())
              .WillByDefault(Return(mi::InputDeviceInfo{"another_device","dev-2", mi::DeviceCapability::keyboard}));
++        ON_CALL(another_device, get_pointer_settings())
++            .WillByDefault(Return(mir::optional_value<mi::PointerSettings>()));
++        ON_CALL(another_device, get_touchpad_settings())
++            .WillByDefault(Return(mir::optional_value<mi::TouchpadSettings>()));
          ON_CALL(third_device,get_device_info())
              .WillByDefault(Return(mi::InputDeviceInfo{"third_device","dev-3", mi::DeviceCapability::keyboard}));
++        ON_CALL(third_device, get_pointer_settings())
++            .WillByDefault(Return(mir::optional_value<mi::PointerSettings>()));
++        ON_CALL(third_device, get_touchpad_settings())
++            .WillByDefault(Return(mir::optional_value<mi::TouchpadSettings>()));
++
++        ON_CALL(touchpad, get_device_info())
++            .WillByDefault(Return(mi::InputDeviceInfo{"touchpad", "dev-4", mi::DeviceCapability::touchpad|mi::DeviceCapability::pointer}));
++        ON_CALL(touchpad, get_pointer_settings())
++            .WillByDefault(Return(mi::PointerSettings()));
++        ON_CALL(touchpad, get_touchpad_settings())
++            .WillByDefault(Return(mi::TouchpadSettings()));
+     }
      void capture_input_sink(Nice<MockInputDevice>& dev, mi::InputSink*& sink, mi::EventBuilder*& builder)
+     {
--        using namespace ::testing;
          ON_CALL(dev,start(_,_))
              .WillByDefault(Invoke([&sink,&builder](mi::InputSink* input_sink, mi::EventBuilder* event_builder)
+                                   {
@@ -143,8 +164,6 @@
  TEST_F(InputDeviceHubTest, input_device_hub_starts_device)
+ {
--    using namespace ::testing;
--
      EXPECT_CALL(device,start(_,_));
      hub.add_device(mt::fake_shared(device));
@@ -152,8 +171,6 @@
  TEST_F(InputDeviceHubTest, input_device_hub_stops_device_on_removal)
+ {
--    using namespace ::testing;
--
      EXPECT_CALL(device,stop());
      hub.add_device(mt::fake_shared(device));
@@ -162,7 +179,6 @@
  TEST_F(InputDeviceHubTest, input_device_hub_ignores_removal_of_unknown_devices)
+ {
--    using namespace ::testing;
      EXPECT_CALL(device,start(_,_)).Times(0);
      EXPECT_CALL(device,stop()).Times(0);
@@ -172,7 +188,6 @@
  TEST_F(InputDeviceHubTest, input_device_hub_start_stop_happens_in_order)
+ {
--    using namespace ::testing;
      InSequence seq;
      EXPECT_CALL(device, start(_,_));
@@ -199,8 +214,6 @@
  TEST_F(InputDeviceHubTest, observers_receive_devices_on_add)
+ {
--    using namespace ::testing;
--
      std::shared_ptr<mi::Device> handle_1, handle_2;
      InSequence seq;
@@ -239,8 +252,6 @@
  TEST_F(InputDeviceHubTest, observers_receive_device_changes)
+ {
--    using namespace ::testing;
--
      InSequence seq;
      EXPECT_CALL(mock_observer, changes_complete());
      EXPECT_CALL(mock_observer, device_added(WithName("device")));
@@ -257,8 +268,6 @@
  TEST_F(InputDeviceHubTest, input_sink_posts_events_to_input_dispatcher)
+ {
--    using namespace ::testing;
--
      mi::InputSink* sink;
      mi::EventBuilder* builder;
      std::shared_ptr<mi::Device> handle;
@@ -283,7 +292,6 @@
  TEST_F(InputDeviceHubTest, forwards_touch_spots_to_visualizer)
+ {
--    using namespace ::testing;
      mi::InputSink* sink;
      mi::EventBuilder* builder;
@@ -352,7 +360,6 @@
  TEST_F(InputDeviceHubTest, confines_pointer_movement)
+ {
--    using namespace ::testing;
      geom::Point confined_pos{10, 18};
      ON_CALL(mock_region,confine(_))
@@ -375,8 +382,6 @@
  TEST_F(InputDeviceHubTest, forwards_pointer_updates_to_cursor_listener)
+ {
--    using namespace ::testing;
--
      auto x = 12.2f, y = 14.3f;
      mi::InputSink* sink;
@@ -391,6 +396,38 @@
      sink->handle_input(*event);
+ }
++TEST_F(InputDeviceHubTest, forwards_pointer_settings_to_input_device)
++{
++    std::shared_ptr<mi::Device> dev;
++    ON_CALL(mock_observer, device_added(_)).WillByDefault(SaveArg<0>(&dev));
++
++    hub.add_observer(mt::fake_shared(mock_observer));
++    hub.add_device(mt::fake_shared(touchpad));
++    observer_loop.trigger_server_actions();
++
++    EXPECT_CALL(touchpad, apply_settings(Matcher<mi::PointerSettings const&>(_)));
++
++    auto conf = dev->pointer_configuration();
++    dev->apply_pointer_configuration(conf.value());
++    multiplexer.dispatch(mir::dispatch::FdEvent::readable);
++}
++
++TEST_F(InputDeviceHubTest, forwards_touchpad_settings_to_input_device)
++{
++    std::shared_ptr<mi::Device> dev;
++    ON_CALL(mock_observer, device_added(_)).WillByDefault(SaveArg<0>(&dev));
++
++    hub.add_observer(mt::fake_shared(mock_observer));
++    hub.add_device(mt::fake_shared(touchpad));
++    observer_loop.trigger_server_actions();
++
++    EXPECT_CALL(touchpad, apply_settings(Matcher<mi::TouchpadSettings const&>(_)));
++
++    auto conf = dev->touchpad_configuration();
++    dev->apply_touchpad_configuration(conf.value());
++    multiplexer.dispatch(mir::dispatch::FdEvent::readable);
++}
++
  TEST_F(InputDeviceHubTest, input_sink_tracks_modifier)
+ {
      using namespace ::testing;