From 6cc40b12357ae339abcb44517810af304bab77ea Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 13 Feb 2016 08:17:20 -0600 Subject: [PATCH] Improve stability of the Wii U Gamecube Controller adapter under Android. Under failure conditions of the GC Adapter, When interface count is zero and we can't open the device. Then there were race conditions on shutdown of the threads which could result in crashing. Make adapter opening more robust like the Mayflash DolphinBar. Make shutdown more robust by making the read thread control the write thread. Make sure that there is actual data to be written when kicking the write thread. So it doesn't attempt a write a shutdown. Make a toast on screen to tell the user that the adapter needs to be unplugged and plugged back in again for it to work. --- .../dolphinemu/utils/Java_GCAdapter.java | 76 +++++---- Source/Core/InputCommon/GCAdapter_Android.cpp | 151 ++++++++++-------- 2 files changed, 128 insertions(+), 99 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/Java_GCAdapter.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/Java_GCAdapter.java index 87d816d93e..37ab4c6b5f 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/Java_GCAdapter.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/Java_GCAdapter.java @@ -9,6 +9,7 @@ import android.hardware.usb.UsbDeviceConnection; import android.hardware.usb.UsbEndpoint; import android.hardware.usb.UsbInterface; import android.hardware.usb.UsbManager; +import android.widget.Toast; import org.dolphinemu.dolphinemu.NativeLibrary; import org.dolphinemu.dolphinemu.services.USBPermService; @@ -76,32 +77,16 @@ public class Java_GCAdapter { } public static int Input() { - if (usb_in != null) - { - int read = usb_con.bulkTransfer(usb_in, controller_payload, controller_payload.length, 16); - return read; - } - else - { - // TODO Is this right? - return 0; - } + int read = usb_con.bulkTransfer(usb_in, controller_payload, controller_payload.length, 16); + return read; } public static int Output(byte[] rumble) { - if (usb_out != null) - { - int size = usb_con.bulkTransfer(usb_out, rumble, 5, 16); - return size; - } - else - { - // TODO Is this right? - return 0; - } + int size = usb_con.bulkTransfer(usb_out, rumble, 5, 16); + return size; } - public static void OpenAdapter() + public static boolean OpenAdapter() { HashMap devices = manager.getDeviceList(); Iterator it = devices.entrySet().iterator(); @@ -113,20 +98,47 @@ public class Java_GCAdapter { if (manager.hasPermission(dev)) { usb_con = manager.openDevice(dev); - UsbConfiguration conf = dev.getConfiguration(0); - usb_intf = conf.getInterface(0); - usb_con.claimInterface(usb_intf, true); - for (int i = 0; i < usb_intf.getEndpointCount(); ++i) - if (usb_intf.getEndpoint(i).getDirection() == UsbConstants.USB_DIR_IN) - usb_in = usb_intf.getEndpoint(i); - else - usb_out = usb_intf.getEndpoint(i); - InitAdapter(); - return; + Log.info("GCAdapter: Number of configurations: " + dev.getConfigurationCount()); + Log.info("GCAdapter: Number of interfaces: " + dev.getInterfaceCount()); + + if (dev.getConfigurationCount() > 0 && dev.getInterfaceCount() > 0) + { + UsbConfiguration conf = dev.getConfiguration(0); + usb_intf = conf.getInterface(0); + usb_con.claimInterface(usb_intf, true); + + Log.info("GCAdapter: Number of endpoints: " + usb_intf.getEndpointCount()); + + if (usb_intf.getEndpointCount() == 2) + { + for (int i = 0; i < usb_intf.getEndpointCount(); ++i) + if (usb_intf.getEndpoint(i).getDirection() == UsbConstants.USB_DIR_IN) + usb_in = usb_intf.getEndpoint(i); + else + usb_out = usb_intf.getEndpoint(i); + + InitAdapter(); + return true; + } + else + { + usb_con.releaseInterface(usb_intf); + } + } + + NativeLibrary.sEmulationActivity.runOnUiThread(new Runnable() + { + @Override + public void run() + { + Toast.makeText(NativeLibrary.sEmulationActivity, "GameCube Adapter couldn't be opened. Please re-plug the device.", Toast.LENGTH_LONG).show(); + } + }); + usb_con.close(); } } - } + return false; } } diff --git a/Source/Core/InputCommon/GCAdapter_Android.cpp b/Source/Core/InputCommon/GCAdapter_Android.cpp index 4fa887cd23..46ec417f2b 100644 --- a/Source/Core/InputCommon/GCAdapter_Android.cpp +++ b/Source/Core/InputCommon/GCAdapter_Android.cpp @@ -40,12 +40,12 @@ static std::atomic s_controller_payload_size{0}; // Output handling static std::mutex s_write_mutex; static u8 s_controller_write_payload[5]; +static std::atomic s_controller_write_payload_size{0}; // Adapter running thread static std::thread s_read_adapter_thread; static Common::Flag s_read_adapter_thread_running; -static std::thread s_write_adapter_thread; static Common::Flag s_write_adapter_thread_running; static Common::Event s_write_happened; @@ -77,6 +77,47 @@ static void ScanThreadFunc() NOTICE_LOG(SERIALINTERFACE, "GC Adapter scanning thread stopped"); } +static void Write() +{ + Common::SetCurrentThreadName("GC Adapter Write Thread"); + NOTICE_LOG(SERIALINTERFACE, "GC Adapter write thread started"); + + JNIEnv* env; + g_java_vm->AttachCurrentThread(&env, NULL); + jmethodID output_func = env->GetStaticMethodID(s_adapter_class, "Output", "([B)I"); + + while (s_write_adapter_thread_running.IsSet()) + { + s_write_happened.Wait(); + int write_size = s_controller_write_payload_size.load(); + if (write_size) + { + jbyteArray jrumble_array = env->NewByteArray(5); + jbyte* jrumble = env->GetByteArrayElements(jrumble_array, NULL); + + { + std::lock_guard lk(s_write_mutex); + memcpy(jrumble, s_controller_write_payload, write_size); + } + + env->ReleaseByteArrayElements(jrumble_array, jrumble, 0); + int size = env->CallStaticIntMethod(s_adapter_class, output_func, jrumble_array); + // Netplay sends invalid data which results in size = 0x00. Ignore it. + if (size != write_size && size != 0x00) + { + ERROR_LOG(SERIALINTERFACE, "error writing rumble (size: %d)", size); + Reset(); + } + } + + Common::YieldCPU(); + } + + g_java_vm->DetachCurrentThread(); + + NOTICE_LOG(SERIALINTERFACE, "GC Adapter write thread stopped"); +} + static void Read() { Common::SetCurrentThreadName("GC Adapter Read Thread"); @@ -93,76 +134,56 @@ static void Read() // Get function pointers jmethodID getfd_func = env->GetStaticMethodID(s_adapter_class, "GetFD", "()I"); jmethodID input_func = env->GetStaticMethodID(s_adapter_class, "Input", "()I"); - jmethodID openadapter_func = env->GetStaticMethodID(s_adapter_class, "OpenAdapter", "()V"); + jmethodID openadapter_func = env->GetStaticMethodID(s_adapter_class, "OpenAdapter", "()Z"); - env->CallStaticVoidMethod(s_adapter_class, openadapter_func); + bool connected = env->CallStaticBooleanMethod(s_adapter_class, openadapter_func); - // Reset rumble once on initial reading - ResetRumble(); - - while (s_read_adapter_thread_running.IsSet()) + if (connected) { - int read_size = env->CallStaticIntMethod(s_adapter_class, input_func); + s_write_adapter_thread_running.Set(true); + std::thread write_adapter_thread(Write); - jbyte* java_data = env->GetByteArrayElements(*java_controller_payload, nullptr); - { - std::lock_guard lk(s_read_mutex); - memcpy(s_controller_payload, java_data, 0x37); - s_controller_payload_size.store(read_size); - } - env->ReleaseByteArrayElements(*java_controller_payload, java_data, 0); + // Reset rumble once on initial reading + ResetRumble(); - if (first_read) + while (s_read_adapter_thread_running.IsSet()) { - first_read = false; - s_fd = env->CallStaticIntMethod(s_adapter_class, getfd_func); + int read_size = env->CallStaticIntMethod(s_adapter_class, input_func); + + jbyte* java_data = env->GetByteArrayElements(*java_controller_payload, nullptr); + { + std::lock_guard lk(s_read_mutex); + memcpy(s_controller_payload, java_data, 0x37); + s_controller_payload_size.store(read_size); + } + env->ReleaseByteArrayElements(*java_controller_payload, java_data, 0); + + if (first_read) + { + first_read = false; + s_fd = env->CallStaticIntMethod(s_adapter_class, getfd_func); + } + + Common::YieldCPU(); } - Common::YieldCPU(); + // Terminate the write thread on leaving + if (s_write_adapter_thread_running.TestAndClear()) + { + s_controller_write_payload_size.store(0); + s_write_happened.Set(); // Kick the waiting event + write_adapter_thread.join(); + } } + s_fd = 0; + s_detected = false; + g_java_vm->DetachCurrentThread(); NOTICE_LOG(SERIALINTERFACE, "GC Adapter read thread stopped"); } -static void Write() -{ - Common::SetCurrentThreadName("GC Adapter Write Thread"); - NOTICE_LOG(SERIALINTERFACE, "GC Adapter write thread started"); - - JNIEnv* env; - g_java_vm->AttachCurrentThread(&env, NULL); - jmethodID output_func = env->GetStaticMethodID(s_adapter_class, "Output", "([B)I"); - - while (s_write_adapter_thread_running.IsSet()) - { - jbyteArray jrumble_array = env->NewByteArray(5); - jbyte* jrumble = env->GetByteArrayElements(jrumble_array, NULL); - - s_write_happened.Wait(); - { - std::lock_guard lk(s_write_mutex); - memcpy(jrumble, s_controller_write_payload, 5); - } - - env->ReleaseByteArrayElements(jrumble_array, jrumble, 0); - int size = env->CallStaticIntMethod(s_adapter_class, output_func, jrumble_array); - // Netplay sends invalid data which results in size = 0x00. Ignore it. - if (size != 0x05 && size != 0x00) - { - ERROR_LOG(SERIALINTERFACE, "error writing rumble (size: %d)", size); - Reset(); - } - - Common::YieldCPU(); - } - - g_java_vm->DetachCurrentThread(); - - NOTICE_LOG(SERIALINTERFACE, "GC Adapter write thread stopped"); -} - void Init() { if (s_fd) @@ -189,14 +210,14 @@ void Init() void Setup() { s_fd = 0; + s_detected = true; + + // Make sure the thread isn't in the middle of shutting down while starting a new one + if (s_read_adapter_thread_running.TestAndClear()) + s_read_adapter_thread.join(); s_read_adapter_thread_running.Set(true); s_read_adapter_thread = std::thread(Read); - - s_write_adapter_thread_running.Set(true); - s_write_adapter_thread = std::thread(Write); - - s_detected = true; } void Reset() @@ -207,12 +228,6 @@ void Reset() if (s_read_adapter_thread_running.TestAndClear()) s_read_adapter_thread.join(); - if (s_write_adapter_thread_running.TestAndClear()) - { - s_write_happened.Set(); // Kick the waiting event - s_write_adapter_thread.join(); - } - for (int i = 0; i < MAX_SI_CHANNELS; i++) s_controller_type[i] = ControllerTypes::CONTROLLER_NONE; @@ -323,6 +338,7 @@ void Output(int chan, u8 rumble_command) { std::lock_guard lk(s_write_mutex); memcpy(s_controller_write_payload, rumble, 5); + s_controller_write_payload_size.store(5); } s_write_happened.Set(); } @@ -349,6 +365,7 @@ void ResetRumble() { std::lock_guard lk(s_read_mutex); memcpy(s_controller_write_payload, rumble, 5); + s_controller_write_payload_size.store(5); } s_write_happened.Set(); }