adafruit / Adafruit_TinyUSB_Arduino

Arduino library for TinyUSB
MIT License
463 stars 119 forks source link

Arduino Mouse.h and Keyboard.h compatible API and other topics #54

Closed cyborg5 closed 4 years ago

cyborg5 commented 4 years ago

There is a lot of legacy code that uses the standard Arduino Mouse.h and Keyboard.h API. However many new boards are using TinyUSB rather than the Arduino Stack. I've created a library that has the same interface as Mouse.h and Keyboard.h but then translates those methods into Tiny USB HID methods. You include my "TinyUSB_Mouse.h" and/or "TinyUSB_Keyboard.h" and it automatically detects whether or not you are using the Arduino Stack or the TinyUSB Stack and auto selects between the two.

One difference that I noticed between TinyUSB and the Arduino HID.h is the way you store USB descriptors. Under TinyUSB you use an array of descriptors but Arduino HID.h uses a linked list. As you add new functions, it simply appends the new descriptor to the list. At first I was concerned that I would have to create three separate libraries. One of them for mouse, another for keyboard, and a third one for mouse and keyboard together. However it's working okay with just the two libraries. You can include them both and it works. That's because each has its own separate instance of "Adafruit_USBD_HID usb_hid;"

I still have to wonder if it would be better to create a single HID object and append the descriptors as needed.

I'm opening this issue so that we can have a discussion about the pros and cons of the array versus the linked list.

Meanwhile here is a link to my libraries. I would also like to know if you would welcome having them added to your main repository? I think it would be a very useful feature. I would be happy to submit it as a pull request to add it if you are open to that. https://github.com/cyborg5/TinyUSB_Mouse_and_Keyboard

hathach commented 4 years ago

@cyborg5 this is very thoughtful and useful, it will ease the transition and allow user to switch stack easily. Thank you very much, I haven't looked at Arduino HID implementation yet. Give me a bit of time to examine it then I will get back to you. Though I am very hesitated to use any dynamic method like linked list, I would prefer to stay away from that. Though I will need to look at it first to see the pros and cons.

cyborg5 commented 4 years ago

I thought I had my mouse and keyboard translation] working that would translate standard Arduino Mouse.h and Keyboard.h API into TinyUSB API but I kept getting intermittent results. Finally decided to go ahead and combine them into a single library and for now it's working (sort of). There are several occasions where a Serial.println("whatever"); doesn't appear after using one of the mouse or keyboard methods. Sometimes you need a delay and other times you don't.

Also did some experiments putting Serial.println() commands into your sample sketches for HID mouse and HID keyboard. Again I'm getting intermittent failures.

Can you think of some code I can put after each call to your report methods that would ensure we are free to proceed with other things such as Serial.print?

I'm already doing

while (! usb_hid.ready()) delay(1);

before every report call. Do I need to do that after each report as well to make sure we are free to move on? Would appreciate any advice you can give to make the code more reliable when doing other USB things such as serial output.

hathach commented 4 years ago

@cyborg5 Could you post your complete sketch with a bit of comment where the what cause the issue here. Also the board you are working on as well to make sure we both have the same setup. Due to the timezone difference (I am on UTC+7), I am about to go to bed. I will try my best to analyze it tomorrow or so. This is also a good chance for me to look at the Arduino HID API and your integration layer altogether.

PS: the TinyUSB processing function is always run within yield() or delay() in case of SAMD21/51 . So if you are blocking wait for an USB event (Serial connection etc ..) those should be called in the wait code.

cyborg5 commented 4 years ago

One of the problems I'm having is that it only fails intermittently so I can't consistently reproduce the results. One day it works fine and the next day it doesn't. Here is a modified version of your "hid_composite" sample sketch where I added a serial output after the key is released. Right away I don't get any serial output without that while statement at the top.

When I first tried to run the sketch it would not do the output after the key release. Then I put in a delay just prior to the Serial.println("Test complete"); and it worked. Then I removed the delay and it continues to work.


/*********************************************************************
 Adafruit invests time and resources providing this open source code,
 please support Adafruit and open-source hardware by purchasing
 products from Adafruit!

 MIT license, check LICENSE for more information
 Copyright (c) 2019 Ha Thach for Adafruit Industries
 All text above, and the splash screen below must be included in
 any redistribution
*********************************************************************/

#include "Adafruit_TinyUSB.h"

/* This sketch demonstrates multiple report USB HID.
 * Press button pin will move
 * - mouse toward bottom right of monitor
 * - send 'a' key
 *
 * Depending on the board, the button pin
 * and its active state (when pressed) are different
 */
#if defined ARDUINO_SAMD_CIRCUITPLAYGROUND_EXPRESS
  const int pin = 4; // Left Button
  bool activeState = true;
#elif defined ARDUINO_NRF52840_FEATHER
  const int pin = 7; // UserSw
  bool activeState = false;
#else
  const int pin = 6;               //Modified by cyborg5
  bool activeState = false;
#endif

// Report ID
enum
{
  RID_KEYBOARD = 1,
  RID_MOUSE
};

// HID report descriptor using TinyUSB's template
uint8_t const desc_hid_report[] =
{
  TUD_HID_REPORT_DESC_KEYBOARD( HID_REPORT_ID(RID_KEYBOARD), ),
  TUD_HID_REPORT_DESC_MOUSE   ( HID_REPORT_ID(RID_MOUSE), )
};

// USB HID object
Adafruit_USBD_HID usb_hid;

