Fix keyboard mode reports not working properly by refactoring HID int…

…erface

fw/Config/LUFAConfig.h

@@ -70,11 +70,11 @@
 		#define USE_FLASH_DESCRIPTORS
 //		#define USE_EEPROM_DESCRIPTORS
 //		#define NO_INTERNAL_SERIAL
-		#define FIXED_CONTROL_ENDPOINT_SIZE      8
+		#define FIXED_CONTROL_ENDPOINT_SIZE      64
 //		#define DEVICE_STATE_AS_GPIOR            {Insert Value Here}
 		#define FIXED_NUM_CONFIGURATIONS         1
 //		#define CONTROL_ONLY_DEVICE
-//		#define INTERRUPT_CONTROL_ENDPOINT
+		#define INTERRUPT_CONTROL_ENDPOINT
 //		#define NO_DEVICE_REMOTE_WAKEUP
 //		#define NO_DEVICE_SELF_POWER
 

fw/Descriptors.cpp

@@ -1,7 +1,7 @@
 #include "Descriptors.h"
 
-const USB_Descriptor_HIDReport_Datatype_t* HIDReport;
-uint16_t SizeOfHIDReport;
+const USB_Descriptor_HIDReport_Datatype_t* JoystickHIDReport;
+uint16_t SizeOfJoystickHIDReport;
 const USB_Descriptor_Device_t* DeviceDescriptor;
 const USB_Descriptor_Configuration_t* ConfigurationDescriptor;
 uint8_t LedStringCount;
@@ -60,12 +60,42 @@ uint16_t CALLBACK_USB_GetDescriptor(const uint16_t wValue,
       }
       break;
     case HID_DTYPE_HID:
-      Address = &ConfigurationDescriptor->HID_HIDData;
-      Size    = sizeof(USB_HID_Descriptor_HID_t);
+      switch (wIndex)
+      {
+        case INTERFACE_ID_Joystick:
+          Address = &ConfigurationDescriptor->HID_JoystickHID;
+          Size    = sizeof(USB_HID_Descriptor_HID_t);
+          break;
+        case INTERFACE_ID_Keyboard:
+          Address = &ConfigurationDescriptor->HID_KeyboardHID;
+          Size    = sizeof(USB_HID_Descriptor_HID_t);
+          break;
+        case INTERFACE_ID_Mouse:
+          Address = &ConfigurationDescriptor->HID_MouseHID;
+          Size    = sizeof(USB_HID_Descriptor_HID_t);
+          break;
+        default:
+          break;
+      }
       break;
     case HID_DTYPE_Report:
-      Address = HIDReport;
-      Size    = SizeOfHIDReport;
+      switch (wIndex)
+      {
+        case INTERFACE_ID_Joystick:
+          Address = JoystickHIDReport;
+          Size    = SizeOfJoystickHIDReport;
+          break;
+        case INTERFACE_ID_Keyboard:
+          Address = &KeyboardHIDReport;
+          Size    = sizeof(KeyboardHIDReport);
+          break;
+        case INTERFACE_ID_Mouse:
+          Address = &MouseHIDReport;
+          Size    = sizeof(MouseHIDReport);
+          break;
+        default:
+          break;
+      }
       break;
     default:
       break;

fw/Descriptors.h

@@ -76,7 +76,6 @@ HID_RI_END_COLLECTION(0)
 HID_RI_USAGE_PAGE(8, 0x01), \
 HID_RI_USAGE(8, 0x06), \
 HID_RI_COLLECTION(8, 0x01), \
-  HID_RI_REPORT_ID(8, HID_REPORTID_KeyboardReport), \
   HID_RI_USAGE_PAGE(8, 0x07), \
   HID_RI_USAGE_MINIMUM(8, 0x00), \
   HID_RI_USAGE_MAXIMUM(8, 0xFF), \
@@ -87,35 +86,21 @@ HID_RI_COLLECTION(8, 0x01), \
   HID_RI_INPUT(8, HID_IOF_DATA | HID_IOF_ARRAY | HID_IOF_ABSOLUTE), \
 HID_RI_END_COLLECTION(0)
 
-#define HID_KBM_DESCRIPTOR(MaxKeys) \
+#define HID_MOUSE_DESCRIPTOR \
 HID_RI_USAGE_PAGE(8, 0x01), \
-HID_RI_REPORT_ID(8, HID_REPORTID_KeyboardReport), \
+HID_RI_USAGE(8, 0x02), \
 HID_RI_COLLECTION(8, 0x01), \
-  HID_RI_USAGE(8, 0x02), \
-  HID_RI_COLLECTION(8, 0x01), \
-    HID_RI_USAGE(8, 0x01), \
-    HID_RI_COLLECTION(8, 0x00), \
-      HID_RI_USAGE(8, 0x30), \
-      HID_RI_USAGE(8, 0x31), \
-      HID_RI_LOGICAL_MINIMUM(8, -1), \
-      HID_RI_LOGICAL_MAXIMUM(8, 1), \
-      HID_RI_PHYSICAL_MINIMUM(8, -1), \
-      HID_RI_PHYSICAL_MAXIMUM(8, 1), \
-      HID_RI_REPORT_COUNT(8, 0x02), \
-      HID_RI_REPORT_SIZE(8, 8), \
-      HID_RI_INPUT(8, HID_IOF_DATA | HID_IOF_VARIABLE | HID_IOF_RELATIVE), \
-    HID_RI_END_COLLECTION(0), \
-  HID_RI_END_COLLECTION(0), \
-  HID_RI_USAGE(8, 0x06), \
-  HID_RI_COLLECTION(8, 0x01), \
-    HID_RI_USAGE_PAGE(8, 0x07), \
-    HID_RI_USAGE_MINIMUM(8, 0x00), \
-    HID_RI_USAGE_MAXIMUM(8, 0xFF), \
-    HID_RI_LOGICAL_MINIMUM(8, 0x00), \
-    HID_RI_LOGICAL_MAXIMUM(16, 0xFF), \
-    HID_RI_REPORT_COUNT(8, MaxKeys), \
+  HID_RI_USAGE(8, 0x01), \
+  HID_RI_COLLECTION(8, 0x02),\
+    HID_RI_USAGE(8, 0x31), \
+    HID_RI_USAGE(8, 0x30), \
+    HID_RI_LOGICAL_MINIMUM(8, -1), \
+    HID_RI_LOGICAL_MAXIMUM(8, 1), \
+    HID_RI_PHYSICAL_MINIMUM(8, -1), \
+    HID_RI_PHYSICAL_MAXIMUM(8, 1), \
+    HID_RI_REPORT_COUNT(8, 0x02), \
     HID_RI_REPORT_SIZE(8, 0x08), \
-    HID_RI_INPUT(8, HID_IOF_DATA | HID_IOF_ARRAY | HID_IOF_ABSOLUTE), \
+    HID_RI_INPUT(8, HID_IOF_DATA | HID_IOF_VARIABLE | HID_IOF_RELATIVE), \
   HID_RI_END_COLLECTION(0), \
 HID_RI_END_COLLECTION(0)
 
@@ -144,12 +129,24 @@ extern const USB_Descriptor_String_t PROGMEM ProductString;
 // descriptor contains several sub-descriptors which vary between
 // devices, and which describe the device's usage to the host.
 
+// Each interface can only support one IN and OUT endpoint,
+// so for KBM support we need to use multiple interfaces as annoying as it is.
+// Technically keyboard data could be sent in the joystick IN endpoint, but might as well do things "cleanly".
 typedef struct {
   USB_Descriptor_Configuration_Header_t Config;
-  USB_Descriptor_Interface_t HID_Interface;
-  USB_HID_Descriptor_HID_t HID_HIDData;
-  USB_Descriptor_Endpoint_t HID_ReportINEndpoint;
-  USB_Descriptor_Endpoint_t HID_ReportOUTEndpoint;
+
+  USB_Descriptor_Interface_t HID_JoystickInterface;
+  USB_HID_Descriptor_HID_t HID_JoystickHID;
+  USB_Descriptor_Endpoint_t HID_JoystickReportINEndpoint;
+  USB_Descriptor_Endpoint_t HID_JoystickReportOUTEndpoint;
+
+  USB_Descriptor_Interface_t HID_KeyboardInterface;
+  USB_HID_Descriptor_HID_t HID_KeyboardHID;
+  USB_Descriptor_Endpoint_t HID_KeyboardReportINEndpoint;
+
+  USB_Descriptor_Interface_t HID_MouseInterface;
+  USB_HID_Descriptor_HID_t HID_MouseHID;
+  USB_Descriptor_Endpoint_t HID_MouseReportINEndpoint;
 } USB_Descriptor_Configuration_t;
 
 // HID class report descriptor. This is a special descriptor
