Add support for TinyUSB

[Spegs21]

This creates other issues with missing identifiers due to differences in HID libraries and USBAPI.h between architectures.

[Spegs21]

Issues created:

RawHID.cpp

• no instance of constructor "arduino::PluggableUSBModule::PluggableUSBModule" matches the argument list -- argument types are: (int, int, uint8_t [1])
• identifier "EP_TYPE_INTERRUPT_IN" is undefined
• identifier "TRANSFER_PGM" is undefined
• identifier "HID_REPORT_TYPE_FEATURE" is undefined
• identifier "HID_REPORT_TYPE_OUTPUT" is undefined

RawHID.h

• identifier "TRANSFER_RELEASE" is undefined

[Spegs21]

I forgot the parenthesis and killed Uno/Mega sketches.

[Spegs21]

This now compiles for Arduino Zero (Native USB Port) when I comment out the EEPROM references. It might be kind of hacky but I didn't know how else to do it. You'll want to check it over.

[Spegs21]

I got this to build for a Adafruit Trinket M0 target but even more changes needed made, changes that conflict with the Arduino SAMD core.

Adafruit doesn't define Serial as a Uart interface. It's defined in their USBAPI.h instead. Also their PluggableUSBModule constructor wants a uint32_t * epType instead of unsigned int * like the Arduino SAMD core.

[Legion2]

I created an issue in the arduino core api repo arduino/ArduinoCore-API#141

[Spegs21]

I also have a PR open for the missing HID defines in the SAMD core arduino/ArduinoCore-samd#602

[Spegs21]

I purchased a Trinket M0 and created the board definition for it. I was able to upload the sketch for Commander Core. Windows recognizes it as Commander Core when installing the driver but it doesn't show as Commander Core in the device manager. It shows as an 'HID compliant vender-defined device' but has the correct VID and PID. It doesn't show in iCUE either. Here are the logs I can find referencing it.

2021-02-22T13:16:12 I cue.dev.hidenumerator: Found device VID 6940 PID 3098 Usage Page 65472 Usage 3072
2021-02-22T13:16:12 I cue.dev.mgr: Candidate device for QMap(("bus", "hid")("devInstanceId", "hid\vid_1b1c&pid_0c1a&mi_02\7&19b9f7ab&0&0000")("hid-direction", "input")("hid-type", "report")("hid-usage", "3072")("hid-usagepage", "65472")("model-name", "")("usb-pid", "3098")("usb-vid", "6940")("vendor", "")) not found.
2021-02-22T13:16:12 I cue.dev.mgr: No manifest for QMap(("bus", "hid")("devInstanceId", "hid\vid_1b1c&pid_0c1a&mi_02\7&19b9f7ab&0&0000")("hid-direction", "input")("hid-type", "report")("hid-usage", "3072")("hid-usagepage", "65472")("model-name", "")("usb-pid", "3098")("usb-vid", "6940")("vendor", ""))
2021-02-22T13:16:12 I cue.dev.mgr: Candidate device for QMap(("bus", "hid")("devInstanceId", "hid\vid_1b1c&pid_0c1a&mi_02\7&19b9f7ab&0&0000")("hid-direction", "input")("hid-type", "report")("hid-usage", "3072")("hid-usagepage", "65472")("model-name", "")("usb-pid", "3098")("usb-vid", "6940")("vendor", "")) not found.
2021-02-22T13:16:12 I cue.dev.mgr: No manifest for QMap(("bus", "hid")("devInstanceId", "hid\vid_1b1c&pid_0c1a&mi_02\7&19b9f7ab&0&0000")("hid-direction", "input")("hid-type", "report")("hid-usage", "3072")("hid-usagepage", "65472")("model-name", "")("usb-pid", "3098")("usb-vid", "6940")("vendor", ""))
2021-02-22T13:16:12 I cue.dev.mgr: Candidate device for QMap(("bus", "hid")("devInstanceId", "hid\vid_1b1c&pid_0c1a&mi_02\7&19b9f7ab&0&0000")("hid-direction", "output")("hid-type", "report")("hid-usage", "3072")("hid-usagepage", "65472")("model-name", "")("usb-pid", "3098")("usb-vid", "6940")("vendor", "")) not found.
2021-02-22T13:16:12 I cue.dev.mgr: No manifest for QMap(("bus", "hid")("devInstanceId", "hid\vid_1b1c&pid_0c1a&mi_02\7&19b9f7ab&0&0000")("hid-direction", "output")("hid-type", "report")("hid-usage", "3072")("hid-usagepage", "65472")("model-name", "")("usb-pid", "3098")("usb-vid", "6940")("vendor", ""))
2021-02-22T13:16:12 I cue.dev.mgr: Candidate device for QMap(("bus", "hid")("devInstanceId", "hid\vid_1b1c&pid_0c1a&mi_02\7&19b9f7ab&0&0000")("hid-direction", "output")("hid-type", "report")("hid-usage", "3072")("hid-usagepage", "65472")("model-name", "")("usb-pid", "3098")("usb-vid", "6940")("vendor", "")) not found.
2021-02-22T13:16:12 I cue.dev.mgr: No manifest for QMap(("bus", "hid")("devInstanceId", "hid\vid_1b1c&pid_0c1a&mi_02\7&19b9f7ab&0&0000")("hid-direction", "output")("hid-type", "report")("hid-usage", "3072")("hid-usagepage", "65472")("model-name", "")("usb-pid", "3098")("usb-vid", "6940")("vendor", ""))
2021-02-22T13:16:12 I cue.devices: Device vid=1b1c pid=c1a ignored by HidDeviceEnumerator

