Simpler "plumbing" (#3471)

include/platform/mir/input/platform.h

@@ -41,7 +41,6 @@ namespace input
 class InputDevice;
 class InputReport;
 class InputDeviceRegistry;
-class LedObserverRegistrar;
 
 enum class PlatformPriority : uint32_t
 {
@@ -98,8 +97,7 @@ typedef mir::UniqueModulePtr<Platform>(*CreatePlatform)(
     std::shared_ptr<EmergencyCleanupRegistry> const& emergency_cleanup_registry,
     std::shared_ptr<InputDeviceRegistry> const& input_device_registry,
     std::shared_ptr<ConsoleServices> const& console,
-    std::shared_ptr<InputReport> const& report,
-    std::shared_ptr<LedObserverRegistrar> const& led_observer_registrar);
+    std::shared_ptr<InputReport> const& report);
 
 typedef void(*AddPlatformOptions)(
     boost::program_options::options_description& config);
@@ -139,8 +137,7 @@ mir::UniqueModulePtr<mir::input::Platform> create_input_platform(
     std::shared_ptr<mir::EmergencyCleanupRegistry> const& emergency_cleanup_registry,
     std::shared_ptr<mir::input::InputDeviceRegistry> const& input_device_registry,
     std::shared_ptr<mir::ConsoleServices> const& console,
-    std::shared_ptr<mir::input::InputReport> const& report,
-    std::shared_ptr<mir::input::LedObserverRegistrar> const& led_observer_registrar);
+    std::shared_ptr<mir::input::InputReport> const& report);
 
 /**
  * Function used to add additional configuration options

src/include/server/mir/input/input_probe.h

@@ -37,7 +37,6 @@ namespace input
 class InputReport;
 class Platform;
 class InputDeviceRegistry;
-class LedObserverRegistrar;
 
 mir::UniqueModulePtr<Platform> probe_input_platforms(
     options::Option const& options,
@@ -46,8 +45,7 @@ mir::UniqueModulePtr<Platform> probe_input_platforms(
     std::shared_ptr<ConsoleServices> const& console,
     std::shared_ptr<InputReport> const& input_report,
     std::vector<std::shared_ptr<SharedLibrary>> const& loaded_platforms,
-    SharedLibraryProberReport & prober_report,
-    std::shared_ptr<LedObserverRegistrar> const& led_observer_registrar);
+    SharedLibraryProberReport & prober_report);
 
 /// Tries to create an input platform from the graphics module, otherwise returns a null pointer
 auto input_platform_from_graphics_module(
@@ -56,8 +54,7 @@ auto input_platform_from_graphics_module(
     std::shared_ptr<EmergencyCleanupRegistry> const& emergency_cleanup,
     std::shared_ptr<InputDeviceRegistry> const& device_registry,
     std::shared_ptr<ConsoleServices> const& console,
-    std::shared_ptr<InputReport> const& input_report,
-    std::shared_ptr<LedObserverRegistrar> const& led_observer_registrar)
+    std::shared_ptr<InputReport> const& input_report)
 -> mir::UniqueModulePtr<Platform>;
 }
 }

src/platforms/evdev/libinput_device.cpp

@@ -25,7 +25,6 @@
 #include "mir/input/pointer_settings.h"
 #include "mir/input/touchpad_settings.h"
 #include "mir/input/input_device_info.h"
-#include "mir/input/led_observer_registrar.h"
 #include "mir/events/event_builders.h"
 #include "mir/geometry/displacement.h"
 #include "mir/dispatch/dispatchable.h"
@@ -105,44 +104,28 @@ auto get_axis_source(libinput_event_pointer* pointer) -> MirPointerAxisSource
     default:                                        return mir_pointer_axis_source_none;
     }
 }
+}
 
-class LibInputDeviceLedObserver : public mir::input::LedObserver
+void mie::LibInputDevice::leds_set(KeyboardLeds leds)
 {
-public:
-    explicit LibInputDeviceLedObserver(mie::LibInputDevice const* device) : device{device} {}
-
-    void leds_set(mir::input::KeyboardLeds leds) override
-    {
-        int led = 0;
-        if (contains(leds, mir::input::KeyboardLed::caps_lock))
-            led |= LIBINPUT_LED_CAPS_LOCK;
-        if (contains(leds, mir::input::KeyboardLed::num_lock))
-            led |= LIBINPUT_LED_NUM_LOCK;
-        if (contains(leds, mir::input::KeyboardLed::scroll_lock))
-            led |= LIBINPUT_LED_SCROLL_LOCK;
-
-        libinput_device_led_update(device->device(), static_cast<libinput_led>(led));
-    }
+    int led = 0;
+    if (contains(leds, mir::input::KeyboardLed::caps_lock))
+        led |= LIBINPUT_LED_CAPS_LOCK;
+    if (contains(leds, mir::input::KeyboardLed::num_lock))
+        led |= LIBINPUT_LED_NUM_LOCK;
+    if (contains(leds, mir::input::KeyboardLed::scroll_lock))
+        led |= LIBINPUT_LED_SCROLL_LOCK;
 
-private:
-    mie::LibInputDevice const* device;
-};
+    libinput_device_led_update(device(), static_cast<libinput_led>(led));
 }
 
-mie::LibInputDevice::LibInputDevice(
-    std::shared_ptr<mi::InputReport> const& report,
-    std::shared_ptr<LedObserverRegistrar> const& led_observer_registrar,
-    LibInputDevicePtr dev)
-    : report{report}, led_observer_registrar{led_observer_registrar},
-      device_{std::move(dev)}, pointer_pos{0, 0}, button_state{0}
+mie::LibInputDevice::LibInputDevice(std::shared_ptr<mi::InputReport> const& report, LibInputDevicePtr dev)
+    : report{report}, device_{std::move(dev)}, pointer_pos{0, 0}, button_state{0}
 {
     update_device_info();
 }
 
-mie::LibInputDevice::~LibInputDevice()
-{
-    try_stop_observing_leds();
-}
+mie::LibInputDevice::~LibInputDevice() = default;
 
 void mie::LibInputDevice::start(InputSink* sink, EventBuilder* builder)
 {
@@ -724,21 +707,3 @@ void mie::LibInputDevice::apply_settings(mi::TouchscreenSettings const& settings
 {
     touchscreen = settings;
 }
-
-void mie::LibInputDevice::associate_to_id(MirInputDeviceId id)
-{
-    try_stop_observing_leds();
-    led_observer = std::make_shared<LibInputDeviceLedObserver>(this);
-    led_observer_registrar->register_interest(led_observer, id);
-    device_id = id;
-}
-
-void mie::LibInputDevice::try_stop_observing_leds()
-{
-    if (led_observer && device_id)
-    {
-        led_observer_registrar->unregister_interest(*led_observer, device_id.value());
-        led_observer = nullptr;
-        device_id = std::nullopt;
-    }
-}

src/platforms/evdev/libinput_device.h

@@ -23,8 +23,9 @@
 #include "mir/input/event_builder.h"
 #include "mir/input/input_device.h"
 #include "mir/input/input_device_info.h"
-#include "mir/input/touchscreen_settings.h"
+#include "mir/input/keyboard_leds.h"
 #include "mir/input/led_observer_registrar.h"
+#include "mir/input/touchscreen_settings.h"
 #include "mir/geometry/point.h"
 
 #include <vector>
@@ -42,17 +43,12 @@ namespace input
 {
 class OutputInfo;
 class InputReport;
-class LedObserverRegistrar;
-class LedObserver;
 namespace evdev
 {
-class LibInputDevice : public input::InputDevice
+class LibInputDevice : public input::InputDevice, public mir::input::LedObserver
 {
 public:
-    LibInputDevice(
-        std::shared_ptr<InputReport> const& report,
-        std::shared_ptr<LedObserverRegistrar> const& led_observer_registrar,
-        LibInputDevicePtr dev);
+    LibInputDevice(std::shared_ptr<InputReport> const& report, LibInputDevicePtr dev);
     ~LibInputDevice();
     void start(InputSink* sink, EventBuilder* builder) override;
     void stop() override;
@@ -63,7 +59,8 @@ class LibInputDevice : public input::InputDevice
     void apply_settings(TouchpadSettings const&) override;
     optional_value<TouchscreenSettings> get_touchscreen_settings() const override;
     void apply_settings(TouchscreenSettings const&) override;
-    void associate_to_id(MirInputDeviceId id);
+
+    void leds_set(KeyboardLeds leds) override;
 
     void process_event(libinput_event* event);
     ::libinput_device* device() const;
@@ -84,8 +81,6 @@ class LibInputDevice : public input::InputDevice
     OutputInfo get_output_info() const;
 
     std::shared_ptr<InputReport> const report;
-    std::shared_ptr<LedObserverRegistrar> led_observer_registrar;
-    std::shared_ptr<LedObserver> led_observer;
     LibInputDevicePtr const device_;
 
     InputSink* sink{nullptr};
@@ -108,10 +103,8 @@ class LibInputDevice : public input::InputDevice
         bool down_notified = false;
     };
     std::map<MirTouchId,ContactData> last_seen_properties;
-    std::optional<MirInputDeviceId> device_id;
 
     void update_contact_data(ContactData &data, MirTouchAction action, libinput_event_touch* touch);
-    void try_stop_observing_leds();
 };
 }
 }

src/platforms/evdev/platform.cpp

@@ -28,7 +28,6 @@
 
 #include "mir/input/input_device_registry.h"
 #include "mir/input/input_report.h"
-#include "mir/input/device.h"
 #include "mir/fd.h"
 #include "mir/raii.h"
 
@@ -129,13 +128,11 @@ mie::Platform::Platform(
         std::shared_ptr<InputDeviceRegistry> const& registry,
         std::shared_ptr<InputReport> const& report,
         std::unique_ptr<udev::Context>&& udev_context,
-        std::shared_ptr<ConsoleServices> const& console,
-        std::shared_ptr<LedObserverRegistrar> const& led_observer_registrar) :
+        std::shared_ptr<ConsoleServices> const& console) :
     report(report),
     udev_context(std::move(udev_context)),
     input_device_registry(registry),
     console{console},
-    led_observer_registrar{led_observer_registrar},
     platform_dispatchable{std::make_shared<md::MultiplexingDispatchable>()}
 {
 }
@@ -430,14 +427,9 @@ void mie::Platform::device_added(libinput_device* dev)
 
     try
     {
-        auto new_device = std::make_shared<mie::LibInputDevice>(
-            report, led_observer_registrar, std::move(device_ptr));
-        devices.emplace_back(new_device);
+        devices.emplace_back(std::make_shared<mie::LibInputDevice>(report, std::move(device_ptr)));
 
-        auto weak_device = input_device_registry->add_device(devices.back());
-
-        if (auto device = weak_device.lock())
-            new_device->associate_to_id(device->id());
+        input_device_registry->add_device(devices.back());
 
         log_info("Opened device: %s", describe(dev).c_str());
     }

src/platforms/evdev/platform.h

@@ -49,7 +49,6 @@ class ActionQueue;
 namespace input
 {
 class InputDeviceRegistry;
-class LedObserverRegistrar;
 namespace evdev
 {
 
@@ -62,8 +61,7 @@ class Platform : public input::Platform
         std::shared_ptr<InputDeviceRegistry> const& registry,
         std::shared_ptr<InputReport> const& report,
         std::unique_ptr<udev::Context>&& udev_context,
-        std::shared_ptr<ConsoleServices> const& console,
-        std::shared_ptr<LedObserverRegistrar> const& led_observer_registrar);
+        std::shared_ptr<ConsoleServices> const& console);
     std::shared_ptr<mir::dispatch::Dispatchable> dispatchable() override;
     void start() override;
     void stop() override;
@@ -81,7 +79,6 @@ class Platform : public input::Platform
     std::shared_ptr<udev::Context> const udev_context;
     std::shared_ptr<InputDeviceRegistry> const input_device_registry;
     std::shared_ptr<ConsoleServices> const console;
-    std::shared_ptr<LedObserverRegistrar> const led_observer_registrar;
     std::shared_ptr<dispatch::MultiplexingDispatchable> const platform_dispatchable;
     std::shared_ptr<::libinput> lib;
     std::shared_ptr<dispatch::ReadableFd> libinput_dispatchable;

src/platforms/evdev/platform_factory.cpp

@@ -44,16 +44,14 @@ mir::UniqueModulePtr<mi::Platform> create_input_platform(
     std::shared_ptr<mir::EmergencyCleanupRegistry> const& /*emergency_cleanup_registry*/,
     std::shared_ptr<mi::InputDeviceRegistry> const& input_device_registry,
     std::shared_ptr<mir::ConsoleServices> const& console,
