core
core copied to clipboard
Add overkiz support for Atlantic Shogun ZoneControl 2.0 (AtlanticPassAPCHeatingAndCoolingZone)
Proposed change
There was a previous implementation of the widget AtlanticPassAPCHeatingAndCoolingZone, but it was lacking cooling features, and was only reusing AtlanticPassAPCHeatingZone logic.
There was a lack of support for the devices not supporting the setDerogatedTargetTemperature command (see #89746).
This PR handles that, it is mainly a fix for the Atlantic Shogun Zone Control 2.0, but could work for other devices. As of right now, I only found one other device using the same widget : Atlantic Alféa Extensa Duo (see discussion here : https://github.com/iMicknl/ha-tahoma/issues/381), for this device the behavior will stay the same as before (good or bad as it is).
Here what can be done :
- change of temperature (will change manual cooling and manual heating temperature)
- modes :
OFFdoes not really stop de HVAC, but stop the airflow to the specific zone (= STOP on the zone thermostat)HEATorCOOLorDRYING, the only other available mode for the zone, will be the currently active mode on the main ZoneControl unit
- preset :
Manual: allow to set temperatureInternal_Schedule: will follow comfort/eco rules set internaly on the main ZoneControl unit, temperature change is ignored on this mode
In addition Auto HVAC mode has been added, when the Atlantic Pass APC device can be configured with.
Here what may be done in a future PR :
- disable change of temperature for DRYING mode
- follow virtually the device preset (comfort, eco) when targetTemperature = presetTargetTemperature
- it means the
internal_schedulepreset info is lost, so that when we selectinternal_schedule, it will immediately goes toComfortorEco, maybe aSchedule ComfortandSchedule Ecopreset could be added instead to solve the issue
- it means the
- When Comfort preset or Eco preset is selected, allow the change of the preset temperature (but only for the current mode as opposition to Manual which changes for all modes)
See #89746 for more informations about these changes.
For other devices, that support the setDerogatedTargetTemperature command, the behavior has not been changed and it will fallback to the AtlanticPassAPCHeatingZone like before (so no breaking changes).
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [x] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes #89746
- This PR is related to issue:
- Link to documentation pull request:
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] I have followed the perfect PR recommendations
- [x] The code has been formatted using Ruff (
ruff format homeassistant tests) - [ ] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [ ] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [x] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest. - [x] New or updated dependencies have been added to
requirements_all.txt.
Updated by runningpython3 -m script.gen_requirements_all. - [x] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [x] Untested files have been added to
.coveragerc.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Hey there @imicknl, @vlebourl, @tetienne, @nyrodev, mind taking a look at this pull request as it has been labeled with an integration (overkiz) you are listed as a code owner for? Thanks!
Code owner commands
Code owners of overkiz can trigger bot actions by commenting:
@home-assistant closeCloses the pull request.@home-assistant rename Awesome new titleRenames the pull request.@home-assistant reopenReopen the pull request.@home-assistant unassign overkizRemoves the current integration label and assignees on the pull request, add the integration domain after the command.@home-assistant add-label needs-more-informationAdd a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.@home-assistant remove-label needs-more-informationRemove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.
@Tronix117 Thanks for your first PR here!
This PR is breaking everything on my setup regarding the device you're updating. Before, I had HAVC Mode enabled (Auto, Heating, Off) Now, I've got nothing
Before I had Preset Eco, comfort, away, antifrost, externe, home, Now, I only have manual and schedule
When I try to set to manual, I have error:
2024-02-15 22:12:15.804 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [140617948070336] 'manual'
Traceback (most recent call last):
File "/workspaces/home-assistant-core/homeassistant/components/websocket_api/commands.py", line 240, in handle_call_service
response = await hass.services.async_call(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspaces/home-assistant-core/homeassistant/core.py", line 2291, in async_call
response_data = await coro
^^^^^^^^^^
File "/workspaces/home-assistant-core/homeassistant/core.py", line 2328, in _execute_service
return await target(service_call)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspaces/home-assistant-core/homeassistant/helpers/service.py", line 904, in entity_service_call
single_response = await _handle_entity_call(
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspaces/home-assistant-core/homeassistant/helpers/service.py", line 974, in _handle_entity_call
result = await task
^^^^^^^^^^
File "/workspaces/home-assistant-core/homeassistant/components/climate/__init__.py", line 709, in async_handle_set_preset_mode_service
await self.async_set_preset_mode(preset_mode)
File "/workspaces/home-assistant-core/homeassistant/components/overkiz/climate_entities/atlantic_pass_apc_heating_and_cooling_zone.py", line 165, in async_set_preset_mode
return await super().async_set_preset_mode(preset_mode)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspaces/home-assistant-core/homeassistant/components/overkiz/climate_entities/atlantic_pass_apc_heating_zone.py", line 156, in async_set_preset_mode
await self.async_set_heating_mode(PRESET_MODES_TO_OVERKIZ[preset_mode])
~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
KeyError: 'manual'
Min/max available temperarture was also set up directly in the code, because AFAIK it was done that way in tahoma code too.
If I understand correctly, you're trying to fix a problem for a specific device, which seem to fall under the same widget name than me (and the one that is currenlty working on HA)
If the diagnostic you're trying to fix is the one posted here, I think we have to choose the correct device class using controllableName
On my case, it's AtlanticPassAPCHeatingAndCoolingZoneComponent
On the diagnostic posted, it's io:AtlanticPassAPCZoneControlZoneComponent
In other words, it means we have to something like WIDGET_AND_PROTOCOL_TO_CLIMATE_ENTITY to match the Widget and then the controllablename.
Or maybe we can do directly on the controllableName.
Ok thanks for your feedback.
That's weird because I first check if you support the missing command for the device I'm trying to fix (the one used to set the temperature on your device), if it's the case I fallback to the previous behavior. Obviously it seems that I introduced a mistake somewhere, I'll check that next week.
Edit: I know what the issue is, it's from the initialized attributes, I need to do them in the init function.
The initial code is also not perfect for your device that is reversible (to cooling) if the thermostat is changed. I'll try to do something for that too (but in a separated PR)
@nyroDev You were right about controllableName, I missed that in your diagnostic export.
I changed the code to be able to choose the class using the controllableName in addition to the widget.
However I still kept (and also fix the issue you had), the check I do on the SET_DEROGATED_TARGET_TEMPERATURE presence. That way I'm sure it will behave good even if controllableName is not what we expect on an "unknown yet" device.
Not sure if I shall still keep it, since it increase complexity a bit...
Could you please @nyroDev check if the new changes I made, is working perfectly (like before) for you ? Thanks
Could you please @nyroDev check if the new changes I made, is working perfectly (like before) for you ? Thanks
The current code works as before for me, thanks for that ;)
Thanks for the review, it seems pretty good, strings instead of constants are there because there is an issue with the CI on the other repo preventing the pyoverkiz release (@iMickNL could you take a look ?) I'll include the changes asap
@Tronix117 what do you mean? The latest pyoverkiz has been published a few days ago and should include your changes.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1: