home-assistant / frontend

:lollipop: Frontend for Home Assistant
https://demo.home-assistant.io
Other
4.06k stars 2.78k forks source link

Incorrect target_humidity_min is shown if target_humidity is not set #14995

Closed jbouwh closed 1 year ago

jbouwh commented 1 year ago

Checklist

Describe the issue you are experiencing

With #14575 the target_humidity is always shown. To solve that now the target_humidity_min shows when target_humidity is None. This is not correct. In cases where target_humidity I would like to see a similar behaviour as where the climate control has no actual temperature. It can be set from the UI.

Describe the behavior you expected

I do not expect to see target_humidity_min as active target_humidity, in cases where the mode of the humidifier not supports a target_humidty and sets it to null (None).

Steps to reproduce the issue

1. 2. 3. ...

What version of Home Assistant Core has the issue?

Home Assistant 2023.1.0.dev6

What was the last working version of Home Assistant Core?

No response

In which browser are you experiencing the issue with?

No response

Which operating system are you using to run this browser?

No response

State of relevant entities

No response

Problem-relevant frontend configuration

No response

Javascript errors shown in your browser console/inspector

No response

Additional information

No response

github-actions[bot] commented 1 year ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Shulyaka commented 1 year ago

Sorry, I think I have missed this issue. The https://github.com/home-assistant/frontend/pull/14575 only fixes a layout issue on certain conditions (a humidifier card didn't show the value in 'panel' mode while showing in non-panel mode), it wasn't supposed to bring any functionality change.

I think we don't actually support humidifiers (or modes) that don't allow setting target humidity, so the behavior in this case is undefined and not documented. We may think of a proper way to support those. Is a null target humidity a proper way to communicate to the frontend that setting of a target humidity is not supported? Could there be downsides? For example, what if we have "write-only" humidifiers that don't provide their target humidity but allow setting one?

jbouwh commented 1 year ago

I think we don't actually support humidifiers (or modes) that don't allow setting target humidity, so the behavior in this case is undefined and not documented.

Well some humidifiers have an auto mode and manual modes, with the manual modes the target humidity becomes unknown, because it is not controlled. In that case we do not want to show the minimum humidity. Actually, the humidifier I own myself (xiaomi-miio) is of such type.

jbouwh commented 1 year ago

Another case where it goes wrong is where the device is switched off. The target humidity then is no longer exposed and set to None (unknown) but the UI shows the minimum humidity when the target humidity is set to None

Shulyaka commented 1 year ago

If by 'off' you mean it is unavailable, then it is actually the issue in the backend integration, it should report the correct state as unavailable.

jbouwh commented 1 year ago

When the device is off the target_humidity is not exposed as an attribute when it is set to None, this will cause that the frontend shows the minimal humidity instead, which is incorrect.

jbouwh commented 1 year ago

@Shulyaka Do you think you can solve this issue, may be we should not show min_humidity and max_humidity if target_humidity is None?