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 Xiaomi Mi Air Dehumidifier #28690

Closed
wants to merge 1 commit into from
Closed

Add Xiaomi Mi Air Dehumidifier #28690

wants to merge 1 commit into from

Conversation

stkang
Copy link

@stkang stkang commented Nov 11, 2019

Description:

Add Device Xiaomi Mi Air Dehumidifier

Related issue (if applicable):
fixes #

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Example entry for configuration.yaml (if applicable):

climate:
  - platform: xiaomi_miio
    name: xiaomi_dehumidifier
    host: 192.168.100.112
    token: !secret xiaomi_dehumidifier_token
    model: 'nwt.derh.wdh318efw1'

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@probot-home-assistant
Copy link

Hey there @rytilahti, @syssi, mind taking a look at this pull request as its been labeled with a integration (xiaomi_miio) you are listed as a codeowner for? Thanks!

@MartinHjelmare MartinHjelmare changed the title Support for Xiaomi Mi Air Dehumidifier Add Xiaomi Mi Air Dehumidifier Nov 11, 2019
@springstan
Copy link
Member

@stkang please format your code with black, e.g. run black --fast homeassistant in your development environment :)

@sasukebinbin
Copy link

Can't wait to see this supported by HA

Copy link
Member

@springstan springstan left a comment

Choose a reason for hiding this comment

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

I have some suggestions please take a look :)

ATTR_COMPRESSOR_STATUS = 'compressor_status'
ATTR_DEFROST_STATUS = 'defrost_status'
ATTR_FAN_ST = 'fan_st'
ATTR_ALARM = 'alarm'
Copy link
Member

Choose a reason for hiding this comment

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

You could move these constants or even more into a separate file e.g. const.py and import them into this file.

self._skip_update = False
return

try:
Copy link
Member

Choose a reason for hiding this comment

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

Only wrap one statement where an exception could occur with a try-catch statement

@property
def temperature_unit(self):
"""Return the unit of measurement which this thermostat uses."""
return TEMP_CELSIUS
Copy link
Member

Choose a reason for hiding this comment

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

Does this model only support measuring the temperature in celsius?

ATTR_DEFROST_STATUS: 'defrost_status',
ATTR_FAN_ST: 'fan_st',
ATTR_ALARM: 'alarm',
}
Copy link
Member

Choose a reason for hiding this comment

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

Looks like 1-1 mapping to me. What purpose does this have?

async def async_setup_platform(hass, config, async_add_entities,
discovery_info=None):
"""Set up the miio fan device from config."""
from miio import Device, DeviceException
Copy link
Member

Choose a reason for hiding this comment

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

Please move all imports to the top of the file, see #27284 for more details.

@property
def min_humidity(self):
"""Return the minimum humidity."""
return 40
Copy link
Member

Choose a reason for hiding this comment

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

What about other models? Do the support a different min/max humidity?

@syssi
Copy link
Member

syssi commented Dec 8, 2019

@stkang Could you care about the review of @springstan? I would like to see this device supported.

@sasukebinbin
Copy link

I have used the py file to create a custom component, it works great.

@stkang
Copy link
Author

stkang commented Dec 9, 2019

@syssi @sasukebinbin @springstan

I can't build a development environment for HA.
(I have a lot of custom components.)

However, this code works.

This code is based on the existing miio_fan.

If you really need this component, please fix the above CI error and make a new Pull Request.
(anyone is fine)

@springstan
Copy link
Member

@stkang you should not build your dev environment on top of your existing home assistant instance. Instead you should set up a fresh development environment like it is mentionend in the developer documentation.

@balloob
Copy link
Member

balloob commented Jan 10, 2020

This PR seems to have gone stale. Closing it.

@balloob balloob closed this Jan 10, 2020
@lock lock bot locked and limited conversation to collaborators Jan 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants