make USB HID period configurable (and default is now 8ms) (#998)

* make USB HID period configurable (and default is now 8ms)

* add HID_SetInterval to target.h

* fix lint error (whitespace/comments)

* add HID_SetInterval to test stubs

* moeder's solution for USB HID compilation on 7e/f7

* usbhid: simplify period selection to 125/250/500/1000 Hz, and make mixer run in-sync with protocol
mixer timing code is copied from sbus/sumd

* fix lint error

* USBHID: try to improve synchronisation to host polling

* usbhid: decrease ROM use (esp. for modular) and hopefully not break anything
stubs might not be the right place to declare HID_prevXferComplete on platforms that don't use devo_hid.c,
but I'm not sure where else makes sense

* USBHID: try using a callback for better timing

* usb hid: ensure clock is stopped after deinit

* revert d47172 and 99fb44

Co-authored-by: somewhatlurker <[email protected]>

src/protocol/exports.ld

@@ -21,6 +21,8 @@ EXTERN(TELEMETRY_SetUpdated)
 
 EXTERN(USB_Enable)
 EXTERN(USB_Disable)
+EXTERN(HID_SetInterval)
+EXTERN(HID_prevXferComplete)
 
 EXTERN(usbd_dev)
 EXTERN(USB_Product_Name)

src/protocol/usbhid.c

@@ -18,6 +18,28 @@
 #include "mixer.h"
 #include "config/model.h"
 
+static const char * const usbhid_opts[] = {
+  _tr_noop("Period (Hz)"),  "125", "250", "500", "1000", NULL,
+  NULL
+};
+enum {
+    PROTO_OPTS_PERIOD,
+    LAST_PROTO_OPT,
+};
+ctassert(LAST_PROTO_OPT <= NUM_PROTO_OPTS, too_many_protocol_opts);
+
+# define USBHID_PERIOD_MAX_INDEX 3
+static u16 period_index_to_ms(s16 idx)
+{
+    switch (idx) {
+        case 3: return 1;
+        case 2: return 2;
+        case 1: return 4;
+        default: return 8;
+    }
+    return 8;
+}
+
 //To change USBHID_MAX_CHANNELS you must change the Report_Descriptor in hid_usb_desc.c as well
 #define USBHID_ANALOG_CHANNELS 8
 #define USBHID_DIGITAL_CHANNELS 4
@@ -29,7 +51,6 @@
 //if sizeof(packet) changes, must change wMaxPacketSize to match in Joystick_ConfigDescriptor
 static s8 packet[USBHID_ANALOG_CHANNELS + 1];
 static u8 num_channels;
-extern void HID_Write(s8 *packet, u8 size);
 
 static void build_data_pkt()
 {
@@ -57,13 +78,47 @@ static void build_data_pkt()
     packet[USBHID_ANALOG_CHANNELS] = digital;
 }
 
+static enum {
+    ST_DATA1,
+    ST_DATA2,
+} state;
+
+static u16 mixer_runtime;
+// ms suffix on usbhid_period_ms to indicate that it's in milliseconds not microseconds like other protocols
+static u16 usbhid_period_ms;
 static u16 usbhid_cb()
 {
-    build_data_pkt();
-
-    HID_Write(packet, sizeof(packet));
+    // wait until endpoint is ready for writing before preparing data
+    // if the host is polling slower than our clock, this will just delay us a bit
+    // it does increase the chance of mixers not completing in time for 1ms period though...
+    if (!HID_prevXferComplete) return 100;
+
+    u16 protoopts_period = period_index_to_ms(Model.proto_opts[PROTO_OPTS_PERIOD]);
+    if (usbhid_period_ms != protoopts_period) {
+        usbhid_period_ms = protoopts_period;
+        // HID should be restarted when period changes
+        // this lets us update the endpoint descriptor's bInterval field
+        HID_Disable();
+        HID_SetInterval(usbhid_period_ms);
+        HID_Enable();
+    }
+    switch (state) {
+        case ST_DATA1:
+            CLOCK_RunMixer();    // clears mixer_sync, which is then set when mixer update complete
+            state = ST_DATA2;
+            return mixer_runtime;
 
-    return 50000;
+        case ST_DATA2:
+            if (mixer_sync != MIX_DONE && mixer_runtime < 2000) mixer_runtime += 50;
+            build_data_pkt();
+            HID_Write(packet, sizeof(packet));
+            state = ST_DATA1;
+            // return with - 200 in case host is polling slightly faster than our clock
+            // this doesn't guarantee perfect timing, but it should be sufficient to
+            // catch most variations and get us back to waiting for the host
+            return usbhid_period_ms * 1000 - mixer_runtime - 200;
+    }
+    return usbhid_period_ms * 1000 - 200;   // avoid compiler warning
 }
 
 static void deinit()
@@ -75,7 +130,11 @@ static void deinit()
 static void initialize()
 {
     CLOCK_StopTimer();
+    state = ST_DATA1;
+    mixer_runtime = 50;
     num_channels = Model.num_channels;
+    usbhid_period_ms = period_index_to_ms(Model.proto_opts[PROTO_OPTS_PERIOD]);
+    HID_SetInterval(usbhid_period_ms);
     HID_Enable();
     CLOCK_StartTimer(1000, usbhid_cb);
 }
@@ -91,6 +150,10 @@ uintptr_t USBHID_Cmds(enum ProtoCmds cmd)
         case PROTOCMD_DEFAULT_NUMCHAN: return 6;
         case PROTOCMD_CHANNELMAP: return UNCHG;
         case PROTOCMD_TELEMETRYSTATE: return PROTO_TELEM_UNSUPPORTED;
+        case PROTOCMD_GETOPTIONS:
+            if (Model.proto_opts[PROTO_OPTS_PERIOD] > USBHID_PERIOD_MAX_INDEX)
+                Model.proto_opts[PROTO_OPTS_PERIOD] = USBHID_PERIOD_MAX_INDEX;
+            return (uintptr_t)usbhid_opts;
         default: break;
     }
     return 0;

