hpeyerl/venstar_colortouch

T5800 support

Closed this issue · 6 comments

I have an older Colortouch T5800 that I'd like to use with your code (for HomeAssistant). I noticed a few things that need to be somehow 'optional' for it to work with the T5800:

  1. In the update_info() function, both hum_setpoint and dehum_setpoint do not exist in the JSON returned by the device, making this call fail.

Example JSON:
$ curl "http://thermostatip/query/info"
{"name":"HOME","mode":3,"state":1,"fan":0,"fanstate":1,"tempunits":0,"schedule":1,"schedulepart":2,"away":0,"spacetemp":65.0,"heattemp":68.0,"cooltemp":80.0,"cooltempmin":65.0,"cooltempmax":99.0,"heattempmin":35.00,"heattempmax":80.0,"setpointdelta":4.0,"availablemodes":0}

  1. In the get_thermostat_sensor() function, trying to get the 'hum' sensor will also cause a problem on the T5800.

I'm attaching a diff that seems to fix these issues.

Thank you for your work!

--- venstarcolortouch.py.orig   2018-01-15 13:29:44.061887273 -0500
+++ venstarcolortouch.py        2018-01-15 14:24:49.799229048 -0500
@@ -138,8 +138,14 @@
         self.tempunits = self.get_info("tempunits")
         self.away = self.get_info("away")
         self.schedule = self.get_info("schedule")
-        self.hum_setpoint = self.get_info("hum_setpoint")
-        self.dehum_setpoint = self.get_info("dehum_setpoint")
+        if 'hum_setpoint' in self._info:
+          self.hum_setpoint = self.get_info("hum_setpoint")
+        else:
+          self.hum_setpoint = None
+        if 'dehum_setpoint' in self._info:
+          self.dehum_setpoint = self.get_info("dehum_setpoint")
+        else:
+          self.dehum_setpoint = None
         #
         return True
 
@@ -166,7 +172,10 @@
 
     def get_thermostat_sensor(self, attr):
         if self._sensors != None and self._sensors["sensors"] != None and len(self._sensors["sensors"]) > 0:
-            return self._sensors["sensors"][0][attr]
+            if attr in self._sensors["sensors"][0]:
+              return self._sensors["sensors"][0][attr]
+            else:
+              return None
         else:
             return None

That looks like the right thing to do. Can you submit a pull request? It'd be good if there were a comment in there indicating it was due to the keys missing on a T5800.

@Cinntax probably cares about this for home-assistant.

Oh nice- the more the merrier.
@rbebeau - i already have a pending pull request for home-assistant's dev branch. Looks like i just missed the cut for 0.61.
home-assistant/core#11639

Also, we're discussing it here:
https://community.home-assistant.io/t/venstar-thermostat/20668/8

A couple of things:

  1. The hass reviewers don't want I/O in our constructor- i believe there is still I/O in the venstarColorTouch constructor (not in the hass component at least). Now that we're talking about another release, we may as well take that out (the call to login()) that is.

  2. Right now, the hass component assumes we can set the humidity. @rbebeau are you aware of anything else that doesn't apply to you with your model? I think maybe the easiest thing right now would be simply a configuration.yaml variable to disable humidity settings.

@Cinntax

As far as I know, anything humidity related is pretty much the only thing that is not supported for the T5800 model. A yaml config variable sounds good to me. I'm not sure what other thermostat models (if any) would be the same as mine.

Okay cool- actually, as I got to putting the "flag" in the hass component, I thought it might be cleaner if we have the library "tell" the component what it supports. So i'm thinking i can modify the library here to put a "supports humidity" or something of that nature, and it will return true if the thermostat supports it, and it's enabled.

I've pushed 0.4 to pypi with changes from @rbebeau.

I assume this issue has been resolved so I'm going to close it.