home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
71.11k stars 29.79k forks source link

[HomeKit] [Humidifier] min_humidity & max_humidity of humidifier not treated correctly by homekit integration #55173

Closed yaroslavbon closed 3 years ago

yaroslavbon commented 3 years ago

The problem

I have a humidifier in my system with the followiing attributes:

image

When added into homekit, min_humidity & max_humidity are ignored, and instead of expected humidity scale from 40 to 80 % in the Apple Home app, I have a slider that shows values from 0 to 100 % note: value set from home app correctly transposes to home assistant scale, e.g. if we set desired humidity to 100% in apple home app it will set humidifier to 80% in home assistant.

I'm not a phyton dev, but I checked homeassistant/components/homekit/type_humidifiers.py and saw the min&max attributes retrieval mechanism:

image

Can it be that this mechanism incorrectly retrieves min&max values from humidifier attributes and fallbacks to default ones?

JFYI, I'm using custom component for humidifier integration (hass-xiaomi-miot), but I believe it should not matter since it provides necessary attributes.

Thanks in advance!

What is version of Home Assistant Core has the issue?

core-2021.8.8

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

HomeKit

Link to integration documentation on our website

https://www.home-assistant.io/integrations/homekit/

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

probot-home-assistant[bot] commented 3 years ago

homekit documentation homekit source (message by IssueLinks)

probot-home-assistant[bot] commented 3 years ago

Hey there @bdraco, mind taking a look at this issue as it has been labeled with an integration (homekit) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

bdraco commented 3 years ago

Please turn in debug logging for pyhap and post the response that is being sent when iOS asks for /accessories

yaroslavbon commented 3 years ago

Hi @bdraco, here you go: pyhap_accessories.log

bdraco commented 3 years ago
{
    "aid": 2082452784,
    "services": [{
        "iid": 1,
        "type": "3E",
        "characteristics": [{
            "iid": 2,
            "type": "14",
            "perms": ["pw"],
            "format": "bool"
        }, {
            "iid": 3,
            "type": "20",
            "perms": ["pr"],
            "format": "string",
            "value": "deerma"
        }, {
            "iid": 4,
            "type": "21",
            "perms": ["pr"],
            "format": "string",
            "value": "deerma.humidifier.jsq3"
        }, {
            "iid": 5,
            "type": "23",
            "perms": ["pr"],
            "format": "string",
            "value": "Humidifier"
        }, {
            "iid": 6,
            "type": "30",
            "perms": ["pr"],
            "format": "string",
            "value": "humidifier.deerma_jsq3_53b6_humidifier"
        }, {
            "iid": 7,
            "type": "52",
            "perms": ["pr"],
            "format": "string",
            "value": "2.2.0"
        }]
    }, {
        "iid": 8,
        "type": "BD",
        "characteristics": [{
            "iid": 9,
            "type": "10",
            "perms": ["pr", "ev"],
            "format": "float",
            "minStep": 1,
            "maxValue": 100,
            "minValue": 0,
            "unit": "percentage",
            "value": 0
        }, {
            "iid": 10,
            "type": "B3",
            "perms": ["pr", "ev"],
            "format": "uint8",
            "valid-values": [0, 1, 2, 3],
            "value": 0
        }, {
            "iid": 11,
            "type": "B4",
            "perms": ["pr", "pw", "ev"],
            "format": "uint8",
            "valid-values": [1],
            "value": 1
        }, {
            "iid": 12,
            "type": "B0",
            "perms": ["pr", "pw", "ev"],
            "format": "uint8",
            "valid-values": [0, 1],
            "value": 0
        }, {
            "iid": 13,
            "type": "CA",
            "perms": ["pr", "pw", "ev"],
            "format": "float",
            "minStep": 1,
            "maxValue": 80,
            "minValue": 40,
            "unit": "percentage",
            "value": 60
        }]
    }]
}

CA is Relative Humidity Humidifier Threshold

The accessory looks like its being constructed correctly.

Does 0% map to 40 and 100% map to 80 when you test it ?

yaroslavbon commented 3 years ago

