MislavMandaric / home-assistant-vaillant-vsmart

Home Assistant custom component for Vaillant vSMART.
MIT License
57 stars 12 forks source link

Incorrect current_temperature value in the new boiler device #269

Open janchlebek opened 8 months ago

janchlebek commented 8 months ago

Version of the custom_component

0.9.0

Describe the bug

Device was split in two as expected, all values show properly including the actual set temperature of the water boiler, however the current_temperature does not show real value but the same value as the set value. Also when changing the set value of the climate entity it appears as it did not work and stays on previous value (due to delay on the API I believe). Reloading the integration speeds up this and correct value is shown immediately.

Not sure what was the driver to separate the device but I find the new setup with two climate entities a little more impractical, given the limited control over water heating I believe a simple toggle for HWB and a number entity would be sufficient and less confusing instead of whole climate entity which by nature has way more options then needed.

MislavMandaric commented 8 months ago

Hey @janchlebek.

current_temperature does not show real value but the same value as the set value

You caught my shortcut there 😅 Unfortunately, my boiler doesn't have a water temperature sensor, so I had an option of either showing nothing, or pretending the expected temperature is what current temperature is. I went with the later.

If I manage to get an API response for a boiler with water temperature sensor, I'll gladly fix that bug and expose the real current water temperature.

when changing the set value of the climate entity it appears as it did not work and stays on previous value (due to delay on the API I believe).

I'm aware of this and I'll use #21 to track it. I'll have to dig around a bit for HA best practices on this, as it should be a problem most cloud polling integrations face.

I find the new setup with two climate entities a little more impractical

Thank you for the feedback, I really appreciate it. Separating the boiler from the thermostat made it much easier for me to wrap my head around it and I'd encourage you to try it out a bit before making a final judgement on this change 😄

janchlebek commented 8 months ago

You caught my shortcut there 😅 Unfortunately, my boiler doesn't have a water temperature sensor, so I had an option of either showing nothing, or pretending the expected temperature is what current temperature is. I went with the later.

😇 If I manage to get an API response for a boiler with water temperature sensor, I'll gladly fix that bug and expose the real current water temperature.

let's see if you can find something in the API. I'm aware of this and I'll use #21 to track it. I'll have to dig around a bit for HA best practices on this, as it should be a problem most cloud polling integrations face.

I just noticed it only happens with the HW climate entity, the main one for heating does not behave like this and the change in UI is instant.

Thank you for the feedback, I really appreciate it. Separating the boiler from the thermostat made it much easier for me to wrap my head around it and I'd encourage you to try it out a bit before making a final judgement on this change 😄

Of course I will give it a go - my opinion is based on the fact that hot water temperature is something that you normally set to a given value and typically never change - basically the only thing needed here in most cases is the HWB activation to override scheduling, especially while the climate entity does not read the real water temperature so it cannot react to actual status (like thermostat). Hence the whole climate entity seems a little too much for that (of course it will do the job as well). 😉

Anyway, don't take the above as complaints, love the integration and big thanks to you for keeping it alive! 😎

compunix-be commented 8 months ago

Also loving this integration. I disagree with hot water temperature never change. to reduce gas consumption, I have use cases where I want the temperature to be different, so controllable from remote. I assume that I have a temperature sensor to my external boiler, so if needed, I can test things out.

janchlebek commented 8 months ago

@MislavMandaric: I am reverting to my comment above: I understand and in fact like the idea of having the water heating separated from main heating. But having the HWB as a simple toggle was better in my opinion for the reason below, the mode setting of heating being now part of the water heater makes it a little hard:

The problem is that whenever using the new HWB mode setting on the water heater thermostat to (temporarily) override the scheduled water heating you have to first record the actual mode of the water heater in order to be able to put it back where it was.

For example now in winter typically the main thermostat is in Heating mode but in summer it is in hot water only mode. This means that automating HWB would be different in summer or winter since you need to know to which mode to set it when deactivating HWB.

