Ignore IMU-only Controllers in GUI

[Fredrum]

Hi Florian!

• I discovered that the DS4 actually needs two SDL joysticks, one for IMU/Motion and one for standard inputs.

• It was confusing having 2x Chiaki Controllers, 2x SDL Controllers and 2x SDL_Joysticks for the DS4 so for this pr I have changed the 'controller' names to 'chiaki_controller' and 'sdl_controller' to be able to immediately see which is which.

• I'm trying to channel everything through the Events now. Even the initial startup controller setup.

• SDL keeps and uses a no-reuse and continually increasing 'instance_id' for joysticks. I have been using that whenever possible instead of SDL_NumJoysticks(). I'm using that fact to avoid having to re-order lists when controller un-plugs and re-plugs. I just keep going with the higher numbers. This saves me some complexity.

• There's still something that causes a segfault every 3rd or 4th time I pull out the controller usb cord while playing. I'm guessing some pointer thing I'm doing wrong. Let me know if you spot it but I'll keep trying to find it too so that can be solid.

• I don't need to 'push' right for you to see this work? There should only be 1 commit with this pr. Four files.

---

Update,

so apart from being able to filter off the Motion control Events from the secondary DS4 controller I also wanted to get to a place where I didn't have the instability I had before. See https://github.com/thestr4ng3r/chiaki/issues/384 and https://github.com/thestr4ng3r/chiaki/issues/383

I don't get those problems now with this system. I believe the problem was that since it wasn't handling the two sub-devices random things happened when one unplugged a controller and plugged back in. Sometimes it got stuck in that motion sensor spamming mode and sometimes I couldn't reconnect with a controller at all.

SO this system was a result of me just reading about how people used SDL for controller handling. For example to handle all through the Events.

But basically the main things I wanted to achieve was to get rid of the buggy behavior described in those two 'issues' tickets. And then when I realized about the two SDL controllers for the DS4 it made sense to do a filter for that.

Fix #383 Fix #384

[Fredrum]

Ok I hope and think, fingers crossed, that this one should be good? Just let me know if it still isn't?

Iv'e been watching about re-basing and since they seemed to say that it was dangerous to do against a remote directly I have done it via a local master that I pulled from your master just now.

Then rebased my feature(rpi01) and my master against each other.

I did get some strange merge conflict that I wasn't able to get rid of without a push --force. That was in settingsdialog.cpp and seemed to be just a single whitespace thing. I even tried copying and pasting the entirety of your file since I hadn't worked in it.

[thestr4ng3r]

Nope, still messed up. You can easily check this yourself. For example here on the pr on github, it should only show the commits you added yourself and the diff should also only contain these changes.

Iv'e been watching about re-basing and since they seemed to say that it was dangerous to do against a remote directly I have done it via a local master that I pulled from your master just now.

I wouldn't consider rebasing against a remote branch to be dangerous, there isn't really any difference in whether the branch to rebase on is remote or not. Assuming you have git@github.com:thestr4ng3r/chiaki saved as for example upstream and git@github.com:Fredrum/chiaki as origin and you are currently on your branch that has your changes, you can do git fetch upstream, then git rebase -i upstream/master and it will show you a list of commits that it would put on top of upstream/master. Delete all the lines that are not exactly your commits, save and quit and you will have a nice history. After this, force push is naturally necessary because the history is different.

[Fredrum]

Ok, now the four files I have changed are the same ones I see under the PR 'Files Changed' and it looks like the stuff Iv'e done too.

I think I got rid of the segfault thing too.

It's quite a radical change from how it was before so I'm hoping I haven't broken anything that I couldn't test. Like on the Android side or anything like that?

[thestr4ng3r]

Yes, now the history is perfect. Could you please edit the pr description with a short and concise explanation of what exactly you are trying to achieve? All I can tell is that you would like to filter out the imu controller and I think this could be done by just adding approximately three lines in UpdateGamepads() so I'm not sure what all the changes are for.

[Fredrum]

Just added another bit to this OP. Let me know if that's not clear or you have questions?

[thestr4ng3r]

Thanks, I see now. So what I would do before considering all these changes would be to make a clean new branch from the current chiaki master and only apply the filtering exactly before this line: https://github.com/thestr4ng3r/chiaki/blob/777c3a649a7bfb0108bb5610d381c8679e2b75be/gui/src/controllermanager.cpp#L59 and check if that can already fix all of your issues. The filter should however not use SDL_JoystickName() as that can be abitrary info. SDL_JoystickGetGUID() et al. seem fitting here.

[Fredrum]

Just tried checking the GUIDs but unfortunately they are the same for both sub-controllers,

GUID Name:  030000004c050000c405000011810000
[I] Connected: Sony Computer Entertainment Wireless Controller
GUID Name:  030000004c050000c405000011810000
[I] Connected: Sony Computer Entertainment Wireless Controller Motion Sensors

But I see that for the DS4 it returns '0' (zero) from, int SDL_JoystickNumButtons(SDL_Joystick* joystick)

I'm not sure if that's 100% safe to use as an identifier either, but do you feel that would be better?

[thestr4ng3r]

I would combine checking the guid and NumButtons and only if both criteria match, ignore the controller. Did you also have the chance to check how the guids look for controllers that are not the DS4, or another DS4 of the same generation? So we know that our check both doesn't cover more controllers than we want and it is not specific to only the controller that you own. Also, maybe SDL_JoystickGetDeviceGUID() is interesting too.

[Fredrum]

HI so the GUID's are not telling me anything about if the controller has a separate SDL controller motion component. Both my ds4 pads are the same model and have the same guid. I have two xbox one controllers that have different guid from the ds4's but the same as each other. That command SDL_JoystickGetDeviceGUID() gave me the same guid as the other command. There's a long community list that is kept up to date here where you can see loads of guid's and what they are, https://github.com/gabomdq/SDL_GameControllerDB/blob/master/gamecontrollerdb.txt

So now I'm only checking against '0 buttons'. I think this is probably something that can be added to if people are having trouble with some other controller that is not following the same pattern. I guess we could make a list with all the guid's that have the Motion Sensors and add a lookup to that and combine with the button check? We could include all those PS4 controllers to start with and probably the PS3 dual shock ones too.

And your suggestion seems to work fine! :) Much less code change. Although I learned quite a bit about SDL controllers doing all that other stuff.

And because this was now such a small edit Iv'e thrown in two other things that I wanted to also do after the controller fix.

1. Add a double click action for the stream window. I'm wanting to be able to use only a gamepad, no KB+M, to start a game session on the Pi. And that bit was missing. With this I can use the DS4s touchpad to double click to toggle full screen. Similar to VLC or Netflix.

2. The Audio menu had ended up after the 'About Chiaki' menu and it irked me. So I just moved that up one step.

[thestr4ng3r]

Yes, checking against a set of guids sounds good to me. Please move the other changes to separate prs. If possible, individual pull requests should be as small as possible.

[Fredrum]

Just re-did this one with just the SDL bits, including a guid lookup.

[Fredrum]

With the other two PRs, do you need them separated out? So each ontop of your Master? Or can they be gradually appending each other?

[thestr4ng3r]

With the other two PRs, do you need them separated out? So each ontop of your Master? Or can they be gradually appending each other?

PRs should always be based on the target branch they should be merged into, which in this case is master here. Except in cases where the contents of the PR depend on something else but that is not the case here. So yes, each independently on top of master.

[Fredrum]

Ok that should be it now!? (I tested it as well)