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

Added context manager for Device ('with'-support) #108

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

RobertoRoos
Copy link
Contributor

I added __enter__ and __exit__ methods to enable context manager support. This makes uses of a device more pythonic.

I also renamed device to Device, since it's a class. I added an alias with a warning for device, so existing code would keep running.

Having said this out loud, the class renaming should have been a separate PR. Let me know and I'll split it again.

@RobertoRoos
Copy link
Contributor Author

Usage is now:

h = hid.Device()
with h.open(...):
    # pass

I would prefer:

with hid.open(...) as h:
    # pass

Much like opening a file.
Something like this could be more easily implemented with #109 .

@prusnak
Copy link
Member

prusnak commented Feb 11, 2021

Can you please add backwards compatibility for the rename, so also the old device would still work the same way? Maybe just device = Device would suffice.

@RobertoRoos
Copy link
Contributor Author

@prusnak I've added exactly that.
device extends Device with a warning in the constructor.

Copy link
Contributor

@jonasmalacofilho jonasmalacofilho left a comment

Choose a reason for hiding this comment

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

Hi,

I'm sorry for the unsolicited review, but as the maintainer of project that depends on cython-hidapi, I reviewed this and also tested it for backwards compatibility.

I left a few comments, please take a look at them when possible.

hid.pyx Outdated Show resolved Hide resolved
hid.pyx Outdated Show resolved Hide resolved
docs/examples.md Outdated Show resolved Hide resolved
hid.pyx Outdated Show resolved Hide resolved
@RobertoRoos
Copy link
Contributor Author

Reverted all (potentially) breaking changes.

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.

None yet

3 participants