HID fixes

Fix incorrect GetReportDescr args. We want the interface index.
HID BOOT mouse will now always work
HID BOOT keyboard will now always work

Usb.h

@@ -28,6 +28,7 @@ e-mail   :  support@circuitsathome.com
 #include "printhex.h"
 #include "message.h"
 #include "hexdump.h"
+#include "sink_parser.h"
 #include "max3421e.h"
 #include "address.h"
 #include "avrpins.h"

confdescparser.h

@@ -20,7 +20,6 @@ e-mail   :  support@circuitsathome.com
 
 #define __CONFDESCPARSER_H__
 
-
 class UsbConfigXtracter {
 public:
         //virtual void ConfigXtract(const USB_CONFIGURATION_DESCRIPTOR *conf) = 0;
@@ -54,11 +53,12 @@ class ConfigDescParser : public USBReadParser {
         uint8_t ifaceNumber; // Interface number
         uint8_t ifaceAltSet; // Interface alternate settings
 
+        bool UseOr;
         bool ParseDescriptor(uint8_t **pp, uint16_t *pcntdn);
-
         void PrintHidDescriptor(const USB_HID_DESCRIPTOR *pDesc);
 
 public:
+        void SetOR(void) { UseOr = true; }
         ConfigDescParser(UsbConfigXtracter *xtractor);
         virtual void Parse(const uint16_t len, const uint8_t *pbuf, const uint16_t &offset);
 };
@@ -68,12 +68,14 @@ ConfigDescParser<CLASS_ID, SUBCLASS_ID, PROTOCOL_ID, MASK>::ConfigDescParser(Usb
 theXtractor(xtractor),
 stateParseDescr(0),
 dscrLen(0),
-dscrType(0) {
+dscrType(0),
+UseOr(false) {
         theBuffer.pValue = varBuffer;
         valParser.Initialize(&theBuffer);
         theSkipper.Initialize(&theBuffer);
 };
 
+
 template <const uint8_t CLASS_ID, const uint8_t SUBCLASS_ID, const uint8_t PROTOCOL_ID, const uint8_t MASK>
 void ConfigDescParser<CLASS_ID, SUBCLASS_ID, PROTOCOL_ID, MASK>::Parse(const uint16_t len, const uint8_t *pbuf, const uint16_t &offset) {
         uint16_t cntdn = (uint16_t) len;
@@ -88,8 +90,8 @@ void ConfigDescParser<CLASS_ID, SUBCLASS_ID, PROTOCOL_ID, MASK>::Parse(const uin
   compare masks for them. When the match is found, calls EndpointXtract passing buffer containing endpoint descriptor */
 template <const uint8_t CLASS_ID, const uint8_t SUBCLASS_ID, const uint8_t PROTOCOL_ID, const uint8_t MASK>
 bool ConfigDescParser<CLASS_ID, SUBCLASS_ID, PROTOCOL_ID, MASK>::ParseDescriptor(uint8_t **pp, uint16_t *pcntdn) {
-        USB_CONFIGURATION_DESCRIPTOR* ucd= reinterpret_cast<USB_CONFIGURATION_DESCRIPTOR*>(varBuffer);
+        USB_CONFIGURATION_DESCRIPTOR* ucd = reinterpret_cast<USB_CONFIGURATION_DESCRIPTOR*> (varBuffer);
-        USB_INTERFACE_DESCRIPTOR* uid= reinterpret_cast<USB_INTERFACE_DESCRIPTOR*>(varBuffer);
+        USB_INTERFACE_DESCRIPTOR* uid = reinterpret_cast<USB_INTERFACE_DESCRIPTOR*> (varBuffer);
         switch(stateParseDescr) {
                 case 0:
                         theBuffer.valueSize = 2;
@@ -139,9 +141,13 @@ bool ConfigDescParser<CLASS_ID, SUBCLASS_ID, PROTOCOL_ID, MASK>::ParseDescriptor
                                                 break;
                                         if((MASK & CP_MASK_COMPARE_SUBCLASS) && uid->bInterfaceSubClass != SUBCLASS_ID)
                                                 break;
-                                        if((MASK & CP_MASK_COMPARE_PROTOCOL) && uid->bInterfaceProtocol != PROTOCOL_ID)
+                                        if(UseOr) {
-                                                break;
+                                                if((!((MASK & CP_MASK_COMPARE_PROTOCOL) && uid->bInterfaceProtocol)))
-
+                                                        break;
+                                        } else {
+                                                if((MASK & CP_MASK_COMPARE_PROTOCOL) && uid->bInterfaceProtocol != PROTOCOL_ID)
+                                                        break;
+                                        }
                                         isGoodInterface = true;
                                         ifaceNumber = uid->bInterfaceNumber;
                                         ifaceAltSet = uid->bAlternateSetting;

hid.cpp

@@ -1,7 +1,7 @@
 #include "hid.h"
 
 //get HID report descriptor
-
+/* WRONG! Endpoint is _ALWAYS_ ZERO for HID! We want the _INTERFACE_ value here!
 uint8_t HID::GetReportDescr(uint8_t ep, USBReadParser *parser) {
         const uint8_t constBufLen = 64;
         uint8_t buf[constBufLen];
@@ -12,6 +12,18 @@ uint8_t HID::GetReportDescr(uint8_t ep, USBReadParser *parser) {
         //return ((rcode != hrSTALL) ? rcode : 0);
         return rcode;
 }
+*/
+uint8_t HID::GetReportDescr(uint16_t wIndex, USBReadParser *parser) {
+        const uint8_t constBufLen = 64;
+        uint8_t buf[constBufLen];
+
+        uint8_t rcode = pUsb->ctrlReq(bAddress, 0x00, bmREQ_HIDREPORT, USB_REQUEST_GET_DESCRIPTOR, 0x00,
+                HID_DESCRIPTOR_REPORT, wIndex, 128, constBufLen, buf, (USBReadParser*)parser);
+
+        //return ((rcode != hrSTALL) ? rcode : 0);
+        return rcode;
+}
+
 //uint8_t HID::getHidDescr( uint8_t ep, uint16_t nbytes, uint8_t* dataptr )
 //{
 //    return( pUsb->ctrlReq( bAddress, ep, bmREQ_GET_DESCR, USB_REQUEST_GET_DESCRIPTOR, 0x00, HID_DESCRIPTOR_HID, 0x0000, nbytes, dataptr ));

hid.h

@@ -174,7 +174,7 @@ public:
         uint8_t GetIdle(uint8_t iface, uint8_t reportID, uint8_t* dataptr);
         uint8_t SetIdle(uint8_t iface, uint8_t reportID, uint8_t duration);
 
-        uint8_t GetReportDescr(uint8_t ep, USBReadParser *parser = NULL);
+        uint8_t GetReportDescr(uint16_t wIndex, USBReadParser *parser = NULL);
 
         uint8_t GetHidDescr(uint8_t ep, uint16_t nbytes, uint8_t* dataptr);
         uint8_t GetReport(uint8_t ep, uint8_t iface, uint8_t report_type, uint8_t report_id, uint16_t nbytes, uint8_t* dataptr);

hidboot.h

@@ -28,6 +28,14 @@ e-mail   :  support@circuitsathome.com
 #define UHS_HID_BOOT_KEY_ZERO2          0x62
 #define UHS_HID_BOOT_KEY_PERIOD         0x63
 
+// Don't worry, GCC will optimize the result to a final value.
+#define bitsEndpoints(p) ((((p) & HID_PROTOCOL_KEYBOARD)? 2 : 0) | (((p) & HID_PROTOCOL_MOUSE)? 1 : 0))
+#define totalEndpoints(p) ((bitsEndpoints(p) == 3) ? 3 : 2)
+#define epMUL(p) ((((p) & HID_PROTOCOL_KEYBOARD)? 1 : 0) + (((p) & HID_PROTOCOL_MOUSE)? 1 : 0))
+
+// Already defined in hid.h
+// #define HID_MAX_HID_CLASS_DESCRIPTORS 5
+
 struct MOUSEINFO {
 
         struct {
@@ -167,11 +175,6 @@ protected:
         };
 };
 
-#define bitsEndpoints(p) (((p & HID_PROTOCOL_KEYBOARD)? 2 : 0)+((p & HID_PROTOCOL_MOUSE)? 1 : 0))
-#define totalEndpoints(p) ((bitsEndpoints(p) == 3) ? 3 : 2)
-#define epMUL(p) (((p & HID_PROTOCOL_KEYBOARD)? 1 : 0)+((p & HID_PROTOCOL_MOUSE)? 1 : 0))
-#define HID_MAX_HID_CLASS_DESCRIPTORS 5
-
 template <const uint8_t BOOT_PROTOCOL>
 class HIDBoot : public HID //public USBDeviceConfig, public UsbConfigXtracter
 {
@@ -342,38 +345,53 @@ uint8_t HIDBoot<BOOT_PROTOCOL>::Init(uint8_t parent, uint8_t port, bool lowspeed
         USBTRACE2("NC:", num_of_conf);
 
         // GCC will optimize unused stuff away.
-        if(BOOT_PROTOCOL & HID_PROTOCOL_KEYBOARD) {
+        if((BOOT_PROTOCOL & (HID_PROTOCOL_KEYBOARD | HID_PROTOCOL_MOUSE)) == (HID_PROTOCOL_KEYBOARD | HID_PROTOCOL_MOUSE)) {
-                USBTRACE("HID_PROTOCOL_KEYBOARD\r\n");
+                USBTRACE("HID_PROTOCOL_KEYBOARD AND MOUSE\r\n");
+                ConfigDescParser<
+                        USB_CLASS_HID,
+                        HID_BOOT_INTF_SUBCLASS,
+                        HID_PROTOCOL_KEYBOARD | HID_PROTOCOL_MOUSE,
+                        CP_MASK_COMPARE_ALL > confDescrParser(this);
+                confDescrParser.SetOR(); // Use the OR variant.
                 for(uint8_t i = 0; i < num_of_conf; i++) {
-                        ConfigDescParser<
+                        pUsb->getConfDescr(bAddress, 0, i, &confDescrParser);
-                                USB_CLASS_HID,
-                                HID_BOOT_INTF_SUBCLASS,
-                                HID_PROTOCOL_KEYBOARD,
-                                CP_MASK_COMPARE_ALL> confDescrParserA(this);
-
-                        pUsb->getConfDescr(bAddress, 0, i, &confDescrParserA);
                         if(bNumEP == (uint8_t) (totalEndpoints(BOOT_PROTOCOL)))
                                 break;
                 }
-        }
+        } else {
+                // GCC will optimize unused stuff away.
+                if(BOOT_PROTOCOL & HID_PROTOCOL_KEYBOARD) {
+                        USBTRACE("HID_PROTOCOL_KEYBOARD\r\n");
+                        for(uint8_t i = 0; i < num_of_conf; i++) {
+                                ConfigDescParser<
+                                        USB_CLASS_HID,
+                                        HID_BOOT_INTF_SUBCLASS,
+                                        HID_PROTOCOL_KEYBOARD,
+                                        CP_MASK_COMPARE_ALL> confDescrParserA(this);
 
-        // GCC will optimize unused stuff away.
+                                pUsb->getConfDescr(bAddress, 0, i, &confDescrParserA);
-        if(BOOT_PROTOCOL & HID_PROTOCOL_MOUSE) {
+                                if(bNumEP == (uint8_t) (totalEndpoints(BOOT_PROTOCOL)))
-                USBTRACE("HID_PROTOCOL_MOUSE\r\n");
+                                        break;
-                for(uint8_t i = 0; i < num_of_conf; i++) {
+                        }
-                        ConfigDescParser<
+                }
-                                USB_CLASS_HID,
-                                HID_BOOT_INTF_SUBCLASS,
-                                HID_PROTOCOL_MOUSE,
-                                CP_MASK_COMPARE_ALL> confDescrParserB(this);
 
-                        pUsb->getConfDescr(bAddress, 0, i, &confDescrParserB);
+                // GCC will optimize unused stuff away.
-                        if(bNumEP == ((uint8_t) (totalEndpoints(BOOT_PROTOCOL))))
+                if(BOOT_PROTOCOL & HID_PROTOCOL_MOUSE) {
-                                break;
+                        USBTRACE("HID_PROTOCOL_MOUSE\r\n");
+                        for(uint8_t i = 0; i < num_of_conf; i++) {
+                                ConfigDescParser<
+                                        USB_CLASS_HID,
+                                        HID_BOOT_INTF_SUBCLASS,
+                                        HID_PROTOCOL_MOUSE,
+                                        CP_MASK_COMPARE_ALL> confDescrParserB(this);
 
+                                pUsb->getConfDescr(bAddress, 0, i, &confDescrParserB);
+                                if(bNumEP == ((uint8_t) (totalEndpoints(BOOT_PROTOCOL))))
+                                        break;
+
+                        }
                 }
         }
-
         USBTRACE2("bNumEP:", bNumEP);
 
         if(bNumEP != (uint8_t) (totalEndpoints(BOOT_PROTOCOL))) {
@@ -408,13 +426,21 @@ uint8_t HIDBoot<BOOT_PROTOCOL>::Init(uint8_t parent, uint8_t port, bool lowspeed
                 rcode = SetIdle(i, 0, 0);
                 USBTRACE2("SET_IDLE rcode:", rcode);
                 if(rcode) goto FailSetIdle;
+                // Get the RPIPE and just throw it away.
+                SinkParser<USBReadParser, uint16_t, uint16_t> sink;
+                rcode = GetReportDescr(i, &sink);
+                USBTRACE2("RPIPE rcode:", rcode);
         }
 
+        // Get RPIPE and throw it away.
+
         if(BOOT_PROTOCOL & HID_PROTOCOL_KEYBOARD) {
-                // Wake keyboard interface by twinkling up to 7 LEDs
+                // Wake keyboard interface by twinkling up to 5 LEDs that are in the spec.
-                rcode = 0x80; // Reuse rcode.
+                // kana, compose, scroll, caps, num
+                rcode = 0x20; // Reuse rcode.
                 while(rcode) {
                         rcode >>= 1;
+                        // Ignore any error returned, we don't care if LED is not supported
                         SetReport(0, 0, 2, 0, 1, &rcode); // Eventually becomes zero (All off)
                         delay(25);
                 }
@@ -472,7 +498,8 @@ template <const uint8_t BOOT_PROTOCOL>
 void HIDBoot<BOOT_PROTOCOL>::EndpointXtract(uint8_t conf, uint8_t iface, uint8_t alt, uint8_t proto, const USB_ENDPOINT_DESCRIPTOR *pep) {
 
         // If the first configuration satisfies, the others are not considered.
-        if(bNumEP > 1 && conf != bConfNum)
+        //if(bNumEP > 1 && conf != bConfNum)
+        if(bNumEP == totalEndpoints(BOOT_PROTOCOL))
                 return;
 
         bConfNum = conf;