Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Venstar integration report higher than normal temperatures with 5.x firmwares #74115

Closed
wkethman opened this issue Jun 28, 2022 · 27 comments · Fixed by #74271
Closed

Venstar integration report higher than normal temperatures with 5.x firmwares #74115

wkethman opened this issue Jun 28, 2022 · 27 comments · Fixed by #74271
Labels
integration: venstar

Comments

@wkethman
Copy link

The problem

Venstar integration is reporting much higher than normal values - attempted deleting and re-initializing the integration without luck

Home assistant entity values:
hvac_modes: heat, cool, off, auto
min_temp: 44.5
max_temp: 95
fan_modes: on, auto
preset_modes: none, away, temperature
current_temperature: 174
temperature: 179.5
fan_mode: auto
hvac_action: idle
preset_mode: temperature
fan_state: 0
hvac_mode: 0
friendly_name: DOWNSTAIRS
supported_features: 25

Venstar local API call - http://192.168.***.***/query/info
{"name":"DOWNSTAIRS","mode":2,"state":0,"activestage":0,"fan":0,"fanstate":0,"tempunits":0,"schedule":0,"schedulepart":1,"away":0,"spacetemp":80.0,"heattemp":61.0,"cooltemp":82.0,"cooltempmin":35.0,"cooltempmax":99.0,"heattempmin":35.0,"heattempmax":99.0,"setpointdelta":2,"availablemodes":0}

What version of Home Assistant Core has the issue?

Home Assistant Core 2022.6.7

What was the last working version of Home Assistant Core?

Home Assistant Core 2022.6.7

What type of installation are you running?

Home Assistant Container

Integration causing the issue

Venstar

Link to integration documentation on our website

https://www.home-assistant.io/integrations/venstar/

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

Unfortunately no

Additional information

No response
@probot-home-assistant
Copy link

venstar documentation
venstar source
(message by IssueLinks)

@probot-home-assistant
Copy link

Hey there @garbled1, mind taking a look at this issue as it has been labeled with an integration (venstar) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@numero-trey
Copy link

I'm also having this issue.

The Venstar integration is treating the F temps out of my thermostat as C and converting them F resulting in the temps @wkethman is reporting above.

Digging in the code,

core/homeassistant/components/venstar/sensor.py

Line 110 in 1d5b874

if coordinator.client.tempunits == coordinator.client.TEMPUNITS_F:
is comparing tempunits in the API response with VenstarDataUpdateCoordinator.TEMPUNITS_F which I can't find a definition for. According to my API response, it should be 0.

@wkethman
Copy link
Author

wkethman commented Jun 28, 2022 via email

@wkethman
Copy link
Author

wkethman commented Jun 28, 2022 via email

@numero-trey
Copy link

numero-trey commented Jun 28, 2022 •
edited

I found a temporary workaround. Put the thermostat in Celsius mode and HA will properly convert units to your display setting. tempunits in the API response changes to 1 once the units are changed on the thermostat.

On my Venstar Explorer Mini T2000, hold Mode + Fan for 5 seconds till the menu appears, press Mode until you get to setting 22, then use the arrows to change it to "C". Press Mode + Fan for 5 seconds to exit the menu.

This is a critical bug for me. My thermostat self-heats when it turns on the compressor causing it to misread, so I have to use an automation to turn the thermostat off and on based on a separate temp sensor.

@numero-trey
Copy link

An additional find on this bug: My automation actions stopped working. I set the mode and target temperature to turn my AC off and on, but now those actions don't change the thermostat. I even tried setting the high/low temps like the Venstar API wants with no change. These automations have worked for weeks.

Manual mode control from a dash widget still works.

@L1gerZer0
Copy link

I have 4 T2000 thermostats and it appears that they finally updated the API to report the temp in °F as configured in the stat. This is causing the integration to convert them even though it no longer needs to.

@roverte
Copy link

roverte commented Jun 28, 2022

It sounds like the issue has already been identified, but I'm experiencing the exact same behavior on my setup. Just started last night on all of my Venstar thermostats.

