ebaauw / homebridge-hue

Homebridge plugin for Philips Hue
Apache License 2.0
898 stars 91 forks source link

Use floor temperature if temperature measurement is floor sensor (Support Elko Super TR Thermostat) #1011

Closed torandreroland closed 3 years ago

torandreroland commented 3 years ago

Issue

Hi! Thank you for a wonderful plugin.

I was wondering if you could add support for reporting temperature using floor temperature if the temperature measurement is done using the floor sensor for the Elko Super RF Thermostat.

Here is an example of the sensor information as reported by the deCONZ REST-API:

{ "config": { "heatsetpoint": 2500, "locked": false, "mode": null, "offset": 0, "on": true, "reachable": true, "temperaturemeasurement": "floor sensor" }, "ep": 1, "etag": "31921b7fe84c562bbee8074187212d28", "lastseen": "2021-08-04T18:07Z", "manufacturername": "ELKO", "modelid": "Super TR", "name": "Thermostat", "state": { "floortemperature": 2550, "heating": false, "lastupdated": "2021-08-04T18:07:24.190", "on": true, "temperature": 2290 }, "type": "ZHAThermostat", "uniqueid": "00:0d:6f:00:0f:41:4e:3d-01-0201" }

Basically I want Characteristic.CurrentTemperature to be mapped from state.floortemperature if config.temperaturemeasurement is "floor sensor" or else from state.temperature. This may be exclusive logic for the given combination of manufacture name and model id.

Obviously it would be best if the mapping is dynamically updated in accordance with an updated type of temperature measurement, i.e. be mapped from either floor temperature or temperature when this is updated on the thermostat. But I am more than happy if the type of floor measurement is only evaluated when starting the plugin.

ebaauw commented 3 years ago

Shouldn't that be handled by deCONZ?

torandreroland commented 3 years ago

I guess you can make that argument.

My understanding from the discussion of extending the support for the thermostat in the rest-api, was that they deliberately decided against it because of compatibility with other home automation systems.

From a HomeKit perspective my opinion is that the currentTemperature characteristic should use temperature from the sensor that is actively deciding if the thermostat should be heating. Of course this could also be a separate characteristic (which I believe was the compatibility need for other home automation system to keep this separate in the api), but this would not be available in the Apple Home App since that is adhering strictly to the characteristics of a thermostat per the HAP-specification.

ebaauw commented 3 years ago

From a HomeKit perspective my opinion is that the currentTemperature characteristic should use temperature from the sensor that is actively deciding if the thermostat should be heating.

But that has nothing to do with HomeKit. You want state.temperature to reflect the temperature used by the thermostat.

Of course this could also be a separate characteristic (which I believe was the compatibility need for other home automation system to keep this separate in the api), but this would not be available in the Apple Home App since that is adhering strictly to the characteristics of a thermostat per the HAP-specification.

The proper way to expose this in HomeKit would be a separate Temperature Sensor service for state.floortemperature. It would need to be exposed on a separate accessory, or Eve history would break. I’m not sure if I can get Homebridge Hue to expose two different services/accessories from the same resource, though.

My understanding from the discussion of extending the support for the thermostat in the rest-api, was that they deliberately decided against it because of compatibility with other home automation systems.

What’s the deCONZ issue for this discussion?

torandreroland commented 3 years ago

Initial support for the Elko Super TR thermostat was added following issue 1291. Support for the floor sensor was added following issue 3123. If you see comment 699442563 from arnerek, he is basically proposing handling this directly in the API (which you and I also would have preferred), but then after some consideration they seem to land on exposing different sensors in the API as seen in comment 699691385 by SwoopX due to being the safer variant (my understanding is that this is considered more compatible for a range of home automation systems / use cases).

My view is that the temperature reported in the thermostat service should be mapped to the sensor deciding the boolean state of the heating. So since this is not handled in the API (which I agree it should have been, but they decided against it), I hope that we can amend this in the "Homebridge-layer".

ebaauw commented 3 years ago

I hope that we can amend this in the "Homebridge-layer".

I really don't want to maintain a Homebridge "layer" with functionality. In my view, Homebridge plugins should expose devices to HomeKit, mapping native attributes and commands to their HomeKit equivalents. Any special functionality should be handled in the device or in HomeKit automations.

As you indicate, the mapping of the reported temperature to the sensor, based on the state of the heating, is a generic device feature, not specific to HomeKit.

torandreroland commented 3 years ago

I created issue 5208 in the deconz-rest-plugin repository to follow up this. Thank you for your clear stance on this in the discussion of issue 3123 in that repository.

ebaauw commented 3 years ago

Closeing this for now. Please re-open or create a new issue when deCONZ has been updated.