Expanding In/Out Reports on Keil USB HID example code

Hi ULI,

> I did all the stuff above, and in principal, it works too, but only for the Report Sizes 1,2,3 and 4. (and yes, I enlarged EP0Buf to 64 (resp MAX_PACKET_SIZE0)

Sound like the limit of 4 bytes comes from wMaxPacketSize of the interrupt IN endpoint, doesn't it?

C:\Keil\ARM\Boards\Keil\MCB1700\USBHID\usbdesc.c

const U8 USB_ConfigDescriptor[] = {
  ...
  ...
/* Endpoint, HID Interrupt In */
  USB_ENDPOINT_DESC_SIZE,            /* bLength */
  USB_ENDPOINT_DESCRIPTOR_TYPE,      /* bDescriptorType */
  USB_ENDPOINT_IN(1),                /* bEndpointAddress */
  USB_ENDPOINT_TYPE_INTERRUPT,       /* bmAttributes */
  WBVAL(0x0004),                     /* wMaxPacketSize */   <--------- 0x0040 ie. 64 bytes
  0x20,          /* 32ms */          /* bInterval */

Tsuneo

grrrrr............................
I was close to losing consciousness upon reading this..

THANK YOU TSUNEO ONCE MORE !!!!!!!!!!!!!
I had marked all the spots of modification then with the ARM 7 stuff,
oviously all except one ...

in general, what is your opinion regarding these samples in quality?
from my viewpoint, they could a lot better regarding modularization and
ease of use...
eg my current (solved) problem: it would have been a lot easier to have just
U8 InReport[REPORT_SIZE]
and only one single point for definition and modification

or, eg if looking at the string descriptors ...

how to handle a "reallife" serial number properly ?
this sample won't work if it is stored in an EEProm

what do you think of my idea to solve this ?
keep all strings in ordinary const char and, upon startup (before USBInit !!)
go through them and expand them to wide strings in the given string descriptor structure. Upon extraction, I can easily fill in my real serial number from elsewhere. does this sound OK ?

again: a big thanx

ULI

PS to Garrett: apologies for stumbling in

> in general, what is your opinion regarding these samples in quality?

Not so bad, but not so good.
I recommend the programmers of KEIL stack to read as many stack code from others.
Most of USB MCU manufacturers release their own USB stack.
Also, examine the interfaces of commercial USB stacks.
These materials will give many suggestion on the organization of the stack.

> keep all strings in ordinary const char and, upon startup (before USBInit !!)
go through them and expand them to wide strings in the given string descriptor structure. Upon extraction, I can easily fill in my real serial number from elsewhere. does this sound OK ?

It's OK.
One step advance, this string expansion process is directly integrated in Get_Descriptor( string ) handler, so that the string expansion goes on demand. And then, you can save RAM space for the expanded strings.

If this stack would have an extra hook for SETUP handler, we could modify the behavior of the stack without changing the stack code at all. This kind of hook is seen on other stacks, like Microchip, ST micro, EZUSB, etc.

Tsuneo

thank you once more!

Strings on demand sounds smart.

My problem is that a REAL understanding from bottom to top is still not existent, despite your posts and Axelson's books.
Nowadays, all the sales guys say: get this library, get this component, stick it together, add some glue code and then lets sell it. No spare time to go into detail, no way to become a real expert, especially if in the meantime (last time I was at USB in detail was March or so) so many other topics were hot and needed immediate attention.

regarding the KEIL code, I think its limitations show up once you want to transform it into "real code". from my point of view, it should be easy to separate the sources and have the option to treat them in a "library style" (eg separate folders, maybe even in compiled form)
this is not possible here as they have mixed up everything. (eg the core includes demo.h, and all resides in one folder) First thing for me to do after I got it to basically communicate will be "cleaning up and separating" (and hoping it will not be broken afterwards)

btw, for these tests I always use the HID Exerciser. (cool tool)

<<<Sometimes>>> (not sure about the circumstances and not regularly) upon arrival of one of my (Keil driven) devices, the detection (readout of strings) fails and the exerciser only lists VID+PID in hex. Any idea what this might be? This seems never to happen with other devices.

Another point I always have to add:

why not post your paypal account here ? ;)) Your comments are so helpful ...and probably take so much of your time...keeping you from "real" work. Or maybe write a book ;)