@wkethman
Copy link
Author

wkethman commented Jun 28, 2022 via email

@L1gerZer0
Copy link

It would make more sense that this is an update of the api on Venstar causing the issue in Home Assistant since no recent updates to Home Assistant have happened. Seems most likely that Venstar HA integration needs updating to reflect now accurate reporting of Venstar
…
On Tue, Jun 28, 2022, 10:05 AM roverte @.> wrote: It sounds like the issue has already been identified, but I'm experiencing the exact same behavior on my setup. Just started last night on all of my Venstar thermostats. — Reply to this email directly, view it on GitHub <#74115 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACGJVVQYR7KOB77Z5TXF7TVRMIDLANCNFSM52B2CKJQ . You are receiving this because you were mentioned.Message ID: @.>

This is correct. Previously my T2000 devices were at a 4.xx firmware rev and now they are all 5.28. This was a problem for me once in the past that Venstar just pushes firmware updates automatically without letting us choose when to update.

@vegascom-jeff
Copy link

vegascom-jeff commented Jun 28, 2022 •
edited

Exact same problem, started yesterday around 3pm PDT.

@vegascom-jeff
Copy link

core/homeassistant/components/venstar/sensor.py

Line 110 in 1d5b874

if coordinator.client.tempunits == coordinator.client.TEMPUNITS_F:

is comparing tempunits in the API response with VenstarDataUpdateCoordinator.TEMPUNITS_F which I can't find a definition for. According to my API response, it should be 0.

I believe client.TEMPUNITS_F is referencing the underlying control library (venstar_colortouch). It seems to be correct (F=0):

https://github.com/hpeyerl/venstar_colortouch/blob/master/src/venstarcolortouch/venstarcolortouch.py

@wkethman
Copy link
Author

wkethman commented Jun 28, 2022 via email

@radio-ed
Copy link

I have four T2000 t-stats that all started showing temps 90 degrees higher than actual at 1:30 this morning. They all show firmware 5.28 via the skyport app. I contacted venstar and it sounds like they made some changes and did a push. Here is their reply if it could help the developers:

"The v5.28 firmware for this thermostat series corrected an issue in how temperatures were reported via the API. Rather than reporting all temperatures in degC only, the API now reports in the units in use at the thermostat. Your application should read "tempunits" (0 = degF, 1 = degC) to properly interpret what units are in use and then optionally convert them to whatever is required by your application.

Sorry if this change resulted in unexpected results in your app.

Thank you,
Venstar Support Team"

@vegascom-jeff
Copy link

This looks to be the root of the problem. The underlying venstar communication library, venstar_colortouch, coded around the fact that the API was not correctly returning C or F so now that it is, it's broken:

https://github.com/hpeyerl/venstar_colortouch/blob/97fecae2d1fc4b047d611b7c1d1bd1a0e09da117/src/venstarcolortouch/venstarcolortouch.py#L211

    # T2xxx, T3xxx thermostats (and maybe more) always use Celsius in the API regardless of the display units
    # So handle this case accordingly
    if self.model.startswith(("T2", "T3")):
        # Always degC
        self.tempunits = self.TEMPUNITS_C
        logging.debug("Detected thermostat model %s, using temp units of Celsius", self.model)
    elif self.model in ["VYG-4900-VEN", "VYG-4800-VEN", "VYG-3900", "COLORTOUCH"]:
        # Same as display units
        self.tempunits = self.display_tempunits
    elif self.get_info("heattempmax") >= 40:
        # Heat max temp over 40, only possible if degF
        logging.warning("Unknown thermostat model %s, inferring API tempunits of Fahrenheit", self.model)
        self.tempunits = self.TEMPUNITS_F
    else:
        logging.warning("Unknown thermostat model %s, inferring API tempunits of Celsius", self.model)
        self.tempunits = self.TEMPUNITS_C
    return True

@wkethman wkethman mentioned this issue Jun 28, 2022
Closed
@wkethman
Copy link
Author

@vegascom-jeff - I just discovered this as well - I don't know how to test/make changes to this library but I did create an issue

