From 94f5f29573726112e26d3015749736a7d0a4993c Mon Sep 17 00:00:00 2001 From: Ameer Date: Thu, 16 Jul 2020 13:00:04 -0400 Subject: [PATCH 1/3] Refactor adapter code --- src/input_common/gcadapter/gc_adapter.cpp | 184 +++++----------------- src/input_common/gcadapter/gc_adapter.h | 37 +---- 2 files changed, 43 insertions(+), 178 deletions(-) diff --git a/src/input_common/gcadapter/gc_adapter.cpp b/src/input_common/gcadapter/gc_adapter.cpp index 898a278a9..0fc609184 100644 --- a/src/input_common/gcadapter/gc_adapter.cpp +++ b/src/input_common/gcadapter/gc_adapter.cpp @@ -24,12 +24,9 @@ Adapter::Adapter() { } LOG_INFO(Input, "GC Adapter Initialization started"); - current_status = NO_ADAPTER_DETECTED; - get_origin.fill(true); - const int init_res = libusb_init(&libusb_ctx); if (init_res == LIBUSB_SUCCESS) { - StartScanThread(); + Setup(); } else { LOG_ERROR(Input, "libusb could not be initialized. failed with error = {}", init_res); } @@ -37,9 +34,9 @@ Adapter::Adapter() { GCPadStatus Adapter::GetPadStatus(std::size_t port, const std::array& adapter_payload) { GCPadStatus pad = {}; + const size_t offset = 1 + (9 * port); - ControllerTypes type = ControllerTypes(adapter_payload[1 + (9 * port)] >> 4); - adapter_controllers_status[port] = type; + adapter_controllers_status[port] = static_cast(adapter_payload[offset] >> 4); static constexpr std::array b1_buttons{ PadButton::PAD_BUTTON_A, PadButton::PAD_BUTTON_B, PadButton::PAD_BUTTON_X, @@ -54,14 +51,19 @@ GCPadStatus Adapter::GetPadStatus(std::size_t port, const std::array& ad PadButton::PAD_TRIGGER_L, }; + static constexpr std::array axes{ + PadAxes::StickX, PadAxes::StickY, PadAxes::SubstickX, + PadAxes::SubstickY, PadAxes::TriggerLeft, PadAxes::TriggerRight, + }; + if (adapter_controllers_status[port] == ControllerTypes::None && !get_origin[port]) { // Controller may have been disconnected, recalibrate if reconnected. get_origin[port] = true; } if (adapter_controllers_status[port] != ControllerTypes::None) { - const u8 b1 = adapter_payload[1 + (9 * port) + 1]; - const u8 b2 = adapter_payload[1 + (9 * port) + 2]; + const u8 b1 = adapter_payload[offset + 1]; + const u8 b2 = adapter_payload[offset + 2]; for (std::size_t i = 0; i < b1_buttons.size(); ++i) { if ((b1 & (1U << i)) != 0) { @@ -74,21 +76,13 @@ GCPadStatus Adapter::GetPadStatus(std::size_t port, const std::array& ad pad.button |= static_cast(b2_buttons[j]); } } - - pad.stick_x = adapter_payload[1 + (9 * port) + 3]; - pad.stick_y = adapter_payload[1 + (9 * port) + 4]; - pad.substick_x = adapter_payload[1 + (9 * port) + 5]; - pad.substick_y = adapter_payload[1 + (9 * port) + 6]; - pad.trigger_left = adapter_payload[1 + (9 * port) + 7]; - pad.trigger_right = adapter_payload[1 + (9 * port) + 8]; + for (PadAxes axis : axes) { + const int index = static_cast(axis); + pad.axis_values[index] = adapter_payload[offset + 3 + index]; + } if (get_origin[port]) { - origin_status[port].stick_x = pad.stick_x; - origin_status[port].stick_y = pad.stick_y; - origin_status[port].substick_x = pad.substick_x; - origin_status[port].substick_y = pad.substick_y; - origin_status[port].trigger_left = pad.trigger_left; - origin_status[port].trigger_right = pad.trigger_right; + origin_status[port].axis_values = pad.axis_values; get_origin[port] = false; } } @@ -101,82 +95,46 @@ void Adapter::PadToState(const GCPadStatus& pad, GCState& state) { state.buttons.insert_or_assign(button_value, pad.button & button_value); } - state.axes.insert_or_assign(static_cast(PadAxes::StickX), pad.stick_x); - state.axes.insert_or_assign(static_cast(PadAxes::StickY), pad.stick_y); - state.axes.insert_or_assign(static_cast(PadAxes::SubstickX), pad.substick_x); - state.axes.insert_or_assign(static_cast(PadAxes::SubstickY), pad.substick_y); - state.axes.insert_or_assign(static_cast(PadAxes::TriggerLeft), pad.trigger_left); - state.axes.insert_or_assign(static_cast(PadAxes::TriggerRight), pad.trigger_right); + for (size_t i = 0; i < pad.axis_values.size(); ++i) { + state.axes.insert_or_assign(static_cast(i), pad.axis_values[i]); + } } void Adapter::Read() { LOG_DEBUG(Input, "GC Adapter Read() thread started"); - int payload_size_in, payload_size_copy; + int payload_size; std::array adapter_payload; - std::array adapter_payload_copy; std::array pads; while (adapter_thread_running) { libusb_interrupt_transfer(usb_adapter_handle, input_endpoint, adapter_payload.data(), - sizeof(adapter_payload), &payload_size_in, 16); - payload_size_copy = 0; - // this mutex might be redundant? - { - std::lock_guard lk(s_mutex); - std::copy(std::begin(adapter_payload), std::end(adapter_payload), - std::begin(adapter_payload_copy)); - payload_size_copy = payload_size_in; - } + sizeof(adapter_payload), &payload_size, 16); - if (payload_size_copy != sizeof(adapter_payload_copy) || - adapter_payload_copy[0] != LIBUSB_DT_HID) { - LOG_ERROR(Input, "error reading payload (size: {}, type: {:02x})", payload_size_copy, - adapter_payload_copy[0]); + if (payload_size != sizeof(adapter_payload) || adapter_payload[0] != LIBUSB_DT_HID) { + LOG_ERROR(Input, "error reading payload (size: {}, type: {:02x}) Possible disconnect?", + payload_size, adapter_payload[0]); adapter_thread_running = false; // error reading from adapter, stop reading. break; } for (std::size_t port = 0; port < pads.size(); ++port) { - pads[port] = GetPadStatus(port, adapter_payload_copy); + pads[port] = GetPadStatus(port, adapter_payload); if (DeviceConnected(port) && configuring) { if (pads[port].button != 0) { pad_queue[port].Push(pads[port]); } - // Accounting for a threshold here because of some controller variance - if (pads[port].stick_x > origin_status[port].stick_x + pads[port].THRESHOLD || - pads[port].stick_x < origin_status[port].stick_x - pads[port].THRESHOLD) { - pads[port].axis = GCAdapter::PadAxes::StickX; - pads[port].axis_value = pads[port].stick_x; - pad_queue[port].Push(pads[port]); - } - if (pads[port].stick_y > origin_status[port].stick_y + pads[port].THRESHOLD || - pads[port].stick_y < origin_status[port].stick_y - pads[port].THRESHOLD) { - pads[port].axis = GCAdapter::PadAxes::StickY; - pads[port].axis_value = pads[port].stick_y; - pad_queue[port].Push(pads[port]); - } - if (pads[port].substick_x > origin_status[port].substick_x + pads[port].THRESHOLD || - pads[port].substick_x < origin_status[port].substick_x - pads[port].THRESHOLD) { - pads[port].axis = GCAdapter::PadAxes::SubstickX; - pads[port].axis_value = pads[port].substick_x; - pad_queue[port].Push(pads[port]); - } - if (pads[port].substick_y > origin_status[port].substick_y + pads[port].THRESHOLD || - pads[port].substick_y < origin_status[port].substick_y - pads[port].THRESHOLD) { - pads[port].axis = GCAdapter::PadAxes::SubstickY; - pads[port].axis_value = pads[port].substick_y; - pad_queue[port].Push(pads[port]); - } - if (pads[port].trigger_left > pads[port].TRIGGER_THRESHOLD) { - pads[port].axis = GCAdapter::PadAxes::TriggerLeft; - pads[port].axis_value = pads[port].trigger_left; - pad_queue[port].Push(pads[port]); - } - if (pads[port].trigger_right > pads[port].TRIGGER_THRESHOLD) { - pads[port].axis = GCAdapter::PadAxes::TriggerRight; - pads[port].axis_value = pads[port].trigger_right; - pad_queue[port].Push(pads[port]); + // Accounting for a threshold here to ensure an intentional press + for (size_t i = 0; i < pads[port].axis_values.size(); ++i) { + const u8 value = pads[port].axis_values[i]; + const u8 origin = origin_status[port].axis_values[i]; + + if (value > origin + pads[port].THRESHOLD || + value < origin - pads[port].THRESHOLD) { + pads[port].axis = static_cast(i); + pads[port].axis_value = pads[port].axis_values[i]; + pad_queue[port].Push(pads[port]); + } } } PadToState(pads[port], state[port]); @@ -185,42 +143,11 @@ void Adapter::Read() { } } -void Adapter::ScanThreadFunc() { - LOG_INFO(Input, "GC Adapter scanning thread started"); - - while (detect_thread_running) { - if (usb_adapter_handle == nullptr) { - std::lock_guard lk(initialization_mutex); - Setup(); - } - std::this_thread::sleep_for(std::chrono::milliseconds(500)); - } -} - -void Adapter::StartScanThread() { - if (detect_thread_running) { - return; - } - if (!libusb_ctx) { - return; - } - - detect_thread_running = true; - detect_thread = std::thread(&Adapter::ScanThreadFunc, this); -} - -void Adapter::StopScanThread() { - detect_thread_running = false; - detect_thread.join(); -} - void Adapter::Setup() { - // Reset the error status in case the adapter gets unplugged - if (current_status < 0) { - current_status = NO_ADAPTER_DETECTED; - } - + // Initialize all controllers as unplugged adapter_controllers_status.fill(ControllerTypes::None); + // Initialize all ports to store axis origin values + get_origin.fill(true); // pointer to list of connected usb devices libusb_device** devices{}; @@ -229,8 +156,6 @@ void Adapter::Setup() { const ssize_t device_count = libusb_get_device_list(libusb_ctx, &devices); if (device_count < 0) { LOG_ERROR(Input, "libusb_get_device_list failed with error: {}", device_count); - detect_thread_running = false; // Stop the loop constantly checking for gc adapter - // TODO: For hotplug+gc adapter checkbox implementation, revert this. return; } @@ -244,9 +169,6 @@ void Adapter::Setup() { } libusb_free_device_list(devices, 1); } - // Break out of the ScanThreadFunc() loop that is constantly looking for the device - // Assumes user has GC adapter plugged in before launch to use the adapter - detect_thread_running = false; } bool Adapter::CheckDeviceAccess(libusb_device* device) { @@ -331,24 +253,14 @@ void Adapter::GetGCEndpoint(libusb_device* device) { sizeof(clear_payload), nullptr, 16); adapter_thread_running = true; - current_status = ADAPTER_DETECTED; adapter_input_thread = std::thread([=] { Read(); }); // Read input } Adapter::~Adapter() { - StopScanThread(); Reset(); } void Adapter::Reset() { - std::unique_lock lock(initialization_mutex, std::defer_lock); - if (!lock.try_lock()) { - return; - } - if (current_status != ADAPTER_DETECTED) { - return; - } - if (adapter_thread_running) { adapter_thread_running = false; } @@ -356,7 +268,6 @@ void Adapter::Reset() { adapter_controllers_status.fill(ControllerTypes::None); get_origin.fill(true); - current_status = NO_ADAPTER_DETECTED; if (usb_adapter_handle) { libusb_release_interface(usb_adapter_handle, 1); @@ -409,24 +320,7 @@ const std::array& Adapter::GetPadState() const { } int Adapter::GetOriginValue(int port, int axis) const { - const auto& status = origin_status[port]; - - switch (static_cast(axis)) { - case PadAxes::StickX: - return status.stick_x; - case PadAxes::StickY: - return status.stick_y; - case PadAxes::SubstickX: - return status.substick_x; - case PadAxes::SubstickY: - return status.substick_y; - case PadAxes::TriggerLeft: - return status.trigger_left; - case PadAxes::TriggerRight: - return status.trigger_right; - default: - return 0; - } + return origin_status[port].axis_values[axis]; } } // namespace GCAdapter diff --git a/src/input_common/gcadapter/gc_adapter.h b/src/input_common/gcadapter/gc_adapter.h index 3586c8bda..869eafeec 100644 --- a/src/input_common/gcadapter/gc_adapter.h +++ b/src/input_common/gcadapter/gc_adapter.h @@ -47,20 +47,9 @@ enum class PadAxes : u8 { }; struct GCPadStatus { - u16 button{}; // Or-ed PAD_BUTTON_* and PAD_TRIGGER_* bits - u8 stick_x{}; // 0 <= stick_x <= 255 - u8 stick_y{}; // 0 <= stick_y <= 255 - u8 substick_x{}; // 0 <= substick_x <= 255 - u8 substick_y{}; // 0 <= substick_y <= 255 - u8 trigger_left{}; // 0 <= trigger_left <= 255 - u8 trigger_right{}; // 0 <= trigger_right <= 255 + u16 button{}; // Or-ed PAD_BUTTON_* and PAD_TRIGGER_* bits - static constexpr u8 MAIN_STICK_CENTER_X = 0x80; - static constexpr u8 MAIN_STICK_CENTER_Y = 0x80; - static constexpr u8 MAIN_STICK_RADIUS = 0x7f; - static constexpr u8 C_STICK_CENTER_X = 0x80; - static constexpr u8 C_STICK_CENTER_Y = 0x80; - static constexpr u8 C_STICK_RADIUS = 0x7f; + std::array axis_values{}; // Triggers and sticks, following indices defined in PadAxes static constexpr u8 THRESHOLD = 10; // 256/4, at least a quarter press to count as a press. For polling mostly @@ -78,11 +67,6 @@ struct GCState { enum class ControllerTypes { None, Wired, Wireless }; -enum { - NO_ADAPTER_DETECTED = 0, - ADAPTER_DETECTED = 1, -}; - class Adapter { public: /// Initialize the GC Adapter capture and read sequence @@ -111,12 +95,6 @@ private: void PadToState(const GCPadStatus& pad, GCState& state); void Read(); - void ScanThreadFunc(); - /// Begin scanning for the GC Adapter. - void StartScanThread(); - - /// Stop scanning for the adapter - void StopScanThread(); /// Resets status of device connected to port void ResetDeviceType(std::size_t port); @@ -133,19 +111,11 @@ private: /// For use in initialization, querying devices to find the adapter void Setup(); - int current_status = NO_ADAPTER_DETECTED; libusb_device_handle* usb_adapter_handle = nullptr; - std::array adapter_controllers_status{}; - - std::mutex s_mutex; std::thread adapter_input_thread; bool adapter_thread_running; - std::mutex initialization_mutex; - std::thread detect_thread; - bool detect_thread_running = false; - libusb_context* libusb_ctx; u8 input_endpoint = 0; @@ -153,10 +123,11 @@ private: bool configuring = false; - std::array, 4> pad_queue; std::array state; std::array get_origin; std::array origin_status; + std::array, 4> pad_queue; + std::array adapter_controllers_status{}; }; } // namespace GCAdapter From 1e7bed0a456a6771377a6ee9c8a45afb4c2b80a2 Mon Sep 17 00:00:00 2001 From: ameerj Date: Fri, 17 Jul 2020 12:10:32 -0400 Subject: [PATCH 2/3] std::size_t where appropriate, make error message more clear if can't read --- src/input_common/gcadapter/gc_adapter.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/input_common/gcadapter/gc_adapter.cpp b/src/input_common/gcadapter/gc_adapter.cpp index 0fc609184..02d06876f 100644 --- a/src/input_common/gcadapter/gc_adapter.cpp +++ b/src/input_common/gcadapter/gc_adapter.cpp @@ -34,7 +34,7 @@ Adapter::Adapter() { GCPadStatus Adapter::GetPadStatus(std::size_t port, const std::array& adapter_payload) { GCPadStatus pad = {}; - const size_t offset = 1 + (9 * port); + const std::size_t offset = 1 + (9 * port); adapter_controllers_status[port] = static_cast(adapter_payload[offset] >> 4); @@ -77,7 +77,7 @@ GCPadStatus Adapter::GetPadStatus(std::size_t port, const std::array& ad } } for (PadAxes axis : axes) { - const int index = static_cast(axis); + const std::size_t index = static_cast(axis); pad.axis_values[index] = adapter_payload[offset + 3 + index]; } @@ -112,7 +112,8 @@ void Adapter::Read() { sizeof(adapter_payload), &payload_size, 16); if (payload_size != sizeof(adapter_payload) || adapter_payload[0] != LIBUSB_DT_HID) { - LOG_ERROR(Input, "error reading payload (size: {}, type: {:02x}) Possible disconnect?", + LOG_ERROR(Input, + "Error reading payload (size: {}, type: {:02x}) Is the adapter connected?", payload_size, adapter_payload[0]); adapter_thread_running = false; // error reading from adapter, stop reading. break; From 68d6d3e1738f022aa0915d70b8d0c1eb567d0ceb Mon Sep 17 00:00:00 2001 From: ameerj Date: Sun, 19 Jul 2020 11:49:26 -0400 Subject: [PATCH 3/3] Fix axis thresholding while polling axes were very sensitive when mapping controls. --- src/input_common/gcadapter/gc_adapter.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/input_common/gcadapter/gc_adapter.h b/src/input_common/gcadapter/gc_adapter.h index 869eafeec..bed81915c 100644 --- a/src/input_common/gcadapter/gc_adapter.h +++ b/src/input_common/gcadapter/gc_adapter.h @@ -49,11 +49,8 @@ enum class PadAxes : u8 { struct GCPadStatus { u16 button{}; // Or-ed PAD_BUTTON_* and PAD_TRIGGER_* bits - std::array axis_values{}; // Triggers and sticks, following indices defined in PadAxes - static constexpr u8 THRESHOLD = 10; - - // 256/4, at least a quarter press to count as a press. For polling mostly - static constexpr u8 TRIGGER_THRESHOLD = 64; + std::array axis_values{}; // Triggers and sticks, following indices defined in PadAxes + static constexpr u8 THRESHOLD = 50; // Threshold for axis press for polling u8 port{}; PadAxes axis{PadAxes::Undefined};