What I would find very interesting is a list of all your USB children ;)))
-> thinking of all the devices you were responsible for.
I bet, there are some Tsuneo driven parts in my house :D
-> probably not possible due to nda limitations ;((

Vielen Dank!
Uli

Hi Tsuneo,

I implemented the changes but it does not seem to be working quite right. I used SimpleHIDWrite to test the new application (loop back on EP1). I hope this is okay? The repeated IN reports have stopped - in fact I did not appear to be receiving any reports at the PC. What I expected to see was the written command followed by a single identical report being read back from the device. I could see the written report but no read report - hence my assessment that the changes are not working ...

I am working with 8 byte packets IN and OUT so I have ...

usbcfg.h
#define USB_MAX_PACKET0     8

usbdesc.c
/* USB Configuration Descriptor */
/*   All Descriptors (Configuration, Interface, Endpoint, Class, Vendor) */
const U8 USB_ConfigDescriptor[] = {
/* Configuration 1 */
  USB_CONFIGUARTION_DESC_SIZE,       /* bLength */
  USB_CONFIGURATION_DESCRIPTOR_TYPE, /* bDescriptorType */
  WBVAL(                             /* wTotalLength */
    USB_CONFIGUARTION_DESC_SIZE +
    USB_INTERFACE_DESC_SIZE     +
    HID_DESC_SIZE               +
    USB_ENDPOINT_DESC_SIZE              +        // added
        USB_ENDPOINT_DESC_SIZE                   //     added
  ),
  0x01,                              /* bNumInterfaces */
  0x01,                              /* bConfigurationValue: 0x01 is used to select this configuration */
  0x00,                              /* iConfiguration: no string to describe this configuration */
  USB_CONFIG_BUS_POWERED /*|*/       /* bmAttributes */
/*USB_CONFIG_REMOTE_WAKEUP*/,
  USB_CONFIG_POWER_MA(100),          /* bMaxPower, device power consumption is 100 mA */

/* Interface 0, Alternate Setting 0, HID Class */
  USB_INTERFACE_DESC_SIZE,           /* bLength */
  USB_INTERFACE_DESCRIPTOR_TYPE,     /* bDescriptorType */
  0x00,                              /* bInterfaceNumber */
  0x00,                              /* bAlternateSetting */
//  0x01,                              /* bNumEndpoints */
  0x02,                              /* bNumEndpoints */
  USB_DEVICE_CLASS_HUMAN_INTERFACE,  /* bInterfaceClass */
  HID_SUBCLASS_NONE,                 /* bInterfaceSubClass */
  HID_PROTOCOL_NONE,                 /* bInterfaceProtocol */
  0x04,                              /* iInterface */
/* HID Class Descriptor */
/* HID_DESC_OFFSET = 0x0012 */
  HID_DESC_SIZE,                     /* bLength */
  HID_HID_DESCRIPTOR_TYPE,           /* bDescriptorType */
  WBVAL(0x0100), /* 1.00 */          /* bcdHID */
  0x00,                              /* bCountryCode */
  0x01,                              /* bNumDescriptors */
  HID_REPORT_DESCRIPTOR_TYPE,        /* bDescriptorType */
  WBVAL(HID_REPORT_DESC_SIZE),       /* wDescriptorLength */
/* Endpoint, HID Interrupt In */
  USB_ENDPOINT_DESC_SIZE,            /* bLength */
  USB_ENDPOINT_DESCRIPTOR_TYPE,      /* bDescriptorType */
  USB_ENDPOINT_IN(1),                /* bEndpointAddress */
  USB_ENDPOINT_TYPE_INTERRUPT,       /* bmAttributes */
//  WBVAL(0x0004),                     /* wMaxPacketSize */
  WBVAL(0x0008),                     /* wMaxPacketSize */
  0x20,          /* 32ms */          /* bInterval */
//  0x01,          /* 1ms */          /* bInterval */

/* Endpoint, HID Interrupt Out */
  USB_ENDPOINT_DESC_SIZE,            /* bLength */
  USB_ENDPOINT_DESCRIPTOR_TYPE,      /* bDescriptorType */
  USB_ENDPOINT_OUT(1),               /* bEndpointAddress */
  USB_ENDPOINT_TYPE_INTERRUPT,       /* bmAttributes */
//  WBVAL(0x0004),                     /* wMaxPacketSize */
  WBVAL(0x0008),                     /* wMaxPacketSize */
  0x20,          /* 32ms */          /* bInterval */
//  0x01,          /* 1ms */          /* bInterval */

/* Terminator */
  0                                  /* bLength */
};

usbuser.c
static OS_SEM USB_IN_EP1_Semaphore;                     // semaphore for IN EP1

/*
 *  USB Endpoint 1 Task
 *   Handles USB Endpoint 1 Events
 */

#if (USB_EP_EVENT & (1 << 1))
__task void USB_EndPoint1 (void) {
  U16 evt;
  U32 report_size;      // added


  for (;;) {
    os_evt_wait_or(0xFFFF, 0xFFFF);         /* Wait for an Event */
    evt = os_evt_get();                     /* Get Event Flags */

    if (evt & USB_EVT_OUT) {                                 // OUT EP1 interrupt comes
      report_size = USB_ReadEP( HID_EP_OUT, &OutReport[0] );     //   read out the output report
     // usually, your firmware parse the output report, here
     // in this snippet, just copy output report to input one directly, for loopback
//      SetOutReport();

     memcpy( InReport, OutReport, report_size );
                                                             // send an input report to IN EP
      os_sem_wait( USB_IN_EP1_Semaphore, 0xffff );           //   wait until IN EP is available
//      USB_WriteEP( HID_EP_IN, InReport, HID_INPUT_REPORT_BYTES ); //   pass it to the endpoint
      USB_WriteEP( HID_EP_IN, InReport, sizeof(InReport) ); //   pass it to the endpoint
    }


        if (evt & USB_EVT_IN) {
                os_sem_send( USB_IN_EP1_Semaphore );                   //   IN EP1 is free

/*
      GetInReport();
//      USB_WriteEP(HID_EP_IN, &InReport, sizeof(InReport));
      USB_WriteEP(HID_EP_IN, &InReport[0], sizeof(InReport));
*/
        }
  }
}
#endif




/*
 *  USB Core Task
 *   Handles USB Core Events
 */

__task void USB_Core (void) {
#if (USB_CONFIGURE_EVENT || USB_INTERFACE_EVENT || USB_FEATURE_EVENT)
  U16 evt;
#endif

  for (;;) {

    os_evt_wait_or(0xFFFF, 0xFFFF);         /* Wait for an Event */

#if (USB_CONFIGURE_EVENT || USB_INTERFACE_EVENT || USB_FEATURE_EVENT)
    evt = os_evt_get();                     /* Get Event Flags */
#endif

#if USB_CONFIGURE_EVENT
    if (evt & USB_EVT_SET_CFG) {
      if (USB_Configuration) {                          /* Check if USB is configured */

       os_sem_init( USB_IN_EP1_Semaphore, 1 );    // IN EP1 is available now

/*
        GetInReport();
//        USB_WriteEP(HID_EP_IN, &InReport, sizeof(InReport));
        USB_WriteEP(HID_EP_IN, &InReport[0], sizeof(InReport));
*/

      }
    }
#endif

#if USB_INTERFACE_EVENT
    if (evt & USB_EVT_SET_IF) {
    }
#endif

#if USB_FEATURE_EVENT
    if (evt & USB_EVT_SET_FEATURE) {
    }
    if (evt & USB_EVT_CLR_FEATURE) {
    }
#endif

  }
}

hiduser.c

BOOL HID_GetReport (void) {

  /* ReportID = SetupPacket.wValue.WB.L; */
  switch (SetupPacket.wValue.WB.H) {
    case HID_REPORT_INPUT:

//      GetInReport();
//        memcpy(EP0Buf,InReport,sizeof(InReport));
      memcpy( EP0Buf, InReport, SetupPacket.wLength );
      break;
    case HID_REPORT_OUTPUT:
      return (__FALSE);        /* Not Supported */
    case HID_REPORT_FEATURE:
      /* EP0Buf[] = ...; */
      /* break; */
      return (__FALSE);        /* Not Supported */
  }
  return (__TRUE);
}



BOOL HID_SetReport (void) {

  /* ReportID = SetupPacket.wValue.WB.L; */
  switch (SetupPacket.wValue.WB.H) {
    case HID_REPORT_INPUT:
      return (__FALSE);        /* Not Supported */
    case HID_REPORT_OUTPUT:
//        memcpy(OutReport,EP0Buf,sizeof(OutReport));
      memcpy( OutReport, EP0Buf, SetupPacket.wLength );
//      SetOutReport();
      break;
    case HID_REPORT_FEATURE:
      return (__FALSE);        /* Not Supported */
  }
  return (__TRUE);
}

I did not implement the feature report handling at this stage.
I commented everything out in SetOutReport and GetInReport

I will continue to check over the code but if you can spot anything obvious or perhaps point me in a certain direction that would be great. I've probably done something silly or missed something!

Or possibly SimpleHODWRite not the right program for testing? But I think it should work okay?

Many thanks in advance!
Gareth.

Gareth,

what compiler for PC are you using?
I can send you my bare HID testprogram built with VC6

basically, after device detection and FileOpen it is just a matter of WriteFile/ReadFile, nothing else.

In case you want it, just send me a message via my contact form in order to transfer your mail address. you can find the form if you just take my last name and add .de

regards
Ulrich

Hi Tsuneo (and Ulrich)

Thank you for both taking the trouble to read this thread!
As you can probably tell - I am a little new to programming for USB and just knowing someone is out there watching is a great comfort!!!

I am delighted to say that I've worked through everything again (and again ...) and I seem to have solved my problems (for now).

a) I was not using the array pointer properly when filling IN EP1 (cut and paste error - doh!!)
b) also I may have been using the 'Set Report' command rather then 'Write' command in SimpleHIDWrite program ...

Anyway the main problem was the line of code which sent the copied InReport to the EP.

So I now have looping reports working properly with only one IN report send to the PC on receipt of an OUT report from the PC!!

HOORAY! It's a good step as I now have full control of when IN reports are sent. I now just have to fit the exact functionality required for my project.

I will keep you posted as to how I'm getting on!

MANY THANKS ONCE AGAIN!
Best regards,
Gareth.

Hi Tsuneo,

Following my success earlier today I have sorted out the information requests from the host - everything seems to be working very nicely!

I'm about to start work on sending reports when information changes ...
In your example snippet you show a dedicated task. I assume that it is okay to place this task pretty much anywhere?

I was going to create a task (which repeats, say every 50ms) for monitoring variable changes and place this in the main part of the application program separate from the USB parts. I assume it's okay to do this? It looks like the use of the semaphore which is set every IN request allows for 'remote' processing of the EP1.

I plan to implement this tomorrow.
Any comments you might have would be very welcome!

Thanks,
Gareth.

Congratulation!!
I'm glad you got good start of your USB life ;-)

> I assume that it is okay to place this task pretty much anywhere?

It's right.

Tsuneo