Add support for Xiaomi Zero Fog Free Humidifier #33736

iromeo · 2020-04-06T10:54:32Z

Breaking change

Proposed change

Add support for: Xiaomi Zero Fog Free Humidifier support (shuii.humidifier.jsq001)

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example entry for `configuration.yaml`:

# Example configuration.yaml
fan:
  - platform: xiaomi_miio
    name: 'Air Humidifier'
    host: !secret humidifier_ip
    token: !secret humidifier_token
    model: shuii.humidifier.jsq001

Additional information

My PR to python-miio is already accepted and released in 0.5.0.0 (see rytilahti/python-miio#642) This PR adds itegration for the device type.

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Added documentation for Xiaomi Zero Fog Humidifier (shuii.humidifier.jsq001) home-assistant.io#12716

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

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

Documentation added/updated for www.home-assistant.io

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

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

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

…ntegration

…'hardware_version'

homeassistant · 2020-04-06T10:54:34Z

Hi @iromeo,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

probot-home-assistant · 2020-04-06T10:54:35Z

This pull request needs to be manually signed off by @home-assistant/core before it can get merged.

probot-home-assistant · 2020-04-06T10:54:40Z

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!

iromeo · 2020-04-06T11:04:15Z

Also, there is shuii.humidifier.jsq002 model (syssi/xiaomi_airpurifier#50), but issue author doesn't respond and I cannot add support for it without access to the real device.

MartinHjelmare

This integration and platform needs to be refactored first to follow our design guidelines regarding fan modes, state attributes and entity access, before any more additions are made.

MartinHjelmare · 2020-04-06T11:50:32Z

homeassistant/components/xiaomi_miio/fan.py

 }

 AVAILABLE_ATTRIBUTES_AIRHUMIDIFIER = {
    **AVAILABLE_ATTRIBUTES_AIRHUMIDIFIER_COMMON,
+    ATTR_TARGET_HUMIDITY: "target_humidity",
+    ATTR_HARDWARE_VERSION: "hardware_version",
+    ATTR_USE_TIME: "use_time",


Is this a relative time? Relative times are not allowed in the state machine. We only allow absolute utc timestamps.

This change is a refactoring of existing attributes, I just moved some unsupported attributes from common to more specific attributes sets. use_time attribute was added in 25.03.2018 by @syssi (see e36f27d). So I assume that it is in UTC.

Unfortunately this platform isn't following our design guidelines so we can not trust that the existing code is ok.

You recently accepted PR #31729 with the same code.

I could revert this change to the state before my commit if you want. But I don't think that copy-paste is better than trivial refactoring of the code working for more than 2 years.

We won't accept this PR until the integration and platform have been refactored. That should be done in a separate PR first.

This integration and platform needs to be refactored first to follow our design guidelines regarding fan modes, state attributes and entity access, before any more additions are made.

@rytilahti, @syssi As code owners of the integration do you understand how it should be refactored to address this request?

@syssi is more knowledgeable on this specific case, but some things I feel should be refactored:

Moving supported features, available attributes towards the backend library.

This involves, e.g., moving the list of available fan speeds to the backend, like was done recently for the vacuum platform.

Homeassistant has only 3 different fan speeds (+ off), so rest of those need to be exposed over separate services (if wanted).

Moving supported models also towards the backend. This will simplify future developments when the backend lib is the only source of truth.

I think reading somewhere that sensors are preferred over custom state attributes, but cannot find the reference from the docs at the moment. So, these attributes are the ones that should be exposed through the main device: https://developers.home-assistant.io/docs/entity_fan/#properties

This would mean creating new entities for those custom attributes, that are linked to the main device.

We can adapt the backend to report what type of attribute something is, that can then be used inside homeassistant to create sensors, binary_sensors, switches, etc. depending on the type.

I'm not sure what Martin means with entity access, maybe that's related?

Firmware information etc. should only be exposed to the device registry (https://developers.home-assistant.io/docs/device_registry_index/)

Lot of common code could be shared with all the platforms, e.g., the initialization is almost always the same, but is still c&p in different platforms.

Ah, found it (thanks @MartinHjelmare on that other PR: https://developers.home-assistant.io/docs/entity_sensor/#available-device-classes .

So in this case, to my understanding, we should have the following new entities (depending on the supported features of the device):

sensors:

temperature

humidity

binary_sensors:

child lock

led

no water

lid open

buzzer

unknown:

mode

led brightness

One more point, this architecture issue is relevant to this, too: home-assistant/architecture#310

@rytilahti I think that's a great plan and summary. It's not a small task though, so it will take some time and need to be taken step wise. 👍

With entity access I meant accessing the entity directly from outside the entity instead of via core or dispatcher. Direct entity access from outside is fragile since it only works after the entity has been added to home assistant. Now we also have a service helper that platforms can use to easily register services that need to call into an entity method.

core/homeassistant/components/xiaomi_miio/fan.py

Lines 573 to 574 in 0793b5a

await getattr(device, method["method"])(**params)

update_tasks.append(device.async_update_ha_state(True))

Here's the docs for the service helper:
https://developers.home-assistant.io/docs/dev_101_services#entity-services

And here's a real example how to use the service helper:

core/homeassistant/components/wled/light.py

Lines 56 to 71 in 34ecc80

platform = entity_platform.current_platform.get()

platform.async_register_entity_service(

SERVICE_EFFECT,

{

vol.Optional(ATTR_EFFECT): vol.Any(cv.positive_int, cv.string),

vol.Optional(ATTR_INTENSITY): vol.All(

vol.Coerce(int), vol.Range(min=0, max=255)

),

vol.Optional(ATTR_REVERSE): cv.boolean,

vol.Optional(ATTR_SPEED): vol.All(

vol.Coerce(int), vol.Range(min=0, max=255)

),

},

"async_effect",

)

MartinHjelmare · 2020-04-06T11:51:12Z

homeassistant/components/xiaomi_miio/fan.py

    ATTR_TRANS_LEVEL: "trans_level",
    ATTR_BUTTON_PRESSED: "button_pressed",
 }

 AVAILABLE_ATTRIBUTES_AIRHUMIDIFIER_CA_AND_CB = {
    **AVAILABLE_ATTRIBUTES_AIRHUMIDIFIER_COMMON,
+    ATTR_TARGET_HUMIDITY: "target_humidity",
+    ATTR_HARDWARE_VERSION: "hardware_version",


Static info is not meant for state attributes unless it's essential for automations.

These 2 attributes were moved from AVAILABLE_ATTRIBUTES_AIRHUMIDIFIER_COMMON to more specific implementations because they are not common for all humidifiers and not supported by shuii.humidifier.jsq001.

MartinHjelmare · 2020-04-06T11:52:11Z

homeassistant/components/xiaomi_miio/fan.py

+    **AVAILABLE_ATTRIBUTES_AIRHUMIDIFIER_COMMON,
+    ATTR_LED: "led",
+    ATTR_NO_WATER: "no_water",
+    ATTR_LID_OPENED: "lid_opened",


lid_opened seems like it should be a binary sensor. We want to minimize state attributes and instead break out to as many individual sensors as possible.

Yes, it could be a binary sensor. Could you guide me with some example of how I could replace with sate with a binary sensor?

MartinHjelmare · 2020-04-06T11:55:07Z

homeassistant/components/xiaomi_miio/fan.py

+            self._led_brightness_enum = AirhumidifierJsqLedBrightness
+            self._led_brightness_default = 0
+            self._operation_mode_enum = AirhumidifierJsqOperationMode
+            self._speed_list = [mode.name for mode in AirhumidifierJsqOperationMode]


We only allow the modes in the fan base entity integration:

core/homeassistant/components/fan/__init__.py

Lines 35 to 38 in cedf7e3

SPEED_OFF = "off"

SPEED_LOW = "low"

SPEED_MEDIUM = "medium"

SPEED_HIGH = "high"

The device does not fall into such a specification

Same issue as in #31729 (review)

starkillerOG · 2020-04-20T07:42:26Z

Maybe relevant for this PR too: I am busy with supporting Config Flow for the Xiaomi Miio platform, see PR: #32091

stale · 2020-05-20T08:29:58Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

iromeo added 2 commits April 5, 2020 19:36

Feature: Added Xiaomi Zero Fog Humidifier (shuii.humidifier.jsq001) i…

8cb064c

…ntegration

Fixed: AttributeError: 'AirHumidifierStatus' object has no attribute …

280356d

…'hardware_version'

homeassistant added the cla-needed label Apr 6, 2020

probot-home-assistant bot added the integration: xiaomi_miio label Apr 6, 2020

probot-home-assistant bot assigned rytilahti and syssi Apr 6, 2020

homeassistant added cla-signed and removed cla-needed labels Apr 6, 2020

iromeo mentioned this pull request Apr 6, 2020

Add Xiaomi Zero Fog Humidifier (shuii.humidifier.jsq001) support (#52) syssi/xiaomi_airpurifier#55

Merged

MartinHjelmare requested changes Apr 6, 2020

View reviewed changes

springstan changed the title ~~Xiaomi Zero Fog Free Humidifier support (shuii.humidifier.jsq001)~~ Add support for Xiaomi Zero Fog Free Humidifier Apr 6, 2020

rytilahti mentioned this pull request Apr 13, 2020

Add Xiaomi miio Alarm Control Panel #32091

Merged

20 tasks

MartinHjelmare self-assigned this Apr 19, 2020

rytilahti mentioned this pull request Apr 22, 2020

STYJ02YM Vacuum Integration #27268

Closed

stale bot added the stale label May 20, 2020

stale bot closed this May 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Xiaomi Zero Fog Free Humidifier #33736

Add support for Xiaomi Zero Fog Free Humidifier #33736

iromeo commented Apr 6, 2020

homeassistant commented Apr 6, 2020

probot-home-assistant bot commented Apr 6, 2020

probot-home-assistant bot commented Apr 6, 2020

iromeo commented Apr 6, 2020

MartinHjelmare left a comment

MartinHjelmare Apr 6, 2020

iromeo Apr 6, 2020 •

edited

Loading

MartinHjelmare Apr 6, 2020

iromeo Apr 6, 2020

MartinHjelmare Apr 6, 2020 •

edited

Loading

iromeo Apr 6, 2020

rytilahti Apr 6, 2020 •

edited

Loading

rytilahti Apr 6, 2020 •

edited

Loading

rytilahti Apr 6, 2020

MartinHjelmare Apr 6, 2020

MartinHjelmare Apr 6, 2020

iromeo Apr 6, 2020

MartinHjelmare Apr 6, 2020

iromeo Apr 6, 2020

MartinHjelmare Apr 6, 2020

iromeo Apr 6, 2020 •

edited

Loading

starkillerOG commented Apr 20, 2020

stale bot commented May 20, 2020

	await getattr(device, method["method"])(**params)
	update_tasks.append(device.async_update_ha_state(True))

	platform = entity_platform.current_platform.get()

	platform.async_register_entity_service(
	SERVICE_EFFECT,
	{
	vol.Optional(ATTR_EFFECT): vol.Any(cv.positive_int, cv.string),
	vol.Optional(ATTR_INTENSITY): vol.All(
	vol.Coerce(int), vol.Range(min=0, max=255)
	),
	vol.Optional(ATTR_REVERSE): cv.boolean,
	vol.Optional(ATTR_SPEED): vol.All(
	vol.Coerce(int), vol.Range(min=0, max=255)
	),
	},
	"async_effect",
	)

	SPEED_OFF = "off"
	SPEED_LOW = "low"
	SPEED_MEDIUM = "medium"
	SPEED_HIGH = "high"

Add support for Xiaomi Zero Fog Free Humidifier #33736

Add support for Xiaomi Zero Fog Free Humidifier #33736

Conversation

iromeo commented Apr 6, 2020

Breaking change

Proposed change

Type of change

Example entry for configuration.yaml:

Additional information

Checklist

homeassistant commented Apr 6, 2020

probot-home-assistant bot commented Apr 6, 2020

probot-home-assistant bot commented Apr 6, 2020

iromeo commented Apr 6, 2020

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iromeo Apr 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare Apr 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rytilahti Apr 6, 2020 • edited Loading

Choose a reason for hiding this comment

rytilahti Apr 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iromeo Apr 6, 2020 • edited Loading

Choose a reason for hiding this comment

starkillerOG commented Apr 20, 2020

stale bot commented May 20, 2020

Example entry for `configuration.yaml`:

iromeo Apr 6, 2020 •

edited

Loading

MartinHjelmare Apr 6, 2020 •

edited

Loading

rytilahti Apr 6, 2020 •

edited

Loading

rytilahti Apr 6, 2020 •

edited

Loading

iromeo Apr 6, 2020 •

edited

Loading