Usb core fixes

[NicoHood]

Backported a few fixes from https://github.com/NicoHood/HID

• Flexible, global Endpoint size (USB_EP_SIZE)
• Made Magic Key selection more flexible
• Added some useful definitions
• Minor fixes (e.g. HID reports >255 supported now)
• Added u2 Series support

Replaces older
#3562

[NicoHood]

@ArduinoBot build this please

[matthijskooijman]

@ArduinoBot, build this please.

[ArduinoBot]

Build successful. Please test this code using one of the following:
http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-3640-BUILD-375-linux32.tar.xz
http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-3640-BUILD-375-linux64.tar.xz
http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-3640-BUILD-375-windows.zip
http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-3640-BUILD-375-macosx.zip

[facchinm]

Some quick comments:

• magic handling maybe needs a PR on its own
• inlining Recv has any drawback? I would not merge it blindly, need some test
• 16 byte endpoint seems ok
• u2 support seems ok (but I cannot test it for two weeks)
• HID reports > 255 bytes will not work, as the macro is only called once with an uint8_t parameter. Will merge if complete and tested with a descriptor larger then 255 byte

[NicoHood]

Thx for the comments

• Why a different PR? It is in a special commit. If the commit is not okay, suggest something to improve.
• It always worked for me. Just looks like a typo. Its only called in the same file so there should not be a problem. There is also no header definition for this function.
• Good
• u2 support works for me and for a lot of other people. I did not change anything from my HID-Project which worked for month now. If it would not work there is no negative thing for normal Arduino users anyways. Its like additional Attiny support. Feel free to checkout HoodLoader2 though :)
• oh right you changed this. We could keep it like this or make the descriptor length uin16_t. This adds one byte but would make some special descriptors work. I have to admit that its very unlikely to happen but with some gamepad configs I exceeded this limit. But then other OS problem occur with too many interfaces. So that decision is up to you.

[facchinm]

• I was thinking about a different PR because I wanted to merge this directly, but that commit is a bit out of scope and maybe needs some more discussion (first of all we need to decide if we'll ever ship a bootloader with the bugfix)
• Recv is surely static; I was concerned about inlining it but I'm doing some test and I don't see any growth in RAM usage, so it is ok for me
• I've also performed some test with uint16_t and madly long descriptors and it seems to work well after another patch, so I'm 👍 for including also this

[facchinm]

The patch for descriptors > 255 is here if you want to give it a try 😉

[NicoHood]

Feel free to discuss it now. Not all MCUs have a 0x8000 ram position, so the u2 Series would be totally incompatible without that fix. And for my Bootloader I changed the address to RAMEND now.

I dont want to start a discussion here about a new bootloader or not which keeps the PR from being merged. I want to make the PR the way that if you do want a new bootloader, you are able to change the key very easy. You could also think about adding HL2 as bootloader https://github.com/NicoHood/HoodLoader2/tree/fast-usbserial If you ship a new bootloader or not can be discussed later.

We could make the definition only point at the 0x8000 (without the (uint16_t) thing) so you can use a global boards.txt definition to pass the new bootkey location (extra_flags). Then you can have a IDE submenu for choosing old vs new bootloader. I made it this way to be able to also use 8bit bootkeys since this saves some bytes on the u2 Series.

But since its not so heavy I'd probably go for a definition of the position itself and if it should be backed up or not. Or maybe simply back it up if its unequal RAMEND so simplify it. (updating soon, so you can see what I mean).

[NicoHood]

Updated the commits, now you can add via board.txt:

HoodLoader2atmega16u2.build.extra_flags={build.usb_flags} -DMAGIC_KEY_POS=(RAMEND-1)

Very simple to change the position now and to fix this issue. No additional variants file is even needed.
That seems good to me, you agree?

[NicoHood]

Thanks so much!

One thing I want to mention is that this is still incorrect:
https://github.com/NicoHood/Arduino/blob/60e64cfd353fa61bf792e0477183d6ca4d4bf272/hardware/arduino/avr/cores/arduino/USBCore.cpp#L70
The problem is that we need to detect if HID is used or not etc. If its an CDC device only the number must be different than if its CDC + HID. It works still with wrong values but is not USB conform.

I'd now focus on the HID libraries. There is also stuff to improve.