Merge branch 'comex-wiimote-fixes'

Should fix issue 6574.

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;
 }
@@ -58,6 +58,9 @@ bool Wiimote::IsConnected() const
 	return false;
 }
 
+void Wiimote::IOWakeup()
+{}
+
 int Wiimote::IORead(u8* buf)
 {
 	return 0;

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<Wiimote*> & 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);
@@ -182,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;
 

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 <typename T>
 void ProcessWiimotes(bool new_scan, T& callback);
@@ -459,7 +460,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 +536,7 @@ bool Wiimote::Connect()
 	return true;
 }
 
-void Wiimote::Disconnect()
+void Wiimote::DisconnectInternal()
 {
 	if (!IsConnected())
 		return;
@@ -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).",
+						WARN_LOG(WIIMOTE, "Packet ignored. This may indicate a problem.");
-							WIIMOTE_DEFAULT_TIMEOUT);
+					*/
-
 					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

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());
 }
 
@@ -62,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;
@@ -71,9 +73,8 @@
 	memcpy(wm->input, data, length);
 	wm->inputlen = length;
 
-	(void)wm->Read();
-
 	(void)UpdateSystemActivity(UsrActivity);
+	CFRunLoopStop(CFRunLoopGetCurrent());
 }
 
 - (void) l2capChannelClosed: (IOBluetoothL2CAPChannel *) l2capChannel
@@ -98,7 +99,7 @@
 
 	WARN_LOG(WIIMOTE, "Lost channel to wiimote %i", wm->index + 1);
 
-	wm->Disconnect();
+	wm->DisconnectInternal();
 }
 @end
 
@@ -139,14 +140,18 @@ void WiimoteScanner::FindWiimotes(std::vector<Wiimote*> & found_wiimotes, Wiimot
 	[bti setDelegate: sbt];
 	[bti setInquiryLength: 2];
 
-	if ([bti start] == kIOReturnSuccess)
+	if ([bti start] != kIOReturnSuccess)
-		[bti retain];
+	{
-	else
 		ERROR_LOG(WIIMOTE, "Unable to do bluetooth discovery");
+		return;
+	}
 
+	do
+	{
 		CFRunLoopRun();
+	}
+	while(!sbt->done);
 
-	[bti stop];
 	int found_devices = [[bti foundDevices] count];
 
 	if (found_devices)
@@ -160,7 +165,7 @@ void WiimoteScanner::FindWiimotes(std::vector<Wiimote*> & found_wiimotes, Wiimot
 			continue;
 
 		Wiimote *wm = new Wiimote();
-		wm->btd = dev;
+		wm->btd = [dev retain];
 		
 		if(IsBalanceBoardName([[dev name] UTF8String]))
 		{
@@ -184,32 +189,37 @@ bool WiimoteScanner::IsReady() const
 }
 
 // Connect to a wiimote with a known address.
-bool Wiimote::Connect()
+bool Wiimote::ConnectInternal()
 {
 	if (IsConnected())
 		return false;
 
 	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);
-		Disconnect();
+		DisconnectInternal();
-		
 		[cbt release];
+		[ichan release];
+		[cchan release];
 		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]);
 
@@ -220,21 +230,20 @@ bool Wiimote::Connect()
 }
 
 // Disconnect a wiimote.
-void Wiimote::Disconnect()
+void Wiimote::DisconnectInternal()
 {
-	if (btd != NULL)
+	[ichan closeChannel];
-		[btd closeConnection];
-
-	if (ichan != NULL)
 	[ichan release];
-
-	if (cchan != NULL)
-		[cchan release];
-
-	btd = NULL;
-	cchan = NULL;
 	ichan = NULL;
 
+	[cchan closeChannel];
+	[cchan release];
+	cchan = NULL;
+
+	[btd closeConnection];
+	[btd release];
+	btd = NULL;
+
 	if (!IsConnected())
 		return;
 
@@ -248,18 +257,19 @@ bool Wiimote::IsConnected() const
 	return m_connected;
 }
 
+void Wiimote::IOWakeup()
+{
+	CFRunLoopStop(m_wiimote_thread_run_loop);
+}
+
 int Wiimote::IORead(unsigned char *buf)
 {
-	int bytes;
+	input = buf;
+	inputlen = -1;
 
-	if (!IsConnected())
+	CFRunLoopRun();
-		return 0;
 
-	bytes = inputlen;
+	return inputlen;
-	memcpy(buf, input, bytes);
-	inputlen = 0;
-
-	return bytes;
 }
 
 int Wiimote::IOWrite(const unsigned char *buf, int len)

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,9 +49,17 @@ Wiimote::Wiimote()
 	, m_last_input_report()
 	, m_channel(0)
 	, m_rumble_state()
-	, m_run_thread(false)
+	, 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,12 +67,12 @@ Wiimote::Wiimote()
 Wiimote::~Wiimote()
 {
 	StopThread();
-
-	if (IsConnected())
-		Disconnect();
-	
 	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
@@ -85,6 +93,7 @@ void Wiimote::WriteReport(Report rpt)
 	}
 
 	m_write_reports.Push(std::move(rpt));
+	IOWakeup();
 }
 
 // to be called from CPU thread