// the setup function runs once when you press reset or power the board
void setup()
{
  usb_hid.setPollInterval(2);
  usb_hid.setReportDescriptor(desc_hid_report, sizeof(desc_hid_report));

  usb_hid.begin();

  // Set up button, pullup opposite to active state
  pinMode(pin, activeState ? INPUT_PULLDOWN : INPUT_PULLUP);

  Serial.begin(115200);
  while (!Serial) delay(1); //added by cyborg5. Will not produce output unless present.
  Serial.println("Adafruit TinyUSB HID Composite example");

  // wait until device mounted
  while( !USBDevice.mounted() ) delay(1);
}

void loop()
{
  // poll gpio once each 10 ms
  delay(10);

  // Whether button is pressed
  bool btn_pressed = (digitalRead(pin) == activeState);

  // Remote wakeup
  if ( USBDevice.suspended() && btn_pressed )
  {
    // Wake up host if we are in suspend mode
    // and REMOTE_WAKEUP feature is enabled by host
    USBDevice.remoteWakeup();
  }

  /*------------- Mouse -------------*/
  if ( usb_hid.ready() && btn_pressed )
  {
    int8_t const delta = 5;
    usb_hid.mouseMove(RID_MOUSE, delta, delta); // right + down

    // delay a bit before attempt to send keyboard report
    delay(10);
  }

  /*------------- Keyboard -------------*/
  if ( usb_hid.ready() )
  {
    // use to prevent sending multiple consecutive zero report
    static bool has_key = false;

    if ( btn_pressed )
    {
      uint8_t keycode[6] = { 0 };
      keycode[0] = HID_KEY_A;

      usb_hid.keyboardReport(RID_KEYBOARD, 0, keycode);

      has_key = true;
    }else
    {
      // send empty key report if previously has key pressed
      if (has_key) {
        usb_hid.keyboardRelease(RID_KEYBOARD);
        Serial.println("Test complete");//Added by cyborg5. Only works intermittently.
      }
      has_key = false;
    }
  }
}

One consistently reproducible result is if you move the Serial.begin and the while (! Serial) delay (1); to the top of the "setup" before you do any of the HID set up, the serial print works but none of your HID calls work. I don't think the order in which you begin serial or your HID should matter. The Arduino Mouse and Keyboard libraries are not dependent on the order in which you begin them.

Right now the sketch I posted up above as well as the three sketches in my examples folder of my library are all working on both Feather M0 Express and Feather M4 Express.

https://github.com/cyborg5/TinyUSB_Mouse_and_Keyboard

hathach commented 4 years ago

One of the problems I'm having is that it only fails intermittently so I can't consistently reproduce the results. One day it works fine and the next day it doesn't. Here is a modified version of your "hid_composite" sample sketch where I added a serial output after the key is released. Right away I don't get any serial output without that while statement at the top.

That's happen since the Serial.println() is executed before the device recognized/mounted/enumerated to PC. The message in the loop() should appear regardless of the while(!Serial) delay(1) here.

One consistently reproducible result is if you move the Serial.begin and the while (! Serial) delay (1); to the top of the "setup" before you do any of the HID set up, the serial print works but none of your HID calls work. I don't think the order in which you begin serial or your HID should matter. The Arduino Mouse and Keyboard libraries are not dependent on the order in which you begin them.

TinyUSB dynamically configure its interface depending on the user setup(). The process would be something like this

  1. Powering up: the CDC (Serial) will be added
  2. Setup() is invoke allow user sketch to add more driver such as MSC, HID
  3. Meanwhile the host will start the enumeration process.

Here is the interesting part: most of the time the setup() will run fast enough that the enumeration won't even started (normally after 10-20 ms). However, if you blocking wait for Serial() as you said before initializing HID or any other class. TinyUSB will only present its self to PC it is CDC only, therefore HID won't be usable. Therefore it is advisable to initialize USB class driver before other thing. In some cases, where this is not possible (meaning starting up take too long), we can force re-enumeration by calling USBDevice.detach() then USBDevice.attach(). This to PC as we unplug and re-plug the device.

Sorry for a complicated process, I hope I explain myself well enough. I tried to simplify it but USB dynamic configuration is rather complicated.

Right now the sketch I posted up above as well as the three sketches in my examples folder of my library are all working on both Feather M0 Express and Feather M4 Express.

https://github.com/cyborg5/TinyUSB_Mouse_and_Keyboard

Thanks I will checkout your library and educate myself with the Arduino HID API as well. Will get back on this.

hathach commented 4 years ago

PS: I probably need to write document for the library, though I am bit overwhelmed at the moment.

hathach commented 4 years ago

Your example works greats, I think it is very useful and helpful for user that is migrating to TinyUSB. Like adding MSC but still re-use the good-old Mouse/Keyboard API.

One difference that I noticed between TinyUSB and the Arduino HID.h is the way you store USB descriptors. Under TinyUSB you use an array of descriptors but Arduino HID.h uses a linked list. As you add new functions, it simply appends the new descriptor to the list. At first I was concerned that I would have to create three separate libraries. One of them for mouse, another for keyboard, and a third one for mouse and keyboard together. However it's working okay with just the two libraries. You can include them both and it works. That's because each has its own separate instance of "Adafruit_USBD_HID usb_hid;"

I have revised the hid report linked list by Arduino stack. It is internal implementation, I think we don't really have to do it that way. TinyUSB insists that HID Report descriptor must be defined by application, this allow lots of flexibility for application creativitiy. HID has too many profiles that can packed into the same HID interface (each has its own report ID), there is no way we could define them all within the stack. But they all share the same API sendReport(report_id, data) which we could support. I will add an gamepad and media control example to elaborate this.

Yes, having sketch to define the descriptor is kind of weird, however, it is only done once, and TinyUSB provide some most-used template to go with, I think most user won't ever bother to look at it twice.

PS: If you need an clarification and/or help to troubleshoot any issue with the stack, I am happy to help.

hathach commented 4 years ago

closed since this issue is fully resolved. Thank you for the issue.