Reduce number of pre-defined Gamepad instances
Opened this issue · 14 comments
HID/src/SingleReport/SingleGamepad.cpp
Lines 149 to 152 in 7e37414
HID/src/SingleReport/SingleGamepad.h
Lines 50 to 53 in 7e37414
I think it would be better to reduce the number of pre-defined single-report Gamepad instances to 2.
Having 4 does not make much sense IMHO as even on the most capable 32U4 there are only 3 usable endpoints, so besides barring the possibility to use the Gamepad with one or two different devices, the 4 instances cannot even show up all together.
Having two - or even a single one - is not a limitation, since additional devices can be declared in user sketches, if more are needed.
They cannot be declared in the user sketch, since they need a different id, or am I wrong?
But you are right. with CDC Serial only 3 are possible at maximum. But What I suggest is, that we document on how to disable the CDC Serial somewhere in the wiki. Cause I've seen people asking for more controllers, so I guess it is okay to have them.
What is the negative effect of having more than 2? Do they consume more memory? I think not, correct me if I am wrong.
Uhm... I cannot test right now, but as the declaration is just a simple SingleGamepad_ GamepadX, I don't see any problems with declaring them in user sketches.
Documenting how to disable the CDC serial is a good idea (there are some hints here btw), but still it sounds odd to me to ship with something that requires fiddling with the Arduino source files to be usable. If someone is smart enough to do that, I'm sure they can also add/uncomment a few lines in the HID Library to enable as many Gamepads as they want (but still, I'm pretty convinced the latter can be done fully in the user sketch).
On the contrary, a beginner expects the library to work out of the box, so maybe they start using a Gamepad, then add a keyboard which won't show up at all, to their surprise. This is because even if only Gamepad1.begin() is called, all the 3 available endpoints will instantly get used by Gamepads.
This is because even if only Gamepad1.begin() is called, all the 3 available endpoints will instantly get used by Gamepads.
I dont think this is true. If that is the case, we'd have problems since the last 3-4 years or so.
Exactly, I tested it and that's my experience.
That's what's causing the problems in #139, I suspect (My logs are in that issue).
Can this be solved by creating extra files with:
SingleGamepad1.cpp (and 2,3,4)
#include "SingleGamepad.h"
SingleGamepad_ Gamepad1; // and 2,3,4Then the compiler should only create those instances if they are referenced.
Uhm... I cannot test right now, but as the declaration is just a simple
SingleGamepad_ GamepadX, I don't see any problems with declaring them in user sketches.
I have tested this and it works fine. Still haven't had the time to test the rest though.
Please dont disable the extra end points. I also think documenting how to disable cdc is best.
I also would love to see the multi-report endpoint working a multiple gamepads. why limit the multi-report end point to just one gamepad?
@alijani1 I think (but its been a long time) that it just did not work under windows in multireports.
@alijani1 Can you help testing what @NicoHood proposed above?
If that doesn't work, I am still convinced only 1/2 instances should be predefined. Then we would have docs saying "you can have up to 5 if you do the following: 1. disable cdc; 2. enable more in the lib", with the latter that could be made into just enabling a #define in HID-Settings.h.
The point is: not everybody wants to use 5 gamepads or 0. For instance a board of mine needs 2 gamepads, keyboard and consumer. As it is now, as soon as I init the first gamepad, all the rest disappears.
@alijani1 I think (but its been a long time) that it just did not work under windows in multireports.
Perhaps it was windows 7 or 8. I use other libraries with windows 10 and multi-report gamepad descriptors work fine on Windows 10. In linux there is an issue as Linux only supports multiple HIDs on a single usb natively only if the quirk flag HID_QUIRK_MULTI_INPUT is set in its hid-ids list. hid-quriks.c has all the VID/PID combinations that have the flag set. if you dont use one of the preset ones, you have to set the usb quirk flag manually in start up script.
@alijani1 Can you help testing what @NicoHood proposed above?
I will try, but I know in my case, I was able to use keyboard HID functions even after I do gamepad1.begin. I have to double check as I may have disabled gamepad3 in that project. Will let you know once I get to review and also try singlegamepad extra files.
Can this be solved by creating extra files with:
SingleGamepad1.cpp (and 2,3,4)
#include "SingleGamepad.h" SingleGamepad_ Gamepad1; // and 2,3,4Then the compiler should only create those instances if they are referenced.
I have finally tested this myself - not thoroughly though - and it seems to work fine. PR submitted (shouldn't hurt in any case).
It looks like the ability to disable the CDC serial has finally been merged into the Arduino mainline. Let's hope for a release soon.