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

[WIP] Feature/typings #65

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

Conversation

cojmeister
Copy link
Contributor

Added typings to the repo
Need to add more things.
This is still a work in progress.
@kakopappa I left some todos with questions.
What do you think about adding enums?
Feel free to leave asmany comments as you want

print('device_id: {} adjusted brightness level: {}'.format(device_id, brightness))
return True, brightness


def set_color(device_id, r, g, b):
# TODO: is there a bit type (0-256)?
Copy link
Contributor

Choose a reason for hiding this comment

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

did you find an alternative to int? looks like only int valid here

print(device_id, range_value, instance_id)
return True, range_value, instance_id


def mode_value(device_id, mode_value, instance_id):
def mode_value(device_id: str, mode_value: Any, instance_id: str) -> tuple[bool, Any, str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think mode_value can be str here.

Copy link
Contributor

@kakopappa kakopappa left a comment

Choose a reason for hiding this comment

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

Looks like you have spent a significant time refactoring the codebase to support typings. overall code is much better and readable now. Please let me know when you finish the TODOs

Thank you so much!

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