-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
Daikin Airbase Zone Temperature Control #146279
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
base: dev
Are you sure you want to change the base?
Conversation
Hey there @fredrike, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
@fredrike Hopefully this second attempt is up to scratch. Let me know what you think |
Okay so I checked the previous PR and we proposed to use a climate entity, which Erik showed was still quite usable, why did we decide not to go with that one and go with a service call? A service call isn't very user friendly as you need to automate with it per se and you can't just drop it in a card on your dashboard. WDYT? |
Last comment from Erik was to implement as service call so have done so here. Agree service call isn't very user friendly and in parallel have been investigating how to address the limitations previously raised (e.g. no current temp for zone) but to no avail. Keen to get thoughts on implementing this capability as a "feature" in the climate card. However, this gets a bit complicated with zone switches on top of set temperature, so I'm not sure if these should be conditionally combined or available as separate elements. Thoughts? Once I get some feedback on the interface I'll raise a separate PR after this is merged. |
How do you mean? Is this a frontend question or something for core? |
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.
Pull Request Overview
This pull request adds the ability to control individual zone temperatures on compatible Daikin Airbase systems. Key changes include:
- A new custom service implementation in services.py (with retry mechanisms and error handling) and corresponding YAML service definition.
- Updated tests in tests/components/daikin/test_services.py covering various scenarios for zone temperature setting.
- Enhancements in the climate component to expose zone temperature state attributes and updates in init.py to correctly register the service and coordinator.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/components/daikin/test_services.py | Added extensive test cases for the new service functionality. |
homeassistant/components/daikin/services.yaml | Introduced the service schema for setting zone temperature. |
homeassistant/components/daikin/services.py | Implemented the set_zone_temperature service with validation, retry mechanism, and logging. |
homeassistant/components/daikin/climate.py | Updated extra_state_attributes to report zone temperatures based on the current HVAC mode. |
homeassistant/components/daikin/init.py | Modified integration setup to initialize hass.data and register the new custom service. |
Comments suppressed due to low confidence (1)
homeassistant/components/daikin/init.py:68
- Ensure that the initialization of hass.data[DOMAIN] during the integration setup is performed in a thread-safe manner to avoid potential race conditions in concurrent environments.
if DOMAIN not in hass.data:
So you mean it either supports on off or on off and setting temperature? I guess then it still makes sense. But I still feel like you have concerns, and since I don't want to send you into the woods with an impossible task, be sure to voice the concerns so I can also see what you're seeing :) |
Correct
No concerns from me; this PR shouldn't cause any issues, but I would like to get feedback and suggestions on the frontend approach before implementing. What is the best way to engage the community for this? |
I'm running your PR as a custom component and can confirm it causes no issues with my setup. I like the way you combine the front end setup in your first example. Ideally if its possible to obtain the zone temp sensor data we could create individual climate entities for each room, but I don't think this is available from the API? |
Nice! Does your system support the set temperature for zones? How did you find the error handling?
Noted. I think combining zone on/off and set temp into a single looks better so I might try that approach first. Yeah unfortunately the zone sensors aren't exposed in the known endpoints (note: there is no official docs from Daikin). I tried brute-forcing possible combinations but discovered no useful endpoints. Hence implementing this control as a "feature" above. |
Well, a thing is that the community doesn't really decide how an entity is implemented. That is all decided by the available data and the behaviour of the device. For example, a climate entity would allow users to directly use voice commands out of the box, while if it was a number or an action, it requires effort from the user to support that. |
Yes I have 6 individual zones and have no issues changing the temperature of a running zone +- 2deg, If I modify a zone that is not on, it sets the temp and also turns the zone on. If I try to adjust outside of 2deg, it throws an error no problem. So all works great.
Yep I've had no luck finding it either. I started trying p1p2mqtt and have bought a faikin, but have come back to this component for now. Thanks for your work on this PR! |
I'm still not sure what you exactly mean with your screenshot as it looks like custom cards. But so far I understand:
We should then attach the zone each to their own device, so users can assign their device to the right areas. Does this align with what you're seeing? |
Ideally I'd like to update the existing climate card with new attributes to expose to the user as "features" to control zones.
Correct. Additionally, zones can be turned on and off separate to the climate system being turned off or on (to a selected mode).
Since the zones are dependant on the climate entity, my thinking on the approach is to:
|
That would require an architectural decision and IMO we can do this without these features as they would be their own device and climate entity |
My personal preference would be for this to be implemented as a feature of climate entities and leverage the climate Lovelace card for frontend. Could I seek an architecture review of this approach? |
Proposed change
Add ability to set the temperature for zones of compatible Daikin Airbase systems.
Type of change
Additional information
This PR adds a new service
daikin.set_zone_temperature
that allows users to control individual zone temperatures on compatible Daikin Airbase systems. The service includes:The service includes test cases covering:
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: