Merge pull request #13970 from jordan-woyak/wmreal-iowin-fixes

WiimoteReal: Windows improvements.

Source/Core/Common/WindowsDevice.cpp

@@ -7,7 +7,10 @@
 
 #include <string>
 
+#include "Hidclass.h"
+
 #include "Common/CommonFuncs.h"
+#include "Common/Logging/Log.h"
 
 namespace Common
 {
@@ -33,6 +36,52 @@ std::wstring GetDeviceProperty(const HDEVINFO& device_info, const PSP_DEVINFO_DA
 
   return std::wstring(unicode_buffer.data());
 }
+
+std::optional<std::wstring> GetPropertyHelper(auto function, auto dev,
+                                              const DEVPROPKEY* requested_property,
+                                              DEVPROPTYPE expected_type)
+{
+  DEVPROPTYPE type{};
+  ULONG buffer_size{};
+
+  if (const auto result = function(dev, requested_property, &type, nullptr, &buffer_size, 0);
+      result != CR_SUCCESS && result != CR_BUFFER_SMALL)
+  {
+    WARN_LOG_FMT(COMMON, "CM_Get_DevNode_Property returned: {}", result);
+    return std::nullopt;
+  }
+  if (type != expected_type)
+  {
+    WARN_LOG_FMT(COMMON, "CM_Get_DevNode_Property unexpected type: 0x{:x}", type);
+    return std::nullopt;
+  }
+
+  std::optional<std::wstring> property;
+  // FYI: It's legal to write the null terminator at data()[size()] of std::basic_string.
+  property.emplace(buffer_size / sizeof(WCHAR) - 1, L'\0');
+  if (const auto result = function(dev, requested_property, &type,
+                                   reinterpret_cast<BYTE*>(property->data()), &buffer_size, 0);
+      result != CR_SUCCESS)
+  {
+    ERROR_LOG_FMT(COMMON, "CM_Get_DevNode_Property returned: {}", result);
+    return std::nullopt;
+  }
+  return property;
+}
+
+std::optional<std::wstring> GetDevNodeStringProperty(DEVINST dev,
+                                                     const DEVPROPKEY* requested_property)
+{
+  return GetPropertyHelper(CM_Get_DevNode_Property, dev, requested_property, DEVPROP_TYPE_STRING);
+}
+
+std::optional<std::wstring> GetDeviceInterfaceStringProperty(LPCWSTR iface,
+                                                             const DEVPROPKEY* requested_property)
+{
+  return GetPropertyHelper(CM_Get_Device_Interface_Property, iface, requested_property,
+                           DEVPROP_TYPE_STRING);
+}
+
 }  // namespace Common
 
 #endif

Source/Core/Common/WindowsDevice.h

@@ -5,6 +5,7 @@
 
 #ifdef _WIN32
 
+#include <optional>
 #include <string>
 
 #ifndef WIN32_LEAN_AND_MEAN
@@ -24,6 +25,13 @@ namespace Common
 // Obtains a device property and returns it as a wide string.
 std::wstring GetDeviceProperty(const HANDLE& device_info, const PSP_DEVINFO_DATA device_data,
                                const DEVPROPKEY* requested_property);
+
+std::optional<std::wstring> GetDevNodeStringProperty(DEVINST device,
+                                                     const DEVPROPKEY* requested_property);
+
+std::optional<std::wstring> GetDeviceInterfaceStringProperty(LPCWSTR iface,
+                                                             const DEVPROPKEY* requested_property);
+
 }  // namespace Common
 
 #endif

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

@@ -6,26 +6,22 @@
 #ifdef _WIN32
 #include <windows.h>
 
-#include "Common/StringUtil.h"
-#include "Core/HW/WiimoteCommon/WiimoteHid.h"
+#include "Common/SocketContext.h"
 #include "Core/HW/WiimoteReal/WiimoteReal.h"
+#include "Core/USBUtils.h"
 
 namespace WiimoteReal
 {
-// Different methods to send data Wiimote on Windows depending on OS and Bluetooth Stack
-enum WinWriteMethod
-{
-  WWM_WRITE_FILE_LARGEST_REPORT_SIZE,
-  WWM_WRITE_FILE_ACTUAL_REPORT_SIZE,
-  WWM_SET_OUTPUT_REPORT
-};
+class WiimoteScannerWindows;
 
 class WiimoteWindows final : public Wiimote
 {
+  friend WiimoteScannerWindows;
+
 public:
-  WiimoteWindows(const std::basic_string<TCHAR>& path, WinWriteMethod initial_write_method);
+  WiimoteWindows(std::wstring hid_iface);
   ~WiimoteWindows() override;
-  std::string GetId() const override { return WStringToUTF8(m_devicepath); }
+  std::string GetId() const override;
 
 protected:
   bool ConnectInternal() override;
@@ -36,11 +32,19 @@ protected:
   int IOWrite(u8 const* buf, size_t len) override;
 
 private:
-  std::basic_string<TCHAR> m_devicepath;  // Unique Wiimote reference
-  HANDLE m_dev_handle;                    // HID handle
-  OVERLAPPED m_hid_overlap_read;          // Overlap handles
-  OVERLAPPED m_hid_overlap_write;
-  WinWriteMethod m_write_method;  // Type of Write Method to use
+  // These return 0 on error. -1 on no data.
+  int OverlappedRead(u8* data, DWORD size);
+  int OverlappedWrite(const u8* data, DWORD size);
+
+  const std::wstring m_hid_iface;
+
+  HANDLE m_dev_handle{INVALID_HANDLE_VALUE};
+  HANDLE m_wakeup_event{INVALID_HANDLE_VALUE};
+
+  OVERLAPPED m_hid_overlap_read{};
+  OVERLAPPED m_hid_overlap_write{};
+
+  std::atomic_bool m_is_connected{};
 };
 
 class WiimoteScannerWindows final : public WiimoteScannerBackend
@@ -49,8 +53,15 @@ public:
   WiimoteScannerWindows();
   bool IsReady() const override;
   void FindWiimotes(std::vector<Wiimote*>&, Wiimote*&) override;
+  void FindAttachedDevices(std::vector<Wiimote*>&, Wiimote*&) override;
   void Update() override;
   void RequestStopSearching() override {}
+
+  static void FindAndAuthenticateWiimotes();
+  static void RemoveRememberedWiimotes();
+
+private:
+  void FindWiimoteHIDDevices(std::vector<Wiimote*>&, Wiimote*&);
 };
 }  // namespace WiimoteReal
 

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

@@ -168,8 +168,6 @@ void Wiimote::Shutdown()
 
   StopThread();
   ClearReadQueue();
-
-  NOTICE_LOG_FMT(WIIMOTE, "Disconnected real wiimote.");
 }
 
 // to be called from CPU thread
@@ -209,8 +207,8 @@ void Wiimote::WriteReport(Report rpt)
   const auto report_time =
       Core::IsCPUThread() ? core_timing.GetTargetHostTime(core_timing.GetTicks()) : Clock::now();
 
-  m_write_thread.EmplaceItem(report_time, std::move(rpt));
-  IOWakeup();
+  m_write_reports.Emplace(report_time, std::move(rpt));
+  m_write_event.Set();
 }
 
 // to be called from CPU thread
@@ -512,11 +510,13 @@ void Wiimote::Prepare()
   // Set reporting mode to non-continuous core buttons and turn on rumble.
   Report mode_report = {WR_SET_REPORT | BT_OUTPUT, u8(OutputReportID::ReportMode), 1,
                         u8(InputReportID::ReportCore)};
-  m_write_thread.EmplaceItem(now, std::move(mode_report));
+  m_write_reports.Emplace(now, std::move(mode_report));
 
   // Request status and turn off rumble.
   Report req_status_report = {WR_SET_REPORT | BT_OUTPUT, u8(OutputReportID::RequestStatus), 0};
-  m_write_thread.EmplaceItem(now + std::chrono::milliseconds{200}, std::move(req_status_report));
+  m_write_reports.Emplace(now + std::chrono::milliseconds{200}, std::move(req_status_report));
+
+  m_write_event.Set();
 }
 
 void Wiimote::EmuStop()
@@ -689,15 +689,12 @@ void WiimoteScanner::ThreadFunc()
       g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); });
     }
 
-    // Does stuff needed to detect disconnects on Windows
+    // Currently does nothing. To be removed.
     for (const auto& backend : m_backends)
       backend->Update();
 
     CheckForDisconnectedWiimotes();
 
