Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes off-by-one when looping over a Usage range. #63

Merged
merged 1 commit into from
Jul 14, 2020

Conversation

j3hyde
Copy link
Contributor

@j3hyde j3hyde commented Jul 10, 2020

I found that when finding usages in a report, that usage ranges were incorrectly reported. Given the descriptor below one less usage is reported than should be:

    0xa1, 0x02,       //     COLLECTION (Logical)
    0x05, 0x0a,       //     USAGE_PAGE (Ordinal)
    0x19, 0x01,       //     USAGE_MINIMUM (1)
    0x29, 0x08,       //     USAGE_MAXIMUM (8)
    0x05, 0x08,       //     USAGE_PAGE (LED)
    0x19, 0x4b,       //     USAGE_MINIMUM (Generic Indicator)
    0x29, 0x4b,       //     USAGE_MAXIMUM (Generic Indicator)
    0x95, 0x08,       //     REPORT_COUNT (8)
    0x75, 0x01,       //     REPORT_SIZE (1)
    0x15, 0x00,       //     LOGICAL_MINIMUM (0)
    0x25, 0x01,       //     LOGICAL_MAXIMUM (1)
    0x91, 0x02,       //     OUTPUT (Data, Var, Abs)
    0xc0,             //     END_COLLECTION (Logical)

Then, for example, using pywinusb:

>>> from pywinusb import hid
>>> device = hid.find_all_hid_devices()[-1]
>>> device.open()
>>> report = device.find_output_reports()[0]
>>> report
HID report object (Output report, id=0x0a), 7 items included

Investigating, I found that the usages are iterated using range() which goes from the minimum inclusive to the maximum exclusive while HID Usage Minimum is inclusive and Usage Maximum is also inclusive.

This PR adds one to the max to ensure that python range() generates the complete list of usage_ids.

@j3hyde
Copy link
Contributor Author

j3hyde commented Jul 10, 2020

Would fix #64

@rene-aguirre
Copy link
Owner

Thanks for the patch @j3hyde ! I guess this wasn't noticed before as most users are dealing with raw reports.

@rene-aguirre rene-aguirre merged commit bbf8c54 into rene-aguirre:master Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants