thestr4ng3r / chiaki

Moved to https://git.sr.ht/~thestr4ng3r/chiaki - Free and Open Source PS4 Remote Play Client
https://git.sr.ht/~thestr4ng3r/chiaki
2.17k stars 373 forks source link

fixed issue where random controller is selected each session #380

Closed tomblind closed 3 years ago

tomblind commented 3 years ago

This PR fixes an issue where, when multiple controllers are found by SDL, a random one is selected each session.

I was surprised by the cause of his bug. While QSet doesn't guarantee any order, usually the hashing algorithms for these kinds of containers tend to be deterministic. But in my case (on Raspberry Pi), even though the controllers from SDL were inserted in the same order every time, the first controller pulled out was completely random between sessions.

It's possible issues like #287 are actually related to this. In my case, I had an additional device attached which was picked up by SDL as a controller, even though it wasn't one. So initially it seemed like the controller stopped working randomly.

Fredrum commented 3 years ago

Maybe also one of the issues I posted last night? https://github.com/thestr4ng3r/chiaki/issues/384 Although I don't think I have anything else plugged that should be confused for a controller. Only M+KB

Fredrum commented 3 years ago

Hi @tomblind the thing I'm just looking at might be meshing in with what you're doing here.

The problem I'm seeing with the 'spamming' Gyro/Accel input seems to have something to do with that SDL handles motion data as an additional Joystick that gets initialized. So for a DS4 Controller there will be two Joysticks, one for the regular input and one for the motion input.

And this is why I was seeing motion data on the stick x/y member variables.

I'm just trying to figure out how to separate them out.

Also it seems like the motion one won't actually get activated until there's a signal threshold that's been crossed. It will be registered, but not activated. Something like that.

@thestr4ng3r I don't think the current Chiaki controller handles motion data (consciously) is that correct? Since this is currently causing a fair bit of problems when I'm playing should we just try to mute the motion signal for now or how would you like to proceed?

EDIT: There's some work on SDL that seems close to making it in but for the Pi version at least I'm not sure I'd like to wait for them as they seem to be taking their time sorting this out: https://bugzilla.libsdl.org/show_bug.cgi?id=4815

Fredrum commented 3 years ago

So far I can only match with the name but I saw a post with a different controller that had the same match string. (https://discourse.libsdl.org/t/additions-i-intend-to-make-to-the-gamecontroller-system/26654/3)

You can try this out: I'm testing different checks to see if there's anything other than the name that can be used. Looking in hid-sony.h it might be that the kernel driver adds that on so potentially could be ok.

void ControllerManager::UpdateAvailableControllers()
{
#ifdef CHIAKI_GUI_ENABLE_SDL_GAMECONTROLLER
    QSet<SDL_JoystickID> current_controllers;

    for(int i=0; i<SDL_NumJoysticks(); i++)
    {   
        printf("\nN Joysticks:  %d\n\n", SDL_NumJoysticks());
        const char name[256] = "";
        const char* pName = name;
        pName = SDL_JoystickNameForIndex(i);
            printf("JS Instance Name for %d:  %s\n", i, pName);

        //SDL_GameController* ctlr = SDL_GameControllerFromInstanceID(i);
        SDL_GameController* ctlr = SDL_GameControllerOpen(i);
        SDL_Joystick* joy = SDL_GameControllerGetJoystick(ctlr);
        bool attch = SDL_JoystickGetAttached(joy);
            printf("JS %d attach state:  %d\n", i, attch);
        int nAxes = SDL_JoystickNumAxes(joy);
            printf("JS %d N Axes:  %d\n", i, nAxes);

        QString ctrl_name(pName);
        QString ps4_motion_match("Motion Sensors");
        if(ctrl_name.contains(ps4_motion_match, Qt::CaseInsensitive))
            printf("Joystick %d is actually Motion Sensors\n", i);
        else
            printf("Joystick %d is Not motion sensors\n", i);
thestr4ng3r commented 3 years ago

@tomblind Is your issue fixed after the lastest change on master?

tomblind commented 3 years ago

@thestr4ng3r I'll give it a try later, but looking at that commit I don't think it will address my issue. That patch is specifically ignoring certain controllers with no buttons. In my situation, I had a different device (a custom controller using a TeensyLC) which showed up as multiple controllers, each with buttons.

This issue is actually easy to reproduce simply by plugging in multiple real controllers. I tested with an xbox and ps4 controller plugged in at the same time, and each session a random one was selected.

thestr4ng3r commented 3 years ago

Ok. I think it will be better to just connect all controllers instead of changing the data structure here.

tomblind commented 3 years ago

Just to confirm: I tested the latest changes with 2 regular controllers (an xbox and ps4 controller) and the issue still exists.

I'll modify this PR to connect all valid controllers and we'll see how that works.

tomblind commented 3 years ago

@thestr4ng3r Ok, I've updated the PR to connect all controllers.

thestr4ng3r commented 3 years ago

Tested and works, thanks!