-    if (m_scan_mode.load() == WiimoteScanMode::DO_NOT_SCAN)
-      continue;
-
     // If we don't want Wiimotes in ControllerInterface, we may not need them at all.
     if (!Config::Get(Config::MAIN_CONNECT_WIIMOTES_FOR_CONTROLLER_INTERFACE))
     {
@@ -715,11 +712,22 @@ void WiimoteScanner::ThreadFunc()
         continue;
     }
 
+    // Stop scanning if not in continuous mode.
+    auto scan_mode = WiimoteScanMode::SCAN_ONCE;
+    m_scan_mode.compare_exchange_strong(scan_mode, WiimoteScanMode::DO_NOT_SCAN);
+
     for (const auto& backend : m_backends)
     {
       std::vector<Wiimote*> found_wiimotes;
       Wiimote* found_board = nullptr;
-      backend->FindWiimotes(found_wiimotes, found_board);
+
+      // When not scanning we still look for already attached devices.
+      // This allows Windows and DolphinBar remotes to be quickly discovered.
+      if (scan_mode == WiimoteScanMode::DO_NOT_SCAN)
+        backend->FindAttachedDevices(found_wiimotes, found_board);
+      else
+        backend->FindWiimotes(found_wiimotes, found_board);
+
       {
         std::unique_lock wm_lk(g_wiimotes_mutex);
 
@@ -745,10 +753,6 @@ void WiimoteScanner::ThreadFunc()
         }
       }
     }
-
-    // Stop scanning if not in continuous mode.
-    auto scan_mode = WiimoteScanMode::SCAN_ONCE;
-    m_scan_mode.compare_exchange_strong(scan_mode, WiimoteScanMode::DO_NOT_SCAN);
   }
 
   {
@@ -764,38 +768,54 @@ void WiimoteScanner::ThreadFunc()
 bool Wiimote::Connect(int index)
 {
   m_index = index;
-
-  if (!m_run_thread.IsSet())
-  {
-    m_run_thread.Set();
-    StartThread();
-    m_thread_ready_event.Wait();
-  }
-
+  StartThread();
   return IsConnected();
 }
 
 void Wiimote::StartThread()
 {
-  // Note that the read thread starts the writing worker thread.
-  m_read_thread = std::thread(&Wiimote::ReadThreadFunc, this);
+  if (m_write_thread.joinable())
+    return;
+
+  m_run_thread.Set();
+  // Note that the write thread starts the read thread.
+  m_write_thread = std::thread(&Wiimote::WriteThreadFunc, this);
+
+  m_thread_ready_event.Wait();
 }
 
 void Wiimote::StopThread()
 {
-  if (!m_run_thread.TestAndClear())
+  if (!m_write_thread.joinable())
     return;
 
-  IOWakeup();
+  m_run_thread.Clear();
+  m_write_event.Set();
 
-  // Note that the read thread stops the writing worker thread.
-  m_read_thread.join();
+  // Note that the write thread stops the read thread.
+  m_write_thread.join();
 }
 
 void Wiimote::ReadThreadFunc()
 {
   Common::SetCurrentThreadName("Wiimote Read Thread");
 
+  while (m_run_thread.IsSet())
+  {
+    if (!Read())
+    {
+      WARN_LOG_FMT(WIIMOTE, "Wiimote::Read failed on Wiimote {}.", m_index + 1);
+      m_run_thread.Clear();
+      m_write_event.Set();
+      break;
+    }
+  }
+}
+
+void Wiimote::WriteThreadFunc()
+{
+  Common::SetCurrentThreadName("Wiimote Write Thread");
+
   bool ok = ConnectInternal();
 
   if (!ok)
@@ -812,19 +832,55 @@ void Wiimote::ReadThreadFunc()
     return;
   }
 
-  m_write_thread.Reset("Wiimote Write Thread", std::bind_front(&Wiimote::Write, this));
+  std::thread read_thread{&Wiimote::ReadThreadFunc, this};
+
+  // Windows and also the DolphinBar require performing a write
+  //  to detect disconnections in a timely manner
+  // If we haven't written a report in some time, attempt a rumble-off report.
+  // This also has a minor benefit of preventing rumble from being stuck on.
+  constexpr auto WRITE_TEST_INTERVAL = std::chrono::milliseconds{1000};
+
+  TimePoint last_write_time = Clock::now();
 
   while (m_run_thread.IsSet())
   {
-    if (!Read())
+    bool write_success = false;
+
+    if (!m_write_reports.Empty())
     {
-      ERROR_LOG_FMT(WIIMOTE, "Wiimote::Read failed. Disconnecting Wiimote {}.", m_index + 1);
+      // Send a normal report.
+      write_success = Write(m_write_reports.Front());
+      m_write_reports.Pop();
+    }
+    else if (Clock::now() - last_write_time >= WRITE_TEST_INTERVAL)
+    {
+      // We haven't written in a while, test a write so we can check for a disconnect.
+      DEBUG_LOG_FMT(WIIMOTE, "Sending periodic write test for Wiimote {}.", m_index + 1);
+      const u8 rumble_off[] = {WR_SET_REPORT | BT_OUTPUT, u8(OutputReportID::Rumble), 0x00};
+
+      write_success = IOWrite(std::data(rumble_off), std::size(rumble_off)) > 0;
+    }
+    else
+    {
+      // Nothing to do. Wait a while for a kick.
+      m_write_event.WaitFor(WRITE_TEST_INTERVAL);
+      continue;
+    }
+
+    last_write_time = Clock::now();
+
+    if (!write_success)
+    {
+      WARN_LOG_FMT(WIIMOTE, "Wiimote::Write failed on Wiimote {}.", m_index + 1);
+      m_run_thread.Clear();
       break;
     }
   }
 
-  m_write_thread.StopAndCancel();
+  IOWakeup();
+  read_thread.join();
 
+  NOTICE_LOG_FMT(WIIMOTE, "Disconnecting Wiimote {}.", m_index + 1);
   DisconnectInternal();
 }
 
