home-assistant / homebridge-homeassistant

DEPRECATED in favor of native HomeKit support. -  Homebridge plugin for Home Assistant
https://www.home-assistant.io/components/homekit/
Other
683 stars 144 forks source link

Climate adjustment causing temperature adjustment to 65 degrees #247

Closed heathpitts closed 6 years ago

heathpitts commented 6 years ago

I installed the latest version of Homebridge-homeassistant yesterday. Really nice having the temperature displayed properly when set to Fahrenheit. Problem now is that when I turn the temp down using the HomeKit app, it immediately changes it to 65 degrees. It could be set on 72 but it immediately gets set to 65. These are Ecobee 3 thermostats if that matters. All works as expected from the home Assistant web UI. Anyone else seeing this?

mceres commented 6 years ago

In my case it displays the temperature almost correct (off by 1 degree most of the time, probably rounding issue) but the mayor issue is that when I change the temperature it doesn’t change it on my thermostat. Homeassistant is working fine. I am using a Nest

heathpitts commented 6 years ago

Mine actually displays the temp correctly if it is changed in Home Assistant or on the Ecobee app. It just messes up completely if I change it with the Home app

AmpacheUser commented 6 years ago

Changing temperatures doesn't work for me too. The first time I tried, it set the thermostat to 35 Degrees Fahrenheit...haven't tried since.

schmittx commented 6 years ago

I haven't actually tested this, but I think the current code for climate.js needs to be updated. HomeKit only supports Celsius internally so the setTargetTemperture needs to convert it's output to Fahrenheit if the Home Assistant units are F (basically the opposite of what's currently being done by getCurrentTemperature and getTargetTemperature. Additionally, the setCharacteristic calls under onEvent need to apply unit conversions as necessary similar to getCurrentTemperature, getTargetTemperature, etc.

Sorry, that's a lot of text...I can try to dig into this later this week but I wanted to share my thoughts at least...

radiationnow commented 6 years ago

Definitely seems to relate to the Celsius/Fahrenheit issue. Here's what my logs have:

2017-12-03 19:39:52 INFO (MainThread) [homeassistant.core] Bus:Handling <Event call_service[L]: domain=climate, service=set_temperature, service_data=entity_id=climate.bedroom_nest, temperature=18, service_call_id=1965405392-38>
2017-12-03 19:39:52 ERROR (MainThread) [homeassistant.core] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/lib/python3.6/asyncio/tasks.py", line 181, in _step
    result = coro.throw(exc)
  File "/usr/lib/python3.6/site-packages/homeassistant/core.py", line 1031, in _event_to_service_call
    yield from service_handler.func(service_call)
  File "/usr/lib/python3.6/site-packages/homeassistant/components/climate/__init__.py", line 327, in async_temperature_set_service
    yield from climate.async_set_temperature(**kwargs)
  File "/usr/lib/python3.6/asyncio/futures.py", line 331, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.6/asyncio/tasks.py", line 244, in _wakeup
    future.result()
  File "/usr/lib/python3.6/asyncio/futures.py", line 244, in result
    raise self._exception
  File "/usr/lib/python3.6/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/lib/python3.6/site-packages/homeassistant/components/climate/nest.py", line 158, in set_temperature
    self.device.target = temp
  File "/usr/lib/python3.6/site-packages/nest/nest.py", line 510, in target
    self._set('devices/thermostats', data)
  File "/usr/lib/python3.6/site-packages/nest/nest.py", line 168, in _set
    response = self._nest_api._put(path=path, data=data)
  File "/usr/lib/python3.6/site-packages/nest/nest.py", line 1590, in _put
    return self._request('PUT', path, data=data)
  File "/usr/lib/python3.6/site-packages/nest/nest.py", line 1582, in _request
    raise APIError(response)
nest.nest.APIError: Temperature F value is too low: 18.0
chriscrowe commented 6 years ago

Any plans to fix this? This makes Homebridge+Nest nearly useless.

chriscrowe commented 6 years ago

Trying to track down the exact bug here but given the number of layers that these temperatures need to pass through (HomeKit/Siri -> homebridge-homeassistant -> homebridge -> home-assistant -> python-nest) is making that a bit difficult.

Grasping at straws here-- is the issue related to the fact that we're not doing a fahrenheitToCelsius call in the setTargetTemp function?

https://github.com/home-assistant/homebridge-homeassistant/blob/master/accessories/climate.js#L91

I see that it's being called when we fetch temperature (which works correctly through HomeKit/Siri):

https://github.com/home-assistant/homebridge-homeassistant/blob/master/accessories/climate.js#L82

radiationnow commented 6 years ago

We need the reverse - need to do a celsiusToFahrenheit when we are setting target temp, because HomeKit feeds a celsius value to Homebridge, which needs to send fahrenheit to our fahrenheit Home Assistant.

I have an (untested, because I can't figure out how to use it with hassio) patch at https://github.com/radiationnow/homebridge-homeassistant/blob/master/accessories/climate.js

chriscrowe commented 6 years ago

That makes complete sense.

And yeah I'm having the same trouble-- can't figure out how to test anything 😂 Incidentally the SSH add-on isn't even actually giving me root filesystem access, seems I'm in some kind of sandboxed filesystem, otherwise I'd just change the source directly on the device for testing.

core-ssh:~# find / -name "climate.js"
core-ssh:~# 

EDIT: Just read the docs more closely-- seems I'm inside a docker container. Bummer.

chriscrowe commented 6 years ago

Found this:

https://home-assistant.io/developers/hassio/debugging/

I'll test your change after I get this set up.

chriscrowe commented 6 years ago

Proposed solution in pull request is tested by locally modifying files in my homebridge Docker container and running killall homebridge to restart the service. Seems to work as intended.

schmittx commented 6 years ago

Fixed by #268