On Tue, Jan 15, 2019 at 12:49 AM Roderick Colenbrander <thunderbird2k@xxxxxxxxx> wrote: > > Hi Daniel, > > Thanks for sharing your driver. Benjamin already shared some feedback, > since I didn't want to divert from his message, let me reply > separately. > > I'm not sure if this is your first driver or not, but the driver was a > bit hard to review due to the amount of code. One suggestion would be > to split up the driver in more patches. For example Pro controller > could have been its own patch (it is a new device), maybe even > calibration could have been its own patch on top of your "base" > driver. Hi Roderick, Thank you for the feedback. Yes, this is my first driver submission. I'll try to keep future submissions/revisions more split up. > > The Joycons have LEDs, I would expose these through sysfs through LED > class. Other drivers e.g. hid-sony do the same. Allows userspace to > override whatever "player" number. You can of course default to a > player id based on connection order. Hypothetical applications > supporting DualShock4, Xbox 360 gamepad and Switch controllers at the > same time, may want to set cross gamepad controller numbers. > Very cool. I will definitely add that. > At this point the design of the driver is most important, but there > are some style issues as well. Some of the ones I noticed: > - spacing between variable and assignment "int a = 1234;" instead of > "int a = 1234;" > - mixedCase variables (newVal) > Noted, thanks. -Daniel