@@ -965,13 +1021,17 @@ void Refresh()
     s_wiimote_scanner.SetScanMode(WiimoteScanMode::SCAN_ONCE);
 }
 
-bool IsValidDeviceName(const std::string& name)
+bool IsValidDeviceName(std::string_view name)
 {
-  return "Nintendo RVL-CNT-01" == name || "Nintendo RVL-CNT-01-TR" == name ||
-         IsBalanceBoardName(name);
+  return IsWiimoteName(name) || IsBalanceBoardName(name);
 }
 
-bool IsBalanceBoardName(const std::string& name)
+bool IsWiimoteName(std::string_view name)
+{
+  return name == "Nintendo RVL-CNT-01" || name == "Nintendo RVL-CNT-01-TR";
+}
+
+bool IsBalanceBoardName(std::string_view name)
 {
   return "Nintendo RVL-WBC-01" == name;
 }

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

@@ -7,6 +7,7 @@
 #include <memory>
 #include <mutex>
 #include <string>
+#include <string_view>
 #include <thread>
 #include <vector>
 
@@ -145,6 +146,7 @@ private:
   virtual void IOWakeup() = 0;
 
   void ReadThreadFunc();
+  void WriteThreadFunc();
 
   void RefreshConfig();
 
@@ -157,14 +159,16 @@ private:
   // And we track the rumble state to drop unnecessary rumble reports.
   bool m_rumble_state = false;
 
-  std::thread m_read_thread;
+  std::thread m_write_thread;
   // Whether to keep running the thread.
   Common::Flag m_run_thread;
   // Triggered when the thread has finished ConnectInternal.
   Common::Event m_thread_ready_event;
 
   Common::SPSCQueue<Report> m_read_reports;
-  Common::WorkQueueThreadSP<TimedReport> m_write_thread;
+  Common::SPSCQueue<TimedReport> m_write_reports;
+  // Kick the write thread.
+  Common::Event m_write_event;
 
   bool m_speaker_enabled_in_dolphin_config = false;
   int m_balance_board_dump_port = 0;
@@ -185,6 +189,10 @@ public:
   virtual void Update() = 0;
   // requests the backend to stop scanning if FindWiimotes is blocking
   virtual void RequestStopSearching() = 0;
