Feature request: please implement AC swing modes

ElBalsero commented 1 month ago

Hi,

Thanks a lot for all the work on this node so far.

When using the setZoneOverlay for an AC device, the payload contains information about the horizontalSwing and the and the verticalSwing modes. However, the red node interface does not give an option to configure those values.

Is there any possibility that you bring those settings to the red node interface?

I know you do not own an AC from Tado. I can help there as much as needed.

For starters, I can provide the possible values as I see them in the developer console of the Tado web interface:

horizontalSwing: "ON", "OFF", "RIGHT", "MID-RIGHT", "MID", "MID-LEFT", "LEFT" verticalSwing: "ON", "OFF", "DOWN", "MID-DOWN", "MID", "MID-UP", "UP"

Thanks in advance, and please do not hesitate to request any extra info you might need.

Daniel

mattdavis90 commented 4 weeks ago

Hi,

You're welcome - I'm glad the node is useful to you. The underlying library mostly already supports the features you're asking for. Tado made some changes to their API that mean they've mostly moved away from setZoneOverlay towards setZoneOverlays (note the 's'). The underlying library supports that API call (which supports horizontalSwing) but I haven't yet managed to integrate this neatly into Nodered's UI framework. The feature is sort of tracked in #39 and #40

I'm wondering if there's a shortcut version for this ask though which is to just add it to the older API call. I'm not certain when I'll get around to looking at the necessary UI work again to do the multiple overlays.

I'll look into it.

Thanks

ElBalsero commented 3 weeks ago

Hi @mattdavis90,

Thanks for the answer. I see what you mean. I have also had a look to the other issues you are referencing.

It is possible that for this particular thing I am requesting, simply adding the options to the GUI with the API as it is would do the trick. I would be more than happy with that option.

I also understand your dilema about wanting to integrate the new API calls before the old ones are deprecated. I have a suggestion, and I hope not to be saying anything stupid here. If that is the case, I apologize in advance. If I understand correctly you would like to adapt the node UI to the new calls to get all the new juice they are bringing. For the time being you are not sure about the best way to do that and also you do not have a lot of time to dedicate.

That approach would be definitely the perfect one. However, there is also the good one (less than perfect but much better than bad). Instead of adapting the UI framework to the new API calls, just adapt the new API calls to the UI as it is. What I mean is, leave the UI, as it is, use the new API calls to continue giving exactly the same functionality which gives today. To be honest, as it is now, the UI is giving a lot of flexibility already. Sure, it could have more if you are able to adapt it to extract all the juice from the new calls. But as it is... man... it is already very good. This way, everything will still work when the old calls are finally deprecated, and you can continue thinking how to fully integrate them. When you have a good idea for that or someone comes with a good suggestion, you go for the next step and you finally adapt the UI with the new functionalities.

Well... as I said, I hope not to have said something stupid.

In any case, whatever way you go, count on me for testing as much as needed.

Thanks for all your work,

Daniel

mattdavis90 commented 3 weeks ago

Hi,

That's certainly not a stupid idea and is one of the options I've been mulling over. I think the following is my current plan.

On the master branch of the library I've backported this functionality to the singular API call and plan to add the UI elements in this library shortly.
I plan to add the newer multi-overlay API to this library in the exact way you mention. If you know it is there and how to form the messages then you can use the feature but there won't be a UI.
Eventually I hope to add the UI elements.

Time to develop the library and this node isn't on my side and, as you're aware, AC is a feature we've historically had issue with and I can't test. The library change look ok though so I'm hoping to do the NodeRed UI integration tonight for the older API call, and maybe even for the newer call.

mattdavis90 commented 3 weeks ago

I've just pushed changes that should mean that the first two bullet points are now done. I need to add documentation around how to use the non-UI API calls and I need to do some more local testing before I publish to the market but I wanted you to get the code ASAP to help test. Thanks

ElBalsero commented 3 weeks ago

Hey,

That sounds like a great plan.

Oh, wow, your message stating that the changes are already there popped up while writing this.

Let me update and see if I can make some tests...

[edit]: with the excitement I kind of missed the part where you say that you'll publish it only after testing :-) I will just wait until you tell me how to get the new one to test.

