Derive HIDUniversal from HIDComposite

They were mostly the same, I only had to keep the original
version of Poll() because while the differences are very small,
I don't think that unifying them is possible without needlessly
complicating things.
For this to work I had to make the private fields of HIDComposite
protected instead. I took the "+1" in HIDInterface::epIndex[]
from the original HIDUniversal class.

hidcomposite.cpp

@@ -308,7 +308,7 @@ void HIDComposite::EndpointXtract(uint8_t conf, uint8_t iface, uint8_t alt, uint
         if(!piface) {
                 if(bNumIface >= maxHidInterfaces) {
                         // don't overflow hidInterfaces[]
-                        Notify(PSTR("\r\n HIDComposite::EndpointXtract(): Not adding HID interface because we already have "), 0x80);
+                        Notify(PSTR("\r\n EndpointXtract(): Not adding HID interface because we already have "), 0x80);
                         Notify(bNumIface, 0x80);
                         Notify(PSTR(" interfaces and can't hold more. "), 0x80);
                         return;
@@ -329,7 +329,7 @@ void HIDComposite::EndpointXtract(uint8_t conf, uint8_t iface, uint8_t alt, uint
         if(index) {
                 if(bNumEP >= totalEndpoints) {
                         // don't overflow epInfo[] either
-                        Notify(PSTR("\r\n HIDComposite::EndpointXtract(): Not adding endpoint info because we already have "), 0x80);
+                        Notify(PSTR("\r\n EndpointXtract(): Not adding endpoint info because we already have "), 0x80);
                         Notify(bNumEP, 0x80);
                         Notify(PSTR(" endpoints and can't hold more. "), 0x80);
                         return;

hidcomposite.h

@@ -23,6 +23,8 @@ e-mail   :  support@circuitsathome.com
 
 class HIDComposite : public USBHID {
 
+protected:
+
         struct ReportParser {
                 uint8_t rptId;
                 HIDReportParser *rptParser;
@@ -40,7 +42,7 @@ class HIDComposite : public USBHID {
                         uint8_t bmAltSet : 3;
                         uint8_t bmProtocol : 2;
                 };
-                uint8_t epIndex[maxEpPerInterface];
+                uint8_t epIndex[maxEpPerInterface + 1]; // We need to make room for the control endpoint as well
         };
 
         uint8_t bConfNum; // configuration number
@@ -57,10 +59,13 @@ class HIDComposite : public USBHID {
 
         void ZeroMemory(uint8_t len, uint8_t *buf);
 
-protected:
+
         EpInfo epInfo[totalEndpoints];
         HIDInterface hidInterfaces[maxHidInterfaces];
 
+        // FIXME: bHasReportId is never set (except to false in constructor)
+        //        should probably be in EpInfo, /maybe/ in HIDInterface
+        //        but setting it isn't that easy (requires parsing report descriptors)
         bool bHasReportId;
 
         uint16_t PID, VID; // PID and VID of connected device

hiduniversal.cpp

@@ -17,353 +17,6 @@ e-mail   :  support@circuitsathome.com
 
 #include "hiduniversal.h"
 
-HIDUniversal::HIDUniversal(USB *p) :
-USBHID(p),
-qNextPollTime(0),
-pollInterval(0),
-bPollEnable(false),
-bHasReportId(false) {
-        Initialize();
-
-        if(pUsb)
-                pUsb->RegisterDeviceClass(this);
-}
-
-uint16_t HIDUniversal::GetHidClassDescrLen(uint8_t type, uint8_t num) {
-        for(uint8_t i = 0, n = 0; i < HID_MAX_HID_CLASS_DESCRIPTORS; i++) {
-                if(descrInfo[i].bDescrType == type) {
-                        if(n == num)
-                                return descrInfo[i].wDescriptorLength;
-                        n++;
-                }
-        }
-        return 0;
-}
-
-void HIDUniversal::Initialize() {
-        for(uint8_t i = 0; i < MAX_REPORT_PARSERS; i++) {
-                rptParsers[i].rptId = 0;
-                rptParsers[i].rptParser = NULL;
-        }
-        for(uint8_t i = 0; i < HID_MAX_HID_CLASS_DESCRIPTORS; i++) {
-                descrInfo[i].bDescrType = 0;
-                descrInfo[i].wDescriptorLength = 0;
-        }
-        for(uint8_t i = 0; i < maxHidInterfaces; i++) {
-                hidInterfaces[i].bmInterface = 0;
-                hidInterfaces[i].bmProtocol = 0;
-
-                for(uint8_t j = 0; j < maxEpPerInterface; j++)
-                        hidInterfaces[i].epIndex[j] = 0;
-        }
-        for(uint8_t i = 0; i < totalEndpoints; i++) {
-                epInfo[i].epAddr = 0;
-                epInfo[i].maxPktSize = (i) ? 0 : 8;
-                epInfo[i].bmSndToggle = 0;
-                epInfo[i].bmRcvToggle = 0;
-                epInfo[i].bmNakPower = (i) ? USB_NAK_NOWAIT : USB_NAK_MAX_POWER;
-        }
-        bNumEP = 1;
-        bNumIface = 0;
-        bConfNum = 0;
-        pollInterval = 0;
-}
-
-bool HIDUniversal::SetReportParser(uint8_t id, HIDReportParser *prs) {
-        for(uint8_t i = 0; i < MAX_REPORT_PARSERS; i++) {
-                if(rptParsers[i].rptId == 0 && rptParsers[i].rptParser == NULL) {
-                        rptParsers[i].rptId = id;
-                        rptParsers[i].rptParser = prs;
-                        return true;
-                }
-        }
-        return false;
-}
-
-HIDReportParser* HIDUniversal::GetReportParser(uint8_t id) {
-        if(!bHasReportId)
-                return ((rptParsers[0].rptParser) ? rptParsers[0].rptParser : NULL);
-
-        for(uint8_t i = 0; i < MAX_REPORT_PARSERS; i++) {
-                if(rptParsers[i].rptId == id)
-                        return rptParsers[i].rptParser;
-        }
-        return NULL;
-}
-
-uint8_t HIDUniversal::Init(uint8_t parent, uint8_t port, bool lowspeed) {
-        const uint8_t constBufSize = sizeof (USB_DEVICE_DESCRIPTOR);
-
-        uint8_t buf[constBufSize];
-        USB_DEVICE_DESCRIPTOR * udd = reinterpret_cast<USB_DEVICE_DESCRIPTOR*>(buf);
-        uint8_t rcode;
-        UsbDevice *p = NULL;
-        EpInfo *oldep_ptr = NULL;
-        uint8_t len = 0;
-
-        uint8_t num_of_conf; // number of configurations
-        //uint8_t num_of_intf; // number of interfaces
-
-        AddressPool &addrPool = pUsb->GetAddressPool();
-
-        USBTRACE("HU Init\r\n");
-
-        if(bAddress)
-                return USB_ERROR_CLASS_INSTANCE_ALREADY_IN_USE;
-
-        // Get pointer to pseudo device with address 0 assigned
-        p = addrPool.GetUsbDevicePtr(0);
-
-        if(!p)
-                return USB_ERROR_ADDRESS_NOT_FOUND_IN_POOL;
-
-        if(!p->epinfo) {
-                USBTRACE("epinfo\r\n");
-                return USB_ERROR_EPINFO_IS_NULL;
-        }
-
-        // Save old pointer to EP_RECORD of address 0
-        oldep_ptr = p->epinfo;
-
-        // Temporary assign new pointer to epInfo to p->epinfo in order to avoid toggle inconsistence
-        p->epinfo = epInfo;
-
-        p->lowspeed = lowspeed;
-
-        // Get device descriptor
-        rcode = pUsb->getDevDescr(0, 0, 8, (uint8_t*)buf);
-
-        if(!rcode)
-                len = (buf[0] > constBufSize) ? constBufSize : buf[0];
-
-        if(rcode) {
-                // Restore p->epinfo
-                p->epinfo = oldep_ptr;
-
-                goto FailGetDevDescr;
-        }
-
-        // Restore p->epinfo
-        p->epinfo = oldep_ptr;
-
-        // Allocate new address according to device class
-        bAddress = addrPool.AllocAddress(parent, false, port);
-
-        if(!bAddress)
-                return USB_ERROR_OUT_OF_ADDRESS_SPACE_IN_POOL;
-
-        // Extract Max Packet Size from the device descriptor
-        epInfo[0].maxPktSize = udd->bMaxPacketSize0;
-
-        // Assign new address to the device
-        rcode = pUsb->setAddr(0, 0, bAddress);
-
-        if(rcode) {
-                p->lowspeed = false;
-                addrPool.FreeAddress(bAddress);
-                bAddress = 0;
-                USBTRACE2("setAddr:", rcode);
-                return rcode;
-        }
-
-        //delay(2); //per USB 2.0 sect.9.2.6.3
-
-        USBTRACE2("Addr:", bAddress);
-
-        p->lowspeed = false;
-
-        p = addrPool.GetUsbDevicePtr(bAddress);
-
-        if(!p)
-                return USB_ERROR_ADDRESS_NOT_FOUND_IN_POOL;
-
-        p->lowspeed = lowspeed;
-
-        if(len)
-                rcode = pUsb->getDevDescr(bAddress, 0, len, (uint8_t*)buf);
-
-        if(rcode)
-                goto FailGetDevDescr;
-
-        VID = udd->idVendor; // Can be used by classes that inherits this class to check the VID and PID of the connected device
-        PID = udd->idProduct;
-
-        num_of_conf = udd->bNumConfigurations;
-
-        // Assign epInfo to epinfo pointer
-        rcode = pUsb->setEpInfoEntry(bAddress, 1, epInfo);
-
-        if(rcode)
-                goto FailSetDevTblEntry;
-
-        USBTRACE2("NC:", num_of_conf);
-
-        for(uint8_t i = 0; i < num_of_conf; i++) {
-                //HexDumper<USBReadParser, uint16_t, uint16_t> HexDump;
-                ConfigDescParser<USB_CLASS_HID, 0, 0,
-                        CP_MASK_COMPARE_CLASS> confDescrParser(this);
-
-                //rcode = pUsb->getConfDescr(bAddress, 0, i, &HexDump);
-                rcode = pUsb->getConfDescr(bAddress, 0, i, &confDescrParser);
-
-                if(rcode)
-                        goto FailGetConfDescr;
-
-                if(bNumEP > 1)
-                        break;
-        } // for
-
-        if(bNumEP < 2)
-                return USB_DEV_CONFIG_ERROR_DEVICE_NOT_SUPPORTED;
-
-        // Assign epInfo to epinfo pointer
-        rcode = pUsb->setEpInfoEntry(bAddress, bNumEP, epInfo);
-
-        USBTRACE2("Cnf:", bConfNum);
-
-        // Set Configuration Value
-        rcode = pUsb->setConf(bAddress, 0, bConfNum);
-
-        if(rcode)
-                goto FailSetConfDescr;
-
-        for(uint8_t i = 0; i < bNumIface; i++) {
-                if(hidInterfaces[i].epIndex[epInterruptInIndex] == 0)
-                        continue;
-
-                rcode = SetIdle(hidInterfaces[i].bmInterface, 0, 0);
-
-                if(rcode && rcode != hrSTALL)
-                        goto FailSetIdle;
-        }
-
-        USBTRACE("HU configured\r\n");
-
-        OnInitSuccessful();
-
-        bPollEnable = true;
-        return 0;
-
-FailGetDevDescr:
-#ifdef DEBUG_USB_HOST
-        NotifyFailGetDevDescr();
-        goto Fail;
-#endif
-
-FailSetDevTblEntry:
-#ifdef DEBUG_USB_HOST
-        NotifyFailSetDevTblEntry();
-        goto Fail;
-#endif
-
-FailGetConfDescr:
-#ifdef DEBUG_USB_HOST
-        NotifyFailGetConfDescr();
-        goto Fail;
-#endif
-
-FailSetConfDescr:
-#ifdef DEBUG_USB_HOST
-        NotifyFailSetConfDescr();
-        goto Fail;
-#endif
-
-
-FailSetIdle:
-#ifdef DEBUG_USB_HOST
-        USBTRACE("SetIdle:");
-#endif
-
-#ifdef DEBUG_USB_HOST
-Fail:
-        NotifyFail(rcode);
-#endif
-        Release();
-        return rcode;
-}
-
-HIDUniversal::HIDInterface* HIDUniversal::FindInterface(uint8_t iface, uint8_t alt, uint8_t proto) {
-        for(uint8_t i = 0; i < bNumIface && i < maxHidInterfaces; i++)
-                if(hidInterfaces[i].bmInterface == iface && hidInterfaces[i].bmAltSet == alt
-                        && hidInterfaces[i].bmProtocol == proto)
-                        return hidInterfaces + i;
-        return NULL;
-}
-
-void HIDUniversal::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 concidered.
-        if(bNumEP > 1 && conf != bConfNum)
-                return;
-
-        //ErrorMessage<uint8_t>(PSTR("\r\nConf.Val"), conf);
-        //ErrorMessage<uint8_t>(PSTR("Iface Num"), iface);
-        //ErrorMessage<uint8_t>(PSTR("Alt.Set"), alt);
-
-        bConfNum = conf;
-
-        uint8_t index = 0;
-        HIDInterface *piface = FindInterface(iface, alt, proto);
-
-        // Fill in interface structure in case of new interface
-        if(!piface) {
-                if(bNumIface >= maxHidInterfaces) {
-                        // don't overflow hidInterfaces[]
-                        Notify(PSTR("\r\n HIDUniversal::EndpointXtract(): Not adding HID interface because we already have "), 0x80);
-                        Notify(bNumIface, 0x80);
-                        Notify(PSTR(" interfaces and can't hold more. "), 0x80);
-                        return;
-                }
-                piface = hidInterfaces + bNumIface;
-                piface->bmInterface = iface;
-                piface->bmAltSet = alt;
-                piface->bmProtocol = proto;
-                bNumIface++;
-        }
-
-        if((pep->bmAttributes & bmUSB_TRANSFER_TYPE) == USB_TRANSFER_TYPE_INTERRUPT)
-                index = (pep->bEndpointAddress & 0x80) == 0x80 ? epInterruptInIndex : epInterruptOutIndex;
-
-        if(index) {
-                if(bNumEP >= totalEndpoints) {
-                        // don't overflow epInfo[] either
-                        Notify(PSTR("\r\n HIDUniversal::EndpointXtract(): Not adding endpoint info because we already have "), 0x80);
-                        Notify(bNumEP, 0x80);
-                        Notify(PSTR(" endpoints and can't hold more. "), 0x80);
-                        return;
-                }
-                // Fill in the endpoint info structure
-                epInfo[bNumEP].epAddr = (pep->bEndpointAddress & 0x0F);
-                epInfo[bNumEP].maxPktSize = (uint8_t)pep->wMaxPacketSize;
-                epInfo[bNumEP].bmSndToggle = 0;
-                epInfo[bNumEP].bmRcvToggle = 0;
-                epInfo[bNumEP].bmNakPower = USB_NAK_NOWAIT;
-
-                // Fill in the endpoint index list
-                piface->epIndex[index] = bNumEP; //(pep->bEndpointAddress & 0x0F);
-
-                if(pollInterval < pep->bInterval) // Set the polling interval as the largest polling interval obtained from endpoints
-                        pollInterval = pep->bInterval;
-
-                bNumEP++;
-        }
-        //PrintEndpointDescriptor(pep);
-}
-
-uint8_t HIDUniversal::Release() {
-        pUsb->GetAddressPool().FreeAddress(bAddress);
-
-        bNumEP = 1;
-        bAddress = 0;
-        qNextPollTime = 0;
-        bPollEnable = false;
-        return 0;
-}
-
-void HIDUniversal::ZeroMemory(uint8_t len, uint8_t *buf) {
-        for(uint8_t i = 0; i < len; i++)
-                buf[i] = 0;
-}
-
 uint8_t HIDUniversal::Poll() {
         uint8_t rcode = 0;
 
@@ -392,6 +45,11 @@ uint8_t HIDUniversal::Poll() {
                         if(read > constBuffLen)
                                 read = constBuffLen;
 
+                        // TODO: handle read == 0 ? continue like HIDComposite,
+                        //       return early like in the error case above?
+                        //       Either way passing an empty buffer to the functions below
+                        //       probably makes no sense?
+
 #if 0
                         Notify(PSTR("\r\nBuf: "), 0x80);
 
@@ -412,8 +70,3 @@ uint8_t HIDUniversal::Poll() {
         }
         return rcode;
 }
-
-// Send a report to interrupt out endpoint. This is NOT SetReport() request!
-uint8_t HIDUniversal::SndRpt(uint16_t nbytes, uint8_t *dataptr) {
-        return pUsb->outTransfer(bAddress, epInfo[epInterruptOutIndex].epAddr, nbytes, dataptr);
-}

hiduniversal.h

@@ -18,88 +18,43 @@ e-mail   :  support@circuitsathome.com
 #if !defined(__HIDUNIVERSAL_H__)
 #define __HIDUNIVERSAL_H__
 
-#include "usbhid.h"
+#include "hidcomposite.h"
-//#include "hidescriptorparser.h"
 
-class HIDUniversal : public USBHID {
+class HIDUniversal : public HIDComposite {
 
-        struct ReportParser {
+        bool SelectInterface(uint8_t iface __attribute__((unused)), uint8_t proto __attribute__((unused))) final {
-                uint8_t rptId;
+                // the original HIDUniversal didn't have this at all so make it a no-op
-                HIDReportParser *rptParser;
+                // (and made it final so users don't override this - if they want to use
-        } rptParsers[MAX_REPORT_PARSERS];
+                // SelectInterface() they should be deriving from HIDComposite directly)
+                return true;
+        }
 
-        // HID class specific descriptor type and length info obtained from HID descriptor
+        void ParseHIDData(USBHID *hid, uint8_t ep __attribute__((unused)), bool is_rpt_id, uint8_t len, uint8_t *buf) final {
-        HID_CLASS_DESCRIPTOR_LEN_AND_TYPE descrInfo[HID_MAX_HID_CLASS_DESCRIPTORS];
+                // override the HIDComposite version of this method to call the HIDUniversal version
-
+                // (which doesn't use the endpoint), made it final to make sure users
-        // Returns HID class specific descriptor length by its type and order number
+                // of HIDUniversal override the right version of ParseHIDData() (the other one, below)
-        uint16_t GetHidClassDescrLen(uint8_t type, uint8_t num);
+                ParseHIDData(hid, is_rpt_id, len, buf);
-
+        }
-        struct HIDInterface {
-                struct {
-                        uint8_t bmInterface : 3;
-                        uint8_t bmAltSet : 3;
-                        uint8_t bmProtocol : 2;
-                };
-                uint8_t epIndex[maxEpPerInterface + 1]; // We need to make room for the control endpoint as well
-        };
-
-        uint8_t bConfNum; // configuration number
-        uint8_t bNumIface; // number of interfaces in the configuration
-        uint8_t bNumEP; // total number of EP in the configuration
-        uint32_t qNextPollTime; // next poll time
-        uint8_t pollInterval;
-        bool bPollEnable; // poll enable flag
-
-        static const uint16_t constBuffLen = 64; // event buffer length
-
-        void Initialize();
-        HIDInterface* FindInterface(uint8_t iface, uint8_t alt, uint8_t proto);
-
-        void ZeroMemory(uint8_t len, uint8_t *buf);
 
 protected:
-        EpInfo epInfo[totalEndpoints];
-        HIDInterface hidInterfaces[maxHidInterfaces];
-
-        bool bHasReportId;
-
-        uint16_t PID, VID; // PID and VID of connected device
-
-        // HID implementation
-        HIDReportParser* GetReportParser(uint8_t id);
-
-        virtual uint8_t OnInitSuccessful() {
-                return 0;
-        };
-
         virtual void ParseHIDData(USBHID *hid __attribute__((unused)), bool is_rpt_id __attribute__((unused)), uint8_t len __attribute__((unused)), uint8_t *buf __attribute__((unused))) {
                 return;
-        };
+        }
 
 public:
-        HIDUniversal(USB *p);
+        HIDUniversal(USB *p) : HIDComposite(p) {}
 
-        // HID implementation
+        uint8_t Poll() override;
-        bool SetReportParser(uint8_t id, HIDReportParser *prs);
-
-        // USBDeviceConfig implementation
-        uint8_t Init(uint8_t parent, uint8_t port, bool lowspeed);
-        uint8_t Release();
-        uint8_t Poll();
-
-        virtual uint8_t GetAddress() {
-                return bAddress;
-        };
-
-        virtual bool isReady() {
-                return bPollEnable;
-        };
 
         // UsbConfigXtracter implementation
-        void EndpointXtract(uint8_t conf, uint8_t iface, uint8_t alt, uint8_t proto, const USB_ENDPOINT_DESCRIPTOR *ep);
+        void EndpointXtract(uint8_t conf, uint8_t iface, uint8_t alt, uint8_t proto, const USB_ENDPOINT_DESCRIPTOR *ep) override
-
+        {
-        // Send report - do not mix with SetReport()!
+                // If the first configuration satisfies, the others are not considered.
-        uint8_t SndRpt(uint16_t nbytes, uint8_t *dataptr);
+                if(bNumEP > 1 && conf != bConfNum)
+                        return;
+                // otherwise HIDComposite does what HIDUniversal needs
+                HIDComposite::EndpointXtract(conf, iface, alt, proto, ep);
+        }
 };
 
 #endif // __HIDUNIVERSAL_H__