+
+  // Used by Windows to search for HID interfaces of already connected Wii remotes.
+  // hidapi should probably implement the equivalent.
+  virtual void FindAttachedDevices(std::vector<Wiimote*>&, Wiimote*&) {}
 };
 
 enum class WiimoteScanMode
@@ -225,8 +233,10 @@ extern std::unique_ptr<Wiimote> g_wiimotes[MAX_BBMOTES];
 
 void AddWiimoteToPool(std::unique_ptr<Wiimote>);
 
-bool IsValidDeviceName(const std::string& name);
-bool IsBalanceBoardName(const std::string& name);
+bool IsValidDeviceName(std::string_view name);
+bool IsWiimoteName(std::string_view name);
+bool IsBalanceBoardName(std::string_view name);
+
 bool IsNewWiimote(const std::string& identifier);
 
 bool IsKnownDeviceId(const USBUtils::DeviceInfo&);

Source/Core/DolphinQt/Config/WiimoteControllersWidget.cpp

@@ -12,6 +12,8 @@
 #include <QPushButton>
 #include <QRadioButton>
 #include <QScreen>
+#include <QTimer>
+#include <QToolButton>
 #include <QVBoxLayout>
 #include <QVariant>
 
@@ -39,6 +41,10 @@
 #include "DolphinQt/Settings.h"
 #include "DolphinQt/Settings/USBDevicePicker.h"
 
