BUG report: USB Host HID class parser contains an error

[funfunfun]

I'm writing for stm32f417 MCU. The project is generated using STM32CubeMX with firmware package: STM32Cube FW_F4 V1.26.2 (the latest so far).

In the USB Host Library there is a file related to HID class:

<my_proj_path>\Middlewares\ST\STM32_USB_Host_Library\Class\HID\Src\usbh_hid_parser.c 

There is a function:

uint32_t HID_ReadItem(HID_Report_ItemTypedef *ri, uint8_t ndx)

It parses items of incoming reports. And there is a code in it:

/* read data bytes in little endian order */
 for (x = 0U; x < ((ri->size & 0x7U) ? (ri->size / 8U) + 1U : (ri->size / 8U)); x++)
 {
 val = (uint32_t)((uint32_t)(*data) << (x * 8U));
 }
 val = (val >> shift) & ((1U << ri->size) - 1U);

You can see here the multiple type casting. It's just meaningless overhead. But if you iterate through the loop assigning a value to the variable val it'll be overwritten on every iteration. Moreover the data pointer also stays unchanged during the loop.

At least this line should be changed to:

val |= (uint32_t)(*data++) << (x * 8U);

I also suggest you to test this function over different size and shift values (not only <= 8)

WBR

[Sara BEN HADJ YAHYA]

Hello @funfunfun​ ,

Thanks for your feedback?

What kind of error did you get and could you please share your .ioc file to reproduce the issue?,

Thanks,

Sara.

[waclawek.jan]

Hi @Sara BEN HADJ YAHYA​ ,

funfunfun is right, this is an error.

The error will exhibit itself only if there are items with length > 8 or items which cross byte boundaries. In the usage with example HID report descriptors there are no such items, so you won't see the effect of the error with anything you can click in CubeMX, but it will bite when somebody tries to use a custom report descriptor.

JW

[Sara BEN HADJ YAHYA]

Hello,

This is not a CubeMX issue, I'm adding @Imen DAHMEN​  to the loop to investigate the issue.

Regards,

Sara.

[jvavra]

Is there any follow-up here? I am getting what I believe to be a similar error on an F2 device using a custom report descriptor. It causes class parsing errors resulting in intermittent USB enumeration.

[Robmar]

Not yet, maybe by 2030, 2035... or never. Such is STM's commitment to support!

For the mouse operation, the current code, even with this "fix" only retrieves the X delats in the Y var, and one button state, its a mess.

[Pavel A.]

Years ago we borrowed HID parser from the LUFA project . The Cube example code should not be taken too seriously, it just demonstrates the hardware. 

[waclawek.jan]

> intermittent USB enumeration.

The error described here will exhibit itself only if there are items with length > 8 or items which cross byte boundaries.The workaround described above should help with this particular error.

JW

[Teknotronix]

Nice solution!

I tried a number of permutations with the HID_Report_ItemTypedef values thinking I must not be understanding the intended use of the function, but came to the same conclusion that the line you modified was the issue. Thanks for sharing it!

[Robmar]

For the Mouse, with or without this mod, the Y reads correctly with the X value, X reads 0 always, only one button works, and STM's GitHub still doesn't have this fix, if it is a fix, 10 years after STM released the USB library!  Sorry to say, but what a sinking ship disaster is STM "support"!

https://github.com/STMicroelectronics/stm32-mw-usb-host/blob/master/Class/HID/Src/usbh_hid_parser.c