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

airpurifier_miot: Move favorite_rpm from MB4 to Basic #1070

Merged
merged 4 commits into from
Jun 16, 2021
Merged

airpurifier_miot: Move favorite_rpm from MB4 to Basic #1070

merged 4 commits into from
Jun 16, 2021

Conversation

SylvainPer
Copy link
Contributor

favorite_rpm is also a 3H parameter so it should be in the basic part

Tested on my local setup, I wasn't able to read this parameter before the modification with the status() function.

favorite_rpm is also a 3H parameter so it should be in the basic part
@SylvainPer
Copy link
Contributor Author

Sorry but I'm not familiar with this check and I don't understand why it failed.

@rytilahti
Copy link
Owner

The error came from black (so code formatting, likely some whitespace changes). The simplest way to fix is to run pre-commit run -a and it will correct it for you.

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! As it is unclear which devices support this, let's play it safe and avoid crashing just in case that property is not available on some devices.

miio/airpurifier_miot.py Outdated Show resolved Hide resolved
miio/airpurifier_miot.py Outdated Show resolved Hide resolved
SylvainPer and others added 2 commits June 16, 2021 16:39
Co-authored-by: Teemu R. <tpr@iki.fi>
Co-authored-by: Teemu R. <tpr@iki.fi>
@rytilahti
Copy link
Owner

Please run pre-commit run -a in your local copy to fix the linting errors, then this is good to go :-)

@SylvainPer
Copy link
Contributor Author

yes, it's on going:
pi@raspberrypi:~/temp/miio/python-miio $ pre-commit run -a
[INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Initializing environment for https://github.com/psf/black.
[INFO] Initializing environment for https://github.com/pre-commit/mirrors-isort.
[INFO] Initializing environment for https://github.com/pre-commit/mirrors-isort:toml.
[INFO] Initializing environment for https://github.com/PyCQA/doc8.
[INFO] Initializing environment for https://github.com/myint/docformatter.
[INFO] Initializing environment for https://gitlab.com/pycqa/flake8.
[INFO] Initializing environment for https://gitlab.com/pycqa/flake8:flake8-annotations,flake8-bugbear,flake8-builtins,flake8-docstrings,flake8-print,flake8-pytest-style,flake8-return,flake8-simplify.
[INFO] Initializing environment for https://github.com/PyCQA/bandit.
[INFO] Initializing environment for https://github.com/pre-commit/mirrors-mypy.
[INFO] Installing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/psf/black.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...

@SylvainPer
Copy link
Contributor Author

Done for the formatting but I had this at the end:

devtools/containers.py:61: error: Incompatible types in assignment (expression has type "List[_T]", variable has type "Optional[List[Dict[str, Any]]]")
Found 1 error in 1 file (checked 127 source files)

@rytilahti
Copy link
Owner

That is probably dependent on your mypy version; I don't get any errors on devtools locally and the CI passed just fine :-)

@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2021

Codecov Report

Merging #1070 (c5cd458) into master (6ab4665) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1070   +/-   ##
=======================================
  Coverage   75.40%   75.40%           
=======================================
  Files          73       73           
  Lines        8367     8367           
  Branches      745      745           
=======================================
  Hits         6309     6309           
  Misses       1874     1874           
  Partials      184      184           
Impacted Files Coverage Δ
miio/airpurifier_miot.py 89.28% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ab4665...c5cd458. Read the comment docs.

@SylvainPer
Copy link
Contributor Author

Hello @rytilahti ,
Any idea when the next version will be released ?
Is there a way to know which library version is installed ?
I'll update my plugin when the next version will be available but I'll get an error on oldest version.
Thanks

@rytilahti
Copy link
Owner

Hi @SylvainPer, I'll make a release in a couple of weeks when I return from the holidays, if @syssi doesn't beat me.

Wrt. checking the library version, you can use the standard __version__ property to access the version string:

>>> import miio
>>> miio.__version__
'0.5.5.2'

There is some module (maybe even in stdlib?) that allows parsing the string to something that can be used in sane comparisons, but I cannot recall the name of that.

@syssi
Copy link
Collaborator

syssi commented Aug 2, 2021

@rytilahti Feel free to release as soon as possible. I'm at holidays too and cannot provide much support at the moment.

@SylvainPer
Copy link
Contributor Author

Thanks for your return, I'm also at holidays now so enjoy them !

@SylvainPer
Copy link
Contributor Author

SylvainPer commented Aug 2, 2021

There is some module (maybe even in stdlib?) that allows parsing the string to something that can be used in sane comparisons, but I cannot recall the name of that.

found this:

>>> from packaging import version
>>> version.parse("2.3.1") < version.parse("10.1.2")
 True

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

4 participants