+#if defined(_WIN32)
+#include "Core/HW//WiimoteReal/IOWin.h"
+#endif
+
 WiimoteControllersWidget::WiimoteControllersWidget(QWidget* parent) : QWidget(parent)
 {
   CreateLayout();
@@ -191,7 +197,31 @@ void WiimoteControllersWidget::CreateLayout()
   m_bluetooth_adapters_refresh = new NonDefaultQPushButton(tr("Refresh"));
   m_wiimote_sync = new NonDefaultQPushButton(tr("Sync"));
   m_wiimote_reset = new NonDefaultQPushButton(tr("Reset"));
-  m_wiimote_refresh = new NonDefaultQPushButton(tr("Refresh"));
+
+  m_wiimote_refresh_indicator = new QLabel{};
+  m_wiimote_refresh_indicator->hide();
+  m_wiimote_refresh = new QToolButton();
+  auto* const wiimote_refresh_action = new QAction(tr("Refresh"), m_wiimote_refresh);
+  m_wiimote_refresh->setDefaultAction(wiimote_refresh_action);
+  connect(wiimote_refresh_action, &QAction::triggered, this,
+          &WiimoteControllersWidget::OnWiimoteRefreshPressed);
+  m_wiimote_refresh->setPopupMode(QToolButton::ToolButtonPopupMode::MenuButtonPopup);
+
+#if defined(_WIN32)
+  m_wiimote_refresh_indicator->setPixmap(
+      style()->standardIcon(QStyle::SP_BrowserReload).pixmap(16, 16));
+
+  auto* const wiimote_sync_action = new QAction(tr("Sync"), m_wiimote_refresh);
+  m_wiimote_refresh->addAction(wiimote_sync_action);
+  connect(wiimote_sync_action, &QAction::triggered, this,
+          &WiimoteControllersWidget::TriggerHostWiimoteSync);
+
+  auto* const wiimote_reset_action = new QAction(tr("Reset"), m_wiimote_refresh);
+  m_wiimote_refresh->addAction(wiimote_reset_action);
+  connect(wiimote_reset_action, &QAction::triggered, this,
+          &WiimoteControllersWidget::TriggerHostWiimoteReset);
+#endif
+
   m_wiimote_pt_labels[0] = new QLabel(tr("Sync real Wii Remotes and pair them"));
   m_wiimote_pt_labels[1] = new QLabel(tr("Reset all saved Wii Remote pairings"));
   m_wiimote_emu = new QRadioButton(tr("Emulate the Wii's Bluetooth adapter"));
@@ -245,7 +275,13 @@ void WiimoteControllersWidget::CreateLayout()
   m_wiimote_layout->addWidget(m_wiimote_ciface, m_wiimote_layout->rowCount(), 0, 1, -1);
 
   int continuous_scanning_row = m_wiimote_layout->rowCount();
-  m_wiimote_layout->addWidget(m_wiimote_continuous_scanning, continuous_scanning_row, 0, 1, 3);
+
+  auto* const left_of_refresh_button_layout = new QHBoxLayout;
+  left_of_refresh_button_layout->addWidget(m_wiimote_continuous_scanning);
+  left_of_refresh_button_layout->addStretch(1);
+  left_of_refresh_button_layout->addWidget(m_wiimote_refresh_indicator);
+
+  m_wiimote_layout->addLayout(left_of_refresh_button_layout, continuous_scanning_row, 0, 1, 3);
   m_wiimote_layout->addWidget(m_wiimote_refresh, continuous_scanning_row, 3);
 
   m_bluetooth_unavailable = new QLabel(tr("A supported Bluetooth device could not be found.\n"
@@ -287,8 +323,6 @@ void WiimoteControllersWidget::ConnectWidgets()
           &WiimoteControllersWidget::OnBluetoothPassthroughSyncPressed);
   connect(m_wiimote_reset, &QPushButton::clicked, this,
           &WiimoteControllersWidget::OnBluetoothPassthroughResetPressed);
-  connect(m_wiimote_refresh, &QPushButton::clicked, this,
-          &WiimoteControllersWidget::OnWiimoteRefreshPressed);
 
   for (size_t i = 0; i < m_wiimote_groups.size(); i++)
   {
@@ -503,3 +537,40 @@ void WiimoteControllersWidget::SaveSettings()
 
   SConfig::GetInstance().SaveSettings();
 }
+
+#if defined(_WIN32)
+void WiimoteControllersWidget::AsyncRefreshActionHelper(std::invocable<> auto func)
+{
+  m_wiimote_refresh->setEnabled(false);
+  m_wiimote_refresh_indicator->show();
+
+  auto result = std::async(std::launch::async, std::move(func));
+
+  auto* const animation = new QTimer{this};
+  connect(animation, &QTimer::timeout, this, [this, animation, result = std::move(result)] {
+    // Spin the refresh indicator.
+    m_wiimote_refresh_indicator->setPixmap(
+        m_wiimote_refresh_indicator->pixmap().transformed(QTransform().rotate(90)));
+
+    if (result.wait_for(std::chrono::seconds{}) != std::future_status::ready)
+      return;
+
+    // When the async task is done, re-enable the button and hide the indicator.
+    animation->deleteLater();
+    m_wiimote_refresh_indicator->hide();
+    m_wiimote_refresh->setEnabled(true);
+  });
+
+  animation->start(250);
+}
+
+void WiimoteControllersWidget::TriggerHostWiimoteSync()
+{
+  AsyncRefreshActionHelper(WiimoteReal::WiimoteScannerWindows::FindAndAuthenticateWiimotes);
+}
+
+void WiimoteControllersWidget::TriggerHostWiimoteReset()
+{
+  AsyncRefreshActionHelper(WiimoteReal::WiimoteScannerWindows::RemoveRememberedWiimotes);
+}
+#endif

Source/Core/DolphinQt/Config/WiimoteControllersWidget.h

@@ -10,10 +10,12 @@
 #include "Common/WorkQueueThread.h"
 #include "Core/USBUtils.h"
 
+class QAction;
 class QCheckBox;
 class QComboBox;
 class QHBoxLayout;
 class QGridLayout;
+class QToolButton;
 class QGroupBox;
 class QLabel;
 class QPushButton;
@@ -48,6 +50,12 @@ private:
   void ConnectWidgets();
   void LoadSettings(Core::State state);
 
+#if defined(_WIN32)
+  void AsyncRefreshActionHelper(std::invocable<> auto);
+  void TriggerHostWiimoteSync();
+  void TriggerHostWiimoteReset();
+#endif
+
   QGroupBox* m_wiimote_box;
   QGridLayout* m_wiimote_layout;
   std::array<QLabel*, 4> m_wiimote_labels;
@@ -70,6 +78,7 @@ private:
   QCheckBox* m_wiimote_real_balance_board;
   QCheckBox* m_wiimote_speaker_data;
   QCheckBox* m_wiimote_ciface;
-  QPushButton* m_wiimote_refresh;
+  QToolButton* m_wiimote_refresh;
+  QLabel* m_wiimote_refresh_indicator;
   QLabel* m_bluetooth_unavailable;
 };