mattdavis90 commented 3 weeks ago

It looks like everything it working now as expected for me. There were a couple little bugs in testing so worth updating to master when you're testing the functionality. I've updated the README with and Advanced Usage section.

If you configure an Inject node with the following properties and send it to the Tado node then it'll hopefully work.

msg.apiCall = setZoneOverlays msg.payload = [ 12345, [ { "zone_id": 1, "power": "ON", "verticalSwing": "TOP", "temperature": { "celsius": 18 } } ], 3600 ]

That would should set an overlay for the next hour. You'd need to change the 12345 to your home_id as well.

Let me know how you get one. Thanks

mattdavis90 commented 3 weeks ago

Oops, missed your edit

For me, I run in Docker, so I navigate to /data and run npm i git+https://github.com/mattdavis90/node-red-contrib-tado-client.git. Then I restart Node-RED

If you don't run in Docker then I'm not 100% sure where those files will be. You're looking for a directory that contains a directory called node_modules.

If testing this way is a blocker then I can publish - I'm generally just reluctant with anything AC related :smile:

ElBalsero commented 3 weeks ago

I have it as an add on, but don't worry let me investigate a bit how to do it. If I do not find a way tonight, tomorrow I will quickly install a docker for testing purposes. Thanks for this, man.

ElBalsero commented 3 weeks ago

OK. A quick update before going to bed.

I have not been able to find the way to do it in the add on. So I have quickly installed a docker, and there your instructions worked as a charm. I have been able to do a couple of quick tests.

I wanted to test both ways: from the UI and also injecting.

From the UI:

I have not been able to make it work. I see two problems:

1) In the UI itself. No matter what values I choose for the new vertical and horizontal swing boxes, when I click "Done", if I open again the node to see the values, both have been reset to "Off".

2) If I go with the flow and I leave them in "Off", I am getting two errors. One for the verticalSwing and one for the horizontalSwing. Both errors are identical:

code: "typeMismatch"
title: "Invalid value for property setting.verticalSwing"

and

code: "typeMismatch"
title: "Invalid value for property setting.horizontalSwing"

Just thinking outloud: are you sending the value to the API as "Off" or as "OFF"?. I ask because one of my findings in the injection tests is that the API only swallows the values in uppercase.

With injection:

This one is actually working. I have not tested yet extensively, just some initial basic tests, but everything that I tried worked so far.

I did find some curiosities though, which I would like to share with you:

I started with the payload you gave me as example and it seems not to be enough. It complaint that it was missing the values for "mode", "fanLevel", and "horizontalSwing". Apparently those fields are mandatory. Once I set them, everything worked.

Also, in the example payload you had set the "verticalSwing" to "TOP" and that fails because the correct value is "UP". ButI have noticed that in the UI you set it to "UP". So, I assume that was just a typo.

I will try to make some more extensive tests tomorrow after work. If there is any particular one you would like me to try, just let me know it. Otherwise I will tests every single value with the unit off and on.

Thanks again for everything.

mattdavis90 commented 3 weeks ago

Thanks for the thorough testing and write up. I've no idea what's going wrong with the UI but I'll have a look after work and see if I can replicate your findings.

I tested that the UI options still worked for my cases where I'm controlling heating. I wanted to make sure that the new options didn't affect me overriding my zones.

Behind the scenes the values should be OFF but I'll double check tonight that I didn't typo, it strikes me that some of the automatic wiring of the HTML and JS within the Node-RED isn't quite working.

On injection, that's very exciting!!

Ah, I typed that from memory because it was getting late and for some reason TOP is what I thought the option was 😄

That's interesting about the other required fields. I took a guess at what I thought might work. I'm not sure what the best way to document that is. I'm inclined to put it into the library and reference it from this repo to make it most widely available.

I'm so glad it is somewhat working though. Thanks

ElBalsero commented 3 weeks ago

No problem. It is my pleasure to help here.

If you figure out what is going on in the UI, let me know and I will test again. I will give it another try when arriving home after work. Perhaps yesterday night I was to sleepy and I did something wrong.

I will also test more extensively the injection and share the results.

Regarding the way to document it, I think what you describe is good. This is kind of a "hidden" (actually documented) advance feature which allows users to tinker straight away with the library, kind of bypassing the UI. It makes sense to me that the documentation is in the library and then in the repo there could be a brief explanation on how to use the injection to bypass the UI and a reference to the library documentation for all the details on parameters and options.

I am so happy this is working! I have my Tado system integrated in Home Assistant, but the integration there is virtually abandoned and the AC units are not working almost at all already for a while. With this I can make use of all the functionality through this Node Red. I can rebuild all my automations based on this.

I will come back to you later with the results of the tests.

mattdavis90 commented 3 weeks ago

I found the issue. I forgot how the Tado UI worked and free-flow named an HTML input - not allowed :smile: I've fixed that and looks to be working in my Node-RED now and hasn't affected my ability to do overlays on my heating.

I also updated the advanced section in the docs with some links and I realised that you can now call the generic apiCall method on the underlying library meaning that you can actually hit any endpoint you want with this node just handling authentication for you. Pretty neat so I added an example showing how to us it for a "Boost all zones" button and to replicate the "getAirComfort" API call.

I haven't used Home Assistant in quite some time but I just had a look and the Python library sat behind the integration looks pretty good - it doesn't have some of the API calls we have in this library but it looks ok. I guess the Home Automation team maybe aren't exposing some of that.

I'm glad you can make use of this node though!

ElBalsero commented 3 weeks ago

Yep. I can now change the values in the UI and they stay. I have been able to make some test. Good catch.

I have checked the changes in the documentation and the references to the library. It all looks very clear to me, perfectly explained, and very well organized.

I have noticed that when explaining the 'termination' parameter in the library, you mention:

"auto" - this will put the overlay into "TADOMODE" _Note: I haven't been able to replicate this mode in the Tado App so not sure what it does

In case it helps, my understanding of this is that you leave it to the default value which is set in the Tado App configuration. In the app, you can go to the configuration of every room (not device but room) and set a default value for the duration of the overlays.

Here

IMG_6FF0342EED23-1

Which brings you here.

IMG_97FB915BF19A-1

In the Home Assistant integration, the library supports indeed everything. That has been reiterated in different issues opened by different persons in the Github repository when Tado made changes which broke the integration. The issue is that the original developers do not have time any more to support the integration and to bring the changes to the interface that Tado uses. They have apologized several times about... and that is indeed life... circumstances evolve and priorities change. And now no one is really supporting the integration and, opposite to this one, there is no back door to bypass the UI (or interface if you will) like the injection here. And that probably also makes sense in Home Assistant... Or not... I don't know. Anyway... I tried to help but my knowledge and skills do not reach the level to be able to do the changes myself. I can test and help... but not lead. So, I was reading and looking for other ways when I came across your node. And this is great. I can now use the integration with Node Red from Home Assistant to make things work again. All thanks to you and your work here.

Now for the test, I have mostly tested SetZoneOverlay and ClearZoneOverlay, since all the other commands were already working (well... I have not tried them all... but these are the ones we are discussing here). I have tried them combining all the different parameters in different ways and from different initial statuses. These are the issues I have found.

For the UI:

When setting the AC mode to AUTO or DRY, I get consistently the error below (with the variant DRY in the error text for that case), regardless of what the other values are. And it has nothing to do with the temperature. The temperature is set and, in fact, if I simply change the AC mode from AUTO or DRY to anything else, without touching the other values (also not the temperature, it works). I have not been able to understand what is the problem here. I have captured the messages in the Web Client Console (pasting them below under the error in case that they tell you something that they do not tell me) and also tested with injection. The only thing that I found (and you might want to add to the documentation) is that when using the DRY mode, the "fanLevel" parameter has to be passed as an empty string since the parameter is not allowed. But the AUTO is not different of any other and with injection works perfectly without any difference from the rest.

code: "setting.notSupported"
title: "temperature required for mode AUTO"

Web Client Console message for AUTO:

