Fix for exception when checking if fan without speed is on #39096
Conversation
probot-home-assistant
bot
added
bugfix
core
integration: fan
integration: mqtt
small-pr
|
Hey there @home-assistant/core, @emontnemery, mind taking a look at this pull request as its been labeled with an integration ( |
| @@ -335,7 +337,7 @@ def assumed_state(self): | |||
| @property | |||
| def is_on(self): | |||
| """Return true if device is on.""" | |||
| return self._state | |||
| return self._state == STATE_ON | |||
There was a problem hiding this comment.
This change wasn't needed. We prefer to just use a boolean.
There was a problem hiding this comment.
Not strictly needed given the state property uses is_on, but I thought it makes it more clear given the variable is called _state, not _is_on
There was a problem hiding this comment.
We prefer to keep it simple.
There was a problem hiding this comment.
@MartinHjelmare This PR was maybe merged a bit too quickly, do you think it should be reverted?
There was a problem hiding this comment.
The fix for is_on function is good. The changes around the property are not wanted and we should revert those.
|
@kbickar Can you give an example of when |
|
If a fan entity doesn't support set speed it won't have core/homeassistant/components/fan/__init__.py Lines 194 to 195 in 1804772 |
|
Here's an example configuration that would cause the issue: - platform: mqtt
name: "Sonoff Fan"
state_topic: "stat/box_fan/POWER"
command_topic: "cmnd/box_fan/power"As @MartinHjelmare said, since the speed attribute is optional and not set if it's not a supported feature, it generates a KeyError |
|
Thanks @kbickar and @MartinHjelmare! |
…stant#39096) * Fix for exception when checking if fan without speed is on * Organized imports * Space
…stant#39096) * Fix for exception when checking if fan without speed is on * Organized imports * Space
Proposed change
The
is_onfunction was generating an exception for fans without a speed such as an MQTT fanThis change updates that to first check if there's a speed attribute and if it can't find one it uses the state instead.
The MQTT fan component has also been standardized from using a boolean state to using STATE_ON/STATE_OFF
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: