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

Feature/add target temp step #1178

Merged

Conversation

Mneric
Copy link

@Mneric Mneric commented Dec 4, 2023

Motivation:

Changes:

Related issue (check one):

  • fixes #
  • there is no related issue ticket

Checklist (check one):

  • I did not change any code (e.g. documentation changes)
  • The code change is tested and works locally.

Test-Hardware list (for code changes)

HA Version:

  • Core 2023.11.3
  • Supervisor 2023.11.6
  • Operating System 11.1
    Zigbee2MQTT Version: 1.33.2-1
    TRV Hardware: Moes BRT-100-TRV and Avatto ME167

New device mappings

  • I avoided any changes to other device mappings
  • There are no changes in climate.py

Hello,

Here are a few modifications that I applied to my local home assistant.
Everything is not clean enough to be directly applied to your repo but I wanted to know if you are interested in the feature I added before oing further.

I added two features:

Hybrid calibration mode

I added this feature because I was not happy with the way my TRV handles opening/closing its valve.
Often, when the local temperature was close to the target temp, the valve closed itself to 25%. Because of that, the target temperature took a long time to be reached.
This is annoying because my boiler is controled by HA and will be ON if any of my valves is open.

I also want the local temperature on my TRV to be as closed as possible as my temperature sensor.

Because of that I created a hybrid calibration mode.

The idea is simple:

  • The calibration of my TRV is modified by BT to keep its local temperature close to the value reported by my temperature sensor. This is similar to the temperature calibration mode with a "no offset" mode.
  • The target temperature set on the TRV is 3 degree higher that the temperature set to BT if the local temp is lower than the BT target temps. On the other hand, the target temperature set on the TRV is 3 degree lower that the temperature set to BT if the local temp is higher than the BT target temps. This is similar to the agressive temperature offset mode that already exists.

This allow me to have the desired behavior.

This addition is quite messy as I juste added a new value of 2 to self.real_trvs[entity_id]["calibration"] while it was designed to be a boolean.

Target temperature step option

My TRV target temperature step is 1°C.
However, with my previous feature, the effective BT accuracy is the accuracy of the temperature sensor.

I added an option to override the temperature step of the TRV to have a better control on the desired temperature in a room.

Error handling

I also added a error catch in the set_hvac_mode asI was experiencing some crashed in that function when using the Moes TRV with ZHA.

Conclusion

Let me know what you think about those features.
Maybe what I did is not interesting for anyone except me.

@Mneric Mneric marked this pull request as ready for review December 5, 2023 13:34
@KartoffelToby KartoffelToby merged commit d31a70b into KartoffelToby:master Dec 27, 2023
@isibizi
Copy link

isibizi commented Dec 28, 2023

I have already posted this in another issue post, but i think that is the right issue post. After update to 1.50 Beta i have this message in BT

IMG_0312

@mako777
Copy link

mako777 commented Dec 28, 2023

You have to reconfigure your better thermostat entities and select appropriate value in last input select field (the last one on the bottom) and maybe restart HA after.

@KartoffelToby
Copy link
Owner

#1219

Its an ui input bug, will be fixed in the final Release of 1.5.0

But for now enter 0.1 or above to avoid this error

@Mneric
Copy link
Author

Mneric commented Dec 28, 2023

Hello!

Indeed, I am not surprised that some errors may occurs.

As stated in my original comment, my modifications were not production ready. I was a little surprised to see my PR merged without any review. But I guess the plan is to fix it in the beta branch?

@KartoffelToby let me know is you need any help. For example, the desired behavior regarding #1219 is that is should be possible to leave the field empty to disable the override.
I also can handle the franch translation for the things I added.

@KartoffelToby
Copy link
Owner

@Mneric yes i haven't the time the last weeks, and there was some PRs, i decide to merge first and check for bugs and changes in the Beta loop, so feel free to fix this, shoud be the same as the offset field.

@marcanxo
Copy link

marcanxo commented Jan 8, 2024

Missing documentation for this new feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants