wmalgadey / tado_component

Custom home-assistant component for tado (using PyTado)
GNU General Public License v3.0
12 stars 1 forks source link

`unit_of_measurement` of the climate component is incorrect #5

Closed l00mi closed 7 years ago

l00mi commented 7 years ago

Me again!

The climate component currently reports the operation_mode as value. This is not aligned to the reported unit_of_measurement which is °C.

So best to report no unit_of_measurement or change the value to e.g. current_temperature.

wmalgadey commented 7 years ago

the problem is, hass seems to decide which "value" to give to the device. I copied the code for a climate device from generic_thermostat. It has the same functionality (using sensors to create a thermostat).

I have no other climate-devices so I can't test the "normal" behavior of this component. But I will try to look into some other implementations.

l00mi commented 7 years ago

hmm, okay, but you could stop using unit_of_measurement? sorry i have no time to dive into the code myself right now.

wmalgadey commented 7 years ago

I looked into the homematic devices and they just override the sate to return the temperature setting. I don't know how it was intended by the hass developers? The climateDevice base class returns the operation_state as the device state:

    @property
    def state(self):
        """Return the current state."""
        if self.current_operation:
            return self.current_operation
        else:
            return STATE_UNKNOWN

and for the units they return degrees:

    @property
    def unit_of_measurement(self):
        """The unit of measurement to display."""
        return self.hass.config.units.temperature_unit

    @property
    def temperature_unit(self):
        """The unit of measurement used by the platform."""
        raise NotImplementedError

I wouldn't change the units, because there are some depending methods wich relay on degree-Units here (Fahrenheit or Celsius). Maybe it is ok to override the state and return the current_temperature or the tempereature_setting? What would be the best?

l00mi commented 7 years ago

I am not at all an expert on this. I use also the influxdb component too save the measurement data for better graphing. Problem here is it creates a dimension for all with °C with type float. Now this component comes and wants to push a String which breaks. My feeling is that the unit_of_measure should correspond with what is deliverd in the value field. What do you think? We might ask someone more knowledgable here.

wmalgadey commented 7 years ago

yes, you are probably right. I have asked the question on hass gitter, but got no answer. I will just change the state. If somebody else uses this component and has a good point why to do otherwise, we can have a talk about that later :)

wmalgadey commented 7 years ago

Seems to work :)

l00mi commented 7 years ago

Nice. Thank you!