@@ -225,8 +234,8 @@ bool Wiimote::Read()
 	}
 	else if (0 == result)
 	{
-		WARN_LOG(WIIMOTE, "Wiimote::IORead failed. Disconnecting Wiimote %d.", index + 1);
+		ERROR_LOG(WIIMOTE, "Wiimote::IORead failed. Disconnecting Wiimote %d.", index + 1);
-		Disconnect();
+		DisconnectInternal();
 	}
 
 	return false;
@@ -308,21 +317,25 @@ 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))
@@ -480,6 +493,14 @@ 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;
@@ -489,26 +510,69 @@ void Wiimote::StartThread()
 void Wiimote::StopThread()
 {
 	m_run_thread = false;
+	IOWakeup();
 	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()
+{
+	if (!m_thread_ready)
+	{
+		{
+			std::lock_guard<std::mutex> Guard(m_thread_ready_mutex);
+			m_thread_ready = true;
+		}
+		m_thread_ready_cond.notify_all();
+	}
+}
+
+void Wiimote::WaitReady()
+{
+	std::unique_lock<std::mutex> lock(m_thread_ready_mutex);
+	while (!m_thread_ready)
+	{
+		m_thread_ready_cond.wait(lock);
+	}
 }
 
 void Wiimote::ThreadFunc()
 {
 	Common::SetCurrentThreadName("Wiimote Device Thread");
+#if defined(__APPLE__)
+	m_wiimote_thread_run_loop = (CFRunLoopRef) CFRetain(CFRunLoopGetCurrent());
+#endif
+
+	bool ok = ConnectInternal();
+
+	SetReady();
+
+	if (!ok)
+	{
+		return;
+	}
 
 	// main loop
-	while (m_run_thread && IsConnected())
+	while (IsConnected() && m_run_thread)
 	{
-#ifdef __APPLE__
+		if (m_need_prepare)
-		// Reading happens elsewhere on OSX
+		{
-		bool const did_something = Write();
+			m_need_prepare = false;
-#else
+			if (!PrepareOnThread())
-		bool const did_something = Write() || Read();
+			{
-#endif
+				ERROR_LOG(WIIMOTE, "Wiimote::PrepareOnThread failed.  Disconnecting Wiimote %d.", index + 1);
-		if (!did_something)
+				DisconnectInternal();
-			Common::SleepCurrentThread(1);
 			}
+		}
+		Write();
+		Read();
+	}
+
+	DisconnectInternal();
 }
 
 void LoadSettings()
@@ -629,24 +693,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<std::recursive_mutex> lk(g_refresh_lock);
 
 	for (unsigned int i = 0; i < MAX_WIIMOTES; ++i)
 	{
-		if (WIIMOTE_SRC_REAL & g_wiimote_sources[i]
+		if (TryToConnectWiimoteN(wm, i))
-			&& !g_wiimotes[i])
 		{
-			if (wm->Connect() && wm->Prepare(i))
+			wm = NULL;
-			{
-				NOTICE_LOG(WIIMOTE, "Connected to Wiimote %i.", i + 1);
-				
-				std::swap(g_wiimotes[i], wm);
-				g_wiimotes[i]->StartThread();
-				
-				Host_ConnectWiimote(i, true);
-			}
 			break;
 		}
 	}
@@ -662,18 +734,9 @@ void TryToConnectBalanceBoard(Wiimote* wm)
 {
 	std::unique_lock<std::recursive_mutex> lk(g_refresh_lock);
 
-	if (WIIMOTE_SRC_REAL & g_wiimote_sources[WIIMOTE_BALANCE_BOARD]
+	if (TryToConnectWiimoteN(wm, WIIMOTE_BALANCE_BOARD))
-		&& !g_wiimotes[WIIMOTE_BALANCE_BOARD])
 	{
-		if (wm->Connect() && wm->Prepare(WIIMOTE_BALANCE_BOARD))
+		wm = NULL;
-		{
-			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);
-		}
 	}
 	
 	g_wiimote_scanner.WantBB(0 != CalculateWantedBB());
@@ -687,26 +750,13 @@ void DoneWithWiimote(int index)
 {
 	std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
 
-	if (g_wiimotes[index])
+	Wiimote* wm = g_wiimotes[index];
-	{
-		g_wiimotes[index]->StopThread();
 
+	if (wm)
+	{
+		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)
+		TryToConnectWiimote(wm);
-		{
-			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;
-			}
-		}
 	}
 	
 	// else, just disconnect the Wiimote
@@ -763,9 +813,7 @@ void Refresh()
 		{
 			if (g_wiimotes[i])
 			{
-				g_wiimotes[i]->StopThread();
 				g_wiimotes[i]->Prepare(i);
-				g_wiimotes[i]->StartThread();
 			}
 		}
 

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,13 +76,15 @@ 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
 	int int_sock;						// Interrupt socket
+	int wakeup_pipe_w, wakeup_pipe_r;
 
 #elif defined(_WIN32)
 	std::basic_string<TCHAR> devicepath;	// Unique wiimote reference
@@ -98,13 +104,23 @@ private:
 	
 	int IORead(u8* buf);
 	int IOWrite(u8 const* buf, int len);
+	void IOWakeup();
 
 	void ThreadFunc();
+	void SetReady();
+	void WaitReady();
 
 	bool m_rumble_state;
 	
-	bool				m_run_thread;
 	std::thread               m_wiimote_thread;
+	// 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;
 
 	Common::FifoQueue<Report>	m_read_reports;
 	Common::FifoQueue<Report>	m_write_reports;