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

make parse optional #16

Closed
wants to merge 1 commit into from
Closed

make parse optional #16

wants to merge 1 commit into from

Conversation

meeuw
Copy link

@meeuw meeuw commented Nov 11, 2022

I'd like to python-hid-parser with some descriptors that can't be parsed by the default parser (similar to #10).

With these changes ReportDescriptor.print() is usable though and allows me to extend ReportDescriptor and use self._iterate_raw.

@meeuw meeuw requested a review from FFY00 as a code owner November 11, 2022 19:48
@FFY00
Copy link
Member

FFY00 commented Nov 11, 2022

Things will break when using the rest of the functionality when we don't parse, we should have checks for that. However, I think it would be better if instead of allowing the user to disable the parsing, to just delay the parsing until needed.

For that, we should probably turn

https://github.com/usb-tools/python-hid-parser/blob/main/hid_parser/__init__.py#L603-L605

into a NamedTuple

class _Pools(NamedTuple):
    input = _ITEM_POOL
    output = _ITEM_POOL
    feature = _ITEM_POOL

And then make it a property, where we trigger the parsing when accessed

@property
def _pools(self) -> _Pools:
    if not self._parsed:
        self.__pools = _Pools({}, {}, {})
        self._parse()
        self._parsed = True
    return self.__pools

@meeuw
Copy link
Author

meeuw commented Nov 12, 2022

After some hacking I found out that I actually need some information from _parse and I shouldn't re-implement it.

I'll try to create another PR to see if I can fix the parser or at least let it fail gracefully.

@meeuw meeuw closed this Nov 12, 2022
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