-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[tado] Add support for fanLevel, verticalSwing, horizontalSwing, light API #12470
Conversation
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Ping @dfrommi for comments.. |
Ping @Pro for comments / testing .. |
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Ping @Timtam for testing (???) .. |
I'm sorry, but I currently don't own an air conditioner, just heating equipment over here. |
Same here, I also do not have an air conditioner to test. Sorry. |
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Adding @hpalne to this thread.. |
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@Artur-Fedjukevits and @hpalne : my own Smart AC Controller is old, and my A/C unit is very rudimentary, so I cannot test the new functionality. I can only test that it does not (anymore) break the functionality of older systems. i.e. my testing is only 'old-features-still-working' rather than 'new-features-now-working'. => So I would be most grateful if you could test it from your side. :) { PS I am wondering if I could 'trick' Smart AC Controller to think it is connected to a full featured A/C unit.. So perhaps you can tell me your brand of A/C and the model number written on the front or back of its remote control? } |
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg I can try to test it, just need the compiled jar file. |
|
Perfect :) |
@hpalne see the ReadMe about the Channel names.. |
@andrewfg I have the same issue with the Tado Binding and my LG AC. 2022-03-15 16:54:01.787 [WARN ] [ado.internal.handler.TadoZoneHandler] - Could not apply HVAC change on home 899993 and zone 1: Operation updateZoneOverlay failed with error 422 My issue is that I am totally new to GitHub and I am relatively new to Openhab. So if I want to test the new Binding, what do I have to do? Must i copy the jar file to my raspberry pi? If yes, in which folder? Sorry for being a newbie. |
@Freeride79 in your OH Settings UI under Bindings, select to uninstall the release version of the Tado binding. Then take the jar file from the zip file two posts above this, and drop it in the 'add-ons' folder on your OH system. |
After I replaced the jar file, in my case it starts throwing 422 exception again. First capabilities request in "TadoZoneHandler.java" file on line 216 return this capabilities:
And always when I'm trying to change hvacMode for example, in same file on line 112 get this overlay:
I suspect I get 422 exception because we sending unexpected elements even if it is null |
@andrewfg I replaced also the jar file and got the same error message as before. 2022-03-16 06:47:32.175 [WARN ] [ado.internal.handler.TadoZoneHandler] - Could not apply HVAC change on home 899993 and zone 1: Operation updateZoneOverlay failed with error 422 org.openhab.binding.tado.internal.api.ApiException: Operation updateZoneOverlay failed with error 422 |
@Artur-Fedjukevits & @Freeride79 many thanks for testing, and apologies that it did not work; normally I would test on my own system, but unfortunately my A/C doesn't have the same features as yours. So based on your logs, I will write JUnit test module, so I can test without actually having the correct hardware. It might take me a day or two.. |
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
I have just implemented the following changes..
The new JAR is here.. @Freeride79 and @Artur-Fedjukevits can you please confirm the following..
|
@andrewfg I copied the new JAR to my Openhab and tried to test a little bit.
I think it's very important to send the right set of item changes and finalize it with the ZoneOperationMode (in my case Timer) and TimerDuration. If I put those items not at the end of my rule, it does not work. 2022-03-20 09:43:23.153 [DEBUG] [ado.internal.handler.TadoZoneHandler] - Setting overlay of home 899993 and zone 1 with overlay: class Overlay {
} 2022-03-20 09:43:23.533 [DEBUG] [ado.internal.handler.TadoZoneHandler] - Updating state of home 899993 and zone 1 |
@Freeride79 many thanks for the testing. If I understand you correctly, it is basically all now working properly for you??
I think it is actually the other way round i.e. the Operation Mode should be sent first rather than last.. These A/C devices support different capabilities depending on a) whether the Power State is OFF or ON, and b) if the Power State is ON, whether the AC Mode is HEAT, COOL, FAN, DRY, etc. And if you send a command value for (say) a fanLevel that is not in the device's list of capabilities for the current actual Power State and AC Mode, then that command will probably fail. So the correct sequence is as follows..
I would expect that if you do not follow the sequence above, then you will probably get strange or erroneous results. |
@Freeride79 according to the logs that you submitted earlier, the swing options for your A/C are only OFF and ON |
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@Artur-Fedjukevits and @Freeride79 if/when you are happy with the functionality, could you kindly go to the ‘Files Changed’ tab on this page, hit the ‘Review Changes’ button on the top right, and ‘Approve’? That will give a signal to the OH Maintainers that they can start the code review prior to merging this PR into OH Main.. |
@andrewfg yes, for me everything is working fine! Thank you very much. Regarding the sequence you are right but I don't send ON to the Power State cause I thougth this is only readable but thats not my point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works fine with my LG A/C! Thank you very much!
Glad to have been able to help. :)
Hmm. Any explicit setting that you make via OpenHAB, or via the App, or by touching the Smart A/C controller itself, implicitly (automatically) switches the operation mode from 'Schedule' to 'Until change'. And, although I did not try it, I think if you then change the operation mode to 'Manual' or 'Timer', the respective overridden setting value is persisted (??). But obviously if you command the operation mode back to 'Schedule' then the overridden setting value is cancelled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything working fine!
=> This PR is now ready for the formal code review by OH-Maintainers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good. A few minor comments added.
...enhab.binding.tado/src/main/java/org/openhab/binding/tado/internal/api/TadoApiTypeUtils.java
Show resolved
Hide resolved
...enhab.binding.tado/src/main/java/org/openhab/binding/tado/internal/api/TadoApiTypeUtils.java
Show resolved
Hide resolved
.../main/java/org/openhab/binding/tado/internal/builder/AirConditioningZoneSettingsBuilder.java
Show resolved
Hide resolved
...do/src/main/java/org/openhab/binding/tado/internal/handler/TadoStateDescriptionProvider.java
Outdated
Show resolved
Hide resolved
...ab.binding.tado/src/main/java/org/openhab/binding/tado/internal/handler/TadoZoneHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks!
…t API (openhab#12470) * [tado] fix issue openhab#12160 (first attempt) * [tado] code style * [tado] option descriptors * [tado] add level5 * [tado] fan level and swing not allowed on heating or hot water * [tado] warn about possible un-supported state values * [tado] minor formatting * [tado] harmonise getter methods for fan and swing state * [tado] include all options in xml * [tado] remove 's' from capabilities list names (go figure..) * [tado] add OFF option * [tado] add support for light channel, and dynamic channel decriptions * [tado] tweak ReadMe for more clarity * [tado] adopt reviewer suggestions Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch> Signed-off-by: Nick Waterton <n.waterton@outlook.com>
…t API (openhab#12470) * [tado] fix issue openhab#12160 (first attempt) * [tado] code style * [tado] option descriptors * [tado] add level5 * [tado] fan level and swing not allowed on heating or hot water * [tado] warn about possible un-supported state values * [tado] minor formatting * [tado] harmonise getter methods for fan and swing state * [tado] include all options in xml * [tado] remove 's' from capabilities list names (go figure..) * [tado] add OFF option * [tado] add support for light channel, and dynamic channel decriptions * [tado] tweak ReadMe for more clarity * [tado] adopt reviewer suggestions Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
…t API (openhab#12470) * [tado] fix issue openhab#12160 (first attempt) * [tado] code style * [tado] option descriptors * [tado] add level5 * [tado] fan level and swing not allowed on heating or hot water * [tado] warn about possible un-supported state values * [tado] minor formatting * [tado] harmonise getter methods for fan and swing state * [tado] include all options in xml * [tado] remove 's' from capabilities list names (go figure..) * [tado] add OFF option * [tado] add support for light channel, and dynamic channel decriptions * [tado] tweak ReadMe for more clarity * [tado] adopt reviewer suggestions Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch> Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
…t API (openhab#12470) * [tado] fix issue openhab#12160 (first attempt) * [tado] code style * [tado] option descriptors * [tado] add level5 * [tado] fan level and swing not allowed on heating or hot water * [tado] warn about possible un-supported state values * [tado] minor formatting * [tado] harmonise getter methods for fan and swing state * [tado] include all options in xml * [tado] remove 's' from capabilities list names (go figure..) * [tado] add OFF option * [tado] add support for light channel, and dynamic channel decriptions * [tado] tweak ReadMe for more clarity * [tado] adopt reviewer suggestions Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Fixes #12160
Background
As described in #12160 the Tado API has been extended for Air Conditioners to support more advanced features. Specifically these devices now support some additional JSON sub-elements as follows..
fanLevel
element providing extended fan speed possibilities compared to the earlierfanspeed
element.verticalSwing
,horizontalSwing
elements providing extended swing possibilities than the earlierswing
element.light
element for controlling the A/C unit panel light.@Artur-Fedjukevits made a first attempt to support these additional JSON elements; but instead of simply adding support for the new
fanLevel
,verticalSwing
, &horizontalSwing
elements, he mistakenly removed support for the priorfanspeed
&swing
elements. So his solution is not backward compatible with existing older devices.By contrast, this PR is to add support for the new JSON elements, and still retain support for existing devices without breaking compatibility.
Notes:
Signed-off-by: Andrew Fiddian-Green software@whitebear.ch