Yes, indeed these values are mapped as you described. It is just a matter of convenience because it is hard to set the exact desired humidity level with such scaling.

bdraco commented 3 years ago

I think we don't set the maxValue on thermostats per #33367 for that reason

bdraco commented 3 years ago

If you make this change:

diff --git a/homeassistant/components/homekit/type_humidifiers.py b/homeassistant/components/homekit/type_humidifiers.py
index 6371f883b0..27c3713d0e 100644
--- a/homeassistant/components/homekit/type_humidifiers.py
+++ b/homeassistant/components/homekit/type_humidifiers.py
@@ -110,10 +110,6 @@ class HumidifierDehumidifier(HomeAccessory):
             CHAR_CURRENT_HUMIDITY, value=0
         )

-        max_humidity = state.attributes.get(ATTR_MAX_HUMIDITY, DEFAULT_MAX_HUMIDITY)
-        max_humidity = round(max_humidity)
-        max_humidity = min(max_humidity, 100)
-
         min_humidity = state.attributes.get(ATTR_MIN_HUMIDITY, DEFAULT_MIN_HUMIDITY)
         min_humidity = round(min_humidity)
         min_humidity = max(min_humidity, 0)
@@ -123,7 +119,6 @@ class HumidifierDehumidifier(HomeAccessory):
             value=45,
             properties={
                 PROP_MIN_VALUE: min_humidity,
-                PROP_MAX_VALUE: max_humidity,
                 PROP_MIN_STEP: 1,
             },
         )

Does it behave more like you expect?

yaroslavbon commented 3 years ago

I'm sorry, can you suggest how can I find and change this file on running home assistant os? I got root access to os, but can not find this file. I'm not really familiar with HA development processes, but if you share some guide I could definitely try to apply your diff and check whether it helps.

bdraco commented 3 years ago

I put the changes in my dev version https://github.com/bdraco/homekit/commit/274fa349cb907fe4f61da96487a070053d851a99

You can install it as a custom component which will override the included homekit.

mkdir -p /config/custom_components
cd /config/custom_components
git clone https://github.com/bdraco/homekit
cd homekit
git pull origin master

Restart and check

To remove, delete /config/custom_components/homekit and restart

yaroslavbon commented 3 years ago

I did what you said, with your changes it became worse than before, but I suppose it was expected. So with your changes when I set humidity in apple home app to 0% it maps to 40% in home assistant, but when I try to set humidity in apple home app higher than 67% (have no idea why it is exactly 67) humidifier does not respond at all (meaning that 67% in home app = 80% in home assistant).

I tried to remove lower boundary from code as well, and only then it became more or less logical: in apple home app setting values < 40% (min humidity parameter) or > 80% (max humidity) have no effect on humidifier at all. And all values within the range (40 < x < 80) sends correct command and sets exact humidity value in home assistant.

The question is: can we somehow modify this humidity scale in apple home app so it won't be 0-100%, but 40-80% instead?

bdraco commented 3 years ago

Sadly I don't think its possible unless no min/max value is set. If you figure out a way to make it work without breaking other integrations, I'm happy to accept a PR.

yaroslavbon commented 3 years ago

Ok, I will do research and will let you know if I will find a way to restrict those boundaries.

Anyway, do you think that it is worth adding an option that will allow users to decide whether they want to use a scaled 0-100% version (as is) or 0-100% with no min/max humidity set so only applicable values from the slider will send the command to home assistant? No problem if not, at least I can override this logic for myself in custom_components (thanks for that guide).

And overall, thanks for fast response!

bdraco commented 3 years ago

Anyway, do you think that it is worth adding an option that will allow users to decide whether they want to use a scaled 0-100% version (as is) or 0-100% with no min/max humidity set so only applicable values from the slider will send the command to home assistant? No problem if not, at least I can override this logic for myself in custom_components (thanks for that guide).

We generally don't offer multiple options on how to represent a Home Assistant entity in HomeKit as that makes it more difficult to maintain and tends to break if the entity model changes.

yaroslavbon commented 3 years ago

I agree, thanks for your support