{
"type": "MANUAL",
"setting": {
    "type": "AIR_CONDITIONING",
    "power": "ON",
    "mode": "AUTO",
    "temperature": {
        "celsius": 24,
        "fahrenheit": 75.2
    },
    "fanLevel": "LEVEL4",
    "verticalSwing": "ON",
    "horizontalSwing": "ON"
},
"termination": {
    "type": "TIMER",
    "typeSkillBasedApp": "TIMER",
    "durationInSeconds": 3600,
    "expiry": "2024-06-03T22:15:41Z",
    "remainingTimeInSeconds": 3599,
    "projectedExpiry": "2024-06-03T22:15:41Z"
}
}

Web Client Console message for DRY

{
"type": "MANUAL",
"setting": {
    "type": "AIR_CONDITIONING",
    "power": "ON",
    "mode": "DRY",
    "temperature": {
        "celsius": 24,
        "fahrenheit": 75.2
    },
    "verticalSwing": "MID",
    "horizontalSwing": "ON"
},
"termination": {
    "type": "TIMER",
    "typeSkillBasedApp": "TIMER",
    "durationInSeconds": 3600,
    "expiry": "2024-06-03T22:14:56Z",
    "remainingTimeInSeconds": 3599,
    "projectedExpiry": "2024-06-03T22:14:56Z"
}
}

Web Client Console message for HEAT:

{
"type": "MANUAL",
"setting": {
    "type": "AIR_CONDITIONING",
    "power": "ON",
    "mode": "HEAT",
    "temperature": {
        "celsius": 24,
        "fahrenheit": 75.2
    },
    "fanLevel": "LEVEL2",
    "verticalSwing": "OFF",
    "horizontalSwing": "MID_RIGHT"
},
"termination": {
    "type": "TIMER",
    "typeSkillBasedApp": "TIMER",
    "durationInSeconds": 3365,
    "expiry": "2024-06-03T21:59:04Z",
    "remainingTimeInSeconds": 3364,
    "projectedExpiry": "2024-06-03T21:59:04Z"
}
}

When setting Horizontal Swing to "Mid Right" or "Mid Left", I get the error below. And the corresponding one when I set Vertical Swing to "Mid Down" or "Mid Up". All other values work. And after some tests including also injection, I have realized that this one is totally on me. I told you that the strings to pass for the 'composed' values were in the format "MID-LEFT". But that is not true. The correct format, confirmed now by injection, is "MID_LEFT". Sorry for this one.
```
code: "typeMismatch"
title: "Invalid value for property setting.fanLevel"
```
You explain in the documentation why in the fan speed drop down you have a combination of two different sets of values and only one of them is valid, depending on my system. In my case, the ones working for me are Level 1 to Level 3. However, in the Tado interface I also see a Level 4 and I can also confirm that I have successfully injected Level 4. I think you might want to add it in the UI.

For the injection:

Other that the previously mentioned "MID_UP" instead of "MID-UP" and "fanLevel" has to be passed as an empty string when using DRY mode, I have not been able to find anything which is not working.

mattdavis90 commented 3 weeks ago

Wow, that's a thorough write up! Thanks for putting the effort in.

Thank you for the explanation of TADOMODE that makes sense to me. I'll update the documentation and double check that I'm exposing an API to read and set that default value.

Regarding AUTO and DRY that sounds like the underlying library isn't quite sending the correct parameters. I'll double check that temperature is present for AUTO and I'll have the library automatically remove fanLevel for DRY to not complicate the UI or documentation too much further.

Points 2 and 3 are fairly trivial to fix, thanks.

I don't think there's much to change to get this fully working so should be able to do it after work tonight. I'll let you know once I've updated the codebases.

mattdavis90 commented 3 weeks ago

I had a look at the code and temperature is only currently sent for HEAT and COOL so I can quickly fix that.

mattdavis90 commented 3 weeks ago

I found time this morning. All changes should now be pushed

ElBalsero commented 3 weeks ago

Oh, wow. You are fast man.

Regarding the TADOMODE, if you are able to expose it... great! it is always nice to have the option. However, if you are not, that is probably not terrible. My impression is that the idea behind is to blindly delegate the duration to whatever the user sets as default in the Tado app. I was for example using it to make sure that changes never last infinite time.

I have tried to update and testing, but the update is giving me a problem today. Yesterday I simply ran again the same command under the /data directory in the docker container, and that gave me the last updated version with your changes. However, when I try to do that again today, I am getting this:

