From d17e1f94cb1fad4608e10c36c2870655f9882bef Mon Sep 17 00:00:00 2001 From: "Andrew J. Kroll" Date: Wed, 25 Dec 2013 01:08:02 -0500 Subject: [PATCH] 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 | 1 + confdescparser.h | 22 +++++++----- hexdump.h | 2 +- hid.cpp | 14 +++++++- hid.h | 2 +- hidboot.h | 89 +++++++++++++++++++++++++++++++----------------- 6 files changed, 88 insertions(+), 42 deletions(-) diff --git a/Usb.h b/Usb.h index c9dccb53..d903ff51 100644 --- a/Usb.h +++ b/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" diff --git a/confdescparser.h b/confdescparser.h index d7646a98..186de7be 100644 --- a/confdescparser.h +++ b/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::ConfigDescParser(Usb theXtractor(xtractor), stateParseDescr(0), dscrLen(0), -dscrType(0) { +dscrType(0), +UseOr(false) { theBuffer.pValue = varBuffer; valParser.Initialize(&theBuffer); theSkipper.Initialize(&theBuffer); }; + template void ConfigDescParser::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::Parse(const uin compare masks for them. When the match is found, calls EndpointXtract passing buffer containing endpoint descriptor */ template bool ConfigDescParser::ParseDescriptor(uint8_t **pp, uint16_t *pcntdn) { - USB_CONFIGURATION_DESCRIPTOR* ucd= reinterpret_cast(varBuffer); - USB_INTERFACE_DESCRIPTOR* uid= reinterpret_cast(varBuffer); + USB_CONFIGURATION_DESCRIPTOR* ucd = reinterpret_cast (varBuffer); + USB_INTERFACE_DESCRIPTOR* uid = reinterpret_cast (varBuffer); switch(stateParseDescr) { case 0: theBuffer.valueSize = 2; @@ -139,9 +141,13 @@ bool ConfigDescParser::ParseDescriptor break; if((MASK & CP_MASK_COMPARE_SUBCLASS) && uid->bInterfaceSubClass != SUBCLASS_ID) break; - if((MASK & CP_MASK_COMPARE_PROTOCOL) && uid->bInterfaceProtocol != PROTOCOL_ID) - break; - + if(UseOr) { + 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; diff --git a/hexdump.h b/hexdump.h index cf491d94..5d6f8e11 100644 --- a/hexdump.h +++ b/hexdump.h @@ -58,4 +58,4 @@ void HexDumper::Parse(const LEN_TYPE len, con } } -#endif // __HEXDUMP_H__ \ No newline at end of file +#endif // __HEXDUMP_H__ diff --git a/hid.cpp b/hid.cpp index 0405b04b..02c89720 100644 --- a/hid.cpp +++ b/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 )); diff --git a/hid.h b/hid.h index 53ef325c..2037cc15 100644 --- a/hid.h +++ b/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); diff --git a/hidboot.h b/hidboot.h index 700c2e4b..91f9819d 100644 --- a/hidboot.h +++ b/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 class HIDBoot : public HID //public USBDeviceConfig, public UsbConfigXtracter { @@ -342,38 +345,53 @@ uint8_t HIDBoot::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) { - USBTRACE("HID_PROTOCOL_KEYBOARD\r\n"); + if((BOOT_PROTOCOL & (HID_PROTOCOL_KEYBOARD | HID_PROTOCOL_MOUSE)) == (HID_PROTOCOL_KEYBOARD | HID_PROTOCOL_MOUSE)) { + 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< - USB_CLASS_HID, - HID_BOOT_INTF_SUBCLASS, - HID_PROTOCOL_KEYBOARD, - CP_MASK_COMPARE_ALL> confDescrParserA(this); - - pUsb->getConfDescr(bAddress, 0, i, &confDescrParserA); + pUsb->getConfDescr(bAddress, 0, i, &confDescrParser); 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. - if(BOOT_PROTOCOL & HID_PROTOCOL_MOUSE) { - 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, &confDescrParserA); + if(bNumEP == (uint8_t) (totalEndpoints(BOOT_PROTOCOL))) + break; + } + } - pUsb->getConfDescr(bAddress, 0, i, &confDescrParserB); - if(bNumEP == ((uint8_t) (totalEndpoints(BOOT_PROTOCOL)))) - break; + // GCC will optimize unused stuff away. + if(BOOT_PROTOCOL & HID_PROTOCOL_MOUSE) { + 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::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 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 - rcode = 0x80; // Reuse rcode. + // Wake keyboard interface by twinkling up to 5 LEDs that are in the spec. + // 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 void HIDBoot::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;