-    std::shared_ptr<mi::InputReport> const& report,
-    std::shared_ptr<mi::LedObserverRegistrar> const& led_observer_registrar)
+    std::shared_ptr<mi::InputReport> const& report)
 {
     mir::assert_entry_point_signature<mi::CreatePlatform>(&create_input_platform);
     return mir::make_module_ptr<mie::Platform>(
         input_device_registry,
         report,
         std::make_unique<mu::Context>(),
-        console,
-        led_observer_registrar);
+        console);
 }
 
 void add_input_platform_options(

src/platforms/wayland/input.cpp

@@ -30,8 +30,7 @@ mir::UniqueModulePtr<mi::Platform> create_input_platform(
     std::shared_ptr<mir::EmergencyCleanupRegistry> const& /*emergency_cleanup_registry*/,
     std::shared_ptr<mi::InputDeviceRegistry> const& input_device_registry,
     std::shared_ptr<mir::ConsoleServices> const& /*console*/,
-    std::shared_ptr<mi::InputReport> const& /*report*/,
-    std::shared_ptr<mi::LedObserverRegistrar> const& /*led_observer_registrar*/)
+    std::shared_ptr<mi::InputReport> const& /*report*/)
 {
     mir::assert_entry_point_signature<mi::CreatePlatform>(&create_input_platform);
     return mir::make_module_ptr<miw::InputPlatform>(input_device_registry);

src/platforms/x11/input/input.cpp

@@ -30,8 +30,7 @@ mir::UniqueModulePtr<mi::Platform> create_input_platform(
     std::shared_ptr<mir::EmergencyCleanupRegistry> const& /*emergency_cleanup_registry*/,
     std::shared_ptr<mi::InputDeviceRegistry> const& input_device_registry,
     std::shared_ptr<mir::ConsoleServices> const& /*console*/,
-    std::shared_ptr<mi::InputReport> const& /*report*/,
-    std::shared_ptr<mi::LedObserverRegistrar> const& /*led_observer_registrar*/)
+    std::shared_ptr<mi::InputReport> const& /*report*/)
 {
     mir::assert_entry_point_signature<mi::CreatePlatform>(&create_input_platform);
     return mir::make_module_ptr<mix::XInputPlatform>(input_device_registry, mx::X11Resources::instance());

src/server/input/default_configuration.cpp

@@ -252,8 +252,7 @@ mir::DefaultServerConfiguration::the_input_manager()
                     the_console_services(),
                     input_report,
                     the_platform_libaries(),
-                    *the_shared_library_prober_report(),
-                    the_led_observer_registrar());
+                    *the_shared_library_prober_report());
 
                 return std::make_shared<mi::DefaultInputManager>(the_input_reading_multiplexer(), std::move(platform));
             }
