Update climate.py to support setting leavingWaterOffset #180
Conversation
| @@ -119,6 +114,24 @@ def __init__(self, device): | |||
| self._supported_features |= SUPPORT_PRESET_MODE | |||
| _LOGGER.info("Support_preset_mode {}: {}".format(mode,support_preset)) | |||
|
|
|||
| # In theory only one of the three should be settable | |||
There was a problem hiding this comment.
In my case it seems 2 of them are settable, which isn't correctly supported right now
"heating": {
"setpoints": {
"roomTemperature": {
"settable": true,
"requiresReboot": false,
"value": 20,
"maxValue": 23,
"minValue": 18,
"stepValue": 0.5
},
"leavingWaterOffset": {
"settable": true,
"requiresReboot": false,
"value": 0,
"maxValue": 10,
"minValue": -10,
"stepValue": 1
}
}
},
There was a problem hiding this comment.
Hm. That's interesting. On code it defaults to the room temperature then being the setpoint. I think you'd need a separate ClimateEntity then for the leavingWaterOffset (as I suggested in the issue comment). However, I think as a stopgap, the PR at least enables systems where only the leavingWaterOffset is settable to be adjusted (right now it is not possible).
There was a problem hiding this comment.
But this PR does make a lot of code more complex. What about a member to determine which set of data should be used (ATTR_TARGET_ROOM_TEMPERATURE or ATTR_TARGET_LEAVINGWATER_OFFSET or ATTR_TARGET_LEAVINGWATER_TEMPERATURE) and use that in all places. That would enable the creation of multiple ClimateEntities. Just store ATTR_TARGET_LEAVINGWATER_TEMPERATURE, not introduce another enum or flag.
There was a problem hiding this comment.
This week I am pretty busy, maybe next week I have some time to try that approach
There was a problem hiding this comment.
What about a member to determine which set of data should be used (ATTR_TARGET_ROOM_TEMPERATURE or ATTR_TARGET_LEAVINGWATER_OFFSET or ATTR_TARGET_LEAVINGWATER_TEMPERATURE) and use that in all places. That would enable the creation of multiple ClimateEntities. Just store ATTR_TARGET_LEAVINGWATER_TEMPERATURE, not introduce another enum or flag.
Not sure what you mean by that exactly. P.S.: I am running this branch in my system and it works great (the gauge below is the leaving water offset)
|
@sraka1. I replaced speleontra code by your code from your githubrepo in the custom directory in HA. My heatpump operates in roomTemperature mode. After restarting HA I don't see a way to control the leavingWaterOffset. Am I doing something wrong? |
|
Started with this, see https://github.com/jwillemsen/daikin_residential_altherma/tree/jwi-climatecontrolrework. This is really work in progress, only created multiple climate control entities, for each one found. When I have spare time next week I will continue with this. |
Very good thanks. Is the code in your jwi-climatecontrolrework branch testable in HA? Or do I break thing when I use this code? |
|
The code is very experimental, your issue is not fixed yet, would recommend a second HA setup to test this code |
|
Did create a second HA docker instance and installed your code. I got two thermostat instances but as you had warned before the code is experimental and not fixed yet. The leaving water offset climate is not finished yet and got the same properties as room temperature climate entity.
|
|
Thanks for the test, looks the logic correctly detects that there are two setpoints, need to get the correct values, probably monday I have some spare hours |
|
Extended to code to read the correct values, next step is to add support for controlling all of these |
|
Did a swift test of your update. Lookes fine.
Room temperature thermostat appears to work fine. Temperature is settable. |
|
Leaving water offset should now also work |
|
Yes. You did it. Leaving water offset is HA controlable!!!! Very good. Now I can build HA logic, to dynamicly control the offset and stay better on the target temperature. And create runs of weeks or months compensating the influence of rain, wind and sun. What is your release plan of this branch in the production release of the master branch? |
|
A little remark:
Was in the old version:
Is it possible to change the name ClimateControl back to Altherma ? |
That is not easy, with the new approach it can be that within one device there are multiple sensors with the same name (they exist in a group, for example "ClimateControl". This is one of the breaking changes, I want to finish first all rework and get rid of some more code, at that moment I will create a beta release to get more feedback |
|
I am going to close this PR as the refactoring makes this PR obsolete |
See #179
Opening as a draft to validate approach...
@jwillemsen