src/target.h

@@ -293,8 +293,11 @@ void USB_Disable();
 void USB_HandleISR();
 void USB_Connect();
 
+void HID_SetInterval(u8 interval);
 void HID_Enable();
 void HID_Disable();
+void HID_Write(s8 *packet, u8 size);
+extern volatile u8 HID_prevXferComplete;
 
 void MSC_Enable();
 void MSC_Disable();

src/target/drivers/mcu/emu/stubs.c

@@ -36,12 +36,16 @@ void TxName(u8 *var, int len) {
 }
 void MSC_Enable() {}
 void MSC_Disable() {}
+void HID_SetInterval(u8 interval) {
+    (void)interval;
+}
 void HID_Enable() {}
 void HID_Disable() {}
 void HID_Write(s8 *pkt, u8 size) {
     (void)pkt;
     (void)size;
 }
+volatile u8 HID_prevXferComplete;
 void Initialize_ButtonMatrix() {}
 void PWR_Init(void) {}
 unsigned  PWR_ReadVoltage() { return (DEFAULT_BATTERY_ALARM + 1000); }

src/target/drivers/usb/devo_hid.c

@@ -10,7 +10,7 @@ static const char * const usb_strings[] = {
     DeviationVersion
 };
 
-static volatile u8 usb_preXferComplete;
+volatile u8 HID_prevXferComplete;
 
 static const uint8_t hid_report_descriptor[] = {
     0x05, 0x01,                    // USAGE_PAGE (Generic Desktop)
@@ -72,13 +72,14 @@ static const struct {
     }
 };
 
-static const struct usb_endpoint_descriptor hid_endpoint = {
+// this is no longer const so that bInterval can be modified at runtime
+struct usb_endpoint_descriptor hid_endpoint = {
     .bLength = USB_DT_ENDPOINT_SIZE,
     .bDescriptorType = USB_DT_ENDPOINT,
     .bEndpointAddress = 0x81,
     .bmAttributes = USB_ENDPOINT_ATTR_INTERRUPT,
     .wMaxPacketSize = 9,
-    .bInterval = 0x20,
+    .bInterval = 8,
 };
 
 static const struct usb_interface_descriptor hid_iface = {
@@ -136,7 +137,7 @@ static void hid_callback(usbd_device *usbd_dev, uint8_t ep)
     (void)usbd_dev;
     (void)ep;
 
-    usb_preXferComplete = 1;
+    HID_prevXferComplete = 1;
 }
 
 static void hid_set_config(usbd_device *dev, uint16_t wValue)
@@ -152,7 +153,7 @@ static void hid_set_config(usbd_device *dev, uint16_t wValue)
                 USB_REQ_TYPE_TYPE | USB_REQ_TYPE_RECIPIENT,
                 hid_control_request);
 
-    usb_preXferComplete = 1;
+    HID_prevXferComplete = 1;
 }
 
 static void HID_Init()
@@ -165,14 +166,19 @@ static void HID_Init()
 
 void HID_Write(s8 *packet, u8 size)
 {
-    if (usb_preXferComplete) {
-        usb_preXferComplete = 0;
+    if (HID_prevXferComplete) {
+        HID_prevXferComplete = 0;
         usbd_ep_write_packet(usbd_dev, 0x81, packet, size);
     }
 }
 
+void HID_SetInterval(u8 interval)
+{
+    hid_endpoint.bInterval = interval;
+}
+
 void HID_Enable() {
-    usb_preXferComplete = 0;
+    HID_prevXferComplete = 0;
     USB_Enable(1);
     HID_Init();
 }

src/target/tx/opentx/t12/x9d_stubs.c

@@ -24,12 +24,16 @@ void USB_Enable(unsigned use_interrupt) {
     (void)use_interrupt;
 }
 void USB_Disable() {}
+void HID_SetInterval(u8 interval) {
+    (void)interval;
+}
 void HID_Enable() {}
 void HID_Disable() {}
 void HID_Write(s8 *pkt, u8 size) {
     (void)pkt;
     (void)size;
 }
+volatile u8 HID_prevXferComplete;
 void SOUND_Init() {}
 void SOUND_SetFrequency(unsigned frequency, unsigned volume) {
     (void)frequency;

src/target/tx/opentx/x9d/x9d_stubs.c

@@ -20,12 +20,16 @@
 #include "common.h"
 void MSC_Enable() {}
 void MSC_Disable() {}
+void HID_SetInterval(u8 interval) {
+    (void)interval;
+}
 void HID_Enable() {}
 void HID_Disable() {}
 void HID_Write(s8 *pkt, u8 size) {
     (void)pkt;
     (void)size;
 }
+volatile u8 HID_prevXferComplete;
 void SOUND_Init() {}
 void SOUND_SetFrequency(unsigned frequency, unsigned volume) {
     (void)frequency;

src/target/tx/other/test/test_stubs.c

@@ -105,12 +105,16 @@ void TxName(u8 *var, int len) {
 }
 void MSC_Enable() {}
 void MSC_Disable() {}
+void HID_SetInterval(u8 interval) {
+    (void)interval;
+}
 void HID_Enable() {}
 void HID_Disable() {}
 void HID_Write(s8 *pkt, u8 size) {
     (void)pkt;
     (void)size;
 }
+volatile u8 HID_prevXferComplete;
 void Initialize_ButtonMatrix() {}
 void PWR_Init(void) {}
 unsigned  PWR_ReadVoltage() { return (DEFAULT_BATTERY_ALARM + 1000); }