home-assistant / architecture

Repo to discuss Home Assistant architecture
313 stars 100 forks source link

Should climate use capability_attributes #334

Closed Jc2k closed 4 years ago

Jc2k commented 4 years ago

Context

I noticed when debugging a user issue that the homekit integration was failing to export a climate entity because hvac_modes was unavailable. Whilst there is the autostart mitigation it would be better if that metadata was made a capability_attribute. Presumably this would equally apply to the Google Assistant and Alexa use cases too.

Not sure if this requires an arch issue because i don't think its a behaviour change, but it does touch a core entity so playing safe.

Proposal

Basically:

class ClimateDevice(Entity):

    ... snip ....

    @property
    def capability_attributes(self) -> Optional[Dict[str, Any]]:
        """Return the capability attributes."""
        supported_features = self.supported_features
        data = {
            ATTR_HVAC_MODES: self.hvac_modes,
            ATTR_MIN_TEMP: show_temp(
                self.hass, self.min_temp, self.temperature_unit, self.precision
            ),
            ATTR_MAX_TEMP: show_temp(
                self.hass, self.max_temp, self.temperature_unit, self.precision
            ),
        }

        if self.target_temperature_step:
            data[ATTR_TARGET_TEMP_STEP] = self.target_temperature_step

        if supported_features & SUPPORT_TARGET_HUMIDITY:
            data[ATTR_MIN_HUMIDITY] = self.min_humidity
            data[ATTR_MAX_HUMIDITY] = self.max_humidity

        if supported_features & SUPPORT_FAN_MODE:
            data[ATTR_FAN_MODES] = self.fan_modes

        if supported_features & SUPPORT_PRESET_MODE:
            data[ATTR_PRESET_MODES] = self.preset_modes

        if supported_features & SUPPORT_SWING_MODE:
            data[ATTR_SWING_MODES] = self.swing_modes

        return data

There is a branch ready to go if this is something we want to do here.

Consequences

capability_attributes are included in state_attributes so I don't believe this will affect any existing code. But it will mean that integrations like homekit can leverage the metadata that is stored in the entity registry to improve their startup behaviour.

balloob commented 4 years ago

Yes, that is correct. I never got to migrate all the different entities.

Jc2k commented 4 years ago

Awesome. Thanks for the PR as well. Can I do PR's for other entities without an arch issue?

balloob commented 4 years ago

Sure. I just did a couple though https://github.com/home-assistant/home-assistant/pull/30545

Jc2k commented 4 years ago

Nice