If anyone comes up with a short-term fix, work-around, please post for all

@vegascom-jeff
Copy link

@vegascom-jeff - I just discovered this as well - I don't know how to test/make changes to this library but I did create an issue

If anyone comes up with a short-term fix, work-around, please post for all

I just did what @numero-trey suggested and flipped my T2000 in settings to C and that fixes it in HA but breaks it on the display itself, I can tolerate that.

I don't know the deployment process for HA but I'm guessing since this is part of HA core, it might take some time to make it to an actual release depending on how quickly the underlying venstar_colortouch library is patched. My guess is the shortest patch type solution would be for someone to make a copy of the venstar HA code, put it in custom_component, and patch the F/C handling there. Users would then need to deploy the code to their custom_component folder and adjust their config to reference the custom_component. Not particularly easy.

I am no expert though so hopefully there is a better method.

@wkethman
Copy link
Author

Team, found an easy fix for those running Home Assistant in a Docker container (might work for other installs as well) - 

Go to: https://github.com/hpeyerl/venstar_colortouch - Download the source code, can click Code > Download zip
Unzip the source and copy venstarcolortouch folder into your Home Assistant > config directory

In the venstarcolortouch folder under your config directory open venstarcolortouch.py and change the following:

if self.model.startswith(("T2", "T3")):
  # Always degC
  #self.tempunits = self.TEMPUNITS_C
  self.tempunits = self.display_tempunits
  #logging.debug("Detected thermostat model %s, using temp units of Celsius", self.model)

This corrected the issue for me

Sorry, I am a surgeon that just dabbles in software and am not familiar enough with Github and HA Core to make the fix

@hpeyerl
Copy link

hpeyerl commented Jun 29, 2022 •
edited

"great". We should probably wrap that with a check for firmware version. Some people's thermostats can't reach the internet and, thus, update themselves. Looks like we need to add a check for version < 5.28 ?

Anyone want to submit a pull request?

Otherwise I'll try to get to it after my vacation.

@digiblur
Copy link
Contributor

"great". We should probably wrap that with a check for firmware version. Some people's thermostats can't reach the internet and, thus, update themselves. Looks like we need to add a check for version < 5.28 ?

Anyone want to submit a pull request?

Otherwise I'll try to get to it after my vacation.

And probably need to do via model & firmware combo as not all models run the same revision scheme.

@RobDehnert
Copy link

I can confirm that my Venstar T2000 was exhibiting the same behavior. My thermostat appears to have received the v5.28 firmware upgrade on 06/28/2022 at 01:06 CDT. The workaround of setting the thermostat to Celsius is acceptable... for now.

@bdraco bdraco changed the title Venstar integration report higher than normal temperatures Venstar integration report higher than normal temperatures with 5.x firmwares Jun 29, 2022
@bdraco bdraco mentioned this issue Jun 29, 2022
Closed
@bdraco
Copy link
Member

bdraco commented Jun 29, 2022

Duplicate issue reported here with information about the endpoint #74185 (comment)

@gcomeaux
Copy link

Sorry about the duplicate. PR submitted that should address the issue and check for firmware version.
hpeyerl/venstar_colortouch#40

@numero-trey numero-trey mentioned this issue Jun 30, 2022
Merged
@hpeyerl
Copy link

hpeyerl commented Jun 30, 2022

Fixed in https://pypi.org/project/venstarcolortouch/0.17/ thanks to @gcomeaux

@bdraco
Copy link
Member

bdraco commented Jun 30, 2022

Can someone open a PR to bump the library version?

@chrishoage chrishoage mentioned this issue Jun 30, 2022
Merged
22 tasks
@chrishoage
Copy link

To speed things along I created #74271

I will admit I unfortunately have not had time to test this change, I just followed the pattern from #73038

If this is not acceptable please let me know and I will close the PR

@danmalone326 danmalone326 mentioned this issue Jul 5, 2022
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
integration: venstar
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bump venstarcolortouch to 0.17 chrishoage/homeassistant-core