djbulsink / panasonic_ac

Panasonic Comfort Cloud HA component
30 stars 16 forks source link

Make use of hvac_mode attribute when calling set_temperature service #26

Open pmrazovic opened 3 years ago

pmrazovic commented 3 years ago

Hi, first of all, thanks for the great work you did with this component. I really appreciate it!

I have a small suggestion which could improve the integration with some other components, especially schedulers. Service climate.set_temperature accepts several additional data attributes besides the entity_id and temperature. I think it would be great to make use of hvac_mode attribute which would allow us to set the target temperature and start the AC in a single call. As far as I know, this is how most climate integrations should work.

Besides other benefits, this would also allow us to use the integration with increasingly popular scheduler component. The scheduler defines several schedulable actions (e.g. heat, cool) which execute set_temperature service with temperature and hvac_mode attributes (more details here). For example, if you schedule a cool action with target temperature of 21°C, the scheduler will try to execute set_temperature with hvac_mode: cool and temperature: 21, however our climate component will only set the target temperature but won't turn on the AC or set the HVAC mode.

Here is how set_temperature method in climate.py can be updated to make use of hvac_mode:

@api_call_login
def set_temperature(self, **kwargs):
    """Set new target temperature."""
    target_temp = kwargs.get(ATTR_TEMPERATURE)
    if target_temp is None:
        return

    _LOGGER.debug("Set %s temperature %s", self.name, target_temp)

    self._api.set_device(
        self._device['id'],
        temperature = target_temp
    )

    hvac_mode = kwargs.get(ATTR_HVAC_MODE)
    if hvac_mode is None:
        return

    _LOGGER.debug("Set %s mode %s", self.name, hvac_mode)

    if hvac_mode == HVAC_MODE_OFF:
        self._api.set_device(
            self._device['id'],
            power = self._constants.Power.Off
        )
    else:
        self._api.set_device(
            self._device['id'],
            power = self._constants.Power.On,
            mode = self._constants.OperationMode[OPERATION_LIST[hvac_mode]]
        )

I've already tested this implementation and it works correctly (even with scheduler component). I'm not a native Python developer, so maybe this could be done more elegantly.

Let me know what do you think... If you agree with this implementation I can create a PR.

Once again, thanks for the great work!