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 tinymu smart toiletlid #544

Merged
merged 18 commits into from
Sep 11, 2019
Merged

Add tinymu smart toiletlid #544

merged 18 commits into from
Sep 11, 2019

Conversation

scp10011
Copy link
Contributor

@scp10011 scp10011 commented Sep 2, 2019

select toiletlid status

Work: False
State: 1
Ambient Light: White
Filter remaining: 100%
Filter remaining time: 180

set toiletlid ambient light

Support color:
blue
green
orange
powder
purple
red
white
yellow

nozzle clean

def status(self) -> ToiletlidStatus:
"""Retrieve properties."""
properties = AVAILABLE_PROPERTIES[self.model]
_props_per_request = 15

Choose a reason for hiding this comment

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

local variable '_props_per_request' is assigned to but never used

Copy link
Owner

Choose a reason for hiding this comment

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

Do you mind fixing this?

Copy link
Owner

@rytilahti rytilahti left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I left some comments behind that should be handled before we merge this.

miio/toiletlid.py Show resolved Hide resolved
return self.work

@property
def work(self) -> bool:
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a need for this property, considering there's already a more generic is_on?

miio/toiletlid.py Show resolved Hide resolved
def status(self) -> ToiletlidStatus:
"""Retrieve properties."""
properties = AVAILABLE_PROPERTIES[self.model]
_props_per_request = 15
Copy link
Owner

Choose a reason for hiding this comment

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

Do you mind fixing this?

miio/toiletlid.py Show resolved Hide resolved
miio/toiletlid.py Outdated Show resolved Hide resolved
Device response error throws an exception
miio/toiletlid.py Outdated Show resolved Hide resolved
miio/toiletlid.py Outdated Show resolved Hide resolved
…ising a warning and returning "Unknown" is more friendly.
@rytilahti
Copy link
Owner

rytilahti commented Sep 10, 2019

Looks good to me now, thanks! 👍 One small thing, please add the full name of the device to README.md's list of supported devices. Also, if you have example outputs for those commands, please add them there for future reference (and to allow creating unit tests).

@syssi, you are more knowledgeable on props based integrations, ok to merge?

Copy link
Collaborator

@syssi syssi left a comment

Choose a reason for hiding this comment

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

Good job! :-)

@rytilahti
Copy link
Owner

rytilahti commented Sep 11, 2019

Thanks! 🎉 edit: I fixed the conflicts caused by a recent merge, this is now good to go.

@rytilahti rytilahti merged commit a29f30f into rytilahti:master Sep 11, 2019
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.

4 participants