ebaauw / homebridge-hue

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

Map Characteristic.TargetHeatingCoolingState to heat #1058

Closed torandreroland closed 2 years ago

torandreroland commented 2 years ago

Issue

The only valid values in Homebridge Hue for Characteristic.TargetHeatingCoolingState are 'off' and 'heat' (please note that 'cool' and 'auto' are also valid per the HomeKit specification, but they are disabled in Homebridge Hue). However when changing from 'off' to 'heat', this is mapped as 'auto' for config.mode in the Deconz REST API.

Could this be mapped to 'heat'? Alternatively could all values for Characteristic.TargetHeatingCoolingState be considered valid?

For my thermostats (Elko Super TR) only 'off' and 'heat' are considered valid values for config.mode by the Deconz REST API, leading to the HomeKit thermostat service getting non responsive when 'auto' is sent.

According to issue 1032 the current behaviour looks to be deliberate hack, but this inconsistency have undesired side effects.

Log Messages

'[12/12/2021, 7:17:05 PM] [Hue] Ekraveien 18: request 7568: PUT /sensors/27/config {"mode":"auto"} [12/12/2021, 7:17:05 PM] [Hue] Ekraveien 18: request 7568: api error 608: Could not set attribute'

ebaauw commented 2 years ago

The only valid values in Homebridge Hue for Characteristic.TargetHeatingCoolingState are 'off' and 'heat' (please note that 'cool' and 'auto' are also valid per the HomeKit specification, but they are disabled in Homebridge Hue).

Correct, Homebridge Hue doesn't currently support cooling, and AUTO in HomeKit means: automatically switch between HEAT and COOL.

However when changing from 'off' to 'heat', this is mapped as 'auto' for config.mode in the Deconz REST API.

Correct. That's because heat for the Eurotronic Spirit means: maximum heat and auto means temperature-based heat control.

For my thermostats (Elko Super TR) only 'off' and 'heat' are considered valid values for config.mode by the Deconz REST API, leading to the HomeKit thermostat service getting non responsive when 'auto' is sent. According to issue 1032 the current behaviour looks to be deliberate hack, but this inconsistency have undesired side effects.

The inconsistency is in the deCONZ API, probably introduced when adding support for the Elko. I'm happy to change Homebridge Hue, but I don't want to end up whitelisting each thermostat. This really needs to be addressed in deCONZ first.

torandreroland commented 2 years ago

The rest api is consistent here with regard to what the thermostat supports. It only supports off and heat.

ebaauw commented 2 years ago

The REST API is inconsistent in that heat means something different for the Eurotronic vs the Elko.

torandreroland commented 2 years ago

I think it's a bit harsh to require all devices to adhere to Eurotronics handling in the REST API.

How about making 'auto' a valid Characteristic.TargetHeatingCoolingState and map 'auto' to 'auto' and 'heat' to 'heat'?

torandreroland commented 2 years ago

A bit on the side, but still relevant question, is there a good documentation of how different sensor types should be exposed in the REST API? I think this basically boils down to expectations of what values different keys can have...

torandreroland commented 2 years ago

I think I found it on here where 'heat' is a valid value for ZHAThermostat. Given the disclaimer "Supported modes are device dependent", I don't think it's valid to claim the REST API is inconsistent.

But I think mapping Characteristic.TargetHeatingCoolingState.heat to config.mode.auto is inconsistent given 'heat' being a valid value for config.mode.

ebaauw commented 2 years ago

I think it's a bit harsh to require all devices to adhere to Eurotronics handling in the REST API.

I don't. I don't care whether the meaning is changed for the Eurotronic (and others that followed this) or for the Elko. I do think the purpose of the API is to provide a standard interface to clients, hiding the device-specific handling.

How about making 'auto' a valid Characteristic.TargetHeatingCoolingState and map 'auto' to 'auto' and 'heat' to 'heat'?

That's how I implemented it originally, not understanding the meaning of AUTO in HomeKit. Fixed that recently, see https://github.com/ebaauw/homebridge-hue/releases/tag/v0.13.27.

A bit on the side, but still relevant question, is there a good documentation of how different sensor types should be exposed in the REST API? I think this basically boils down to expectations of what values different keys can have...

Spot on! And no, there isn't (any good, that is).

Given the disclaimer "Supported modes are device dependent", I don't think it's valid to claim the REST API is inconsistent.

I really don't care how you want to call it. I care that I don't have to whitelist each thermostat type in Homebridge Hue, tuning the behaviour.

But I think mapping Characteristic.TargetHeatingCoolingState.heat to config.mode.auto is inconsistent given 'heat' being a valid value for config.mode.

Is is, when heat means something else (maximum heating) then HEAT (temperature controlled heating).

ebaauw commented 2 years ago

I started the discussion with the other deCONZ developers, but looks like that's taking forever. Better whitelist the Elko for now.

What does the resource for your thermostat look like? In particular, the manufacturername and modelid.

ebaauw commented 2 years ago

In v0.13.31.

torandreroland commented 2 years ago

Cool. Many thanks!

I made a small donation for your work on this plugin. It's very good and a vital, integral part of my smart home setup.

ebaauw commented 2 years ago

Is it working? Did you test it already; I don't have the device, so cannot test myself.

torandreroland commented 2 years ago

I updated just now. It works like a charm.

Thank you again!