node-red:/data$ npm i git+https://github.com/mattdavis90/node-red-contrib-tado-cl
ient.git                                                                         
npm ERR! code ETARGET                                                            
npm ERR! notarget No matching version found for node-tado-client@^0.15.1.        
npm ERR! notarget In most cases you or one of your dependencies are requesting   
npm ERR! notarget a package version that doesn't exist.                          

npm ERR! A complete log of this run can be found in:                             
npm ERR!     /data/.npm/_logs/2024-06-04T16_29_05_711Z-debug-0.log               
node-red:/data$

Am I doing anything wrong?

mattdavis90 commented 3 weeks ago

Oops, in my haste this morning I clearly forgot to publish the underlying library to npm. I'll fix this when I get home.

ElBalsero commented 3 weeks ago

No problem at all. Just give me a shout when ready and I will do my best to find the time to test it today. I have the gut feeling that there's not going to be a lot more to investigate :-)

mattdavis90 commented 3 weeks ago

That version is published now

mattdavis90 commented 3 weeks ago

It doesn't look like the defaultOverlay settings as Tado call them are available in the library today but I don't think it will be much effort to add them. It may not happen tonight but we'll see :D

mattdavis90 commented 3 weeks ago

I've just got the default overlay calls working and they're all the way through to the UI. You can now get or set a zone's default overlay. Hope this is also useful. Thanks

ElBalsero commented 3 weeks ago

Wow! I really did not expect that to be possible. Amazing! I ran out of time today. I have to postpone the testing until tomorrow after work. Talk to you tomorrow.

ElBalsero commented 3 weeks ago

I have been able to test today.

I was very curious to test the new feature of setting the defaultOverlay from the node. I tested it in the UI and it works like a charm. Amazing! I would have never expected this to actually be possible to set through the API.

I have noticed for the first time (and when I check I see that it has always been like that) that the option of using this default as Type of Termination is not present in the dropdown menu of the UI. Wouldn't it make sense to include it having so the whole range of options?

AUTO and DRY modes can now be set in the UI.

Mid Right and Mid Left for HorizontalSwing and Mid Down and Mid Up for VerticalSwing can now be set in the UI.

Level 4 can now be set for fanLevel and it works as expected when set.

This means that all my previous findings for the UI have been now corrected. I have also run a small regression tests without finding any other issue.

Now for the injection.

You had mention you wanted to remove fanLevel for DRY mode at library level in order not to overcomplicate the documentation. I have noticed that I am still getting the same error if I include it. However, this one is not very critical since the error itself is very clear and self-explanatory to the point that it might not even be needed to include it in the documentation:

code: "setting.notSupported"
title: "fan level not allowed in mode DRY"

I have tried to test the new setZoneDefaultOverlay using injection, but I have not been able to figure out the format of the payload. If you can give me an example, I will do a small test there. I am pretty sure it will work, though.

mattdavis90 commented 3 weeks ago

Hi,

Thanks so much for the testing. You've done such a thorough job! I'm glad everything is now working.

Regarding Type of Termination - the auto/TADO_MODE option is in there but it is currently labelled as Tado Schedule Termination and I've noticed that next_time_block isn't actually on the list - it seems I took it out because it may have stopped working or I misunderstood the TADO_MODE option.

The setZoneDefaultOverlay should take homeId, zoneId, defaultTerminationType and defaultTerminationTimeout as message properties for injection.

ElBalsero commented 3 weeks ago

Hey,

Oh, not at all. It is my pleasure. Thanks a lot to you for all your work here. This is going to make my life so much easier :-)

It is strange. The results I am getting with the termination mode in the UI, are not really coherent with what you describe. Let me clarify:

When I choose Tado Schedule Termination, what it does is to stick to whatever the Smart Schedule in the app says. So, it diverges from the Smart Schedule according to the settings I choose, but only until the next time block in the Smart Schedule. I have tried to change the default overlay in the app, and this setting is not following it. It just changes until the next time block in my Smart Schedule.

When I choose Manual Termination, it sets the time to infinite, again independently of what I have in the default overlay in the app.

And lastly, when I set Time Based Termination, it sets the time selected in the GUI, again ignoring the default overlay set in the app.

