Add presets to Advantage Air climate entities

[Bre77]

Proposed change

The Advantage Air ducted AC products support three different operating modes, "MyZone" (the default), "MyTemp", and "MyAuto". Before this PR, there was no way to switch between these modes in Home Assistant, and the behaviour of the entities may have been unexpected if the preset was changed.

This PR fixes this by adding each of the three operating modes as a preset in the climate entity, and reloading the integration the operating mdoe changes.

MyTemp and MyAuto are only added if the target system supports them. If neither are supported, preset support is not added to the climate entity.

A visual breakdown of these features can be found at https://www.advantageair.com.au/wp-content/uploads/2019/10/MyComfort.pdf

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 # https://community.home-assistant.io/t/additional-controls-for-myplace-hvac-controller-via-advantage-air-integration/421632
• This PR is related to issue:
• Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/23402

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] The code has been formatted using Black (black --fast homeassistant tests)
• [x] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• [x] Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• [ ] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [ ] New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• [ ] Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

• [ ] No score or internal
• [ ] 🥈 Silver
• [ ] 🥇 Gold
• [x] 🏆 Platinum

To help with the load of incoming pull requests:

• [x] I have reviewed two other open pull requests in this repository.

[Bre77]

@bdraco since you have been on an absolute roll with my PRs today (much appreciated!!), This one has had its conflict resolved.

[bdraco]

Looks like there are some tests that need to be fixed here

[Bre77]

@bdraco if you get a chance, this is ready to go again.

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[bdraco]

Not stale, awaits review bandwidth for second opinion

[frenck]

Meanwhile, a wild merge conflict appeared. Could you take a look @Bre77?

Thanks already 👍

../Frenck

[frenck]

Thanks for addressing the conflict @Bre77 👍

../Frenck

[Bre77]

Thanks @frenck So to expedite this for the next person to review, I'll summarise:

The hardware has three operating modes, and this PR adds support for all three. The user can change this opperating mode on the hardware or (with this PR) using Home Assistant. However, in doing so, the "Supported Features" provided by the integration change to match the hardware. Due to the way Home Assistant is written, this works flawlessly within the core, but it may upset voice assistants.

Option 1 This PR is accepted, allowing Supported Features to change dynamically, and provide a great user experience. (my recommendation)

Option 2 Rework this PR so that Supported Features are set on startup, and if the user changes the preset, they get a persistant notification telling them to restart Home Assistant because the integration is now unable to control the hardware.

Option 3 Support all supported features all the time, giving a confusing user experience about what is and is not going to work due to the hardwares operating mode.

Option 4 Not support the two other operating modes of this hardware in core and continue providing it in HACS. (essentially the current state)

In the real world, users will unlikely change the Operating Mode often, so Option 1 and 2 are fine, but if the choice is between voice assistants being unhappy, or the entire integration being unable to operating (which also impacts voice assistants), then Option 1 is the best outcome.

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

[frenck]

allowing Supported Features to change dynamically, and provide a great user experience.

That isn't allowed and thus not an option.

Rework this PR so that Supported Features are set on startup,

Yes, that is how it is supposed to work.

and if the user changes the preset, they get a persistant notification telling them to restart Home Assistant because the integration is now unable to control the hardware.

Reload the integration entry, no need to burden a user with that.

[frenck]

Marking the PR draft, as it needs work (and has merge conflicts). Undraft it when ready.

[Bre77]

I have removed all the dynamic properties, but am struggling to reload the integration entry on state change.

[Bre77]

Ok I think I managed to brute force a working solution. I however have not figured out how to correctly test this

[bdraco]

While I applaud your ingenuity here, I don't think reloading the integration is going to resolve the problem as its still effectively the same problem as the supported features will change after the start event.

I wish I had a solution to offer you but I really don't see a good option here. I'm hoping another reviewer will have an idea.

[Bre77]

While I applaud your ingenuity here, I don't think reloading the integration is going to resolve the problem as its still effectively the same problem as the supported features will change after the start event.

I wish I had a solution to offer you but I really don't see a good option here. I'm hoping another reviewer will have an idea.

Would you say to not reload and just leave it up to the user to restart Home Assistant? @frenck was the one who suggested doing a reload rather than a persistent notification.

[bdraco]

This one is super frustrating (I'm sure for everyone) as there isn't a really good solution that works with our current core design.

Maybe a repair issue would be better but I'll wait for frenck to comment

[Bre77]

So I am intending #91107 to be a stop gap thats easier to approve. This PR will then build upon it (unless its merged first) to add the preset support.

[bdraco]

That's a good call. Good to not hold up the other fixes because we are stuck on the architecture problem

[elupus]

Hmm. Very interesting solution. I actually think reloading would be okey. supported features are allowed to change on load of an integration. It's really really ugly, but it would work and i don't think it breaks the rules per say.

At the moment i don't think we re-sync google assistant/alexa on such changes. But we probably should.

[Bre77]

Hmm. Very interesting solution. I actually think reloading would be okey. supported features are allowed to change on load of an integration. It's really really ugly, but it would work and i don't think it breaks the rules per say.

At the moment i don't think we re-sync google assistant/alexa on such changes. But we probably should.

Also if a user had an issue with their assistant it would be easy for them to setup an automation to run the sync when the preset is changed.

The solution that didn't require a reload was so much cleaner, but "broke the rules".

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[MartinHjelmare]

Converting to draft as I think we need an architecture discussion or a different approach to add these presets.

[Bre77]

Happy Birthday to this pull request. Today it is 1 year old. 🎂

[frenck]

@MartinHjelmare I think we've discussed this at some point. Is that enough to take this PR out of the stuck place it is in?

../Frenck

[MartinHjelmare]

I think we've said something like this:

• Amazon, Google Assistant, Homekit needs to listen to capability attribute changes
• We need a way for integrations to sync capability attributes in the registry

But there's no architecture discussion or solution right now, I think.

[bdraco]

HomeKit can now handle supported features changing. We were able to do it without an architecture change since we had all the data we needed from the state change event.

I don't use the other ones so I'm not in a position to add support there.

[Bre77]

HomeKit can now handle supported features changing. We were able to do it without an architecture change since we had all the data we needed from the state change event.

I don't use the other ones so I'm not in a position to add support there.

Home Assistant already has the service to instruct Google Assistant to reload.

[github-actions[bot]]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!