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

Set zhimi.fan.za4 countdown timer to minutes #1787

Merged

Conversation

alex3305
Copy link
Contributor

This change fixes #1786.

There may be some minor cosmetic tweaks needed for the CLI. For instance, setting the timer still shows 'seconds':

❯ miiocli fan --ip 192.168.10.8 --token REDACTED delay_off 600
Setting delayed turn off to 600 seconds
['ok']

while actually it is in minutes 🤷. But apparently I'm not profound enough in Python to fix this. So any help for this is welcome!

I've added a (working) test and also tested it with my device and that seem to work great. So I was pretty content with myself 😄.


As you can see I've created a simple extension upon the base FanStatus class. I thought this was the most efficient way to implement this (very small) change for a specific device. I've not yet added the decorators per your suggestion as I wanted to get the basic stuff out of the way first. Maybe another extension is needed for those to be correct everywhere, but I want to keep the tasks / PR small, if you don't mind 😉.

Thanks again for the great library 🚀!

@syssi syssi self-assigned this Jun 27, 2023
@syssi
Copy link
Collaborator

syssi commented Jun 27, 2023

Thanks for your contribution! AFAIK the issue affects a bunch of fan models. I will try to provide some more input as soon as possible (probably next week). May be we are able to provide a more generic solution to provide a consistent interface.

@alex3305
Copy link
Contributor Author

@syssi Thanks for your reply and input. Sounds good to me, although I hope this issue doesn't lose it's scope. The countdown timer is now unusable for this device (from HA) and a fix would be very welcome.

@syssi
Copy link
Collaborator

syssi commented Jun 27, 2023

Trust me. In best case we are able to reduce your diff to 2-3 lines of code. :-)

@alex3305
Copy link
Contributor Author

@syssi Did you get around to look at this?

@alex3305
Copy link
Contributor Author

😢 Any updates on this? This functionality is broken for over 3 months now. Which terribly influenced my HA WAF. I hoped it could be done before summer ended, but now I would be happy to have implemented it at all.

pinging @syssi @rytilahti

@alex3305
Copy link
Contributor Author

Still broken

@rytilahti

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.

LGTM and thanks for the PR! I'll get this merged right now so that it will be part of the dev release I am currently preparing.

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 94.44444% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 81.43%. Comparing base (658ee2e) to head (6c2d5b8).
Report is 5 commits behind head on master.

Files Patch % Lines
miio/integrations/zhimi/fan/test_fan.py 92.30% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1787      +/-   ##
==========================================
+ Coverage   81.41%   81.43%   +0.02%     
==========================================
  Files         193      193              
  Lines       18564    18604      +40     
  Branches     4024     4032       +8     
==========================================
+ Hits        15113    15150      +37     
+ Misses       3174     3173       -1     
- Partials      277      281       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rytilahti rytilahti merged commit c97c1e5 into rytilahti:master Mar 13, 2024
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Countdown timer for fan zhimi.fan.za4 should be in minutes
3 participants