From f4586570acba4a02750718caa4482d9034479397 Mon Sep 17 00:00:00 2001 From: comex Date: Sat, 7 Sep 2013 15:43:17 -0400 Subject: [PATCH 1/5] Use SetEvent instead of CancelIoEx for XP compatibility. --- Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp index 206c11ab6482..91bd1bec6c21 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp @@ -560,7 +560,7 @@ bool Wiimote::IsConnected() const void _IOWakeup(HANDLE &dev_handle, OVERLAPPED &hid_overlap_read) { - CancelIoEx(dev_handle, &hid_overlap_read); + SetEvent(hid_overlap_read.hEvent); } // positive = read packet @@ -582,26 +582,22 @@ 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, INFINITE); - if (WAIT_TIMEOUT == wait_result) - { - CancelIo(dev_handle); - } - else if (WAIT_FAILED == wait_result) + + // In case the event was signalled by _IOWakeup before the read completed, cancel it. + CancelIo(dev_handle); + + if (WAIT_FAILED == wait_result) { WARN_LOG(WIIMOTE, "A wait error occurred on reading from Wiimote %i.", index + 1); - CancelIo(dev_handle); } - if (!GetOverlappedResult(dev_handle, &hid_overlap_read, &bytes, TRUE)) + if (!GetOverlappedResult(dev_handle, &hid_overlap_read, &bytes, FALSE)) { auto const overlapped_err = GetLastError(); if (ERROR_OPERATION_ABORTED == overlapped_err) { - /* - if (buf[1] != 0) - WARN_LOG(WIIMOTE, "Packet ignored. This may indicate a problem."); - */ + // It was. return -1; } From b31502893fc074a566358f9f84f6b432a7a6502b Mon Sep 17 00:00:00 2001 From: comex Date: Sat, 7 Sep 2013 16:13:39 -0400 Subject: [PATCH 2/5] Fix lifetime issues in IOWakeup. --- Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp | 22 +++++++++++++ Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp | 27 ++++++++++------ .../Core/Core/Src/HW/WiimoteReal/IOdarwin.mm | 24 +++++++++++++- .../Core/Src/HW/WiimoteReal/WiimoteReal.cpp | 31 ++----------------- .../Core/Src/HW/WiimoteReal/WiimoteReal.h | 3 ++ 5 files changed, 69 insertions(+), 38 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp index 826b9e67ce2a..f06bec2ddea6 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IONix.cpp @@ -133,6 +133,28 @@ void WiimoteScanner::FindWiimotes(std::vector & found_wiimotes, Wiimot } +void Wiimote::InitInternal() +{ + cmd_sock = -1; + int_sock = -1; + + 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}}; +} + +void Wiimote::TeardownInternal() +{ + close(wakeup_pipe_w); + close(wakeup_pipe_r); +} + // Connect to a wiimote with a known address. bool Wiimote::ConnectInternal() { diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp index 91bd1bec6c21..58ebfbfa6493 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOWin.cpp @@ -514,12 +514,6 @@ bool Wiimote::ConnectInternal() } #endif - hid_overlap_read = OVERLAPPED(); - hid_overlap_read.hEvent = CreateEvent(NULL, true, false, NULL); - - hid_overlap_write = OVERLAPPED(); - hid_overlap_write.hEvent = CreateEvent(NULL, true, false, NULL); - // 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 @@ -544,15 +538,30 @@ void Wiimote::DisconnectInternal() CloseHandle(dev_handle); dev_handle = 0; - CloseHandle(hid_overlap_read.hEvent); - CloseHandle(hid_overlap_write.hEvent); - #ifdef SHARE_WRITE_WIIMOTES std::lock_guard lk(g_connected_wiimotes_lock); g_connected_wiimotes.erase(devicepath); #endif } +void Wiimote::InitInternal() +{ + dev_handle = 0; + stack = MSBT_STACK_UNKNOWN; + + hid_overlap_read = OVERLAPPED(); + hid_overlap_read.hEvent = CreateEvent(NULL, true, false, NULL); + + hid_overlap_write = OVERLAPPED(); + hid_overlap_write.hEvent = CreateEvent(NULL, true, false, NULL); +} + +void Wiimote::TeardownInternal() +{ + CloseHandle(hid_overlap_read.hEvent); + CloseHandle(hid_overlap_write.hEvent); +} + bool Wiimote::IsConnected() const { return dev_handle != 0; diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm index 0583cbf94c24..e7e9033338d3 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm @@ -188,6 +188,22 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel return true; } +void Wiimote::InitInternal() +{ + inputlen = 0; + m_connected = false; + m_wiimote_thread_run_loop = NULL; +} + +void Wiimote::TeardownInternal() +{ + if (m_wiimote_thread_run_loop) + { + CFRelease(m_wiimote_thread_run_loop); + m_wiimote_thread_run_loop = NULL; + } +} + // Connect to a wiimote with a known address. bool Wiimote::ConnectInternal() { @@ -226,6 +242,9 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel m_connected = true; [cbt release]; + + m_wiimote_thread_run_loop = (CFRunLoopRef) CFRetain(CFRunLoopGetCurrent()); + return true; } @@ -259,7 +278,10 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel void Wiimote::IOWakeup() { - CFRunLoopStop(m_wiimote_thread_run_loop); + if (m_wiimote_thread_run_loop) + { + CFRunLoopStop(m_wiimote_thread_run_loop); + } } int Wiimote::IORead(unsigned char *buf) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp index 77f84449be6e..ee6de5f19d9c 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.cpp @@ -39,29 +39,12 @@ WiimoteScanner g_wiimote_scanner; Wiimote::Wiimote() : index() -#ifdef __APPLE__ - , btd(), ichan(), cchan(), input(), inputlen(), m_connected() -#elif defined(__linux__) && HAVE_BLUEZ - , cmd_sock(-1), int_sock(-1) -#elif defined(_WIN32) - , dev_handle(0), stack(MSBT_STACK_UNKNOWN) -#endif , m_last_input_report() , m_channel(0) , m_rumble_state() , 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 + InitInternal(); } Wiimote::~Wiimote() @@ -69,10 +52,7 @@ Wiimote::~Wiimote() StopThread(); ClearReadQueue(); m_write_reports.Clear(); -#if defined(__linux__) && HAVE_BLUEZ - close(wakeup_pipe_w); - close(wakeup_pipe_r); -#endif + TeardownInternal(); } // to be called from CPU thread @@ -514,8 +494,6 @@ 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 } @@ -543,9 +521,6 @@ 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(); @@ -565,7 +540,7 @@ void Wiimote::ThreadFunc() if (!PrepareOnThread()) { ERROR_LOG(WIIMOTE, "Wiimote::PrepareOnThread failed. Disconnecting Wiimote %d.", index + 1); - DisconnectInternal(); + break; } } Write(); diff --git a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h index fa94303a665d..e4e965400c6d 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h +++ b/Source/Core/Core/Src/HW/WiimoteReal/WiimoteReal.h @@ -56,6 +56,9 @@ friend class WiimoteEmu::Wiimote; bool ConnectInternal(); void DisconnectInternal(); + void InitInternal(); + void TeardownInternal(); + bool Connect(); // TODO: change to something like IsRelevant From 8b4f0ef034f913f68155e37bbdef1b876c4d8504 Mon Sep 17 00:00:00 2001 From: comex Date: Sat, 7 Sep 2013 16:17:00 -0400 Subject: [PATCH 3/5] IODummy needs it too. --- Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp b/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp index e1ca6d603c9a..ada6237756ab 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp +++ b/Source/Core/Core/Src/HW/WiimoteReal/IODummy.cpp @@ -43,6 +43,12 @@ bool WiimoteScanner::IsReady() const return false; } +void Wiimote::InitInternal() +{} + +void Wiimote::TeardownInternal() +{} + bool Wiimote::ConnectInternal() { return 0; From 22d9331b9601acb6943378977896d12678b8aaef Mon Sep 17 00:00:00 2001 From: comex Date: Sun, 8 Sep 2013 18:15:49 -0400 Subject: [PATCH 4/5] Improve IOdarwin - Add requestAuthentication, which might help someone who can't sync, and better error reporting. --- .../Core/Core/Src/HW/WiimoteReal/IOdarwin.mm | 61 +++++++++++++------ 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm index e7e9033338d3..886dbb9c9cf7 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm @@ -193,6 +193,7 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel inputlen = 0; m_connected = false; m_wiimote_thread_run_loop = NULL; + btd = nil; } void Wiimote::TeardownInternal() @@ -202,6 +203,8 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel CFRelease(m_wiimote_thread_run_loop); m_wiimote_thread_run_loop = NULL; } + [btd release]; + btd = nil; } // Connect to a wiimote with a known address. @@ -214,30 +217,49 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel cchan = ichan = nil; - [btd openL2CAPChannelSync: &cchan - withPSM: kBluetoothL2CAPPSMHIDControl delegate: cbt]; - [btd openL2CAPChannelSync: &ichan - withPSM: kBluetoothL2CAPPSMHIDInterrupt delegate: cbt]; + IOReturn ret = [btd openConnection]; + if (ret) + { + ERROR_LOG(WIIMOTE, "Unable to open Bluetooth connection to wiimote %i: %x", + index + 1, ret); + return false; + } + + ret = [btd requestAuthentication]; + if (ret) + { + WARN_LOG(WIIMOTE, "Unable to request authentication for wiimote %i: %x", + index + 1, ret); + goto bad; + } + + ret = [btd openL2CAPChannelSync: &cchan + withPSM: kBluetoothL2CAPPSMHIDControl delegate: cbt]; + if (ret) + { + ERROR_LOG(WIIMOTE, "Unable to open control channel for wiimote %i: %x", + index + 1, ret); + goto bad; + } // 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) + + ret = [btd openL2CAPChannelSync: &ichan + withPSM: kBluetoothL2CAPPSMHIDInterrupt delegate: cbt]; + if (ret) { - ERROR_LOG(WIIMOTE, "Unable to open L2CAP channels " - "for wiimote %i", index + 1); - DisconnectInternal(); - [cbt release]; - [ichan release]; - [cchan release]; - return false; + WARN_LOG(WIIMOTE, "Unable to open interrupt channel for wiimote %i: %x", + index + 1, ret); + goto bad; } + [ichan retain]; NOTICE_LOG(WIIMOTE, "Connected to wiimote %i at %s", - index + 1, [[btd addressString] UTF8String]); + index + 1, [[btd addressString] UTF8String]); m_connected = true; @@ -246,6 +268,11 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel m_wiimote_thread_run_loop = (CFRunLoopRef) CFRetain(CFRunLoopGetCurrent()); return true; + +bad: + DisconnectInternal(); + [cbt release]; + return false; } // Disconnect a wiimote. @@ -253,15 +280,13 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel { [ichan closeChannel]; [ichan release]; - ichan = NULL; + ichan = nil; [cchan closeChannel]; [cchan release]; - cchan = NULL; + cchan = nil; [btd closeConnection]; - [btd release]; - btd = NULL; if (!IsConnected()) return; From fe0a450ee406dfe1a09c3c385eb4a39096695fef Mon Sep 17 00:00:00 2001 From: comex Date: Sun, 8 Sep 2013 19:32:14 -0400 Subject: [PATCH 5/5] Don't request authentication (aka I should actually test before committing). --- Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm index 886dbb9c9cf7..4f015866bc8f 100644 --- a/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm +++ b/Source/Core/Core/Src/HW/WiimoteReal/IOdarwin.mm @@ -225,14 +225,6 @@ - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel return false; } - ret = [btd requestAuthentication]; - if (ret) - { - WARN_LOG(WIIMOTE, "Unable to request authentication for wiimote %i: %x", - index + 1, ret); - goto bad; - } - ret = [btd openL2CAPChannelSync: &cchan withPSM: kBluetoothL2CAPPSMHIDControl delegate: cbt]; if (ret)