For the setZoneDefaultOverlay, I had more or less come with the four parameters which are needed, but I do not seem to be able to put them together in the required way. I tried several ways. For example:

[
    12345,
    [
        {
            "zone_id": 17,
            "type": "TIMER",
            "durationInSeconds": 1800
        }
    ]
]

But I always get back:

code: "nullable"
title: "Property terminationCondition.type is required"

mattdavis90 commented 3 weeks ago

Hi,

Apologies for the delay in replying. My testing agrees with your own and looking into the app.tado.com implementation I can't actually get it use the defaultOverlay for my zone at all. The web app always uses 10 minutes as the default for all of my zones, I have no idea why.

I wondering if we could change the library to look up the default overlay when the TADO_MODE option is chosen and then use this. Then have the additional 3 explicit options available also.

Apologies, now that the setZoneDefaultOverlay is handled in the NodeRed code it won't be available for injection in the same was as setZoneOverlays - the code will see a recognised API call and then handle the parameters for you based on the NodeRed naming. So to test injection for setZoneDefaultOverlay it'll be something like the following for msg

{
  "apiCall": "setZoneDefaultOverlay",
  "homeId": "1234",
  "zoneId": 1,
  "defaultTerminationType": "TIMER",
  "defaultTerminationTimeout": 1800
}

ElBalsero commented 3 weeks ago

No problem at all man.

Good to know we’re in the same page with the modes. It is weird though that it does not work for you. For me, when the right mode is chosen, it really follows whatever I set in the app.

However, I’ve only tested it for the Tado AC. Let me see if I can find some time this weekend and test it for the heating and thermostat. Perhaps I’ll get different results.

If I still get consistent results, I’ll document my testing step by step with an element you also have (thermostat, radiator module?) and see if we can do identical tests. If the results are still different… well… we’ll have to identify why.

ElBalsero commented 2 weeks ago

Finally I’ve not been able to allocate during the weekend for the testing. Hopefully tomorrow.

mattdavis90 commented 2 weeks ago

No problem. There no rush. I could probably cut the release as is to be honest, it's in a very working state. Certainly the AC controls are much better than they were before

ElBalsero commented 2 weeks ago

That’s a good point. It’s already definitely an improvement. And if we find anything else out, it can always be added.

mattdavis90 commented 2 weeks ago

I've just released the features and the code is on npm and NodeRed palette now. https://github.com/mattdavis90/node-red-contrib-tado-client/releases/tag/v0.11.0

ElBalsero commented 2 weeks ago

Nice. I’ve upgraded to it in my Home Assistant add on.

it’s being challenging to find time this week. hopefully I’ll be able to test the pending things on Thursday. Sorry for the delay on this.

mattdavis90 commented 2 weeks ago

No problem and no rush. The swing feature is implemented now along with the default overlay and the new way to call APIs so I'm pretty happy with the release.

ElBalsero commented 2 weeks ago

OK, back for some tests.

In first place, I have tried to inject setZoneDefaultOverlay. I am consistently getting the error:

code: "accessDenied"
title: "current user is not allowed to access this resource"

Perhaps I am doing anything wrong. I have tried to inject with msg.apiCall and msg.payload and also with only msg.payload. Always the same result.

Now, for the test on using the defaultOverlay, I am not sure what is going on. I have spent several hours and I am not able to reproduce anymore the results that, as I recall, I was getting earlier.

Now setting the TADO_MODE is not going for the default in the app as I think I recall it was doing. What is happening now is that it sets the the overlay until While in Home Mode if I am at home, and While in Away Mode if I am away.

Now I do not know if I made a mistake in my previous test, or something has changed, or my brains are today not really working as they should.

I will leave it for today and try again hopefully tomorrow and see if I have a better day and I am able to make sense out of this.

ElBalsero commented 2 weeks ago

I couldn't stop thinking about this and I was having a look to other libraries.

Perhaps TADO_MODE does exactly that (to keep the overlay until a change between the Tado modes home and away happens) and the one that actually changes to the default set in the app is TADO_DEFAULT.

Any chance we can try to send TADO_DEFAULT and see if it is accepted?

mattdavis90 / node-red-contrib-tado-client