IndigoDomotics / indigo-ecobee-plugin

Plugin for Indigo home automation software to support Ecobee thermostats and remote sensors
1 stars 1 forks source link

Failed to Generate Climate List #5

Closed mrstacey1284 closed 6 years ago

mrstacey1284 commented 6 years ago

Thank you for taking this project over. The fixed authentication issues have been amazing!!

When try to automate the activation of a Climate in Indigo, the climate list does not display anything. I believe I located the issue and was able to fix it in my local version of the plugin. Its a minor fix so I am not sure its worth a pull request. I have describe it below to see it you could implement or or have a better way to address the issue.

On line 288 of plugin.py an array is generated named active_smart_thermostats, however on line 376 of the same file the climateListGenerator function only compares the current thermostat ID to those IDs found in the active_thermostats array. In my case, I am using Ecobee 3 and 4 meaning no climate is ever returned to the Indigo UI. My fix was to simply iterate over both arrays (active_thermostats and active_smart_thermostats) and check of the ID.

FlyingDiver commented 6 years ago

I see. Here's the change I think you're describing:

    def climateListGenerator(self, filter, valuesDict, typeId, targetId):                                                                                                                 
        for t in self.active_thermostats:
            if t.dev.id == targetId:
                retList = get_climates(self.ecobee, t.dev.address)
        for t in self.active_smart_thermostats:
            if t.dev.id == targetId:
                retList = get_climates(self.ecobee, t.dev.address)
        return retList
FlyingDiver commented 6 years ago

However, I should point out that Ecobee 3 and 4 are NOT what the plugin refers to as "Smart Thermostats". From ecobee_devices.py:

class EcobeeThermostat(EcobeeBase):
    ## This is for the Ecobee3 generation and later of products with occupancy detection and remote RF sensors

class EcobeeSmartThermostat(EcobeeBase):
    ## This is the older 'Smart' and 'Smart Si' prior to Ecobee3
FlyingDiver commented 6 years ago

So, if the "Smart" models don't have climate lists, then this change is wrong.

And I can't tell, since all I have are Ecobee 3s (four of them).

rapamatic commented 6 years ago

I have two Smarts... from what I can tell they do have climate lists - at least I see a climate listed under custom states in the indigo UI for my smart thermostats.

mrstacey1284 commented 6 years ago

I am definitely mistaken in my interruption of "smart". I was assuming that "smart" referred to the Ecobee 3, 3 lite, and 4. I should have read the documentation better, sorry. However, if @rapamatic is saying that even the other Ecobee thermostats have Climates, then it might still be applicable. I am just not sure if the climate list is able to be pulled via the API on thermostats other the 3, 3 lite and 4.

FlyingDiver commented 6 years ago

I'm looking at the API now.

FlyingDiver commented 6 years ago

Well, the API docs say:

Default Climates

Smart/Si: There are four default Climates for each Smart/Si Thermostat, with possible climateRef values of "wakeup", "away", "home", and "sleep".

ecobee3: There are three default Climates for each ecobee3 Thermostat, with possible climateRef values of "away", "home", and "sleep".

So it looks like they both should be in the list.

mrstacey1284 commented 6 years ago

I just changed my Ecobee 3 and 4 to "Ecobee Thermostats" in the device list while using the plugin version 1.2.3 without my changes and it works. Which makes sense, since my thermostats are now going to be a part of the "active_thermostats" array. This also fixed reporting inaccuracy that I have had since the original plugin. So for my specific issue can be chocked up to user error.

mrstacey1284 commented 6 years ago

The only difference I had in my edit was logging.

FlyingDiver commented 6 years ago

OK, I'm going to leave the change in since it seems like it should be available for both models.

Looking into this morning's server errors that caused my system, and at least one other, to loose authentication.

mrstacey1284 commented 6 years ago

Thank you for the quick response and your hard work!

FYI, I lost auth as well but assumed it was something on my end since I also lost power around midnight and didn't realize I had got an alert until then. I just double checked the alert time and it was about 11:08pm EST.

FlyingDiver commented 6 years ago

I'm also updating the device type description so that it's clearer which device type to pick.

FlyingDiver commented 6 years ago

Will be in next update.

rapamatic commented 6 years ago

That makes sense. Ecobee's naming conventions are unnecessarily confusing.