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.

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

Device Offset (Through Self-Regulation) Should Always be Applied #467

Closed
mag2352 opened this issue May 30, 2024 · 7 comments
Closed

Device Offset (Through Self-Regulation) Should Always be Applied #467

mag2352 opened this issue May 30, 2024 · 7 comments
Labels
bug Something isn't working developed When development is done and tested

Comments

@mag2352
Copy link
Contributor

mag2352 commented May 30, 2024

Firstly, I'm making a new issue for this because it is largely unrelated to the other issues I am commenting on. This change is a very simple change. I'm using VTherm with some AC/HVAC units in Cooling mode. They have an issue where their internal temperature is very inaccurate in both heating and cooling modes. The current version of VTherm stops using the device temperature in offset calculations if the target temperature is reached. This prevents my HVAC from turning off the compressor (it still blows cold air).

Here is a quick example:
The target temperature is 20 C. The external temperature sensor is reading 22 C. The HVAC internal temperature is reading 24 C. The HVAC would be set to some value under 24 C (let's say 22 C). This is a very basic calculation, but leaving the regulation algorithm aside, this makes sense: 24-22= 2 C offset. Underlying target temperature should be set to 20, but the offset adds 2, the final target temperature on the underlying unit is 22 C. Now, let's consider that the external temperature is reading 19.9 C (some value under the target of 20 C). For simplicity, let's assume the HVAC internal temperature is still reading 24 C. The calculated offset would be 4.1 C, but this wouldn't be applied due to the following lines in the code:

offset_temp = 0
device_temp = 0
if (
# regulation can use the device_temp
self.auto_regulation_use_device_temp
# and we have access to the device temp
and (device_temp := under.underlying_current_temperature) is not None
# and target is not reach (ie we need regulation)
and (
(
self.hvac_mode == HVACMode.COOL
and self.target_temperature < self.current_temperature
)
or (
self.hvac_mode == HVACMode.HEAT
and self.target_temperature > self.current_temperature
)
)
):
offset_temp = device_temp - self.current_temperature

This code makes it so that the offset is not used if the current temperature is less than the target temperature. This results in the underlying entity not having the offset applied, and will result in a smaller target being applied to the underlying entity (the 4.1 C offset is no longer added). This effectively turns the AC back on when it reaches under the VTherm target temperature. The solution is simple, just remove the condition that prevents offsets from being applied if the target is reached. The commit on my fork does this (I can make a PR, but I am waiting for the PR on #240 to be approved).

mag2352/versatile_thermostat@7456a41

To put the preview here, it looks like this after this simple change:

            offset_temp = 0
            device_temp = 0
            if (
                # regulation can use the device_temp
                self.auto_regulation_use_device_temp
                # and we have access to the device temp
                and (device_temp := self._hass.states.get(under._entity_id).attributes.get("current_temperature")) is not None
            ):
                offset_temp = device_temp - self.current_temperature
@jmcollin78 jmcollin78 added the question Further information is requested label Jun 9, 2024
@jmcollin78
Copy link
Owner

Hello, I will have a look more closely. I'm not sure of your asumption.

@mag2352
Copy link
Contributor Author

mag2352 commented Jun 9, 2024

Sounds good. There is no rush on this, as I have this on my branch and it seems to work fine. If this causes issues, I can just use my fork for now. I just find it strange to use the offset setting in the configuration, and then have the VTherm stop using the offset once the target temperature is reached. In my case, this causes my HVAC to turn back on when the offset is no longer being used. It would make the most sense to me to always use the offset calculation.

@kvanbiesen
Copy link

kvanbiesen commented Oct 3, 2024

I read Tru the code and agree with @mag2352,

The offset should always been applied. The moment an external temperature meter AND the corresponding internal Temperature meter are accesbile and return a temperate.....an offset applies. No matter if the state is in cooling or hearing mode. And no conditions. Offset applies always.

Edit: I also noticed that the target temp gets offset. Isn't it better to push the offset difference to the TRV. Most TRV have an offset slider/value. That way the target temperature also shows correctly on the TRV?

@jmcollin78
Copy link
Owner

jmcollin78 commented Oct 16, 2024

Same as #535. Ping @tttopuz

jmcollin78 added a commit that referenced this issue Oct 16, 2024
Co-authored-by: Jean-Marc Collin <jean-marc.collin-extern@renault.com>
@jmcollin78
Copy link
Owner

https://github.com/jmcollin78/versatile_thermostat/releases/tag/6.4.1.beta1

If someone can give it a try, i will put it in the next release. Thank you in advance

@jmcollin78 jmcollin78 added bug Something isn't working developed When development is done and tested and removed question Further information is requested labels Oct 16, 2024
@tttopuz
Copy link

tttopuz commented Oct 16, 2024

Tested and working.

@kvanbiesen
Copy link

kvanbiesen commented Oct 16, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working developed When development is done and tested
Projects
None yet
Development

No branches or pull requests

4 participants