WiimoteReal: Make the connected Wiimote check common

This moves the unordered_map used to store connected Wiimotes IDs to
WiimoteReal, and makes the ID insert/erase logic common so we don't
have to duplicate this code in scanner backends.

Source/Core/Core/HW/WiimoteReal/IOAndroid.h

@@ -7,6 +7,7 @@
 #ifdef ANDROID
 #include <jni.h>
 
+#include "Common/StringUtil.h"
 #include "Core/HW/WiimoteReal/WiimoteReal.h"
 
 namespace WiimoteReal
@@ -16,7 +17,7 @@ class WiimoteAndroid final : public Wiimote
 public:
   WiimoteAndroid(int index);
   ~WiimoteAndroid() override;
-
+  std::string GetId() const override { return "Android " + StringFromInt(m_mayflash_index); }
 protected:
   bool ConnectInternal() override;
   void DisconnectInternal() override;

Source/Core/Core/HW/WiimoteReal/IOLinux.cpp

@@ -14,14 +14,6 @@
 
 namespace WiimoteReal
 {
-// This is used to store the Bluetooth address of connected Wiimotes,
-// so we can ignore Wiimotes that are already connected.
-static std::vector<std::string> s_known_addrs;
-static bool IsNewWiimote(const std::string& addr)
-{
-  return std::find(s_known_addrs.begin(), s_known_addrs.end(), addr) == s_known_addrs.end();
-}
-
 WiimoteScannerLinux::WiimoteScannerLinux() : m_device_id(-1), m_device_sock(-1)
 {
   // Get the id of the first Bluetooth device.
@@ -100,7 +92,6 @@ void WiimoteScannerLinux::FindWiimotes(std::vector<Wiimote*>& found_wiimotes, Wi
       continue;
 
     // Found a new device
-    s_known_addrs.push_back(bdaddr_str);
     Wiimote* wm = new WiimoteLinux(scan_infos[i].bdaddr);
     if (IsBalanceBoardName(name))
     {
@@ -180,10 +171,6 @@ void WiimoteLinux::DisconnectInternal()
 
   m_cmd_sock = -1;
   m_int_sock = -1;
-  char bdaddr_str[18] = {};
-  ba2str(&m_bdaddr, bdaddr_str);
-  s_known_addrs.erase(std::remove(s_known_addrs.begin(), s_known_addrs.end(), bdaddr_str),
-                      s_known_addrs.end());
 }
 
 bool WiimoteLinux::IsConnected() const

Source/Core/Core/HW/WiimoteReal/IOLinux.h

@@ -16,6 +16,12 @@ class WiimoteLinux final : public Wiimote
 public:
   WiimoteLinux(bdaddr_t bdaddr);
   ~WiimoteLinux() override;
+  std::string GetId() const override
+  {
+    char bdaddr_str[18] = {};
+    ba2str(&m_bdaddr, bdaddr_str);
+    return bdaddr_str;
+  }
 
 protected:
   bool ConnectInternal() override;

Source/Core/Core/HW/WiimoteReal/IOWin.cpp

@@ -25,13 +25,9 @@
 #include "Common/CommonFuncs.h"
 #include "Common/CommonTypes.h"
 #include "Common/Logging/Log.h"
-#include "Common/StringUtil.h"
 #include "Common/Thread.h"
 #include "Core/HW/WiimoteReal/IOWin.h"
 
-//#define AUTHENTICATE_WIIMOTES
-#define SHARE_WRITE_WIIMOTES
-
 // Create func_t function pointer type and declare a nullptr-initialized static variable of that
 // type named "pfunc".
 #define DYN_FUNC_DECLARE(func)                                                                     \
@@ -64,11 +60,6 @@ static bool s_loaded_ok = false;
 
 std::unordered_map<BTH_ADDR, std::time_t> g_connect_times;
 
-#ifdef SHARE_WRITE_WIIMOTES
-std::unordered_set<std::basic_string<TCHAR>> g_connected_wiimotes;
-std::mutex g_connected_wiimotes_lock;
-#endif
-
 #define DYN_FUNC_UNLOAD(func) p##func = nullptr;
 
 // Attempt to load the function from the given module handle.
@@ -448,18 +439,7 @@ void WiimoteScannerWindows::FindWiimotes(std::vector<Wiimote*>& found_wiimotes,
 
       WinWriteMethod write_method = GetInitialWriteMethod(IsUsingToshibaStack);
 
-#ifdef SHARE_WRITE_WIIMOTES
-      {
-        std::lock_guard<std::mutex> lk(g_connected_wiimotes_lock);
-        // This device is a Wiimote that is already connected. Skip it.
-        if (g_connected_wiimotes.count(device_path) != 0)
-        {
-          free(detail_data);
-          continue;
-        }
-      }
-#endif
-      if (!IsWiimote(device_path, write_method))
+      if (!IsNewWiimote(UTF16ToUTF8(device_path)) || !IsWiimote(device_path, write_method))
       {
         free(detail_data);
         continue;
@@ -510,19 +490,10 @@ bool WiimoteWindows::ConnectInternal()
   if (IsConnected())
     return true;
 
-#ifdef SHARE_WRITE_WIIMOTES
-  std::lock_guard<std::mutex> lk(g_connected_wiimotes_lock);
-  if (g_connected_wiimotes.count(m_devicepath) != 0)
+  if (!IsNewWiimote(UTF16ToUTF8(m_devicepath)))
     return false;
 
   auto const open_flags = FILE_SHARE_READ | FILE_SHARE_WRITE;
-#else
-  // Having no FILE_SHARE_WRITE disallows us from connecting to the same Wiimote twice.
-  // (And disallows using Wiimotes in use by other programs)
-  // This is what "WiiYourself" does.
-  // Apparently this doesn't work for everyone. It might be their fault.
-  auto const open_flags = FILE_SHARE_READ;
-#endif
 
   m_dev_handle = CreateFile(m_devicepath.c_str(), GENERIC_READ | GENERIC_WRITE, open_flags, nullptr,
                             OPEN_EXISTING, FILE_FLAG_OVERLAPPED, nullptr);
@@ -558,18 +529,15 @@ bool WiimoteWindows::ConnectInternal()
 	}
 #endif
 
-// TODO: thread isn't started here now, do this elsewhere
-// This isn't as drastic as it sounds, since the process in which the threads
-// reside is normal priority. Needed for keeping audio reports at a decent rate
-/*
-  if (!SetThreadPriority(m_wiimote_thread.native_handle(), THREAD_PRIORITY_TIME_CRITICAL))
-  {
-    ERROR_LOG(WIIMOTE, "Failed to set Wiimote thread priority");
-  }
-*/
-#ifdef SHARE_WRITE_WIIMOTES
-  g_connected_wiimotes.insert(m_devicepath);
-#endif
+  // TODO: thread isn't started here now, do this elsewhere
+  // This isn't as drastic as it sounds, since the process in which the threads
+  // reside is normal priority. Needed for keeping audio reports at a decent rate
+  /*
+    if (!SetThreadPriority(m_wiimote_thread.native_handle(), THREAD_PRIORITY_TIME_CRITICAL))
+    {
+      ERROR_LOG(WIIMOTE, "Failed to set Wiimote thread priority");
+    }
+  */
 
   return true;
 }
@@ -581,11 +549,6 @@ void WiimoteWindows::DisconnectInternal()
 
   CloseHandle(m_dev_handle);
   m_dev_handle = nullptr;
-
-#ifdef SHARE_WRITE_WIIMOTES
-  std::lock_guard<std::mutex> lk(g_connected_wiimotes_lock);
-  g_connected_wiimotes.erase(m_devicepath);
-#endif
 }
 
 WiimoteWindows::WiimoteWindows(const std::basic_string<TCHAR>& path,

Source/Core/Core/HW/WiimoteReal/IOWin.h

@@ -7,6 +7,7 @@
 #ifdef _WIN32
 #include <windows.h>
 
+#include "Common/StringUtil.h"
 #include "Core/HW/WiimoteEmu/WiimoteHid.h"
 #include "Core/HW/WiimoteReal/WiimoteReal.h"
 
@@ -25,7 +26,7 @@ class WiimoteWindows final : public Wiimote
 public:
   WiimoteWindows(const std::basic_string<TCHAR>& path, WinWriteMethod initial_write_method);
   ~WiimoteWindows() override;
-
+  std::string GetId() const override { return UTF16ToUTF8(m_devicepath); }
 protected:
   bool ConnectInternal() override;
   void DisconnectInternal() override;

Source/Core/Core/HW/WiimoteReal/IOdarwin_private.h

@@ -23,7 +23,7 @@ class WiimoteDarwin final : public Wiimote
 public:
   WiimoteDarwin(IOBluetoothDevice* device);
   ~WiimoteDarwin() override;
-
+  std::string GetId() const override { return [[m_btd addressString] UTF8String]; }
   // These are not protected/private because ConnectBT needs them.
   void DisconnectInternal() override;
   IOBluetoothDevice* m_btd;

Source/Core/Core/HW/WiimoteReal/IOhidapi.cpp

@@ -3,7 +3,6 @@
 // Refer to the license.txt file included.
 
 #include <algorithm>
-#include <unordered_set>
 
 #include "Common/Assert.h"
 #include "Common/Logging/Log.h"
@@ -11,10 +10,6 @@
 #include "Core/HW/WiimoteEmu/WiimoteHid.h"
 #include "Core/HW/WiimoteReal/IOhidapi.h"
 
-// This is used to store connected Wiimotes' paths, so we don't connect
-// more than once to the same device.
-static std::unordered_set<std::string> s_known_paths;
-
 static bool IsDeviceUsable(const std::string& device_path)
 {
   hid_device* handle = hid_open_path(device_path.c_str());
@@ -42,7 +37,6 @@ namespace WiimoteReal
 {
 WiimoteScannerHidapi::WiimoteScannerHidapi()
 {
-  s_known_paths.clear();
   int ret = hid_init();
   _assert_msg_(WIIMOTE, ret == 0, "Couldn't initialise hidapi.");
 }
@@ -67,7 +61,7 @@ void WiimoteScannerHidapi::FindWiimotes(std::vector<Wiimote*>& wiimotes, Wiimote
     const bool is_wiimote =
         IsValidDeviceName(name) || (device->vendor_id == 0x057e &&
                                     (device->product_id == 0x0306 || device->product_id == 0x0330));
-    if (!is_wiimote || s_known_paths.count(device->path) != 0 || !IsDeviceUsable(device->path))
+    if (!is_wiimote || !IsNewWiimote(device->path) || !IsDeviceUsable(device->path))
       continue;
 
     auto* wiimote = new WiimoteHidapi(device->path);
@@ -87,13 +81,11 @@ void WiimoteScannerHidapi::FindWiimotes(std::vector<Wiimote*>& wiimotes, Wiimote
 
 WiimoteHidapi::WiimoteHidapi(const std::string& device_path) : m_device_path(device_path)
 {
-  s_known_paths.insert(m_device_path);
 }
 
 WiimoteHidapi::~WiimoteHidapi()
 {
   Shutdown();
-  s_known_paths.erase(m_device_path);
 }
 
 bool WiimoteHidapi::ConnectInternal()
@@ -107,7 +99,6 @@ bool WiimoteHidapi::ConnectInternal()
     ERROR_LOG(WIIMOTE, "Could not connect to Wiimote at \"%s\". "
                        "Do you have permission to access the device?",
               m_device_path.c_str());
-    s_known_paths.erase(m_device_path);
   }
   return m_handle != nullptr;
 }

Source/Core/Core/HW/WiimoteReal/IOhidapi.h

@@ -16,7 +16,7 @@ class WiimoteHidapi final : public Wiimote
 public:
   explicit WiimoteHidapi(const std::string& device_path);
   ~WiimoteHidapi() override;
-
+  std::string GetId() const override { return m_device_path; }
 protected:
   bool ConnectInternal() override;
   void DisconnectInternal() override;

Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp

@@ -5,6 +5,7 @@
 #include <algorithm>
 #include <cstdlib>
 #include <queue>
+#include <unordered_set>
 
 #include "Core/HW/WiimoteReal/WiimoteReal.h"
 
@@ -37,6 +38,11 @@ void HandleWiimoteDisconnect(int index);
 
 static bool g_real_wiimotes_initialized = false;
 
+// This is used to store connected Wiimotes' IDs, so we don't connect
+// more than once to the same device.
+static std::unordered_set<std::string> s_known_ids;
+static std::mutex s_known_ids_mutex;
+
 std::mutex g_wiimotes_mutex;
 
 Wiimote* g_wiimotes[MAX_BBMOTES];
@@ -687,6 +693,7 @@ void Initialize(::Wiimote::InitializeMode init_mode)
 {
   if (!g_real_wiimotes_initialized)
   {
+    s_known_ids.clear();
     g_wiimote_scanner.AddScannerBackend(std::make_unique<WiimoteScannerLinux>());
     g_wiimote_scanner.AddScannerBackend(std::make_unique<WiimoteScannerAndroid>());
     g_wiimote_scanner.AddScannerBackend(std::make_unique<WiimoteScannerWindows>());
@@ -790,6 +797,8 @@ static bool TryToConnectWiimoteToSlot(Wiimote* wm, unsigned int i)
       NOTICE_LOG(WIIMOTE, "Connected to Wiimote %i.", i + 1);
       g_wiimotes[i] = wm;
       Host_ConnectWiimote(i, true);
+      std::lock_guard<std::mutex> lk(s_known_ids_mutex);
+      s_known_ids.insert(wm->GetId());
     }
     return true;
   }
@@ -824,6 +833,8 @@ void HandleWiimoteDisconnect(int index)
   std::swap(wm, g_wiimotes[index]);
   if (wm)
   {
+    std::lock_guard<std::mutex> lk(s_known_ids_mutex);
+    s_known_ids.erase(wm->GetId());
     delete wm;
     NOTICE_LOG(WIIMOTE, "Disconnected Wiimote %i.", index + 1);
   }
@@ -897,4 +908,11 @@ bool IsBalanceBoardName(const std::string& name)
   return "Nintendo RVL-WBC-01" == name;
 }
 
+// This is called from the scanner backends (currently on the scanner thread).
+bool IsNewWiimote(const std::string& identifier)
+{
+  std::lock_guard<std::mutex> lk(s_known_ids_mutex);
+  return s_known_ids.count(identifier) == 0;
+}
+
 };  // end of namespace

Source/Core/Core/HW/WiimoteReal/WiimoteReal.h

@@ -31,6 +31,8 @@ public:
   // This needs to be called in derived destructors!
   void Shutdown();
 
+  virtual std::string GetId() const = 0;
+
   void ControlChannel(const u16 channel, const void* const data, const u32 size);
   void InterruptChannel(const u16 channel, const void* const data, const u32 size);
   void Update();
@@ -164,6 +166,7 @@ void ChangeWiimoteSource(unsigned int index, int source);
 
 bool IsValidDeviceName(const std::string& name);
 bool IsBalanceBoardName(const std::string& name);
+bool IsNewWiimote(const std::string& identifier);
 
 #ifdef ANDROID
 void InitAdapterClass();