search

nkgilley / python-ecobee-api

Python API for controlling Ecobee Thermostats
MIT License
44 stars 40 forks source link

- Update set_fan_mode support for holdHours/coolHoldTemp/heatHoldTemp #64

Closed bjpetit closed 3 years ago

bjpetit commented 3 years ago

If holdType is holdHours, setting the fan mode will fail as the Ecobee API requires the holdHours arg to be passed in too. This was causing an error to be returned when setting the fan mode. Modeled change after set_hold_temp. Also cleaned up an equality check which was throwing warnings.

bjpetit commented 3 years ago

This change implies a change in the caller too. I have a change in my home-assistant/core fork that I've been working with. But even without that update, the call works and defaults to 2 hours.

marthoc commented 3 years ago

I have a few comments on this that I will leave in detail tomorrow (on my phone at the moment); my main thinking is that we should make heat_temp and cool_temp kwargs with default values of None so as not to have to artificially pass in 0’s. I haven’t reviewed the implications for this in Home Assistant yet but the changes to various calls should be minimal.

bjpetit commented 3 years ago

Makes sense. I started with the approach in this PR to keep things consistent for callers. But it certainly would be nice to make those args optional, or even remove them completely. There's one call in climate.py, and one in test_climate.py. But if we made the args here optional, I would think the references to the cool and heat temps could be removed from the caller(s) in HA all together.