I have not abstracted the EEPROM, so it's still disabled in the code. Do you think this has something to do with it? What do you think is the best approach to the abstraction?

[Legion2]

What do you think is the best approach to the abstraction?

For CorsairLightingFirmware we need an interface (abstract class) which defines a getter and setter for DeviceId

For FastLEDController we need an interface for loading and storing the channels data

For both interfaces a EEPROM implementation should be provided which can be given in the constructor.
For the FastLEDController we can replace the useEEPROM flag with an empty implementation for the storage interface.

[Legion2]

@Spegs21 With "Commander Core" you mean LIGHTING NODE CORE? Because the Commander Core has a different PID and we are working on support in #211.

[Spegs21]

@Spegs21 With "Commander Core" you mean LIGHTING NODE CORE? Because the Commander Core has a different PID and we are working on support in #211.

Yea, sorry. That was was a test of the Lighting Node Core example.

[Legion2]

please add the samd architecture to the libary.properties and if you can add an example samd board to the CI pipeline so we have a test compile to check that it works with the new architecture.

[Spegs21]

I ended up forking the cmaglie/FlashStorage and added put and get templates which allow the original code to work with changes to which header is included. I tested it the library changes separately and they appear to work.

Unfortunately this didn't help. I'm still not seeing the device in iCUE. One things I've noticed in the iCUE logs is that vendor and model-name are not being reported even through these are set with the proper defines.

[Legion2]

For iCUE to detect the devices, you must define the USB IDs for the arduino boards. The boards are defined here /~https://github.com/Legion2/CorsairLightingProtocolBoards, we need to add also the samd boards

[Spegs21]

I'll add the boards definitions I created for the Trinket M0. I can create one for the Zero as well.

[Legion2]

It does not compile with the CLP Adafruit board

[Spegs21]

There are a few reason for this:

• EPTYPE_DESCRIPTOR_SIZE differs between Arduino:Arduino SAMD and Adafruit:Arduino SAMD
  I don't have a good solution for this yet.
• extern Uart Serial does not exist in the variant.h for the Trinket M0 and it cannot be added because they use it for SERIAL_PORT_USBVIRTUAL
  I added #ifndef USBCON to CorsairLightingProtocolSerial.h/.cpp to work around this but I don't know if that's the right solution. Maybe we could pass a reference to the necessary serial to the CorsairLightingProtocolSerial constructor.

[Legion2]

Yes we can make it as constructor argument, but have to add it to the changelog

[Spegs21]

Yes we can make it as constructor argument, but have to add it to the changelog

