-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add support for AtlanticHeatRecoveryVentilation #379
Conversation
* fix ventilation commands * refactor ventilation command * refactor cooling mode
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Outdated
Show resolved
Hide resolved
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Outdated
Show resolved
Hide resolved
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Outdated
Show resolved
Hide resolved
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Outdated
Show resolved
Hide resolved
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Outdated
Show resolved
Hide resolved
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.
is this still needed?
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Outdated
Show resolved
Hide resolved
I find a typo error in the pyoverkiz library ('O' is missing in VENTILATION) : |
ventilation_mode = self.executor.select_state(OverkizState.IO_VENTILATION_MODE) | ||
prog = ventilation_mode.get(OverkizCommandParam.PROG) | ||
|
||
if prog == OverkizCommandParam.PROG: |
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.
ha-tahoma/custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Line 87 in f0aff74
if prog == OverkizCommandParam.PROG: |
We have to replace "OverkizCommandParam.PROG" by "OverkizCommandParam.ON" because the value to compare is "on"
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Outdated
Show resolved
Hide resolved
Even though we need to fix this typo, this should not matter. The typo is only in our constant, not in the actual exposed value. |
But wouldn't that make that fail ? ventilation_mode = self.executor.select_state(OverkizState.IO_VENTILATION_MODE) |
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.
Hi @iMicknl,
Could you fix the typo error and update with the last commits from master build instead?
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Outdated
Show resolved
Hide resolved
@tillstaff this one is very hard for me to debug without actual access. Is this PR currently working or still broken? I am looking if this one can be migrated to core as well. |
@iMicknl this PR is working, all the commands are working with the corrections in the last comments. One of the log error can be solve with this change : For the other error log, I don't know how we can avoid the "too many request error". |
…y_ventilation.py Co-authored-by: tillstaff <[email protected]>
Did you test the latest version? I just applied your suggestions. Could you check if everything works now? Does the too many request error happen a lot, or only during testing (when you reboot a lot) and maybe even have 2 installations running at the same time? |
Thanks for the commits, I will test this new build. The correction The log error "too many request" appears each time I change the preset or the fan-mode. |
custom_components/tahoma/climate_devices/atlantic_heat_recovery_ventilation.py
Show resolved
Hide resolved
We moved away from this part of the code, thus when we add it to core, this will be fixed as well.
I will have a look, in the end we should group these commands in a single call.. Are the refresh calls needed? |
Yes, there are needed. I made some tests without them and the states of the VMC were totally wrong after that. I made some new traces with the cozytouch app and I confirm that the two first commands (setVentilationMode and setVentilationConfigurationMode) are send in a single call and the two refresh commands are send later in a single call (1.5 seconds after the end of execution of the first commands).
Can we build the same commands requests? |
@tetienne if you can approve, I will merge. Perhaps we can bring this one to core a well? |
) | ||
prog = ventilation_mode.get(OverkizCommandParam.PROG) | ||
|
||
if prog == OverkizCommandParam.ON: |
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.
We can use a dict here no?
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.
I will have a look, thanks!
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.
@tetienne you mean a dict for ventilation_configuration? Since we need to check from two values. (prog and ventilation_configuration).
_attr_hvac_modes = [HVAC_MODE_FAN_ONLY] | ||
_attr_preset_modes = [PRESET_AUTO, PRESET_PROG, PRESET_MANUAL] | ||
_attr_temperature_unit = TEMP_CELSIUS | ||
_attr_supported_features = SUPPORT_PRESET_MODE | SUPPORT_FAN_MODE |
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.
We can use ClimateEntityFeatures here
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.
I will do that for core, but otherwise I need to bump the custom component version etc. :D
OK to be merged. |
Fixes #376