@@ -315,7 +314,8 @@ std::shared_ptr<mi::DefaultInputDeviceHub> mir::DefaultServerConfiguration::the_
                the_input_reading_multiplexer(),
                the_clock(),
                the_key_mapper(),
-               the_server_status_listener());
+               the_server_status_listener(),
+               the_led_observer_registrar());
 
            // lp:1675357: KeyRepeatDispatcher must be informed about removed input devices, otherwise
            // pressed keys get repeated indefinitely

src/server/input/default_input_device_hub.cpp

@@ -32,6 +32,8 @@
 #include "mir/log.h"
 
 #include "boost/throw_exception.hpp"
+#include "mir/input/led_observer_registrar.h"
+#include "mir/input/key_mapper.h"
 
 #include <algorithm>
 #include <atomic>
@@ -199,13 +201,15 @@ mi::DefaultInputDeviceHub::DefaultInputDeviceHub(
     std::shared_ptr<dispatch::MultiplexingDispatchable> const& input_multiplexer,
     std::shared_ptr<time::Clock> const& clock,
     std::shared_ptr<mi::KeyMapper> const& key_mapper,
-    std::shared_ptr<mir::ServerStatusListener> const& server_status_listener)
+    std::shared_ptr<mir::ServerStatusListener> const& server_status_listener,
+    std::shared_ptr<LedObserverRegistrar> led_observer_registrar)
     : seat{seat},
       input_dispatchable{input_multiplexer},
       device_queue(std::make_shared<dispatch::ActionQueue>()),
       clock(clock),
       key_mapper(key_mapper),
       server_status_listener(server_status_listener),
+      led_observer_registrar{std::move(led_observer_registrar)},
       device_id_generator{0}
 {
     input_dispatchable->add_watch(device_queue);
@@ -492,6 +496,11 @@ auto mi::DefaultInputDeviceHub::add_device(std::shared_ptr<InputDevice> const& d
         seat->add_device(*handle);
         dev->start(seat, input_dispatchable);
 
+        if (auto const observer = std::dynamic_pointer_cast<LedObserver>(device))
+        {
+            led_observer_registrar->register_interest(observer, handle->id());
+        }
+
         return handle;
     }
     else
@@ -534,6 +543,11 @@ void mi::DefaultInputDeviceHub::remove_device(std::shared_ptr<InputDevice> const
         BOOST_THROW_EXCEPTION(std::logic_error("Input device not managed by server"));
     }
 
+    if (auto const observer = std::dynamic_pointer_cast<LedObserver>(device))
+    {
+        led_observer_registrar->unregister_interest(*observer, (*pos)->id());
+    }
+
     devices.erase(pos, end(devices));
 }