Actual if we change byte rawCommand[COMMAND_SIZE]; to char rawCommand[COMMAND_SIZE]; in CorsairLightingProtocolSerial.h it seems to fix it. Probably can't do that though without other changes.

[Legion2]

byte is semantically the correct type here. I think we can exclude the file for the Trinket M0 board

[Legion2]

@Spegs21 have you tested this PR with the SAMD boards definitions we added in Legion2/CorsairLightingProtocolBoards#7?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Spegs21]

@Legion2 I rewrote this to use TinyUSB in a class that extends CLPResponse like your CLPHID class. I had some trouble at the start so I set up the NoEeprom example on vscode and made a SWD debugger out of a RPi Pico.

I've got it running now except I can't get it to respond. I've verified the response data is making it to my sendX method but after that I can't confirm what happens other than iCue doesn't see it and I can't get a response from it with and HID python script.

When I send the report I use report_id 0. It's seemed like you maybe send something different in RawHID. Is there a correct value to use here?

[Legion2]

I think I send it without an report_id. How did you setup the USB endpoints, how does your usb report look like?

[Spegs21]

I verified that my report descriptor I made with TinyUSB macros matches yours from RawHID, byte for byte. TinyUSB handles endpoints so I've just created an Adafruit_USBD_HID object and use that.

It's working better than the SAMD USB implementation did though. It's showing Vendor and Model Name in the iCue logs unlike it did before. It is also receiving the request for deviceID.

The version of the TinyUSB library included in the Adafruit core doesn't allow for setting the serial number, newer versions do. I'm wondering is if that has something to do with it.

[Legion2]

I think the difference is, that I don't send a report, I just send the data here

CorsairLightingProtocol/src/RawHID.h

Line 143 in 4593dca

return USB_Send(pluggedEndpoint | TRANSFER_RELEASE, buffer, size);

and do not use the SendReport function
/~https://github.com/arduino/ArduinoCore-avr/blob/2cebe625833afcdb76cc941ccf90e5f5fefc27a9/libraries/HID/src/HID.cpp#L89-L96

[Legion2]

I have created an issue adafruit/Adafruit_TinyUSB_Arduino#153

[Spegs21]

@Legion2 It looks like it already emulates this behavior by skipping the report_ID byte if it's 0.

I enabled debugging for tusb and hooked up serial to my pico probe. It's using EP 1 and 81 depending on how I configure it. I've read the protocol information you linked and it looks like it was using EP 2. Should this matter?

[Legion2]

The endpoint does not matter, but the endpoint configuration is important

[Spegs21]

@Legion2 Finally got this working after installing Wireshark and seeing that the packets being sent from the device were considered malformed. The issue ended up being the HID report I was sending was 16 bytes but the report descriptor called for 64 bytes. I changed the descriptor to 16 and it worked right away.

I'll clean it up and update the PR.

[Spegs21]

@Legion2 I've made the TinyUSB implementation so that it no longer needs custom board definitions. VID, PID, manufacturer, and product are now set at runtime. Any board that is supported by TinyUSB should now work.

I'm wondering if we can just pass the product from the new corsair_product_enum_t to the firmware constructor and look up the firmware version from an array like I do in the TinyUSB implementation. Then if we stored the product ID in the firmware, it could just be looked up the TinyUSB implementation and not passed to its begin function.

It just seems redundant to declare what product we are using multiple times but I know it'll eat up some memory.

[Legion2]

@Spegs21 Is this MR ready for review? Then I can try to look into it tomorrow.

[Spegs21]

@Legion2 Yes, I think it's ready to go.

[Legion2]

This PR looks great, thanks for your work! Could you rebase it, so there are no merge commits.

[Spegs21]

@Legion2 I think I did this right, let me know if there's a problem with it.

[Spegs21]

@Legion2 I enabled workflows on my fork and have fixed any issues and squashed/rebased the commits. It should pass the workflows here now.

[Legion2]

@Spegs21 Thanks for your work, I will create a new release for it this week.

[Spegs21]

@Legion2 Awesome! Thanks for taking the time to review and incorporate these changes.