Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GC USB Adapter configuration improvements #7291

Merged
merged 5 commits into from May 30, 2019

Conversation

VinDuv
Copy link
Contributor

@VinDuv VinDuv commented Jul 28, 2018

Some improvements to the GameCube USB adapter configuration process:

  • If opening the adapter fails, unplugging and replugging it will make Dolphin try again to open it.
  • The adapter configuration dialog will display an error message (coming from libusb) if an error occurs when opening the adapter, instead of displaying “No Adapter Detected”.
  • The adapter configuration dialog will refresh if the adapter state changes. This was (partially I think) implemented in the Wx backend but not the Qt backend.

Caveats:

  • On systems without libusb hotplug, if the open process fails, it will be retried every half-second until it works or the controller is unplugged (On systems with libusb hotplug it will wait for it to be unplugged and replugged before trying again). Unplugging a successfully opened controller on these systems will not be detected until something reads from the controller. (The current system also has this issue)
  • The error message from libusb is not translated. libusb has a couple of built-in translations, which can be activated by calling libusb_setlocale with a locale string. I did not implement this because I wasn’t sure where to put the call and how to get the current locale in a cross-platform manner.

Source/Core/Core/Analytics.cpp Outdated Show resolved Hide resolved
Source/Core/InputCommon/GCAdapter.cpp Outdated Show resolved Hide resolved
Source/Core/InputCommon/GCAdapter.cpp Outdated Show resolved Hide resolved
}

if (error_message)
*error_message = libusb_strerror((enum libusb_error)s_status);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static_cast<libusb_error>(s_status)

@VinDuv
Copy link
Contributor Author

VinDuv commented Jul 30, 2018

Updated PR; I fixed the issues (except for the use of named constants in GCAdapter.cpp, for now) and added a commit to update GCAdapter_Android.cpp; that should hopefully fix the Android port.

@VinDuv
Copy link
Contributor Author

VinDuv commented Jul 31, 2018

Updated PR:

  • Added named constants for non-error adapter status
  • Replaced call of QMetaObject::invokeMethod with QueueOnObject (should also fix build issues with earlier Qt versions)

Copy link
Member

@leoetlino leoetlino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few minor comments.

Your commit titles should be modified to use the infinitive form, e.g. "GCAdapter: Better error reporting" -> "GCAdapter: Improve error reporting" or something like that.

Source/Core/InputCommon/GCAdapter.cpp Outdated Show resolved Hide resolved
Source/Core/InputCommon/GCAdapter.cpp Show resolved Hide resolved
Source/Core/InputCommon/GCAdapter.cpp Outdated Show resolved Hide resolved
Source/Core/InputCommon/GCAdapter.cpp Outdated Show resolved Hide resolved
@VinDuv
Copy link
Contributor Author

VinDuv commented May 26, 2019

Fixed some issues and rebased this PR on top of PR #8070 as they had conflicting changes in GCAdapter.cpp. Will rebase on master when #8070 is merged.

@leoetlino
Copy link
Member

It looks like you forgot the libusb error enum change.

@leoetlino leoetlino dismissed their stale review May 27, 2019 19:09

Will re-review after #8070 is merged.

@leoetlino
Copy link
Member

Could you rebase this again please?

The handle was previously kept open, which was causing future adapter
plug/unplug events to be ignored.
If opening the adapter fails, report the libusb error message in the GUI
instead of “No Adapter Detected”.

The error condition is removed when the adapter is unplugged.
Update the GC adapter config GUI if the adapter is plugged or unplugged.
Detect when the setup function found no adapter, or found one but could
not connect to it, and report the new status in that case.
Fix the Android version of GCAdapter.cpp so it matches the new definitions in GCAdapter.h.
@VinDuv
Copy link
Contributor Author

VinDuv commented May 29, 2019

Done. I’ve also removed the extraneous enum in the static_cast.

@leoetlino leoetlino merged commit 8d8ed37 into dolphin-emu:master May 30, 2019
@VinDuv VinDuv deleted the gc-adapter-error-report branch May 30, 2019 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants