Nut upsdev - Nov 2021 - usbhidparser requirements to fix CyberPower OutputVoltage issue.

I would welcome a little assistance. I am debugging the issue affecting
CyberPower CP*PFCLCD devices as per
https://github.com/networkupstools/nut/issues/439 and others.


The parser HIDParse() in drivers/hidparser.c appears to terminate
processing of a HID report when an INPUT, OUTPUT, or FEATURE item is
encountered (or the data expires) and then saves it for later recall
with a memcpy().

In the case of the CyberPower HID the FEATURE item that is the normal
trigger is half way through the reports which means that any subsequent
items are not associated with the correct HID report. Worse still any
unprocessed items from the preceding report are used instead.

Here is an excerpt (from a CP900EPFCLCD) as an example:

        Report ID (16),
...
        Usage (54h),
        Logical Minimum (260),
        Logical Maximum (270),
        Feature (Variable, No Preferred, Volatile),
        Usage (54h),
        Input (Constant, Variable, No Preferred),
    End Collection,
    Usage (1Ch),
    Collection (Physical),
        Report ID (18),
        Usage (30h),
        Feature (Constant, Variable, No Preferred, Volatile),
        Report Size (8),
        Logical Minimum (0),
        Logical Maximum (255)

LogMin and LogMax items from Report ID 16 (with values 260 and 270) are
associated with reportID 18 not values 0 and 255 as should be the case.

This results in the erroneous behaviour raised in the GitHub issue.

The question is "What are the assumptions or requirements of the Parser
and the HID data that the code supports or should support?"

I understand this may or may not be consistent with the USB Usage Tables for
Power Devices spec but without having any real examples of vendor usb HID's
available it is not possible to work out how to proceed to develop a solution
that doesn't break support for other devices.

Have I missed something? - As it stands it doesnt seem like a reliable way of
delimiting each report.

Thanks
Dave

I did not dig deep into that part of code yet and am a bit vague on USB
internals in general, but from your description and data sample it seems
the proper solution is to raise a flag upon seeing any of those 3
conditions, and keep processing (looking for?) until nearest "end of
collection", and then save (memcpy to process in detail) that whole memory
block to have all the context?

I'd love to see comments from the experts, but a PR with developer-tests
that it fixes the issue (and commented logic for "future us") would be
also
great :)

Thanks,
Jim Klimov


On Tue, Nov 16, 2021, 17:14 Dave Williams <dave at
opensourcesolutions.co.uk>
wrote:
> I would welcome a little assistance. I am debugging the issue affecting
> CyberPower CP*PFCLCD devices as per
> https://github.com/networkupstools/nut/issues/439 and others.
>
>
> The parser HIDParse() in drivers/hidparser.c appears to terminate
> processing of a HID report when an INPUT, OUTPUT, or FEATURE item is
> encountered (or the data expires) and then saves it for later recall
> with a memcpy().
>
> In the case of the CyberPower HID the FEATURE item that is the normal
> trigger is half way through the reports which means that any subsequent
> items are not associated with the correct HID report. Worse still any
> unprocessed items from the preceding report are used instead.
>
> Here is an excerpt (from a CP900EPFCLCD) as an example:
>
>         Report ID (16),
> ...
>         Usage (54h),
>         Logical Minimum (260),
>         Logical Maximum (270),
>         Feature (Variable, No Preferred, Volatile),
>         Usage (54h),
>         Input (Constant, Variable, No Preferred),
>     End Collection,
>     Usage (1Ch),
>     Collection (Physical),
>         Report ID (18),
>         Usage (30h),
>         Feature (Constant, Variable, No Preferred, Volatile),
>         Report Size (8),
>         Logical Minimum (0),
>         Logical Maximum (255)
>
> LogMin and LogMax items from Report ID 16 (with values 260 and 270) are
> associated with reportID 18 not values 0 and 255 as should be the case.
>
> This results in the erroneous behaviour raised in the GitHub issue.
>
> The question is "What are the assumptions or requirements of the
Parser
> and the HID data that the code supports or should support?"
>
> I understand this may or may not be consistent with the USB Usage Tables
> for
> Power Devices spec but without having any real examples of vendor usb
HID's
> available it is not possible to work out how to proceed to develop a
> solution
> that doesn't break support for other devices.
>
> Have I missed something? - As it stands it doesnt seem like a reliable way
> of
> delimiting each report.
>
> Thanks
> Dave
>
> _______________________________________________
> Nut-upsdev mailing list
> Nut-upsdev at alioth-lists.debian.net
> https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/nut-upsdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://alioth-lists.debian.net/pipermail/nut-upsdev/attachments/20211119/b10c200b/attachment.htm>