Merge pull request #6166 from ligfx/invokedeviceschangedcallbacks

ControllerInterface: cleanup callbacks API and logic

10 files changed, 78 insertions, 50 deletions

diff --git a/Source/Core/Core/HW/GCKeyboard.cpp b/Source/Core/Core/HW/GCKeyboard.cpp
index 300ed0608c..b5ec80bf53 100644
--- a/Source/Core/Core/HW/GCKeyboard.cpp
+++ b/Source/Core/Core/HW/GCKeyboard.cpp
@@ -37,7 +37,7 @@ void Initialize()
       s_config.CreateController<GCKeyboard>(i);
   }
 
-  g_controller_interface.RegisterHotplugCallback(LoadConfig);
+  g_controller_interface.RegisterDevicesChangedCallback(LoadConfig);
 
   // Load the saved controller config
   s_config.LoadConfig(true);
diff --git a/Source/Core/Core/HW/GCPad.cpp b/Source/Core/Core/HW/GCPad.cpp
index fcf9f524c5..9cefbb4db1 100644
--- a/Source/Core/Core/HW/GCPad.cpp
+++ b/Source/Core/Core/HW/GCPad.cpp
@@ -34,7 +34,7 @@ void Initialize()
       s_config.CreateController<GCPad>(i);
   }
 
-  g_controller_interface.RegisterHotplugCallback(LoadConfig);
+  g_controller_interface.RegisterDevicesChangedCallback(LoadConfig);
 
   // Load the saved controller config
   s_config.LoadConfig(true);
diff --git a/Source/Core/Core/HW/Wiimote.cpp b/Source/Core/Core/HW/Wiimote.cpp
index 0e0d3f9f78..178ee75540 100644
--- a/Source/Core/Core/HW/Wiimote.cpp
+++ b/Source/Core/Core/HW/Wiimote.cpp
@@ -80,7 +80,7 @@ void Initialize(InitializeMode init_mode)
       s_config.CreateController<WiimoteEmu::Wiimote>(i);
   }
 
-  g_controller_interface.RegisterHotplugCallback(LoadConfig);
+  g_controller_interface.RegisterDevicesChangedCallback(LoadConfig);
 
   LoadConfig();
 
diff --git a/Source/Core/Core/HotkeyManager.cpp b/Source/Core/Core/HotkeyManager.cpp
index 6e96aad1d7..ecaeb58369 100644
--- a/Source/Core/Core/HotkeyManager.cpp
+++ b/Source/Core/Core/HotkeyManager.cpp
@@ -213,7 +213,7 @@ void Initialize()
   if (s_config.ControllersNeedToBeCreated())
     s_config.CreateController<HotkeyManager>();
 
-  g_controller_interface.RegisterHotplugCallback(LoadConfig);
+  g_controller_interface.RegisterDevicesChangedCallback(LoadConfig);
 
   // load the saved controller config
   s_config.LoadConfig(true);
diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
index 1a72fa172c..328258a8b2 100644
--- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
@@ -2,10 +2,12 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
-#include <mutex>
-
 #include "InputCommon/ControllerInterface/ControllerInterface.h"
 
+#include <algorithm>
+
+#include "Common/Logging/Log.h"
+
 #ifdef CIFACE_USE_XINPUT
 #include "InputCommon/ControllerInterface/XInput/XInput.h"
 #endif
@@ -45,6 +47,7 @@ void ControllerInterface::Initialize(void* const hwnd)
     return;
 
   m_hwnd = hwnd;
+  m_is_populating_devices = true;
 
 #ifdef CIFACE_USE_DINPUT
 // nothing needed
@@ -86,6 +89,8 @@ void ControllerInterface::RefreshDevices()
     m_devices.clear();
   }
 
+  m_is_populating_devices = true;
+
 #ifdef CIFACE_USE_DINPUT
   ciface::DInput::PopulateDevices(reinterpret_cast<HWND>(m_hwnd));
 #endif
@@ -111,6 +116,9 @@ void ControllerInterface::RefreshDevices()
 #ifdef CIFACE_USE_PIPES
   ciface::Pipes::PopulateDevices();
 #endif
+
+  m_is_populating_devices = false;
+  InvokeDevicesChangedCallbacks();
 }
 
 //
