auchter / haaska

Home Assistant Alexa Skill Adapter
263 stars 61 forks source link

ValueOutOfRangeError when setting thermostat temperature #52

Closed keatontaylor closed 7 years ago

keatontaylor commented 7 years ago

Not sure why, but all the sudden I am getting messages from alexa that the temperature I requested to set the thermostat to is not within the allowed range. The following code is the culprit.

if new > max_temp or new < min_temp:
    raise ValueOutOfRangeError(min_temp, max_temp)

However, casting new, max_temp and min_temp to floats like this seemed to fix it:

if float(new) > float(max_temp) or float(new) < float(min_temp):
    raise ValueOutOfRangeError(min_temp, max_temp)

I also had to update this code as well:

e.set_temperature(float(new), mode.lower(), state)

I'd be happy to submit a pull request for the changes, but I wanted to get some thoughts on how best to solve this first. @trisk can you chime in?

auchter commented 7 years ago

Thanks for opening this issue; I too noticed this failing as of today. Looking at my logs, it seems that the deltaTemperature and targetTemperature values in calls from Amazon into haaska changed from floats to strings. Odd, and I can't help but wonder if this was an unintentional break on their part.

I think the correct fix is to cast these two values in handle_temperature_adj. It'd be redundant yet harmless if Amazon ends up reverting back to sending floats instead.

keatontaylor commented 7 years ago

So something like this?

    if op is not None and 'deltaTemperature' in payload:
        new = op(temperature, float(payload['deltaTemperature']['value']))
        # Clamp the allowed temperature for relative adjustments
        if temperature != max_temp and temperature != min_temp:
            if new < min_temp:
                new = min_temp
            elif new > max_temp:
                new = max_temp
    else:
        new = float(payload['targetTemperature']['value'])
auchter commented 7 years ago

Yep, looks good. Merged.

Thanks!