In the Vaillant app HWB simply acts as an optional toggle on top of any existing mode and when turned off it switches back to previous mode automatically. This is now not really possible via the new entity and makes the logic for automations more complex. Wouldn't it be possible to expose the HWB as a toggle (ON/OFF) like it was before and how it is designed in the app? Or do you have an idea how to work around this?

21Development commented 2 months ago

@MislavMandaric Further to https://github.com/MislavMandaric/home-assistant-vaillant-vsmart/issues/263 if you can provide me some guidence on any quick way I can check the API response on my boiler/system, I'll gladly experiment and see if the NTC tank temp is exposed.

MislavMandaric commented 2 months ago

You can try running the example.py from the library repo: https://github.com/MislavMandaric/vaillant-netatmo-api/blob/master/example.py

Just be sure to set all the environment variables from lines 13 to 18.

21Development commented 2 months ago

Just be sure to set all the environment variables from lines 13 to 18.

I ran the following:

pip install vaillant-netatmo-api
nano example.py

I pasted the example.py contents then added the following to the top, from my HA install:-

os.environ['CLIENT_ID'] = "na_client_android_vaillant"          
os.environ['CLIENT_SECRET'] = "929a9xxxxxxxxxxxxxxxxx"
os.environ['USERNAME'] = "my email"
os.environ['PASSWORD'] = "my pass"
os.environ['USER_PREFIX'] = "vaillant"
os.environ['APP_VERSION'] = "1.0.4.0"

when running python3 example.py I get the following error message

Traceback (most recent call last):
  File "example.py", line 3, in <module>
    import asyncio
  File "/usr/lib/python3.8/asyncio/__init__.py", line 8, in <module>
    from .base_events import *
  File "/usr/lib/python3.8/asyncio/base_events.py", line 18, in <module>
    import concurrent.futures
  File "/usr/lib/python3.8/concurrent/futures/__init__.py", line 8, in <module>
    from concurrent.futures._base import (FIRST_COMPLETED,
  File "/usr/lib/python3.8/concurrent/futures/_base.py", line 7, in <module>
    import logging
  File "/usr/lib/python3.8/logging/__init__.py", line 26, in <module>
    import sys, os, time, io, re, traceback, warnings, weakref, collections.abc
  File "/usr/lib/python3.8/traceback.py", line 5, in <module>
    import linecache
  File "/usr/lib/python3.8/linecache.py", line 11, in <module>
    import tokenize
  File "/usr/lib/python3.8/tokenize.py", line 35, in <module>
    from token import EXACT_TOKEN_TYPES
ImportError: cannot import name 'EXACT_TOKEN_TYPES' from 'token' (/home/name/.local/lib/python3.8/site-packages/vaillant_netatmo_api/token.py)

What have I missed doing?

MislavMandaric commented 2 months ago

Unfortunately, the docs are misleading. The steps you followed, as written in the readme, are for integrating the library into a separate product, like this HA integration. The steps for contributing to the library itself are missing, and I'll need to add those 😄

But for now, this is how you can run the mentioned example:

  1. Clone the repo (follow steps defined in the Github documentation on how to clone a repo)
  2. Run python -m venv venv from the root of the repo directory
  3. Run . venv/bin/activate
  4. Run pip install -r requirements-dev.txt
  5. Create a new file called .env
  6. Put the environment variables inside this new file in the form of NAME=value (ex. CLIENT_ID= na_client_android_vaillant), with one env var per line
  7. Run ./example.py

You need to run those on your local machine, not on your HA instance. The instructions are for Linux or Mac.

21Development commented 2 months ago

But for now, this is how you can run the mentioned example:

Okay, installed as above and the output is three lines, which presumably is the token.access_token, token.refresh_token and measured.temperature from the vSmart room stat?

5cfbexxxxxxxxxxxxxxxxxxxxx|efc037xxxxxxxxxxxxxxxxxxxxx42da 5cfbexxxxxxxxxxxxxxxxxxxxx|1548fexxxxxxxxxxxxxxxxxxxxx86f6 20.7

What do I add in example.py to get the DHW/tank/NTC temperature ? I think it's code d.04 on the boiler so I can verify output against that.