From 2511bfdb8a9afa3eb916eed0184f578ab5c0f544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Thu, 11 Aug 2016 13:28:25 +0200 Subject: [PATCH] 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 | 3 +- Source/Core/Core/HW/WiimoteReal/IOLinux.cpp | 13 ---- Source/Core/Core/HW/WiimoteReal/IOLinux.h | 6 ++ Source/Core/Core/HW/WiimoteReal/IOWin.cpp | 59 ++++--------------- Source/Core/Core/HW/WiimoteReal/IOWin.h | 3 +- .../Core/HW/WiimoteReal/IOdarwin_private.h | 2 +- Source/Core/Core/HW/WiimoteReal/IOhidapi.cpp | 11 +--- Source/Core/Core/HW/WiimoteReal/IOhidapi.h | 2 +- .../Core/Core/HW/WiimoteReal/WiimoteReal.cpp | 18 ++++++ Source/Core/Core/HW/WiimoteReal/WiimoteReal.h | 3 + 10 files changed, 45 insertions(+), 75 deletions(-) diff --git a/Source/Core/Core/HW/WiimoteReal/IOAndroid.h b/Source/Core/Core/HW/WiimoteReal/IOAndroid.h index 988d8794ed..3f28e532ac 100644 --- a/Source/Core/Core/HW/WiimoteReal/IOAndroid.h +++ b/Source/Core/Core/HW/WiimoteReal/IOAndroid.h @@ -7,6 +7,7 @@ #ifdef ANDROID #include +#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; diff --git a/Source/Core/Core/HW/WiimoteReal/IOLinux.cpp b/Source/Core/Core/HW/WiimoteReal/IOLinux.cpp index f27c5428fd..573dba02b6 100644 --- a/Source/Core/Core/HW/WiimoteReal/IOLinux.cpp +++ b/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 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& 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 diff --git a/Source/Core/Core/HW/WiimoteReal/IOLinux.h b/Source/Core/Core/HW/WiimoteReal/IOLinux.h index 9f74e74a80..83929e6095 100644 --- a/Source/Core/Core/HW/WiimoteReal/IOLinux.h +++ b/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; diff --git a/Source/Core/Core/HW/WiimoteReal/IOWin.cpp b/Source/Core/Core/HW/WiimoteReal/IOWin.cpp index 8a0558e1d1..ab53232d5b 100644 --- a/Source/Core/Core/HW/WiimoteReal/IOWin.cpp +++ b/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 g_connect_times; -#ifdef SHARE_WRITE_WIIMOTES -std::unordered_set> 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& found_wiimotes, WinWriteMethod write_method = GetInitialWriteMethod(IsUsingToshibaStack); -#ifdef SHARE_WRITE_WIIMOTES - { - std::lock_guard 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 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 lk(g_connected_wiimotes_lock); - g_connected_wiimotes.erase(m_devicepath); -#endif } WiimoteWindows::WiimoteWindows(const std::basic_string& path, diff --git a/Source/Core/Core/HW/WiimoteReal/IOWin.h b/Source/Core/Core/HW/WiimoteReal/IOWin.h index ceda5085ad..e562cbd62c 100644 --- a/Source/Core/Core/HW/WiimoteReal/IOWin.h +++ b/Source/Core/Core/HW/WiimoteReal/IOWin.h @@ -7,6 +7,7 @@ #ifdef _WIN32 #include +#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& path, WinWriteMethod initial_write_method); ~WiimoteWindows() override; - + std::string GetId() const override { return UTF16ToUTF8(m_devicepath); } protected: bool ConnectInternal() override; void DisconnectInternal() override; diff --git a/Source/Core/Core/HW/WiimoteReal/IOdarwin_private.h b/Source/Core/Core/HW/WiimoteReal/IOdarwin_private.h index a49eb2befb..05b6503b48 100644 --- a/Source/Core/Core/HW/WiimoteReal/IOdarwin_private.h +++ b/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; diff --git a/Source/Core/Core/HW/WiimoteReal/IOhidapi.cpp b/Source/Core/Core/HW/WiimoteReal/IOhidapi.cpp index e615a69e35..35856fca8d 100644 --- a/Source/Core/Core/HW/WiimoteReal/IOhidapi.cpp +++ b/Source/Core/Core/HW/WiimoteReal/IOhidapi.cpp @@ -3,7 +3,6 @@ // Refer to the license.txt file included. #include -#include #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 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& 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& 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; } diff --git a/Source/Core/Core/HW/WiimoteReal/IOhidapi.h b/Source/Core/Core/HW/WiimoteReal/IOhidapi.h index 5cacf06ccf..9bc6f8a187 100644 --- a/Source/Core/Core/HW/WiimoteReal/IOhidapi.h +++ b/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; diff --git a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp index a184dd2a63..e455047a66 100644 --- a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp +++ b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #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 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()); g_wiimote_scanner.AddScannerBackend(std::make_unique()); g_wiimote_scanner.AddScannerBackend(std::make_unique()); @@ -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 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 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 lk(s_known_ids_mutex); + return s_known_ids.count(identifier) == 0; +} + }; // end of namespace diff --git a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h index f5b1cd1446..bf5d978a11 100644 --- a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h +++ b/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();