@@ -165,30 +173,49 @@ void ControllerInterface::Shutdown()
 
 void ControllerInterface::AddDevice(std::shared_ptr<ciface::Core::Device> device)
 {
-  std::lock_guard<std::mutex> lk(m_devices_mutex);
-  // Try to find an ID for this device
-  int id = 0;
-  while (true)
   {
-    const auto it = std::find_if(m_devices.begin(), m_devices.end(), [&device, &id](const auto& d) {
-      return d->GetSource() == device->GetSource() && d->GetName() == device->GetName() &&
-             d->GetId() == id;
-    });
-    if (it == m_devices.end())  // no device with the same name with this ID, so we can use it
-      break;
-    else
-      id++;
+    std::lock_guard<std::mutex> lk(m_devices_mutex);
+    // Try to find an ID for this device
+    int id = 0;
+    while (true)
+    {
+      const auto it =
+          std::find_if(m_devices.begin(), m_devices.end(), [&device, &id](const auto& d) {
+            return d->GetSource() == device->GetSource() && d->GetName() == device->GetName() &&
+                   d->GetId() == id;
+          });
+      if (it == m_devices.end())  // no device with the same name with this ID, so we can use it
+        break;
+      else
+        id++;
+    }
+    device->SetId(id);
+
+    NOTICE_LOG(SERIALINTERFACE, "Added device: %s", device->GetQualifiedName().c_str());
+    m_devices.emplace_back(std::move(device));
   }
-  device->SetId(id);
-  m_devices.emplace_back(std::move(device));
+
+  if (!m_is_populating_devices)
+    InvokeDevicesChangedCallbacks();
 }
 
 void ControllerInterface::RemoveDevice(std::function<bool(const ciface::Core::Device*)> callback)
 {
-  std::lock_guard<std::mutex> lk(m_devices_mutex);
-  m_devices.erase(std::remove_if(m_devices.begin(), m_devices.end(),
-                                 [&callback](const auto& dev) { return callback(dev.get()); }),
-                  m_devices.end());
+  {
+    std::lock_guard<std::mutex> lk(m_devices_mutex);
+    auto it = std::remove_if(m_devices.begin(), m_devices.end(), [&callback](const auto& dev) {
+      if (callback(dev.get()))
+      {
+        NOTICE_LOG(SERIALINTERFACE, "Removed device: %s", dev->GetQualifiedName().c_str());
+        return true;
+      }
+      return false;
+    });
+    m_devices.erase(it, m_devices.end());
+  }
+
+  if (!m_is_populating_devices)
+    InvokeDevicesChangedCallbacks();
 }
 
 //
@@ -208,23 +235,25 @@ void ControllerInterface::UpdateInput()
 }
 
 //
-// RegisterHotplugCallback
+// RegisterDevicesChangedCallback
 //
-// Register a callback to be called from the input backends' hotplug thread
-// when there is a new device
+// Register a callback to be called when a device is added or removed (as from the input backends'
+// hotplug thread), or when devices are refreshed
 //
-void ControllerInterface::RegisterHotplugCallback(std::function<void()> callback)
+void ControllerInterface::RegisterDevicesChangedCallback(std::function<void()> callback)
 {
-  m_hotplug_callbacks.emplace_back(std::move(callback));
+  std::lock_guard<std::mutex> lk(m_callbacks_mutex);
+  m_devices_changed_callbacks.emplace_back(std::move(callback));
 }
 
 //
-// InvokeHotplugCallbacks
+// InvokeDevicesChangedCallbacks
 //
 // Invoke all callbacks that were registered
 //
-void ControllerInterface::InvokeHotplugCallbacks() const
+void ControllerInterface::InvokeDevicesChangedCallbacks() const
 {
-  for (const auto& callback : m_hotplug_callbacks)
+  std::lock_guard<std::mutex> lk(m_callbacks_mutex);
+  for (const auto& callback : m_devices_changed_callbacks)
     callback();
 }
diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
index 23afab7d67..06fd7c544d 100644
--- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
+++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
@@ -4,14 +4,12 @@
 
 #pragma once
 
-#include <algorithm>
+#include <atomic>
 #include <functional>
-#include <map>
-#include <sstream>
-#include <string>
+#include <memory>
+#include <mutex>
 #include <vector>
 
-#include "Common/CommonTypes.h"
 #include "InputCommon/ControllerInterface/Device.h"
 
 // enable disable sources
@@ -53,12 +51,14 @@ public:
   bool IsInit() const { return m_is_init; }
   void UpdateInput();
 
