From e0214b1a38aaa3cf2f796118fcd700b00587e423 Mon Sep 17 00:00:00 2001 From: comex Date: Wed, 4 Sep 2013 01:03:44 -0400 Subject: [PATCH 1/7] Fix syncing wiimotes on OS X. IOdarwin.mm was assuming that scanning was complete when the run loop was stopped (which the scan callback does), but somebody else was stopping the run loop first, causing the scan to be aborted. Wait until the scan is actually complete. --- Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm index 6e399f16ac..f0d285b274 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm @@ -7,6 +7,7 @@ @interface SearchBT: NSObject { @public unsigned int maxDevices; + bool done; } @end @@ -15,6 +16,7 @@ error: (IOReturn) error aborted: (BOOL) aborted { + done = true; CFRunLoopStop(CFRunLoopGetCurrent()); } @@ -144,9 +146,12 @@ void WiimoteScanner::FindWiimotes(std::vector & found_wiimotes, Wiimot else ERROR_LOG(WIIMOTE, "Unable to do bluetooth discovery"); - CFRunLoopRun(); + do + { + CFRunLoopRun(); + } + while(!sbt->done); - [bti stop]; int found_devices = [[bti foundDevices] count]; if (found_devices) From 0e949afa57c6a7e4532d4d160bd4289decf96e8c Mon Sep 17 00:00:00 2001 From: comex Date: Wed, 4 Sep 2013 01:11:04 -0400 Subject: [PATCH 2/7] Remove dubious retain on OS X. Revert this if the claimed crash actually shows up - or better, figure out the actual cause. --- Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm index f0d285b274..8596b12080 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm @@ -210,11 +210,6 @@ bool Wiimote::Connect() return false; } - // As of 10.8 these need explicit retaining or writing to the wiimote has a very high - // chance of crashing and burning. - [ichan retain]; - [cchan retain]; - NOTICE_LOG(WIIMOTE, "Connected to wiimote %i at %s", index + 1, [[btd addressString] UTF8String]); @@ -227,15 +222,15 @@ bool Wiimote::Connect() // Disconnect a wiimote. void Wiimote::Disconnect() { - if (btd != NULL) - [btd closeConnection]; - if (ichan != NULL) [ichan release]; if (cchan != NULL) [cchan release]; + if (btd != NULL) + [btd closeConnection]; + btd = NULL; cchan = NULL; ichan = NULL; From 906de748bd00a09486d2a16b337751c55ebac8f9 Mon Sep 17 00:00:00 2001 From: comex Date: Wed, 4 Sep 2013 03:10:09 -0400 Subject: [PATCH 3/7] Refactor thread handling to fix OS X bug. On OS X, openL2CAPChannelSync registers events on the current thread's run loop, so Connect needs to be called on a thread that's going to do CFRunLoopRun; this was causing all Wiimote input to be ignored. Easiest way to do that is to use the Wiimote thread, and have Read call CFRunLoopRun to block on events, bringing OS X's Wiimote event loop in line with every other platform's. This also means that the thread can't be stopped and recreated by Prepare, so make Prepare notify it instead, which has the side effect of not making the GUI block on Prepare. (It would be nice if the GUI also did not block on searching for devices, because blocking the GUI is gross, but for now...) --- .../Core/Core/Src/HW/WiimoteReal/IODummy.cpp | 4 +- Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp | 7 +- Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp | 4 +- .../Core/Core/Src/HW/WiimoteReal/IOdarwin.mm | 27 ++- .../Core/Src/HW/WiimoteReal/WiimoteReal.cpp | 163 ++++++++++-------- .../Core/Src/HW/WiimoteReal/WiimoteReal.h | 22 ++- 6 files changed, 128 insertions(+), 99 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp index 59025f9040..870d281326 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp @@ -43,12 +43,12 @@ bool WiimoteScanner::IsReady() const return false; } -bool Wiimote::Connect() +bool Wiimote::ConnectInternal() { return 0; } -void Wiimote::Disconnect() +void Wiimote::DisconnectInternal() { return; } diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp index edd79f2bf6..014eadc9a8 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp @@ -27,8 +27,7 @@ namespace WiimoteReal { WiimoteScanner::WiimoteScanner() - : m_run_thread() - , m_want_wiimotes() + : m_want_wiimotes() , device_id(-1) , device_sock(-1) { @@ -135,7 +134,7 @@ void WiimoteScanner::FindWiimotes(std::vector & found_wiimotes, Wiimot } // Connect to a wiimote with a known address. -bool Wiimote::Connect() +bool Wiimote::ConnectInternal() { sockaddr_l2 addr; addr.l2_family = AF_BLUETOOTH; @@ -168,7 +167,7 @@ bool Wiimote::Connect() return true; } -void Wiimote::Disconnect() +void Wiimote::DisconnectInternal() { close(cmd_sock); close(int_sock); diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp index 5d4d26108f..866284b323 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp @@ -459,7 +459,7 @@ bool WiimoteScanner::IsReady() const } // Connect to a wiimote with a known device path. -bool Wiimote::Connect() +bool Wiimote::ConnectInternal() { if (IsConnected()) return false; @@ -535,7 +535,7 @@ bool Wiimote::Connect() return true; } -void Wiimote::Disconnect() +void Wiimote::DisconnectInternal() { if (!IsConnected()) return; diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm index 8596b12080..17e867bc7a 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm @@ -64,7 +64,7 @@ return; } - if (wm->inputlen != 0) { + if (wm->inputlen != -1) { WARN_LOG(WIIMOTE, "Dropping packet for wiimote %i, queue full", wm->index + 1); return; @@ -73,9 +73,8 @@ memcpy(wm->input, data, length); wm->inputlen = length; - (void)wm->Read(); - (void)UpdateSystemActivity(UsrActivity); + CFRunLoopStop(CFRunLoopGetCurrent()); } - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel @@ -100,7 +99,7 @@ WARN_LOG(WIIMOTE, "Lost channel to wiimote %i", wm->index + 1); - wm->Disconnect(); + wm->DisconnectInternal(); } @end @@ -189,7 +188,7 @@ bool WiimoteScanner::IsReady() const } // Connect to a wiimote with a known address. -bool Wiimote::Connect() +bool Wiimote::ConnectInternal() { if (IsConnected()) return false; @@ -204,12 +203,12 @@ bool Wiimote::Connect() { ERROR_LOG(WIIMOTE, "Unable to open L2CAP channels " "for wiimote %i", index + 1); - Disconnect(); + DisconnectInternal(); [cbt release]; return false; } - + NOTICE_LOG(WIIMOTE, "Connected to wiimote %i at %s", index + 1, [[btd addressString] UTF8String]); @@ -220,7 +219,7 @@ bool Wiimote::Connect() } // Disconnect a wiimote. -void Wiimote::Disconnect() +void Wiimote::DisconnectInternal() { if (ichan != NULL) [ichan release]; @@ -250,16 +249,12 @@ bool Wiimote::IsConnected() const int Wiimote::IORead(unsigned char *buf) { - int bytes; + input = buf; + inputlen = -1; - if (!IsConnected()) - return 0; + CFRunLoopRun(); - bytes = inputlen; - memcpy(buf, input, bytes); - inputlen = 0; - - return bytes; + return inputlen; } int Wiimote::IOWrite(const unsigned char *buf, int len) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp index d2e7e2f57a..aead9d60fa 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp @@ -40,7 +40,7 @@ WiimoteScanner g_wiimote_scanner; Wiimote::Wiimote() : index() #ifdef __APPLE__ - , btd(), ichan(), cchan(), inputlen(), m_connected() + , btd(), ichan(), cchan(), input(), inputlen(), m_connected() #elif defined(__linux__) && HAVE_BLUEZ , cmd_sock(-1), int_sock(-1) #elif defined(_WIN32) @@ -49,7 +49,7 @@ Wiimote::Wiimote() , m_last_input_report() , m_channel(0) , m_rumble_state() - , m_run_thread(false) + , m_need_prepare() { #if defined(__linux__) && HAVE_BLUEZ bdaddr = (bdaddr_t){{0, 0, 0, 0, 0, 0}}; @@ -60,9 +60,6 @@ Wiimote::~Wiimote() { StopThread(); - if (IsConnected()) - Disconnect(); - ClearReadQueue(); m_write_reports.Clear(); } @@ -225,8 +222,8 @@ bool Wiimote::Read() } else if (0 == result) { - WARN_LOG(WIIMOTE, "Wiimote::IORead failed. Disconnecting Wiimote %d.", index + 1); - Disconnect(); + ERROR_LOG(WIIMOTE, "Wiimote::IORead failed. Disconnecting Wiimote %d.", index + 1); + DisconnectInternal(); } return false; @@ -308,27 +305,31 @@ void Wiimote::Update() rpt.data(), rpt.size()); } -bool Wiimote::Prepare(int _index) +void Wiimote::Prepare(int _index) { index = _index; + m_need_prepare = true; +} +bool Wiimote::PrepareOnThread() +{ // core buttons, no continuous reporting - u8 const mode_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_REPORT_MODE, 0, WM_REPORT_CORE}; + u8 static const mode_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_REPORT_MODE, 0, WM_REPORT_CORE}; // Set the active LEDs and turn on rumble. - u8 const led_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_LEDS, u8(WIIMOTE_LED_1 << (index%WIIMOTE_BALANCE_BOARD) | 0x1)}; + u8 static const led_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_LEDS, u8(WIIMOTE_LED_1 << (index%WIIMOTE_BALANCE_BOARD) | 0x1)}; // Turn off rumble - u8 rumble_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_RUMBLE, 0}; + u8 static const rumble_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_RUMBLE, 0}; // Request status report - u8 const req_status_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_REQUEST_STATUS, 0}; + u8 static const req_status_report[] = {WM_SET_REPORT | WM_BT_OUTPUT, WM_REQUEST_STATUS, 0}; // TODO: check for sane response? return (IOWrite(mode_report, sizeof(mode_report)) - && IOWrite(led_report, sizeof(led_report)) - && (SLEEP(200), IOWrite(rumble_report, sizeof(rumble_report))) - && IOWrite(req_status_report, sizeof(req_status_report))); + && IOWrite(led_report, sizeof(led_report)) + && (SLEEP(200), IOWrite(rumble_report, sizeof(rumble_report))) + && IOWrite(req_status_report, sizeof(req_status_report))); } void Wiimote::EmuStart() @@ -480,35 +481,75 @@ void WiimoteScanner::ThreadFunc() NOTICE_LOG(WIIMOTE, "Wiimote scanning has stopped."); } +bool Wiimote::Connect() +{ + m_thread_ready = false; + StartThread(); + WaitReady(); + return IsConnected(); +} + void Wiimote::StartThread() { - m_run_thread = true; m_wiimote_thread = std::thread(std::mem_fun(&Wiimote::ThreadFunc), this); } void Wiimote::StopThread() { - m_run_thread = false; if (m_wiimote_thread.joinable()) m_wiimote_thread.join(); } +void Wiimote::SetReady() +{ + if (!m_thread_ready) + { + { + std::lock_guard Guard(m_thread_ready_mutex); + m_thread_ready = true; + } + m_thread_ready_cond.notify_all(); + } +} + +void Wiimote::WaitReady() +{ + std::unique_lock lock(m_thread_ready_mutex); + while (!m_thread_ready) + { + m_thread_ready_cond.wait(lock); + } +} + void Wiimote::ThreadFunc() { Common::SetCurrentThreadName("Wiimote Device Thread"); + bool ok = ConnectInternal(); + + SetReady(); + + if (!ok) + { + return; + } // main loop - while (m_run_thread && IsConnected()) + while (IsConnected()) { -#ifdef __APPLE__ - // Reading happens elsewhere on OSX - bool const did_something = Write(); -#else - bool const did_something = Write() || Read(); -#endif - if (!did_something) - Common::SleepCurrentThread(1); + if (m_need_prepare) + { + m_need_prepare = false; + if (!PrepareOnThread()) + { + ERROR_LOG(WIIMOTE, "Wiimote::PrepareOnThread failed. Disconnecting Wiimote %d.", index + 1); + DisconnectInternal(); + } + } + Write(); + Read(); } + + DisconnectInternal(); } void LoadSettings() @@ -629,24 +670,32 @@ void ChangeWiimoteSource(unsigned int index, int source) Host_ConnectWiimote(index, true); } +static bool TryToConnectWiimoteN(Wiimote* wm, unsigned int i) +{ + if (WIIMOTE_SRC_REAL & g_wiimote_sources[i] + && !g_wiimotes[i]) + { + if (wm->Connect()) + { + wm->Prepare(i); + NOTICE_LOG(WIIMOTE, "Connected to Wiimote %i.", i + 1); + g_wiimotes[i] = wm; + Host_ConnectWiimote(i, true); + } + return true; + } + return false; +} + void TryToConnectWiimote(Wiimote* wm) { std::unique_lock lk(g_refresh_lock); for (unsigned int i = 0; i < MAX_WIIMOTES; ++i) { - if (WIIMOTE_SRC_REAL & g_wiimote_sources[i] - && !g_wiimotes[i]) + if (TryToConnectWiimoteN(wm, i)) { - if (wm->Connect() && wm->Prepare(i)) - { - NOTICE_LOG(WIIMOTE, "Connected to Wiimote %i.", i + 1); - - std::swap(g_wiimotes[i], wm); - g_wiimotes[i]->StartThread(); - - Host_ConnectWiimote(i, true); - } + wm = NULL; break; } } @@ -661,19 +710,10 @@ void TryToConnectWiimote(Wiimote* wm) void TryToConnectBalanceBoard(Wiimote* wm) { std::unique_lock lk(g_refresh_lock); - - if (WIIMOTE_SRC_REAL & g_wiimote_sources[WIIMOTE_BALANCE_BOARD] - && !g_wiimotes[WIIMOTE_BALANCE_BOARD]) + + if (TryToConnectWiimoteN(wm, WIIMOTE_BALANCE_BOARD)) { - if (wm->Connect() && wm->Prepare(WIIMOTE_BALANCE_BOARD)) - { - NOTICE_LOG(WIIMOTE, "Connected to Balance Board %i.", WIIMOTE_BALANCE_BOARD + 1); - - std::swap(g_wiimotes[WIIMOTE_BALANCE_BOARD], wm); - g_wiimotes[WIIMOTE_BALANCE_BOARD]->StartThread(); - - Host_ConnectWiimote(WIIMOTE_BALANCE_BOARD, true); - } + wm = NULL; } g_wiimote_scanner.WantBB(0 != CalculateWantedBB()); @@ -687,26 +727,13 @@ void DoneWithWiimote(int index) { std::lock_guard lk(g_refresh_lock); - if (g_wiimotes[index]) + Wiimote* wm = g_wiimotes[index]; + + if (wm) { - g_wiimotes[index]->StopThread(); - + g_wiimotes[index] = NULL; // First see if we can use this real Wiimote in another slot. - for (unsigned int i = 0; i < MAX_WIIMOTES; ++i) - { - if (WIIMOTE_SRC_REAL & g_wiimote_sources[i] - && !g_wiimotes[i]) - { - if (g_wiimotes[index]->Prepare(i)) - { - std::swap(g_wiimotes[i], g_wiimotes[index]); - g_wiimotes[i]->StartThread(); - - Host_ConnectWiimote(i, true); - } - break; - } - } + TryToConnectWiimote(wm); } // else, just disconnect the Wiimote @@ -763,9 +790,7 @@ void Refresh() { if (g_wiimotes[i]) { - g_wiimotes[i]->StopThread(); g_wiimotes[i]->Prepare(i); - g_wiimotes[i]->StartThread(); } } diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h index 2ab4e435e2..e48567acbb 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h @@ -52,13 +52,17 @@ public: // connecting and disconnecting from physical devices // (using address inserted by FindWiimotes) + // these are called from the wiimote's thread. + bool ConnectInternal(); + void DisconnectInternal(); + bool Connect(); - void Disconnect(); // TODO: change to something like IsRelevant bool IsConnected() const; - bool Prepare(int index); + void Prepare(int index); + bool PrepareOnThread(); void DisableDataReporting(); void EnableDataReporting(u8 mode); @@ -72,9 +76,10 @@ public: IOBluetoothDevice *btd; IOBluetoothL2CAPChannel *ichan; IOBluetoothL2CAPChannel *cchan; - char input[MAX_PAYLOAD]; + unsigned char* input; int inputlen; bool m_connected; + CFRunLoopRef m_wiimote_thread_run_loop; #elif defined(__linux__) && HAVE_BLUEZ bdaddr_t bdaddr; // Bluetooth address int cmd_sock; // Command socket @@ -100,12 +105,17 @@ private: int IOWrite(u8 const* buf, int len); void ThreadFunc(); + void SetReady(); + void WaitReady(); bool m_rumble_state; - bool m_run_thread; - std::thread m_wiimote_thread; - + std::thread m_wiimote_thread; + volatile bool m_thread_ready; + volatile bool m_need_prepare; + std::mutex m_thread_ready_mutex; + std::condition_variable m_thread_ready_cond; + Common::FifoQueue m_read_reports; Common::FifoQueue m_write_reports; From 02fc68ea5d1466998c7d59b2882a5f9e348de24b Mon Sep 17 00:00:00 2001 From: comex Date: Wed, 4 Sep 2013 03:21:38 -0400 Subject: [PATCH 4/7] While we're at it, explicitly wake up the Wiimote thread rather than using a 1s timeout. This only matters if reads are not constantly being completed by reports anyway, but seems like a good idea. --- .../Core/Core/Src/HW/WiimoteReal/IODummy.cpp | 3 +++ Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp | 25 ++++++++++++++++--- Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp | 20 ++++++++++++--- .../Core/Core/Src/HW/WiimoteReal/IOdarwin.mm | 5 ++++ .../Core/Src/HW/WiimoteReal/WiimoteReal.cpp | 14 ++++++++++- .../Core/Src/HW/WiimoteReal/WiimoteReal.h | 2 ++ 6 files changed, 60 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp index 870d281326..e1ca6d603c 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp @@ -58,6 +58,9 @@ bool Wiimote::IsConnected() const return false; } +void Wiimote::IOWakeup() +{} + int Wiimote::IORead(u8* buf) { return 0; diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp index 014eadc9a8..826b9e67ce 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp @@ -181,26 +181,43 @@ bool Wiimote::IsConnected() const return cmd_sock != -1;// && int_sock != -1; } +void Wiimote::IOWakeup() +{ + char c = 0; + if (write(wakeup_pipe_w, &c, 1) != 1) + { + ERROR_LOG(WIIMOTE, "Unable to write to wakeup pipe."); + } +} + // positive = read packet // negative = didn't read packet // zero = error int Wiimote::IORead(u8* buf) { // Block select for 1/2000th of a second - timeval tv; - tv.tv_sec = 0; - tv.tv_usec = WIIMOTE_DEFAULT_TIMEOUT * 1000; fd_set fds; FD_ZERO(&fds); FD_SET(int_sock, &fds); + FD_SET(wakeup_pipe_r, &fds); - if (select(int_sock + 1, &fds, NULL, NULL, &tv) == -1) + if (select(int_sock + 1, &fds, NULL, NULL, NULL) == -1) { ERROR_LOG(WIIMOTE, "Unable to select wiimote %i input socket.", index + 1); return -1; } + if (FD_ISSET(wakeup_pipe_r, &fds)) + { + char c; + if (read(wakeup_pipe_r, &c, 1) != 1) + { + ERROR_LOG(WIIMOTE, "Unable to read from wakeup pipe."); + } + return -1; + } + if (!FD_ISSET(int_sock, &fds)) return -1; diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp index 866284b323..206c11ab64 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp @@ -142,6 +142,7 @@ namespace WiimoteReal int _IOWrite(HANDLE &dev_handle, OVERLAPPED &hid_overlap_write, enum win_bt_stack_t &stack, const u8* buf, int len); int _IORead(HANDLE &dev_handle, OVERLAPPED &hid_overlap_read, u8* buf, int index); +void _IOWakeup(HANDLE &dev_handle, OVERLAPPED &hid_overlap_read); template void ProcessWiimotes(bool new_scan, T& callback); @@ -557,6 +558,11 @@ bool Wiimote::IsConnected() const return dev_handle != 0; } +void _IOWakeup(HANDLE &dev_handle, OVERLAPPED &hid_overlap_read) +{ + CancelIoEx(dev_handle, &hid_overlap_read); +} + // positive = read packet // negative = didn't read packet // zero = error @@ -575,7 +581,7 @@ int _IORead(HANDLE &dev_handle, OVERLAPPED &hid_overlap_read, u8* buf, int index if (ERROR_IO_PENDING == read_err) { - auto const wait_result = WaitForSingleObject(hid_overlap_read.hEvent, WIIMOTE_DEFAULT_TIMEOUT); + auto const wait_result = WaitForSingleObject(hid_overlap_read.hEvent, INFINITE); if (WAIT_TIMEOUT == wait_result) { CancelIo(dev_handle); @@ -592,10 +598,10 @@ int _IORead(HANDLE &dev_handle, OVERLAPPED &hid_overlap_read, u8* buf, int index if (ERROR_OPERATION_ABORTED == overlapped_err) { + /* if (buf[1] != 0) - WARN_LOG(WIIMOTE, "Packet ignored. This may indicate a problem (timeout is %i ms).", - WIIMOTE_DEFAULT_TIMEOUT); - + WARN_LOG(WIIMOTE, "Packet ignored. This may indicate a problem."); + */ return -1; } @@ -615,6 +621,12 @@ int _IORead(HANDLE &dev_handle, OVERLAPPED &hid_overlap_read, u8* buf, int index return bytes + 1; } +void Wiimote::IOWakeup() +{ + _IOWakeup(dev_handle, hid_overlap_read); +} + + // positive = read packet // negative = didn't read packet // zero = error diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm index 17e867bc7a..c560ceb652 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm @@ -247,6 +247,11 @@ bool Wiimote::IsConnected() const return m_connected; } +void Wiimote::IOWakeup() +{ + CFRunLoopStop(m_wiimote_thread_run_loop); +} + int Wiimote::IORead(unsigned char *buf) { input = buf; diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp index aead9d60fa..ddf54dfc5a 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp @@ -52,6 +52,14 @@ Wiimote::Wiimote() , m_need_prepare() { #if defined(__linux__) && HAVE_BLUEZ + int fds[2]; + if (pipe(fds)) + { + ERROR_LOG(WIIMOTE, "pipe failed"); + abort(); + } + wakeup_pipe_w = fds[1]; + wakeup_pipe_r = fds[0]; bdaddr = (bdaddr_t){{0, 0, 0, 0, 0, 0}}; #endif } @@ -59,9 +67,12 @@ Wiimote::Wiimote() Wiimote::~Wiimote() { StopThread(); - ClearReadQueue(); m_write_reports.Clear(); +#if defined(__linux__) && HAVE_BLUEZ + close(wakeup_pipe_w); + close(wakeup_pipe_r); +#endif } // to be called from CPU thread @@ -82,6 +93,7 @@ void Wiimote::WriteReport(Report rpt) } m_write_reports.Push(std::move(rpt)); + IOWakeup(); } // to be called from CPU thread diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h index e48567acbb..314fb2e83c 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h @@ -84,6 +84,7 @@ public: bdaddr_t bdaddr; // Bluetooth address int cmd_sock; // Command socket int int_sock; // Interrupt socket + int wakeup_pipe_w, wakeup_pipe_r; #elif defined(_WIN32) std::basic_string devicepath; // Unique wiimote reference @@ -103,6 +104,7 @@ private: int IORead(u8* buf); int IOWrite(u8 const* buf, int len); + void IOWakeup(); void ThreadFunc(); void SetReady(); From 872e9ce3da35d3dc56b28a578a41edc6008d49cf Mon Sep 17 00:00:00 2001 From: comex Date: Wed, 4 Sep 2013 04:39:18 -0400 Subject: [PATCH 5/7] Add accidentally omitted code in last commit. (m_wiimote_thread_run_loop was being used but not set, causing Wiimote::IOWakeup to crash on OS X; todo rebase this) --- Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp index ddf54dfc5a..6cb9521981 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp @@ -510,6 +510,10 @@ void Wiimote::StopThread() { if (m_wiimote_thread.joinable()) m_wiimote_thread.join(); +#if defined(__APPLE__) + CFRelease(m_wiimote_thread_run_loop); + m_wiimote_thread_run_loop = NULL; +#endif } void Wiimote::SetReady() @@ -536,6 +540,10 @@ void Wiimote::WaitReady() void Wiimote::ThreadFunc() { Common::SetCurrentThreadName("Wiimote Device Thread"); +#if defined(__APPLE__) + m_wiimote_thread_run_loop = (CFRunLoopRef) CFRetain(CFRunLoopGetCurrent()); +#endif + bool ok = ConnectInternal(); SetReady(); From dc87b6d43196537842bc37d1bd6f3f28f911dd36 Mon Sep 17 00:00:00 2001 From: comex Date: Wed, 4 Sep 2013 05:29:42 -0400 Subject: [PATCH 6/7] Fix OS X code. - Close the connection properly on destruction. - Work around what seems like an Apple bug. --- .../Core/Core/Src/HW/WiimoteReal/IOdarwin.mm | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm index c560ceb652..0583cbf94c 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm @@ -140,10 +140,11 @@ void WiimoteScanner::FindWiimotes(std::vector & found_wiimotes, Wiimot [bti setDelegate: sbt]; [bti setInquiryLength: 2]; - if ([bti start] == kIOReturnSuccess) - [bti retain]; - else + if ([bti start] != kIOReturnSuccess) + { ERROR_LOG(WIIMOTE, "Unable to do bluetooth discovery"); + return; + } do { @@ -164,7 +165,7 @@ void WiimoteScanner::FindWiimotes(std::vector & found_wiimotes, Wiimot continue; Wiimote *wm = new Wiimote(); - wm->btd = dev; + wm->btd = [dev retain]; if(IsBalanceBoardName([[dev name] UTF8String])) { @@ -195,17 +196,27 @@ bool Wiimote::ConnectInternal() ConnectBT *cbt = [[ConnectBT alloc] init]; + cchan = ichan = nil; + [btd openL2CAPChannelSync: &cchan withPSM: kBluetoothL2CAPPSMHIDControl delegate: cbt]; [btd openL2CAPChannelSync: &ichan withPSM: kBluetoothL2CAPPSMHIDInterrupt delegate: cbt]; - if (ichan == NULL || cchan == NULL) + // Apple docs claim: + // "The L2CAP channel object is already retained when this function returns + // success; the channel must be released when the caller is done with it." + // But without this, the channels get over-autoreleased, even though the + // refcounting behavior here is clearly correct. + [ichan retain]; + [cchan retain]; + if (ichan == nil || cchan == nil) { ERROR_LOG(WIIMOTE, "Unable to open L2CAP channels " "for wiimote %i", index + 1); DisconnectInternal(); - [cbt release]; + [ichan release]; + [cchan release]; return false; } @@ -221,19 +232,18 @@ bool Wiimote::ConnectInternal() // Disconnect a wiimote. void Wiimote::DisconnectInternal() { - if (ichan != NULL) - [ichan release]; - - if (cchan != NULL) - [cchan release]; - - if (btd != NULL) - [btd closeConnection]; - - btd = NULL; - cchan = NULL; + [ichan closeChannel]; + [ichan release]; ichan = NULL; + [cchan closeChannel]; + [cchan release]; + cchan = NULL; + + [btd closeConnection]; + [btd release]; + btd = NULL; + if (!IsConnected()) return; From 8992f587202529a94ee9e2fa3c1a01d10ea1c691 Mon Sep 17 00:00:00 2001 From: comex Date: Wed, 4 Sep 2013 05:30:16 -0400 Subject: [PATCH 7/7] Fix Wiimote thread wakeup on externally-triggered destroy. --- Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp | 5 ++++- Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp index 6cb9521981..77f84449be 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp @@ -503,11 +503,14 @@ bool Wiimote::Connect() void Wiimote::StartThread() { + m_run_thread = true; m_wiimote_thread = std::thread(std::mem_fun(&Wiimote::ThreadFunc), this); } void Wiimote::StopThread() { + m_run_thread = false; + IOWakeup(); if (m_wiimote_thread.joinable()) m_wiimote_thread.join(); #if defined(__APPLE__) @@ -554,7 +557,7 @@ void Wiimote::ThreadFunc() } // main loop - while (IsConnected()) + while (IsConnected() && m_run_thread) { if (m_need_prepare) { diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h index 314fb2e83c..fa94303a66 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h @@ -113,8 +113,12 @@ private: bool m_rumble_state; std::thread m_wiimote_thread; - volatile bool m_thread_ready; + // Whether to keep running the thread. + volatile bool m_run_thread; + // Whether to call PrepareOnThread. volatile bool m_need_prepare; + // Whether the thread has finished ConnectInternal. + volatile bool m_thread_ready; std::mutex m_thread_ready_mutex; std::condition_variable m_thread_ready_cond;