LUFA USB descriptor cleanup (#4871)

* Fix indentation

* Fix braces

* Expand descriptor headers

* Align descriptor elements

* Nicer formatting

* Tidy up preprocessor statements

* Remove VERSION_BCD redefine - LUFA_VERSION_INTEGER is currently 0x170418

* Tidy up comments

* Tweak ordering of  HID report elements (no functional changes)

* We don't need all of these newlines

* Move default USB_MAX_POWER_CONSUMPTION closer to where it makes sense

* Ask nicely

* Add some more comments

* Change indentation back to 4 spaces

* Add changelog entry

docs/ChangeLog/20190830/PR4871.md

@@ -0,0 +1,4 @@
+* LUFA USB descriptor cleanup
+    * Some code cleanups related to the USB HID descriptors on AVR keyboards, to make them easier to read and understand
+    * More information: see https://github.com/qmk/qmk_firmware/pull/4871
+    * No behaviour changes anticipated and no keymaps modified

tmk_core/protocol/usb_descriptor.h

@@ -40,17 +40,19 @@
  *
  *  Header file for Descriptors.c.
  */
-
 #ifndef _DESCRIPTORS_H_
     #define _DESCRIPTORS_H_
 
     #include <LUFA/Drivers/USB/USB.h>
+
     #ifdef PROTOCOL_CHIBIOS
         #include "hal.h"
     #endif
 
-typedef struct
+/*
-{
+ * USB descriptor structure
+ */
+typedef struct {
     USB_Descriptor_Configuration_Header_t    Config;
 
     #ifndef KEYBOARD_SHARED_EP
@@ -67,13 +69,14 @@ typedef struct
     USB_Descriptor_Endpoint_t                 Mouse_INEndpoint;
     #endif
 
-#if defined(SHARED_EP_ENABLE)
+    #ifdef SHARED_EP_ENABLE
+    // Shared Interface
     USB_Descriptor_Interface_t                Shared_Interface;
     USB_HID_Descriptor_HID_t                  Shared_HID;
     USB_Descriptor_Endpoint_t                 Shared_INEndpoint;
     #endif
 
-#if defined(RAW_ENABLE)
+    #ifdef RAW_ENABLE
     // Raw HID Interface
     USB_Descriptor_Interface_t                Raw_Interface;
     USB_HID_Descriptor_HID_t                  Raw_HID;
@@ -94,7 +97,6 @@ typedef struct
     // MIDI Audio Control Interface
     USB_Descriptor_Interface_t                Audio_ControlInterface;
     USB_Audio_Descriptor_Interface_AC_t       Audio_ControlInterface_SPC;
-
     // MIDI Audio Streaming Interface
     USB_Descriptor_Interface_t                Audio_StreamInterface;
     USB_MIDI_Descriptor_AudioInterface_AS_t   Audio_StreamInterface_SPC;
@@ -110,14 +112,12 @@ typedef struct
 
     #ifdef VIRTSER_ENABLE
     USB_Descriptor_Interface_Association_t    CDC_Interface_Association;
-
     // CDC Control Interface
     USB_Descriptor_Interface_t                CDC_CCI_Interface;
     USB_CDC_Descriptor_FunctionalHeader_t     CDC_Functional_Header;
     USB_CDC_Descriptor_FunctionalACM_t        CDC_Functional_ACM;
     USB_CDC_Descriptor_FunctionalUnion_t      CDC_Functional_Union;
     USB_Descriptor_Endpoint_t                 CDC_NotificationEndpoint;
-
     // CDC Data Interface
     USB_Descriptor_Interface_t                CDC_DCI_Interface;
     USB_Descriptor_Endpoint_t                 CDC_DataOutEndpoint;
@@ -125,63 +125,80 @@ typedef struct
     #endif
 } USB_Descriptor_Configuration_t;
 
-
+/*
-/* index of interface */
+ * Interface indexes
+ */
 enum usb_interfaces {
-#if !defined(KEYBOARD_SHARED_EP)
+    #ifndef KEYBOARD_SHARED_EP
     KEYBOARD_INTERFACE,
     #else
         #define KEYBOARD_INTERFACE SHARED_INTERFACE
     #endif
+
     // It is important that the Raw HID interface is at a constant
     // interface number, to support Linux/OSX platforms and chrome.hid
     // If Raw HID is enabled, let it be always 1.
-#if defined(RAW_ENABLE)
+    #ifdef RAW_ENABLE
     RAW_INTERFACE,
     #endif
+
     #if defined(MOUSE_ENABLE) && !defined(MOUSE_SHARED_EP)
     MOUSE_INTERFACE,
     #endif
-#if defined(SHARED_EP_ENABLE)
+
+    #ifdef SHARED_EP_ENABLE
     SHARED_INTERFACE,
     #endif
-#if defined(CONSOLE_ENABLE)
+
+    #ifdef CONSOLE_ENABLE
     CONSOLE_INTERFACE,
     #endif
-#if defined(MIDI_ENABLE)
+
+    #ifdef MIDI_ENABLE
     AC_INTERFACE,
     AS_INTERFACE,
     #endif
-#if defined(VIRTSER_ENABLE)
+
+    #ifdef VIRTSER_ENABLE
     CCI_INTERFACE,
     CDI_INTERFACE,
     #endif
+
     TOTAL_INTERFACES
 };
 
     #define NEXT_EPNUM __COUNTER__
 
+/*
+ * Endpoint numbers
+ */
 enum usb_endpoints {
-    __unused_epnum__ = NEXT_EPNUM,   /* EP numbering starts at 1 */
+    __unused_epnum__ = NEXT_EPNUM, // Endpoint numbering starts at 1
-#if !defined(KEYBOARD_SHARED_EP)
+
+    #ifndef KEYBOARD_SHARED_EP
     KEYBOARD_IN_EPNUM = NEXT_EPNUM,
     #else
         #define KEYBOARD_IN_EPNUM SHARED_IN_EPNUM
     #endif
+
     #if defined(MOUSE_ENABLE) && !defined(MOUSE_SHARED_EP)
     MOUSE_IN_EPNUM = NEXT_EPNUM,
     #else
         #define MOUSE_IN_EPNUM SHARED_IN_EPNUM
     #endif
-#if defined(RAW_ENABLE)
+
+    #ifdef RAW_ENABLE
     RAW_IN_EPNUM = NEXT_EPNUM,
     RAW_OUT_EPNUM = NEXT_EPNUM,
     #endif
-#if defined(SHARED_EP_ENABLE)
+
+    #ifdef SHARED_EP_ENABLE
     SHARED_IN_EPNUM = NEXT_EPNUM,
     #endif
-#if defined(CONSOLE_ENABLE)
+
+    #ifdef CONSOLE_ENABLE
     CONSOLE_IN_EPNUM = NEXT_EPNUM,
+
         #ifdef PROTOCOL_CHIBIOS
     // ChibiOS has enough memory and descriptor to actually enable the endpoint
     // It could use the same endpoint numbers, as that's supported by ChibiOS
@@ -191,12 +208,14 @@ enum usb_endpoints {
             #define CONSOLE_OUT_EPNUM CONSOLE_IN_EPNUM
         #endif
     #endif
+
     #ifdef MIDI_ENABLE
     MIDI_STREAM_IN_EPNUM = NEXT_EPNUM,
     MIDI_STREAM_OUT_EPNUM = NEXT_EPNUM,
         #define MIDI_STREAM_IN_EPADDR  (ENDPOINT_DIR_IN  | MIDI_STREAM_IN_EPNUM)
         #define MIDI_STREAM_OUT_EPADDR (ENDPOINT_DIR_OUT | MIDI_STREAM_OUT_EPNUM)
     #endif
+
     #ifdef VIRTSER_ENABLE
     CDC_NOTIFICATION_EPNUM = NEXT_EPNUM,
     CDC_IN_EPNUM = NEXT_EPNUM,
@@ -207,18 +226,18 @@ enum usb_endpoints {
     #endif
 };
 
-#if defined(PROTOCOL_LUFA)
+    #ifdef PROTOCOL_LUFA
-/* LUFA tells us total endpoints including control */
+        // LUFA tells us total endpoints including control
         #define MAX_ENDPOINTS (ENDPOINT_TOTAL_ENDPOINTS - 1)
     #elif defined(PROTOCOL_CHIBIOS)
-/* ChibiOS gives us number of available user endpoints, not control */
+        // ChibiOS gives us number of available user endpoints, not control
         #define MAX_ENDPOINTS USB_MAX_ENDPOINTS
     #endif
-/* TODO - ARM_ATSAM */
 
+    // TODO - ARM_ATSAM
 
     #if (NEXT_EPNUM - 1) > MAX_ENDPOINTS
-# error There are not enough available endpoints to support all functions. Remove some in the rules.mk file. (MOUSEKEY, EXTRAKEY, CONSOLE, NKRO, MIDI, SERIAL, STENO)
+        #error There are not enough available endpoints to support all functions. Please disable one or more of the following: Mouse Keys, Extra Keys, Console, NKRO, MIDI, Serial, Steno
     #endif
 
     #define KEYBOARD_EPSIZE          8
@@ -230,17 +249,5 @@ enum usb_endpoints {
     #define CDC_NOTIFICATION_EPSIZE  8
     #define CDC_EPSIZE              16
 
-uint16_t get_usb_descriptor(const uint16_t wValue,
+uint16_t get_usb_descriptor(const uint16_t wValue, const uint16_t wIndex, const void** const DescriptorAddress);
-                            const uint16_t wIndex,
-                            const void** const DescriptorAddress);
-
-/* new API */
-#if LUFA_VERSION_INTEGER < 0x140302
-    #undef VERSION_BCD
-    #define VERSION_BCD(Major, Minor, Revision) \
-                                              CPU_TO_LE16( ((Major & 0xFF) << 8) | \
-                                                           ((Minor & 0x0F) << 4) | \
-                                                           (Revision & 0x0F) )
-#endif
-
 #endif