-  void RegisterHotplugCallback(std::function<void(void)> callback);
-  void InvokeHotplugCallbacks() const;
+  void RegisterDevicesChangedCallback(std::function<void(void)> callback);
+  void InvokeDevicesChangedCallbacks() const;
 
 private:
-  std::vector<std::function<void()>> m_hotplug_callbacks;
+  std::vector<std::function<void()>> m_devices_changed_callbacks;
+  mutable std::mutex m_callbacks_mutex;
   bool m_is_init;
+  std::atomic<bool> m_is_populating_devices{false};
   void* m_hwnd;
 };
 
diff --git a/Source/Core/InputCommon/ControllerInterface/Device.cpp b/Source/Core/InputCommon/ControllerInterface/Device.cpp
index 3c0add95f2..b8d73d9ab8 100644
--- a/Source/Core/InputCommon/ControllerInterface/Device.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/Device.cpp
@@ -2,12 +2,14 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
+#include "InputCommon/ControllerInterface/Device.h"
+
 #include <memory>
 #include <sstream>
 #include <string>
 #include <tuple>
 
-#include "InputCommon/ControllerInterface/Device.h"
+#include "Common/StringUtil.h"
 
 namespace ciface
 {
@@ -39,6 +41,11 @@ void Device::AddOutput(Device::Output* const o)
   m_outputs.push_back(o);
 }
 
+std::string Device::GetQualifiedName() const
+{
+  return StringFromFormat("%s/%i/%s", this->GetSource().c_str(), GetId(), this->GetName().c_str());
+}
+
 Device::Input* Device::FindInput(const std::string& name) const
 {
   for (Input* input : m_inputs)
diff --git a/Source/Core/InputCommon/ControllerInterface/Device.h b/Source/Core/InputCommon/ControllerInterface/Device.h
index 560a842999..dc3bdbe84f 100644
--- a/Source/Core/InputCommon/ControllerInterface/Device.h
+++ b/Source/Core/InputCommon/ControllerInterface/Device.h
@@ -76,6 +76,7 @@ public:
   void SetId(int id) { m_id = id; }
   virtual std::string GetName() const = 0;
   virtual std::string GetSource() const = 0;
+  std::string GetQualifiedName() const;
   virtual void UpdateInput() {}
   virtual bool IsValid() const { return true; }
   const std::vector<Input*>& Inputs() const { return m_inputs; }
diff --git a/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm b/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm
index b13701209e..a923c1ea26 100644
--- a/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm
+++ b/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm
@@ -156,8 +156,6 @@ static void DeviceRemovalCallback(void* inContext, IOReturn inResult, void* inSe
 
     return false;
   });
-  g_controller_interface.InvokeHotplugCallbacks();
-  NOTICE_LOG(SERIALINTERFACE, "Removed device: %s", GetDeviceRefName(inIOHIDDeviceRef).c_str());
 }
 
 static void DeviceMatchingCallback(void* inContext, IOReturn inResult, void* inSender,
@@ -174,9 +172,6 @@ static void DeviceMatchingCallback(void* inContext, IOReturn inResult, void* inS
   {
     g_controller_interface.AddDevice(std::make_shared<Joystick>(inIOHIDDeviceRef, name));
   }
-
-  NOTICE_LOG(SERIALINTERFACE, "Added device: %s", name.c_str());
-  g_controller_interface.InvokeHotplugCallbacks();
 }
 
 void Init(void* window)
diff --git a/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp b/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp
index c7c97ebd0e..85e4973270 100644
--- a/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp
@@ -92,9 +92,7 @@ static void HotplugThreadFunc()
       g_controller_interface.RemoveDevice([&name](const auto& device) {
         return device->GetSource() == "evdev" && device->GetName() == name && !device->IsValid();
       });
-      NOTICE_LOG(SERIALINTERFACE, "Removed device: %s", name.c_str());
       s_devnode_name_map.erase(devnode);
-      g_controller_interface.InvokeHotplugCallbacks();
     }
     // Only react to "device added" events for evdev devices that we can access.
     else if (strcmp(action, "add") == 0 && access(devnode, W_OK) == 0)
@@ -107,8 +105,6 @@ static void HotplugThreadFunc()
       {
         g_controller_interface.AddDevice(std::move(device));
         s_devnode_name_map.insert(std::pair<std::string, std::string>(devnode, name));
-        NOTICE_LOG(SERIALINTERFACE, "Added new device: %s", name.c_str());
-        g_controller_interface.InvokeHotplugCallbacks();
       }
     }
     udev_device_unref(dev);