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.

Sign up for GitHub

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

Jump to bottom

Add support for dmaker.fan.p9 and dmaker.fan.p10 #819

Merged
merged 6 commits into from
Oct 7, 2020
Merged

Add support for dmaker.fan.p9 and dmaker.fan.p10 #819

merged 6 commits into from
Oct 7, 2020

Conversation

swim2sun
Copy link
Contributor

@swim2sun swim2sun commented Sep 18, 2020 •
edited
Loading

Add support for dmaker.fan.p9 and dmaker.fan.p10.
Fixes #721

@coveralls
Copy link

coveralls commented Sep 18, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.3%) to 74.305% when pulling a05fe07 on swim2sun:master into a87f872 on rytilahti:master.

@swim2sun swim2sun changed the title Add support for dmaker.fan.p9 and dmaker.fan.p10 #721 Add support for dmaker.fan.p9 and dmaker.fan.p10 Sep 18, 2020
rytilahti
rytilahti requested changes Oct 2, 2020
View reviewed changes
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 the PR! Could you please move the miot implementation (and the related tests) into its own file, e.g., fan_miot.py?

@xrays72 xrays72 mentioned this pull request Oct 2, 2020
Closed
@swim2sun swim2sun requested a review from rytilahti October 4, 2020 17:22
@swim2sun
Copy link
Contributor Author

swim2sun commented Oct 4, 2020

Hi, @rytilahti
Thanks for your advice, I have try to refactor it, pls review it.
Actually I am a java programmer, so I am not sure whether my code meets some best practices of Python

rytilahti
rytilahti requested changes Oct 4, 2020
View reviewed changes
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.

Hi @swim2sun, no worries, the code is looking pretty good python code to me! :-)

miio/fan.py Outdated Show resolved Hide resolved
miio/tests/test_fan.py Outdated Show resolved Hide resolved
miio/fan_common.py Show resolved Hide resolved
miio/fan_miot.py Show resolved Hide resolved
miio/fan_miot.py Outdated Show resolved Hide resolved
miio/fan_miot.py Outdated Show resolved Hide resolved
@swim2sun swim2sun requested a review from rytilahti October 5, 2020 04:24
@rytilahti
Copy link
Owner

LGTM! 👍 One small thing, could you update discovery.py for these?

@swim2sun
Copy link
Contributor Author

swim2sun commented Oct 7, 2020

@rytilahti Thanks for your guidance! discovery.py has been updated, pls check

rytilahti
rytilahti approved these changes Oct 7, 2020
View reviewed changes
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.

Looks great, thanks again! 🎉

@rytilahti rytilahti merged commit 9ec9ac4 into rytilahti:master Oct 7, 2020
@biscuitee
Copy link

Hi, thanks for adding dmaker.fan.p9 support. How could I add this support into Home Assistant?

swim2sun added a commit to swim2sun/python-miio that referenced this pull request Oct 13, 2020
@swim2sun
Add support for dmaker.fan.p9 and dmaker.fan.p10 (rytilahti#819)
a37e4c6 
* Add support for dmaker.fan.p9 and dmaker.fan.p10

* fix format issue

* add new supported fan to README

* move fan miot implementation to fan_miot.py

* move FanP9 and FanP10 to fan_miot.py

* add FanMiot to discovery.py
swim2sun added a commit to swim2sun/python-miio that referenced this pull request Oct 13, 2020
@swim2sun
Revert "Add support for dmaker.fan.p9 and dmaker.fan.p10 (rytilahti#819…
fa52c64 
…)"

This reverts commit a37e4c6.
xvlady pushed a commit to xvlady/python-miio that referenced this pull request May 9, 2021
@swim2sun @xvlady
Add support for dmaker.fan.p9 and dmaker.fan.p10 (rytilahti#819)
9ecdcc3 
* Add support for dmaker.fan.p9 and dmaker.fan.p10

* fix format issue

* add new supported fan to README

* move fan miot implementation to fan_miot.py

* move FanP9 and FanP10 to fan_miot.py

* add FanMiot to discovery.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rytilahti rytilahti rytilahti approved these changes

Assignees
No one assigned
Labels
miot new device
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

support dmaker.fan.p9 and dmaker.fan.p10
4 participants
@swim2sun @coveralls @rytilahti @biscuitee