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

Add HID classes #47

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

electretmike
Copy link

Add support for HID class specific descriptors. This is different from #6, as it doesn't include the actual HID report itself. It is assumed that that is already known or created using a different tool.

Copy link
Member

@martinling martinling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see this but I have some issues with naming.

usb_protocol/types/descriptors/hid.py Outdated Show resolved Hide resolved
usb_protocol/emitters/descriptors/hid.py Outdated Show resolved Hide resolved
Copy link
Member

@martinling martinling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more suggestions on the same theme.


@contextmanager
def DescriptorReference(self):
""" Context manager that allows addition of a subordinate report descriptor.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
""" Context manager that allows addition of a subordinate report descriptor.
""" Context manager that allows addition of a descriptor reference.

It can be used with a `with` statement; and yields an HIDDescriptorReferenceEmitter
that can be populated:

with hiddescriptor.DescriptorReference() as r:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with hiddescriptor.DescriptorReference() as r:
with hiddescriptor.DescriptorReference() as r:
r.bDescriptorType = 0x22
r.wDescriptorLength = 0x10

with hiddescriptor.DescriptorReference() as r:
r.wDescriptorLength = 0x10

This adds the relevant descriptor, automatically.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This adds the relevant descriptor, automatically.
This adds the relevant descriptor reference, automatically.

Comment on lines +39 to +42
# This is not really a stand-alone descriptor, but it it is more a reference to a report
# descriptor that can retrieved seperately. It is part of the HIDDescriptor above.
# That descriptor can contain multiple descriptor references. To support this, a seperate
# descriptor format is used.
Copy link
Member

@martinling martinling Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# This is not really a stand-alone descriptor, but it it is more a reference to a report
# descriptor that can retrieved seperately. It is part of the HIDDescriptor above.
# That descriptor can contain multiple descriptor references. To support this, a seperate
# descriptor format is used.
# This is not really a stand-alone descriptor, but rather a reference to a descriptor
# that can be retrieved seperately. It is part of the HIDDescriptor above. The HID
# descriptor can contain multiple descriptor references. To support this, a separate
# descriptor format is used.

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