Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]Compatibility with official Arduino SAMD core #162

Closed
Spegs21 opened this issue Mar 27, 2022 · 5 comments
Closed

[Feature]Compatibility with official Arduino SAMD core #162

Spegs21 opened this issue Mar 27, 2022 · 5 comments

Comments

@Spegs21
Copy link

Spegs21 commented Mar 27, 2022

Is your feature request related to a problem? Please describe.
I've implemented USB HID support through TinyUSB for SAMD boards in CorsairLightingProtocol and users are wanting to use it with official Arduino boards, like the Zero (Legion2/CorsairLightingProtocol#280), but are not able to since the official Arduino core doesn't support TinyUSB.

Describe the solution you'd like
I'd like to bring the ability to use TinyUSB to the Arduino SAMD core. I'm not asking for full support (arduino/ArduinoCore-samd#668).

Describe alternatives you've considered
Use the built in USB core but it adds unnecessary complexity to the project.

Additional context
I originally envisioned using the USE_TINYUSB define in the official Arduino core to disable the built in USB support but I think it can be made more generic using a menu option to toggle the definition of USBCON. I've created and example here.

TinyUSB doesn't seem to care about USBCON so I used what was in place to achieve the goal of disabling the Arduino core USB. I understand that this is probably not the intended use of this define but it works well this way.

Now come changes needed to TinyUSB. I can bypass the TinyUSB is not selected... error by adding addition conditions to the macro:

#if !defined(USE_TINYUSB) && ( (defined(ARDUINO_ARCH_SAMD) && defined(ARDUINO_SAMD_ADAFRUIT)) || \
                               (defined(ARDUINO_ARCH_RP2040) && !defined(ARDUINO_ARCH_MBED)) )
#error TinyUSB is not selected, please select it in "Tools->Menu->USB Stack"
#endif

Finally, since USE_TINYUSB is not defined the tusb_config_samd.h needs updated here:

#if defined(USE_TINYUSB) || !defined(USBCON)

Since the Adafruit SAMD core will still require TinyUSB to be selected and the Arduino SAMD core will not define USBCON, this should work. You could also adopt not defining USBCON in your SAMD core when TinyUSB is selected and simplify the changes made to the Arduino core implementation by only checking for USBCON and not USE_TINYUSB. USBCON now essentially means 'I want to use the Arduino USB core'.

Since I wanted to keep the changes to the Arduino SAMD core generic, this is the best why I could come up with to do this. I realize I'm not explicitly telling the library I want to use TinyUSB but I am now telling it I don't want to use the Arduino USB core. If I've gone through the process of including the library and its functions and I don't want to use the Arduino USB core, I think this should be enough for it to understand its use is desired.

I'm definitely open to other options. I want to see what you think about this @hathach.

@hathach
Copy link
Member

hathach commented Mar 28, 2022

with above changed, have you managed to get tinyusb running on samd core ?

@Spegs21
Copy link
Author

Spegs21 commented Mar 28, 2022

@hathach The example I created in this commit builds just fine but I don't have a board to test it with.

@hathach
Copy link
Member

hathach commented Mar 28, 2022

build just fine is far from working. TinyUSB supported cannot be added without core modification, since core need it for USB Serial and ability to switch between arduino/tinyusb stack much like changes to our own fork.

  • wrap cores/arduino/USB with #ifndef USE_TINYUSB
  • add menu option to switch stack
  • add tinyusb as part of built-in library
  • several other modification to boards.txt and platform.txt

And I don't think Arduino team would agree to adopt required changes, there is little I could do about it. Let me know if you could persuade Arduino team to accept core change

@hathach hathach closed this as completed Mar 28, 2022
@Spegs21
Copy link
Author

Spegs21 commented Mar 28, 2022

@hathach As I said above I'm not asking for full support. My proposal to the Arduino team is simply to give the option to turn off their built in USB core so TinyUSB can be used. As for your list:

  • wrap cores/arduino/USB with #ifndef USE_TINYUSB
    They are already wrapped with #ifdef USBCON with the changes I'm proposing. It would be unnecessary to wrap with #ifndef USE_TINYUSB. Like I said, I'm going for generic changes.
  • add menu option to switch stack
    This is addressed by giving the option to disable the Arduino USB core in a menu, using the USBCON define on the backend, as I did above.
  • add tinyusb as part of built-in library
    This isn't necessary. Disabling of the Arduino USB core allows you to use any USB implementation.
  • several other modification to boards.txt and platform.txt
    These were done with the other changes I made to the Arduino SAMD core.

I'm just asking for these minor changes to TinyUSB so I can say that the changes to the Arduino core will make it work with TinyUSB when working with the Arduino team:

Now come changes needed to TinyUSB. I can bypass the TinyUSB is not selected... error by adding addition conditions to the macro:

#if !defined(USE_TINYUSB) && ( (defined(ARDUINO_ARCH_SAMD) && defined(ARDUINO_SAMD_ADAFRUIT)) || \
                              (defined(ARDUINO_ARCH_RP2040) && !defined(ARDUINO_ARCH_MBED)) )
#error TinyUSB is not selected, please select it in "Tools->Menu->USB Stack"
#endif

Finally, since USE_TINYUSB is not defined the tusb_config_samd.h needs updated here:
#if defined(USE_TINYUSB) || !defined(USBCON)

I can get a board to test the changes.

I think the Arduino team is more likely to accept the changes since they are generic with what I'm proposing.

@hathach
Copy link
Member

hathach commented Mar 28, 2022

please submit an PR for your proposal changes. I will give it a review and see if we could merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants