Add HID OUT Report to the Pluggable HID (Keyboard Leds)

[NicoHood]

It would be nice to be able to check the keyboard leds with the pluggable HID. This would also make RAW HID possible as well (receiving from the Host).

Good Resources:
https://github.com/NicoHood/HID/blob/345dc26257238f8d361d1bed4b56d9d2114c4e99/avr/cores/hid/USB-Core/HID.cpp#L152-L166
https://github.com/NicoHood/HID/blob/345dc26257238f8d361d1bed4b56d9d2114c4e99/avr/cores/hid/USB-Core/HID.h#L172-L218
https://github.com/PaulStoffregen/cores/blob/master/teensy3/usb_dev.c#L483-L488

The code above should better use the wIndex value:
https://github.com/abcminiuser/lufa/blob/master/Demos/Device/LowLevel/KeyboardMouse/KeyboardMouse.c#L146-L156

Also see this:
https://github.com/abcminiuser/lufa/blob/master/Demos/Device/LowLevel/GenericHID/GenericHID.c#L148

This needs to be implemented here:
https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/libraries/HID/HID.cpp#L126-L130
https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/USBAPI.h#L160-L168

https://github.com/abcminiuser/lufa/blob/master/LUFA/Drivers/USB/Class/Device/HIDClassDevice.c#L54

Maybe we could change the pluggable HID this way, that you pass the whole class (this) as pointer and make a function getDescriptor() with the friends attribute, that only the HID class can access it and nothing else? This way we do not need to save pointers for every value (like reportID etc) and also the HIDDescriptor pointer would be not needed anymore. That would require a parent class with virtual functions.
https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/libraries/HID/HID.h#L47-L57

CC @facchinm

Also a quick question:
Why didnt you put extern HID_ HID; inside the HID header, so that the other libraries can include this and do not need to dupe this code? Is it because the user should not accidentally see the HID instance?

Edit: I am currently working on a patch! Updates coming soon!

[NicoHood]

This function was successfully added here:
NicoHood/HID@f34df6e

Notice that I removed the nodes and added the whole HIDDevice class as "node". This gives us the option to read more data (reportID etc) and execute virtual functions such as the setReportData.

This method also uses less flash and less ram. The virtual function adds something back again, so it doesnt matter much.

The static bool HID_Setup(USBSetup& setup, u8 i); function was made static and moved to the HID_ class.

SendReport can now be used via inherited parent. The device no longer need to see the HID instance. This also avoids mistakes in a wrong reportID since its taken by the internal saved ID.
We could also make this function public for advanced users. I'd really go for it.
NicoHood/HID@f34df6e#diff-c8a36f23ff4aeb515a92b4396176d429R54

The saved reportID could also help you to generate a unique ISERIAL ID. That problem solved as well. (or at least given an option to do so)
#3811

Some data in the HIDDevice is public to access it via the linked list pointer. It is inherited private, so the user cannot accidentally access this data. If desired one could add getters for this data but I dont think that someone what to read it, its known at compiletime via #define. This fix was added in a later commit.

Most HID_ class variables and functions are now made static. I hided them from the user by making them private and a friend of the HIDDevice. This way noone can access this, even though noone ever will. Just for the purpose of good code. But we could then remove the instance and use HID_::variable/function to call those static members instead of a singleton we need to add.

I'd like to have a little feedback about the return value here. Does it have to be true?
NicoHood/HID@f34df6e#diff-51a1e639cd8cab75924bf63440fa2a7aR165
You have to: NicoHood/HID@7f407fc

An example for testing is provided.
RawHID should be possible to add now, but it requires (at least under windows) a single report. This means you cannot use it via reportID. This needs a few pluggable HID changes to get this working again properly, but its not much to do. Or you dont use the pluggable HID since its the only thing to "plug" and you could code this statically via a different entpoint/interface. But you get the idea how to implement the report out feature (like the leds).

You should be able to copy the while "HID" folder in my repo. Every modification should be compatible with the internal Arduino library. You need to update the Keyboard and Mouse API though. Example in this commit:
NicoHood/HID@78c763e
Or this from scratch with the real Mouse API: NicoHood/HID@f357dfc

Feedback appreciated.
CC @facchinm @cmaglie

[NicoHood]

I want to note, that my code now also supports the boot protocol for keyboard. More infos here:
NicoHood/HID#34 (comment)

[facchinm]

Hi Nico, I took a look at your code and wow, some fixes are very clever (the HIDDevice base class for example).

However, consider that HID base library is platform-dependant but Mouse and Keyboard (which are included in the base examples) are not so every change to HID must be backported to all supported platforms.

In fact, the main purpose of all the PluggableUSB stuff is to enable devs who want to hack with USB stuff to do this easily without touching the core.

From my point of view you are doing a great job by pushing the limits of this implementation, but since you can achieve all this without touching the core I'd suggest you to publish your library (which will, in fact, become the de facto USB library) in the Lib Manager while keeping the base HID "dumb" but easy.

If you will need, at some points, more invasive changes I'll be glad to review and merge them 😉

[NicoHood]

Sure. I am aware that you'd have to change the whole Pluggable HID again. Since 1.6.6 is not out there, it would still be a chance to do so. That would save me the wrapper/copy.

However I think I will always reach a limit where I do need to change the HID library. For example I will maybe add different HID devices (via new endpoint instead of multireport) to get things like RAW HID working. There is still more stuff to do and to fix. Maybe some fixes can be backported. But you are right, the base library should be kept simple and working. I just thought that some cosmetic things could be changed in the base HID as well, to not have two whole different HID structures. For example my HID Project will now only work for AVR because of this reason.

I will publish my library to the library manager when its time to. The current real problem is, that I need .a linkage for my library to work. Right now I use a custom patched IDE. But this IDE is getting older and older, since we now have a new compiler (written in GO).

I am more or less waiting for those two PRs to be reimplemented in the new compiler:
#3697
#3757

I really need help with this, if you want to see the HID-Project in a non beta state.

[NicoHood]

Closing this as it just adds some overhead that most people dont need. BootKeyboard in the current HID Project now supports such things. You just cant create an API with tons of features but keep it small, so thats fine like this.