@@ -159,8 +156,8 @@ typedef struct {
 // what data (and in what encoding) the device will send, and what it
 // may be sent back from the host. Refer to the HID specification for
 // more details on HID report descriptors.
-extern const USB_Descriptor_HIDReport_Datatype_t* HIDReport;
-extern uint16_t SizeOfHIDReport;
+extern const USB_Descriptor_HIDReport_Datatype_t* JoystickHIDReport;
+extern uint16_t SizeOfJoystickHIDReport;
 extern const USB_Descriptor_Device_t* DeviceDescriptor;
 extern const USB_Descriptor_Configuration_t* ConfigurationDescriptor;
 extern uint8_t LedStringCount;
@@ -171,7 +168,9 @@ extern const USB_Descriptor_String_t** LedStrings;
 // associated with it, which can be used to refer to the interface
 // from other descriptors.
 enum InterfaceDescriptors_t {
-  INTERFACE_ID_Controller = 0, /**< Controller interface descriptor ID */
+  INTERFACE_ID_Joystick = 0, /**< Joystick interface descriptor ID */
+  INTERFACE_ID_Keyboard = 1, /**< Keyboard interface descriptor ID */
+  INTERFACE_ID_Mouse = 2, /**< Mouse interface descriptor ID */
 };
 
 // Enum for the device string descriptor IDs within the device. Each
@@ -183,10 +182,16 @@ enum StringDescriptors_t {
   STRING_ID_Product = 2, // Product string ID
 };
 
-enum HID_ReportID_t {
-  HID_REPORTID_JoystickReport = 1,
-  HID_REPORTID_KeyboardReport = 2,
-  HID_REPORTID_LEDReport = 3
+enum {
+  KEYBOARD_KEYS = 13 // Max used keycodes
+};
+
+const USB_Descriptor_HIDReport_Datatype_t PROGMEM KeyboardHIDReport[] = {
+  HID_KEYBOARD_DESCRIPTOR(KEYBOARD_KEYS)
+};
+
+const USB_Descriptor_HIDReport_Datatype_t PROGMEM MouseHIDReport[] {
+  HID_MOUSE_DESCRIPTOR
 };
 
 // Device descriptor structure. This descriptor, located in FLASH
@@ -212,60 +217,116 @@ constexpr USB_Descriptor_Device_t generate_device_descriptor(uint16_t vid, uint1
   };
 }
 
-// Endpoint address of the HID reporting IN endpoint
-#define HID_IN_EPADDR (ENDPOINT_DIR_IN | 1)
+// Endpoint address of the Joystick HID reporting IN endpoint
+#define JOYSTICK_IN_EPADDR (ENDPOINT_DIR_IN | 1)
+
+// Endpoint address of the Joystick HID reporting OUT endpoint
+#define JOYSTICK_OUT_EPADDR (ENDPOINT_DIR_OUT | 2)
 
-// Endpoint address of the HID reporting OUT endpoint
-#define HID_OUT_EPADDR (ENDPOINT_DIR_OUT | 2)
+// Endpoint address of the Keyboard HID reporting IN endpoint
+#define KEYBOARD_IN_EPADDR (ENDPOINT_DIR_IN | 3)
 
-// Size of Joystick Hid reporting IN endpoint in bytes
-#define HID_EPSIZE 8
+// Endpoint address of the Mouse HID reporting IN endpoint
+#define MOUSE_IN_EPADDR (ENDPOINT_DIR_IN | 4)
+
+// Size of Hid reporting IN/OUT endpoint in bytes
+#define HID_EPSIZE FIXED_CONTROL_ENDPOINT_SIZE
 
 // Configuration descriptor structure. This descriptor, located in
 // FLASH memory, describes the usage of the device in one of its
 // supported configurations, including information about any device
 // interfaces and endpoints. The descriptor is read out by the USB
 // host during the enumeration process when selecting a configuration
 // so that the host may correctly communicate with the USB device.
-constexpr USB_Descriptor_Configuration_t generate_configuration_descriptor(uint16_t hid_report_size) {
+constexpr USB_Descriptor_Configuration_t generate_configuration_descriptor(uint16_t joystick_hid_report_size) {
   return USB_Descriptor_Configuration_t{
     .Config = {
       .Header = {.Size = sizeof(USB_Descriptor_Configuration_Header_t), .Type = DTYPE_Configuration},
       .TotalConfigurationSize = sizeof(USB_Descriptor_Configuration_t),
-      .TotalInterfaces = 1,
+      .TotalInterfaces = 3,
       .ConfigurationNumber = 1,
       .ConfigurationStrIndex = NO_DESCRIPTOR,
       .ConfigAttributes = (USB_CONFIG_ATTR_RESERVED | USB_CONFIG_ATTR_SELFPOWERED),
       .MaxPowerConsumption = USB_CONFIG_POWER_MA(500)
     },
-    .HID_Interface = {
+    .HID_JoystickInterface = {
       .Header = {.Size = sizeof(USB_Descriptor_Interface_t), .Type = DTYPE_Interface},
-      .InterfaceNumber = INTERFACE_ID_Controller,
+      .InterfaceNumber = INTERFACE_ID_Joystick,
       .AlternateSetting = 0x00,
       .TotalEndpoints = 2,
       .Class = HID_CSCP_HIDClass,
       .SubClass = HID_CSCP_NonBootSubclass,
       .Protocol = HID_CSCP_NonBootProtocol,
       .InterfaceStrIndex = NO_DESCRIPTOR
     },
-    .HID_HIDData = {
+    .HID_JoystickHID = {
+      .Header = {.Size = sizeof(USB_HID_Descriptor_HID_t), .Type = HID_DTYPE_HID},
+      .HIDSpec = VERSION_BCD(1,1,1),
+      .CountryCode = 0x00,
+      .TotalReportDescriptors = 1,
+      .HIDReportType = HID_DTYPE_Report,
+      .HIDReportLength = joystick_hid_report_size
+    },
+    .HID_JoystickReportINEndpoint = {
+      .Header = {.Size = sizeof(USB_Descriptor_Endpoint_t), .Type = DTYPE_Endpoint},
+      .EndpointAddress = JOYSTICK_IN_EPADDR,
+      .Attributes = (EP_TYPE_INTERRUPT | ENDPOINT_ATTR_NO_SYNC | ENDPOINT_USAGE_DATA),
+      .EndpointSize = HID_EPSIZE,
+      .PollingIntervalMS = 0x01
+    },
+    .HID_JoystickReportOUTEndpoint = {
+      .Header = {.Size = sizeof(USB_Descriptor_Endpoint_t), .Type = DTYPE_Endpoint},
+      .EndpointAddress = JOYSTICK_OUT_EPADDR,
+      .Attributes = (EP_TYPE_INTERRUPT | ENDPOINT_ATTR_NO_SYNC | ENDPOINT_USAGE_DATA),
+      .EndpointSize = HID_EPSIZE,
+      .PollingIntervalMS = 0x01
+    },
+    .HID_KeyboardInterface = {
+      .Header = {.Size = sizeof(USB_Descriptor_Interface_t), .Type = DTYPE_Interface},
+      .InterfaceNumber = INTERFACE_ID_Keyboard,
+      .AlternateSetting = 0x00,
+      .TotalEndpoints = 1,
+      .Class = HID_CSCP_HIDClass,
+      .SubClass = HID_CSCP_NonBootSubclass,
+      .Protocol = HID_CSCP_NonBootProtocol,
+      .InterfaceStrIndex = NO_DESCRIPTOR
+    },
+    .HID_KeyboardHID = {
       .Header = {.Size = sizeof(USB_HID_Descriptor_HID_t), .Type = HID_DTYPE_HID},
       .HIDSpec = VERSION_BCD(1,1,1),
       .CountryCode = 0x00,
       .TotalReportDescriptors = 1,
       .HIDReportType = HID_DTYPE_Report,
-      .HIDReportLength = hid_report_size
+      .HIDReportLength = sizeof(KeyboardHIDReport)
     },
-    .HID_ReportINEndpoint = {
+    .HID_KeyboardReportINEndpoint = {
       .Header = {.Size = sizeof(USB_Descriptor_Endpoint_t), .Type = DTYPE_Endpoint},
-      .EndpointAddress = HID_IN_EPADDR,
+      .EndpointAddress = KEYBOARD_IN_EPADDR,
       .Attributes = (EP_TYPE_INTERRUPT | ENDPOINT_ATTR_NO_SYNC | ENDPOINT_USAGE_DATA),
       .EndpointSize = HID_EPSIZE,
       .PollingIntervalMS = 0x01
     },
-    .HID_ReportOUTEndpoint = {
+    .HID_MouseInterface = {
+      .Header = {.Size = sizeof(USB_Descriptor_Interface_t), .Type = DTYPE_Interface},
+      .InterfaceNumber = INTERFACE_ID_Mouse,
+      .AlternateSetting = 0x00,
+      .TotalEndpoints = 1,
+      .Class = HID_CSCP_HIDClass,
+      .SubClass = HID_CSCP_NonBootSubclass,
+      .Protocol = HID_CSCP_NonBootProtocol,
+      .InterfaceStrIndex = NO_DESCRIPTOR
+    },
+    .HID_MouseHID = {
+      .Header = {.Size = sizeof(USB_HID_Descriptor_HID_t), .Type = HID_DTYPE_HID},
+      .HIDSpec = VERSION_BCD(1,1,1),
+      .CountryCode = 0x00,
+      .TotalReportDescriptors = 1,
+      .HIDReportType = HID_DTYPE_Report,
+      .HIDReportLength = sizeof(MouseHIDReport)
+    },
+    .HID_MouseReportINEndpoint = {
       .Header = {.Size = sizeof(USB_Descriptor_Endpoint_t), .Type = DTYPE_Endpoint},
-      .EndpointAddress = HID_OUT_EPADDR,
+      .EndpointAddress = MOUSE_IN_EPADDR,
       .Attributes = (EP_TYPE_INTERRUPT | ENDPOINT_ATTR_NO_SYNC | ENDPOINT_USAGE_DATA),
       .EndpointSize = HID_EPSIZE,
       .PollingIntervalMS = 0x01

fw/analog_button.c

@@ -30,10 +30,10 @@ int8_t analog_button_poll(analog_button* self, uint32_t current_value) {
     direction = -1;
   }
 
-  self->raw_state = direction;
+  self->direction = direction;
 
   if (direction != 0) {
-    // turntable is moving
+    // encoder is moving
     // keep updating the new center, and keep extending the sustain timer
     self->center = observed;
     timer_arm(&self->sustain_timer, self->sustain_ms);
@@ -46,8 +46,7 @@ int8_t analog_button_poll(analog_button* self, uint32_t current_value) {
   if (direction == -self->state && self->clear) {
     self->state = direction;
     return 0;
-  }
-  if (direction != 0) {
+  } else if (direction != 0) {
     self->state = direction;
   }
 

fw/analog_button.h

@@ -21,7 +21,7 @@ typedef struct {
   timer sustain_timer;
 
   int8_t state; // -1, 0, 1
-  int8_t raw_state; // state without sustain period
+  int8_t direction; // currently observed direction
 } analog_button;
 extern analog_button button_x;
 extern analog_button button_y;